Skip to content

Disambiguate between rustc vs std having debug assertions in run-make-support and run-make tests #143782

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Jul 11, 2025

NO_DEBUG_ASSERTIONS is set by CI that threads through to the ./configure.py script, which is somewhat fragile and "spooky action at a distance". For fmt-write-bloat, this is actually wrong because the test wants to gate on std being built with debug assertions or not, whereas NO_DEBUG_ASSERTIONS determines rustc being built with debug assertions or not. Instead, use env vars controlled by compiletest, whose debug assertion info comes from bootstrap.

rust/src/ci/run.sh

Lines 137 to 146 in 855e0fe

# Unless explicitly disabled, we want rustc debug assertions on the -alt builds
if [ "$DEPLOY_ALT" != "" ] && [ "$NO_DEBUG_ASSERTIONS" = "" ]; then
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-debug-assertions"
fi
else
# We almost always want debug assertions enabled, but sometimes this takes too
# long for too little benefit, so we just turn them off.
if [ "$NO_DEBUG_ASSERTIONS" = "" ]; then
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-debug-assertions"
fi

NO_DEBUG_ASSERTIONS controls --enable-debug-assertions

o("debug-assertions", "rust.debug-assertions", "build with debugging assertions")

which sets --rust.debug-assertions, which controls rustc debug assertions.

o(
"debug-assertions-std",
"rust.debug-assertions-std",
"build the standard library with debugging assertions",
)

--rust.debug-assertions-std controls std debug assertions.

Noticed while investigating fmt-write-bloat in #143669 (comment).

Best reviewed commit-by-commit.

r? @ChrisDenton (or compiler/bootstrap)

jieyouxu added 2 commits July 11, 2025 19:58
Additionally, `NO_DEBUG_ASSERTIONS` is set by CI that threads through to
the `./configure` script, which is somewhat fragile and "spooky action
at a distance". Instead, use env vars controlled by compiletest, whose
debug assertion info comes from bootstrap.
The test itself is still broken, but fix this gating separately first.
@rustbot rustbot added A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 11, 2025
@jieyouxu
Copy link
Member Author

I believe this gate was wrong ever since the test was introduced, but it was very understandably easy to get wrong because NO_DEBUG_ASSERTIONS is not a great env var name...

Copy link
Member

@ChrisDenton ChrisDenton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from correcting the mistake in the one test, I think this approach will also help avoid confusion in the future. So r=me when CI is green.

@jieyouxu
Copy link
Member Author

PR CI is 📗
@bors r=ChrisDenton rollup

@bors
Copy link
Collaborator

bors commented Jul 11, 2025

📌 Commit 33e6962 has been approved by ChrisDenton

It is now in the queue for this repository.

@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 Jul 11, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 12, 2025
…Denton

Disambiguate between rustc vs std having debug assertions in `run-make-support` and `run-make` tests

`NO_DEBUG_ASSERTIONS` is set by CI that threads through to the `./configure.py` script, which is somewhat fragile and "spooky action at a distance". For `fmt-write-bloat`, this is actually wrong because the test wants to gate on *std* being built with debug assertions or not, whereas `NO_DEBUG_ASSERTIONS` determines *rustc* being built with debug assertions or not. Instead, use env vars controlled by compiletest, whose debug assertion info comes from bootstrap.

https://github.com/rust-lang/rust/blob/855e0fe46e68d94e9f6147531b75ac2d488c548e/src/ci/run.sh#L137-L146

`NO_DEBUG_ASSERTIONS` controls `--enable-debug-assertions`

https://github.com/rust-lang/rust/blob/855e0fe46e68d94e9f6147531b75ac2d488c548e/src/bootstrap/configure.py#L124

which sets `--rust.debug-assertions`, which controls *rustc* debug assertions.

https://github.com/rust-lang/rust/blob/855e0fe46e68d94e9f6147531b75ac2d488c548e/src/bootstrap/configure.py#L125-L129

`--rust.debug-assertions-std` controls *std* debug assertions.

Noticed while investigating `fmt-write-bloat` in rust-lang#143669 (comment).

Best reviewed commit-by-commit.

r? `@ChrisDenton` (or compiler/bootstrap)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants