Skip to content

Conversation

Zalathar
Copy link
Contributor

Running ./x check bootstrap currently doesn't work, because it builds the bootstrap shim binaries with cfg(test), and those binaries can't find a tests submodule when they include shared_helpers.rs via #[path].

This PR fixes that by taking the tests module and moving it to super::tests::shared_helpers_tests instead.

(The extra tests submodule prevents tidy from complaining about unit tests that aren't in a dedicated tests module.)


It would be nice to also run ./x check bootstrap compiletest in CI, so that this and #134848 don't regress, but I didn't want to bundle that change with this fix.

@rustbot
Copy link
Collaborator

rustbot commented Dec 29, 2024

r? @jieyouxu

rustbot has assigned @jieyouxu.
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

@rustbot rustbot added 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) labels Dec 29, 2024
@clubby789
Copy link
Contributor

Ran into this recently; thanks!

r=me when green

@clubby789 clubby789 assigned clubby789 and unassigned jieyouxu Dec 29, 2024
@Zalathar
Copy link
Contributor Author

@bors r=clubby789 rollup

@bors
Copy link
Collaborator

bors commented Dec 29, 2024

📌 Commit 4192293 has been approved by clubby789

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 Dec 29, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 29, 2024
Rollup of 5 pull requests

Successful merges:

 - rust-lang#134799 (nits: Cleanups in `librustdoc::clean`)
 - rust-lang#134851 (docs: inline `alloc::ffi::c_str` types to `alloc::ffi`)
 - rust-lang#134869 (Bump compiler cc)
 - rust-lang#134876 (bootstrap: Consolidate the macros for declaring compiletest test suites)
 - rust-lang#134883 (bootstrap: Fix `./x check bootstrap` by moving `shared_helpers::tests`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fc89cb2 into rust-lang:master Dec 29, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 29, 2024
Rollup merge of rust-lang#134883 - Zalathar:shared-helpers, r=clubby789

bootstrap: Fix `./x check bootstrap` by moving `shared_helpers::tests`

Running `./x check bootstrap` currently doesn't work, because it builds the bootstrap shim binaries with `cfg(test)`, and those binaries can't find a `tests` submodule when they include `shared_helpers.rs` via `#[path]`.

This PR fixes that by taking the tests module and moving it to `super::tests::shared_helpers_tests` instead.

(The extra `tests` submodule prevents tidy from complaining about unit tests that aren't in a dedicated tests module.)

---

It would be nice to also run `./x check bootstrap compiletest` in CI, so that this and rust-lang#134848 don't regress, but I didn't want to bundle that change with this fix.
@rustbot rustbot added this to the 1.85.0 milestone Dec 29, 2024
@Zalathar Zalathar deleted the shared-helpers branch December 29, 2024 21:52
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 25, 2025
…ts, r=Kobzol

Fix `./x check bootstrap` (again)

Redoes rust-lang#134883 and reverts 40c2ca9 from rust-lang#142416. Unfortunately I missed this during review.

- Commit 1: Reverts 40c2ca9 and re-applies rust-lang#134883.
- Commit 2: Check `./x check bootstrap` in `pr-check-1` to catch "obvious" problems like this.

r? Kobzol
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 25, 2025
…ts, r=Kobzol

Fix `./x check bootstrap` (again)

Redoes rust-lang#134883 and reverts 40c2ca9 from rust-lang#142416. Unfortunately I missed this during review.

- Commit 1: Reverts 40c2ca9 and re-applies rust-lang#134883.
- Commit 2: Check `./x check bootstrap` in `pr-check-1` to catch "obvious" problems like this.

r? Kobzol
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 26, 2025
…ts, r=Kobzol

Fix `./x check bootstrap` (again)

Redoes rust-lang#134883 and reverts 40c2ca9 from rust-lang#142416. Unfortunately I missed this during review.

- Commit 1: Reverts 40c2ca9 and re-applies rust-lang#134883.
- Commit 2: Check `./x check bootstrap` in `pr-check-1` to catch "obvious" problems like this.

r? Kobzol
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 26, 2025
…ts, r=Kobzol

Fix `./x check bootstrap` (again)

Redoes rust-lang#134883 and reverts 40c2ca9 from rust-lang#142416. Unfortunately I missed this during review.

- Commit 1: Reverts 40c2ca9 and re-applies rust-lang#134883.
- Commit 2: Check `./x check bootstrap` in `pr-check-1` to catch "obvious" problems like this.

r? Kobzol
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 26, 2025
…ts, r=Kobzol

Fix `./x check bootstrap` (again)

Redoes rust-lang#134883 and reverts 40c2ca9 from rust-lang#142416. Unfortunately I missed this during review.

- Commit 1: Reverts 40c2ca9 and re-applies rust-lang#134883.
- Commit 2: Check `./x check bootstrap` in `pr-check-1` to catch "obvious" problems like this.

r? Kobzol
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 27, 2025
…ts, r=Kobzol

Fix `./x check bootstrap` (again)

Redoes rust-lang#134883 and reverts 40c2ca9 from rust-lang#142416. Unfortunately I missed this during review.

- Commit 1: Reverts 40c2ca9 and re-applies rust-lang#134883.
- Commit 2: Check `./x check bootstrap` in `pr-check-1` to catch "obvious" problems like this.

r? Kobzol
rust-timer added a commit that referenced this pull request Jul 27, 2025
Rollup merge of #144445 - jieyouxu:revert-shared_helpers_tests, r=Kobzol

Fix `./x check bootstrap` (again)

Redoes #134883 and reverts 40c2ca9 from #142416. Unfortunately I missed this during review.

- Commit 1: Reverts 40c2ca9 and re-applies #134883.
- Commit 2: Check `./x check bootstrap` in `pr-check-1` to catch "obvious" problems like this.

r? Kobzol
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. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants