Skip to content

Conversation

@mikeyhew
Copy link
Contributor

@mikeyhew mikeyhew commented Jul 4, 2018

r? @alexcrichton

add-on to #5666

  • Added mod shell_quoting; to testsuite/main.rs, so it actually runs
  • fixed bugs in the test

- Added `mod shell_quoting` to testsuite/main.rs
- fixed up errors in the test, now that the test suite knows about it
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 4, 2018

📌 Commit d1cb78a has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 4, 2018

⌛ Testing commit d1cb78a with merge 623185438493056c5584002c019d8705c4c138d9...

@bors
Copy link
Contributor

bors commented Jul 4, 2018

💔 Test failed - status-appveyor

@mikeyhew
Copy link
Contributor Author

mikeyhew commented Jul 4, 2018

The test is failing on windows because shell-escape uses backslashes on windows, and single-quotes on unix. I'm not sure what the best thing is to do here. We could

  • use #[cfg] attributes to use one version on windows, and a different one on unix, or
  • check that the command is runnable. That's really the important thing here. One way to do that could be to figure out which part of the cargo output is from rustc, and then run the rustc command and make sure the output and exit code is the same.

What do you think?

@alexcrichton
Copy link
Member

I think env vars can be used to trick it into always doing unix escaping, so perhaps that could be used?

@mikeyhew
Copy link
Contributor Author

mikeyhew commented Jul 6, 2018

Sure, I'll try that. Should be the easiest fix.

- this should make it use unix escaping on windows
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 6, 2018

📌 Commit 2e17235 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 6, 2018

⌛ Testing commit 2e17235 with merge 2a39e6c...

bors added a commit that referenced this pull request Jul 6, 2018
Fix the shell_quoting test

r? @alexcrichton

add-on to #5666
- Added `mod shell_quoting;` to testsuite/main.rs, so it actually runs
- fixed bugs in the test
@bors
Copy link
Contributor

bors commented Jul 6, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 2a39e6c to master...

@bors bors merged commit 2e17235 into rust-lang:master Jul 6, 2018
@ehuss ehuss added this to the 1.29.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants