Skip to content

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Oct 1, 2025

Tests should not try to manually enable incremental compilation with -Cincremental, because that typically results in stray directories being created in the repository root.

Also, if the incremental directory is not cleared, there is a risk of interference between successive runs of the same test.

Instead, use the //@ incremental directive, which instructs compiletest to handle the details of passing -Cincremental with a fresh directory.

@rustbot
Copy link
Collaborator

rustbot commented Oct 1, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner 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 Oct 1, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 1, 2025

r? @SparrowLii

rustbot has assigned @SparrowLii.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@Zalathar
Copy link
Contributor Author

Zalathar commented Oct 1, 2025

Currently blocked on #147217, which fixes the offending test.

@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 1, 2025
Copy link
Member

@lqd lqd left a comment

Choose a reason for hiding this comment

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

👍 , r=me when unblocked

View changes since this review

@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Member

jieyouxu commented Oct 1, 2025

We should also change ui tests to not run tests under... weird places, but that's for another PR.

@bjorn3
Copy link
Member

bjorn3 commented Oct 1, 2025

This fixes #128057.

Tests should not try to manually enable incremental compilation with
`-Cincremental`, because that typically results in stray directories being
created in the repository root.

Instead, use the `//@ incremental` directive, which instructs compiletest to
handle the details of passing `-Cincremental` with a fresh directory.
@rustbot
Copy link
Collaborator

rustbot commented Oct 2, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@Zalathar
Copy link
Contributor Author

Zalathar commented Oct 2, 2025

@bors r=lqd rollup

@bors
Copy link
Collaborator

bors commented Oct 2, 2025

📌 Commit 59c4dfe has been approved by lqd

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-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Oct 2, 2025
@Zalathar
Copy link
Contributor Author

Zalathar commented Oct 2, 2025

r? lqd

@rustbot rustbot assigned lqd and unassigned SparrowLii Oct 2, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 2, 2025
Forbid `//@ compile-flags: -Cincremental=` in tests

Tests should not try to manually enable incremental compilation with `-Cincremental`, because that typically results in stray directories being created in the repository root.

Also, if the incremental directory is not cleared, there is a risk of interference between successive runs of the same test.

Instead, use the `//@ incremental` directive, which instructs compiletest to handle the details of passing `-Cincremental` with a fresh directory.
bors added a commit that referenced this pull request Oct 2, 2025
Rollup of 10 pull requests

Successful merges:

 - #146281 (Support `#[rustc_align_static]` inside `thread_local!`)
 - #146535 (mbe: Implement `unsafe` attribute rules)
 - #146585 (indexing: reword help)
 - #147004 (Tweak handling of "struct like start" where a struct isn't supported)
 - #147221 (Forbid `//@ compile-flags: -Cincremental=` in tests)
 - #147225 (Don't enable shared memory by default with Wasm atomics)
 - #147227 (implement `Box::take`)
 - #147231 (Extending `#[rustc_force_inline]` to be applicable to inherent methods)
 - #147233 (Initialize llvm submodule if not already the case to run citool)
 - #147236 (Update books)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 2, 2025
Forbid `//@ compile-flags: -Cincremental=` in tests

Tests should not try to manually enable incremental compilation with `-Cincremental`, because that typically results in stray directories being created in the repository root.

Also, if the incremental directory is not cleared, there is a risk of interference between successive runs of the same test.

Instead, use the `//@ incremental` directive, which instructs compiletest to handle the details of passing `-Cincremental` with a fresh directory.
jhpratt added a commit to jhpratt/rust that referenced this pull request Oct 2, 2025
Forbid `//@ compile-flags: -Cincremental=` in tests

Tests should not try to manually enable incremental compilation with `-Cincremental`, because that typically results in stray directories being created in the repository root.

Also, if the incremental directory is not cleared, there is a risk of interference between successive runs of the same test.

Instead, use the `//@ incremental` directive, which instructs compiletest to handle the details of passing `-Cincremental` with a fresh directory.
bors added a commit that referenced this pull request Oct 2, 2025
Rollup of 9 pull requests

Successful merges:

 - #146281 (Support `#[rustc_align_static]` inside `thread_local!`)
 - #146535 (mbe: Implement `unsafe` attribute rules)
 - #146585 (indexing: reword help)
 - #147004 (Tweak handling of "struct like start" where a struct isn't supported)
 - #147221 (Forbid `//@ compile-flags: -Cincremental=` in tests)
 - #147225 (Don't enable shared memory by default with Wasm atomics)
 - #147227 (implement `Box::take`)
 - #147233 (Initialize llvm submodule if not already the case to run citool)
 - #147236 (Update books)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b78b107 into rust-lang:master Oct 2, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Oct 2, 2025
rust-timer added a commit that referenced this pull request Oct 2, 2025
Rollup merge of #147221 - Zalathar:incremental, r=lqd

Forbid `//@ compile-flags: -Cincremental=` in tests

Tests should not try to manually enable incremental compilation with `-Cincremental`, because that typically results in stray directories being created in the repository root.

Also, if the incremental directory is not cleared, there is a risk of interference between successive runs of the same test.

Instead, use the `//@ incremental` directive, which instructs compiletest to handle the details of passing `-Cincremental` with a fresh directory.
@Zalathar Zalathar deleted the incremental branch October 2, 2025 23:06
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Oct 3, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang/rust#146281 (Support `#[rustc_align_static]` inside `thread_local!`)
 - rust-lang/rust#146535 (mbe: Implement `unsafe` attribute rules)
 - rust-lang/rust#146585 (indexing: reword help)
 - rust-lang/rust#147004 (Tweak handling of "struct like start" where a struct isn't supported)
 - rust-lang/rust#147221 (Forbid `//@ compile-flags: -Cincremental=` in tests)
 - rust-lang/rust#147225 (Don't enable shared memory by default with Wasm atomics)
 - rust-lang/rust#147227 (implement `Box::take`)
 - rust-lang/rust#147233 (Initialize llvm submodule if not already the case to run citool)
 - rust-lang/rust#147236 (Update books)

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-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.

compiletest: -Cincremental in ui tests dump incr build output to source directories
8 participants