Rename the src install build step to rust-src. - #162423
davidv1992 wants to merge 1 commit into
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
06fd35f to
39b6489
Compare
This comment has been minimized.
This comment has been minimized.
39b6489 to
348b669
Compare
There was a problem hiding this comment.
This also should be documented in the bootstrap changelog (https://github.com/rust-lang/rust/blob/20d35a3ae8f310f2a002e5f6e0bc583830010cd4/src/bootstrap/src/utils/change_tracker.rs). I don't think we support a good way to soft-deprecate so probably just changing this is OK? I'll mark @rustbot label +relnotes, so we can call it out as a compatibility note though.
|
|
||
| fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { | ||
| run.path("src") | ||
| run.alias("rust-src") |
There was a problem hiding this comment.
I think this will break
rust/src/bootstrap/mk/Makefile.in
Line 107 in 20d35a3
There was a problem hiding this comment.
Fixed it, and fixed the remaining 4 occurrences I could find. I have taken the approach of everywhere src is used, it is intentional that rust-src is included, but given the size of the src directory, this may not always be the case, so if someone with more domain knowledge can take a look that would be great.
|
Reminder, once the PR becomes ready for a review, use |
|
Error: Unknown labels: a, as, call, can, compatibility, it, note, out, so, though, we Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip. |
348b669 to
0bf5f27
Compare
|
Warning If you are changing how CI LLVM is built or linked, make sure to bump cc @jieyouxu This PR modifies If appropriate, please update |
|
Since rustbot added you into the conversation: @jieyouxu, there is technically a change to a default value in the bootstrap configuration, as rust-src is now its own thing rather than part of src. However, the default behavior of build.tools does not change, rust-src is built when extended=true in both the old and new situation. Should I still add an entry to |
|
@rustbot ready |
|
I think that can't hurt, just in case. (Also that cc is actually for the CI changes not the bootstrap changes, but yeah anyway 😆) |
0bf5f27 to
0dc4b6b
Compare
|
Ok, Added the entry as a warning. |
| @@ -324,12 +324,12 @@ impl CommandLineStep for Src { | |||
| const IS_HOST: bool = true; | |||
|
|
|||
| fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { | |||
| run.path("src") | |||
| run.alias("rust-src") | |||
There was a problem hiding this comment.
cc @Kobzol as well, in case you know of an edge case for this alias.
EDIT: ah, but this is an install alias, which I am much less worried about (as opposed to build/dist aliases).
There was a problem hiding this comment.
Left some comments. I think that @Mark-Simulacrum's comment wasn't correct.
This comment has been minimized.
This comment has been minimized.
…uwer Rollup of 15 pull requests Successful merges: - #160401 (sparc: make ABI consistent with clang) - #161777 (Add Natvis visualiser and debuginfo tests for `f128`) - #162423 (Rename the src install build step to rust-src.) - #162740 (stdarch subtree update) - #162824 (link Enzyme and the offload with in-tree lld if possible) - #161005 (fix: unfulfilled nested dead code lint) - #161803 (Fix docs of make_ascii_lowercase/make_ascii_upercase) - #162256 (Add mentions to sync back `RELEASES.md` to the `main` branch) - #162661 (simplify `Target::GenericParam`) - #162666 (Tidy footnote in `platform-support.md`) - #162803 (docs(num): add documentation for `NonZero::from_str`) - #162906 (Move more `rustdoc-html` tests in the right location) - #162922 (An assortment of polonius tweaks) - #162929 (Update unicode_data to Unicode version 18.0.0) - #162930 (Use niche length type for strlen to guarantee `isize::MAX` bound)
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment has been minimized.
This comment has been minimized.
Rename the src install build step to rust-src. try-job: test_x86_64-fuchsia
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
@bors try jobs=test-x86_64-fuchsia |
This comment has been minimized.
This comment has been minimized.
Rename the src install build step to rust-src. try-job: test-x86_64-fuchsia
|
@bors rollup=never note="modifies ./x install logic" |
|
💔 Test for bc11f45 failed: CI. Failed job:
|
This comment has been minimized.
This comment has been minimized.
|
Could you also update this line from |
This avoids installing the src component from the command line also triggering a whole lot of other things which are behind the src path. Furthermore, it makes the name match the component as distributed in rustup, reducing confusion.
4e32edb to
abcb978
Compare
|
This PR was rebased onto a different main 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. |
View all comments
This avoids installing the src component from the command line also triggering installing a whole lot of other things which are behind the src path. Furthermore, it makes the name match the component as distributed in rustup, reducing confusion.