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

Add src/tools/x to the main workspace #138179

Merged
merged 4 commits into from
Mar 14, 2025
Merged

Add src/tools/x to the main workspace #138179

merged 4 commits into from
Mar 14, 2025

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Mar 7, 2025

The original reason to exclude it was so it could run before submodules
were initialized. However, those have all been converted to subtrees
now, so the entire workspace is always ready to go.

I've also alphabetized the workspace members, as it was an untidy mess. 🧹

@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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 A-tidy Area: The tidy tool 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 Mar 7, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2025

src/tools/x was changed. Bump version of Cargo.toml in src/tools/x so tidy will suggest installing the new version.

The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging.

cc @davidtwco, @wesleywiser

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@cuviper
Copy link
Member Author

cuviper commented Mar 7, 2025

BTW, I think we could also do this for src/bootstrap, except it has some dependency version-pinning that we probably don't want to force on the entire workspace.

src/tools/x was changed. Bump version of Cargo.toml in src/tools/x so tidy will suggest installing the new version.

There's no meaningful change in the tool itself.

@jieyouxu
Copy link
Member

jieyouxu commented Mar 8, 2025

Isn't cargo still a submodule, or does that not matter?

@cuviper
Copy link
Member Author

cuviper commented Mar 8, 2025

It is, but it doesn't matter because it's in an independent workspace.

@rust-log-analyzer

This comment has been minimized.

@cuviper
Copy link
Member Author

cuviper commented Mar 10, 2025

Why on earth are the settings hashed into a test?!?

@jyn514
Copy link
Member

jyn514 commented Mar 10, 2025

because knowing when the settings change allows us to suggest re-running x setup editor

@bors
Copy link
Contributor

bors commented Mar 13, 2025

☔ The latest upstream changes (presumably #138448) made this pull request unmergeable. Please resolve the merge conflicts.

cuviper added 4 commits March 13, 2025 12:18
The original reason to exclude it was so it could run before submodules
were initialized. However, those have all been converted to subtrees
now, so the entire workspace is always ready to go.
@cuviper
Copy link
Member Author

cuviper commented Mar 13, 2025

The conflict was just the removal of src/tools/rls, which my alphabetization tried to reorder.

@jieyouxu
Copy link
Member

I'll test this on a fresh clone w/ no submodules initialized.
r? jieyouxu

@rustbot rustbot assigned jieyouxu and unassigned Mark-Simulacrum Mar 13, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

The changes themselves look good. Should we add a WARN-level change tracker entry, because people would need to adjust their r-a settings, right? EDIT: nvm, the hash differences will let people know, duh.

@jieyouxu
Copy link
Member

jieyouxu commented Mar 14, 2025

For the changes themselves, I did some local testing:

Local testing

I performed a clone of this repo with explicitly cleared git configs (that should not recursively checkout submodules by default).

$ GIT_CONFIG_SYSTEM='' GIT_CONFIG_GLOBAL='' git clone https://github.com/rust-lang/rust.git rust-138179
$ GIT_CONFIG_SYSTEM='' GIT_CONFIG_GLOBAL='' git fetch origin pull/138179/head:pr-138179
$ GIT_CONFIG_SYSTEM='' GIT_CONFIG_GLOBAL='' git checkout pr-138179

With no bootstrap config.toml, I ran:

$ GIT_CONFIG_SYSTEM='' GIT_CONFIG_GLOBAL='' ./x build library --stage=1

I can confirm ./x binary can be built, and that I can further successfully bootstrap a stage 1 compiler and std.

I also redid the fresh clone then tried to run tidy:

$ GIT_CONFIG_SYSTEM='' GIT_CONFIG_GLOBAL='' ./x test tidy

I confirm that I can run tidy checks locally without any submodules being checkout out prior to this invocation, and that tidy completes successfully.

After redoing the clone and an explicit

$ GIT_CONFIG_SYSTEM='' GIT_CONFIG_GLOBAL='' git submodule deinit -f --all

I tried a basic dist flow

$ GIT_CONFIG_SYSTEM='' GIT_CONFIG_GLOBAL='' ./x dist rustc --stage=2

And while I didn't bother waiting it to finish, it got past the stage that would've failed if src/tools/x setup was completely broken.

@jieyouxu
Copy link
Member

Thanks for the cleanup!
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 14, 2025

📌 Commit 496788a has been approved by jieyouxu

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 Mar 14, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2025
Rollup of 16 pull requests

Successful merges:

 - rust-lang#136001 (Overhaul examples for PermissionsExt)
 - rust-lang#136230 (Reword incorrect documentation about SocketAddr having varying layout)
 - rust-lang#136892 (Sync Fuchsia target spec with clang Fuchsia driver)
 - rust-lang#136911 (Add documentation URL to selected jobs)
 - rust-lang#137870 ( Improve HashMap docs for const and static initializers)
 - rust-lang#138179 (Add `src/tools/x` to the main workspace)
 - rust-lang#138389 (use `expect` instead of `allow`)
 - rust-lang#138396 (Enable metrics and verbose tests in PR CI)
 - rust-lang#138398 (atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance)
 - rust-lang#138432 (fix: remove the check of lld not supporting `@response-file)`
 - rust-lang#138434 (Visit `PatField` when collecting lint levels)
 - rust-lang#138441 (update error message)
 - rust-lang#138442 (EUV: fix place of deref pattern's interior's scrutinee)
 - rust-lang#138457 (Remove usage of legacy scheme paths on RedoxOS)
 - rust-lang#138461 (Remove an outdated line from a test comment)
 - rust-lang#138466 (Remove myself from libs review)

Failed merges:

 - rust-lang#138452 (Remove `RUN_CHECK_WITH_PARALLEL_QUERIES`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b9c33fa into rust-lang:master Mar 14, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 14, 2025
Rollup merge of rust-lang#138179 - cuviper:x, r=jieyouxu

Add `src/tools/x` to the main workspace

The original reason to exclude it was so it could run before submodules
were initialized. However, those have all been converted to subtrees
now, so the entire workspace is always ready to go.

I've also alphabetized the workspace members, as it was an untidy mess. 🧹
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Mar 19, 2025
Rollup of 16 pull requests

Successful merges:

 - rust-lang#136001 (Overhaul examples for PermissionsExt)
 - rust-lang#136230 (Reword incorrect documentation about SocketAddr having varying layout)
 - rust-lang#136892 (Sync Fuchsia target spec with clang Fuchsia driver)
 - rust-lang#136911 (Add documentation URL to selected jobs)
 - rust-lang#137870 ( Improve HashMap docs for const and static initializers)
 - rust-lang#138179 (Add `src/tools/x` to the main workspace)
 - rust-lang#138389 (use `expect` instead of `allow`)
 - rust-lang#138396 (Enable metrics and verbose tests in PR CI)
 - rust-lang#138398 (atomic intrinsics: clarify which types are supported and (if applicable) what happens with provenance)
 - rust-lang#138432 (fix: remove the check of lld not supporting `@response-file)`
 - rust-lang#138434 (Visit `PatField` when collecting lint levels)
 - rust-lang#138441 (update error message)
 - rust-lang#138442 (EUV: fix place of deref pattern's interior's scrutinee)
 - rust-lang#138457 (Remove usage of legacy scheme paths on RedoxOS)
 - rust-lang#138461 (Remove an outdated line from a test comment)
 - rust-lang#138466 (Remove myself from libs review)

Failed merges:

 - rust-lang#138452 (Remove `RUN_CHECK_WITH_PARALLEL_QUERIES`)

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-tidy Area: The tidy tool 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.

7 participants