Skip to content
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

Better way of conditioning the sanitizer builds #64166

Merged
merged 1 commit into from
Sep 6, 2019

Conversation

infinity0
Copy link
Contributor

Previously the build would take the presence of the LLVM_CONFIG envvar to
mean that the sanitizers should be built, but this is a common envvar that
could be set for reasons unrelated to the rustc sanitizers.

This commit adds a new envvar RUSTC_BUILD_SANITIZERS and uses it instead.

This PR or similar will be necessary in order to work correctly with rust-lang/compiler-builtins#296

Previously the build would take the presence of the LLVM_CONFIG envvar to
mean that the sanitizers should be built, but this is a common envvar that
could be set for reasons unrelated to the rustc sanitizers.

This commit adds a new envvar RUSTC_BUILD_SANITIZERS and uses it instead.
@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 5, 2019
@cramertj
Copy link
Member

cramertj commented Sep 5, 2019

r? @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Sep 5, 2019

📌 Commit 485697b has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2019
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Sep 6, 2019
Better way of conditioning the sanitizer builds

Previously the build would take the presence of the LLVM_CONFIG envvar to
mean that the sanitizers should be built, but this is a common envvar that
could be set for reasons unrelated to the rustc sanitizers.

This commit adds a new envvar RUSTC_BUILD_SANITIZERS and uses it instead.

This PR or similar will be necessary in order to work correctly with rust-lang/compiler-builtins#296
Centril added a commit to Centril/rust that referenced this pull request Sep 6, 2019
Better way of conditioning the sanitizer builds

Previously the build would take the presence of the LLVM_CONFIG envvar to
mean that the sanitizers should be built, but this is a common envvar that
could be set for reasons unrelated to the rustc sanitizers.

This commit adds a new envvar RUSTC_BUILD_SANITIZERS and uses it instead.

This PR or similar will be necessary in order to work correctly with rust-lang/compiler-builtins#296
bors added a commit that referenced this pull request Sep 6, 2019
Rollup of 9 pull requests

Successful merges:

 - #64067 (Remove no-prefer-dynamic from valgrind tests)
 - #64078 (compiletest: disable -Aunused for run-pass tests)
 - #64096 (Fix regex replacement in theme detection)
 - #64098 (Ensure edition lints and internal lints are enabled with deny-warnings=false)
 - #64166 (Better way of conditioning the sanitizer builds)
 - #64189 (annotate-snippet emitter: Deal with multispans from macros, too)
 - #64202 (Fixed grammar/style in some error messages)
 - #64206 (annotate-snippet emitter: Update an issue number)
 - #64208 (it's more pythonic to use 'is not None' in python files)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Sep 6, 2019
Better way of conditioning the sanitizer builds

Previously the build would take the presence of the LLVM_CONFIG envvar to
mean that the sanitizers should be built, but this is a common envvar that
could be set for reasons unrelated to the rustc sanitizers.

This commit adds a new envvar RUSTC_BUILD_SANITIZERS and uses it instead.

This PR or similar will be necessary in order to work correctly with rust-lang/compiler-builtins#296
bors added a commit that referenced this pull request Sep 6, 2019
Rollup of 8 pull requests

Successful merges:

 - #63565 (Rust 2018: NLL migrate mode => hard error)
 - #63969 (Add missing examples for Option type)
 - #64067 (Remove no-prefer-dynamic from valgrind tests)
 - #64166 (Better way of conditioning the sanitizer builds)
 - #64189 (annotate-snippet emitter: Deal with multispans from macros, too)
 - #64202 (Fixed grammar/style in some error messages)
 - #64206 (annotate-snippet emitter: Update an issue number)
 - #64208 (it's more pythonic to use 'is not None' in python files)

Failed merges:

r? @ghost
@bors bors merged commit 485697b into rust-lang:master Sep 6, 2019
Centril added a commit to Centril/rust that referenced this pull request Sep 16, 2019
…ichton

Fix build script sanitizer check.

rust-lang#64166 changed the way the sanitizer build scripts work. However, they were changed so that they switch between new-style to old-style cargo fingerprints. This trips up on rust-lang/cargo#6779.

It also causes rustbuild to panic.  If you build stage1 std (with sanitizers off), and then enable sanitizers, it panics.  (This is because the build scripts don't declare that they need to re-run.)

This PR will trip rust-lang/cargo#6779 again, unfortunately. I've been having way too many unexplained rebuilds in rust-lang/rust recently, but at least I'll know why this time.

This doesn't fix all problems with the build scripts, but arguably they should be fixed in cargo. For example, the build scripts change which rerun-if statements they declare between runs which triggers rust-lang/cargo#7362.

The test for this is:
1. Turn off sanitizers (which is the default)
2. `./x.py build --stage=1 src/libstd`
3. `./x.py build --stage=1 src/libstd` again should be a null build.
4. Enable sanitizers.
5. `./x.py build --stage=1 src/libstd` should rebuild with sanitizers enabled.
6. `./x.py build --stage=1 src/libstd` again should be a null build. This actually rebuilds due to rust-lang/cargo#7362 because the rerun-if directives changed between step 3 and 5. A 3rd attempt should be a null build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants