-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Some run-make-fulldeps don't fail if -include
fails
#83773
Comments
Ok it looks like make defaults to the environment variable if it's not set? So because x.py is setting RUSTC and RUSTDOC for the bootstrap shim, they get set to reasonable values:
Maybe this doesn't need changes? I'm not sure how important it is that RUSTC and RUSTDOC get set to their new values: rust/src/test/run-make-fulldeps/tools.mk Lines 11 to 12 in 4fa76a4
If it is important, I think the fix is as simple as changing |
Looks like TMPDIR is also set: |
Move rustdoc run-make-fulldeps tests to run-make This cuts the time to run the tests in half, because they don't require building a stage 2 compiler. They were all added to fulldeps before rust-lang#82802 because rustdoc wasn't available in run-make tests. This doesn't change coverage tests, which will be changed soon in a separate PR (rust-lang#83755 (comment)). This also changes some of the `-include` directives, see rust-lang#83773 for what's going on there. r? `@petrochenkov`
Move almost all run-make-fulldeps tests to run-make They pass fine, and this avoids having to build the compiler twice. There are few enough tests left that I think it should be possible to get rid of this test suite altogether, but I expect this PR to fail at least a few times in bors and want to get it merged before tackling further changes. cc rust-lang/rust#83775 Fixes rust-lang/rust#66085. Fixes rust-lang/rust#83773.
I tried this code:
I expected to see this happen: The test fails, because the Makefile includes
../tools.mk
which doesn't exist - it should include../../run-make-fulldeps/tools.mk
instead.Instead, this happened: The test passes.
I can change it to a hard error by changing
-include
toinclude
, but it seems bad to have a test that passes even if the commands it's running don't exist 😟Meta
This is based off of a5029ac.
The text was updated successfully, but these errors were encountered: