-
Notifications
You must be signed in to change notification settings - Fork 998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add test coverage for use of git
without feature flag
#10874
Conversation
e952047
to
dc14fe9
Compare
dc14fe9
to
e1e90ce
Compare
c48e963
to
d71c696
Compare
d71c696
to
0b4365a
Compare
git
without feature flag
b1eaa9e
to
fd318d0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I gather from all the new attrs that The System Works!
let contents = r"#!/bin/sh | ||
echo 'error: `git` operations are not allowed — are you missing a cfg for the `git` feature?' >&2 | ||
exit 127"; | ||
let git = bin_dir.join(format!("git{}", env::consts::EXE_SUFFIX)); | ||
fs_err::write(&git, contents)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's funny, this is all kinds of weird on windows but that's fine because we just need windows to explode (especially because we only need this to break on some platform so the CI throws a fit and tells you to add a cfg).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah haha it doesn't fail with a nice error message on Windows but it does fail loudly..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there something better we could do that remains trivial?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, especially if anything specifically looks for git.exe
(windows PATH resolution is weird and magic about suffixes and there's two different APIs for looking up things on PATH, it's a whole thing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Notably the thing JS does to shim binaries onto path doesn't work with rust's Command because of all this, blech)
Shoves a broken
git
executable onto the front of thePATH
in the test context when thegit
feature is disabled so they fail if they're missing the feature-gate.