Skip to content

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)
bors added a commit that referenced this pull request Jul 12, 2025
Rollup of 15 pull requests

Successful merges:

 - #143554 (slice: Mark `rotate_left`, `rotate_right` unstably const)
 - #143634 (interpret/allocation: expose init + write_wildcards on a range)
 - #143710 (Updates to random number generation APIs)
 - #143774 (constify `From` and `Into`)
 - #143776 (std: move NuttX to use arc4random for random number generation)
 - #143778 (Some const_trait_impl test cleanups)
 - #143782 (Disambiguate between rustc vs std having debug assertions in `run-make-support` and `run-make` tests)
 - #143791 (Update sysinfo version to `0.36.0`)
 - #143796 (Fix ICE for parsed attributes with longer path not handled by CheckAttribute)
 - #143798 (Remove format short command trait)
 - #143803 (New tracking issues for const_ops and const_cmp)
 - #143814 (htmldocck: better error messages for some negative directives)
 - #143817 (Access `wasi_sdk_path` instead of reading environment variable in bootstrap)
 - #143822 (./x test miri: fix cleaning the miri_ui directory)
 - #143823 ([COMPILETEST-UNTANGLE 5/N] Test mode adjustments and other assorted cleanups)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jul 13, 2025
Rollup of 12 pull requests

Successful merges:

 - #143776 (std: move NuttX to use arc4random for random number generation)
 - #143778 (Some const_trait_impl test cleanups)
 - #143782 (Disambiguate between rustc vs std having debug assertions in `run-make-support` and `run-make` tests)
 - #143791 (Update sysinfo version to `0.36.0`)
 - #143796 (Fix ICE for parsed attributes with longer path not handled by CheckAttribute)
 - #143798 (Remove format short command trait)
 - #143803 (New tracking issues for const_ops and const_cmp)
 - #143814 (htmldocck: better error messages for some negative directives)
 - #143817 (Access `wasi_sdk_path` instead of reading environment variable in bootstrap)
 - #143822 (./x test miri: fix cleaning the miri_ui directory)
 - #143823 ([COMPILETEST-UNTANGLE 5/N] Test mode adjustments and other assorted cleanups)
 - #143841 (Label clippy changes with `T-clippy`)

Failed merges:

 - #143850 (Compiletest: Simplify {Html,Json}DocCk directive handling)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3cd37df into rust-lang:master Jul 13, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 13, 2025
rust-timer added a commit that referenced this pull request Jul 13, 2025
Rollup merge of #143782 - jieyouxu:debug-assertions, r=ChrisDenton

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 #143669 (comment).

Best reviewed commit-by-commit.

r? ``@ChrisDenton`` (or compiler/bootstrap)
@jieyouxu jieyouxu deleted the debug-assertions branch July 14, 2025 01:29
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jul 14, 2025
Rollup of 12 pull requests

Successful merges:

 - rust-lang/rust#143776 (std: move NuttX to use arc4random for random number generation)
 - rust-lang/rust#143778 (Some const_trait_impl test cleanups)
 - rust-lang/rust#143782 (Disambiguate between rustc vs std having debug assertions in `run-make-support` and `run-make` tests)
 - rust-lang/rust#143791 (Update sysinfo version to `0.36.0`)
 - rust-lang/rust#143796 (Fix ICE for parsed attributes with longer path not handled by CheckAttribute)
 - rust-lang/rust#143798 (Remove format short command trait)
 - rust-lang/rust#143803 (New tracking issues for const_ops and const_cmp)
 - rust-lang/rust#143814 (htmldocck: better error messages for some negative directives)
 - rust-lang/rust#143817 (Access `wasi_sdk_path` instead of reading environment variable in bootstrap)
 - rust-lang/rust#143822 (./x test miri: fix cleaning the miri_ui directory)
 - rust-lang/rust#143823 ([COMPILETEST-UNTANGLE 5/N] Test mode adjustments and other assorted cleanups)
 - rust-lang/rust#143841 (Label clippy changes with `T-clippy`)

Failed merges:

 - rust-lang/rust#143850 (Compiletest: Simplify {Html,Json}DocCk directive handling)

r? `@ghost`
`@rustbot` modify labels: rollup
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