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

force compiling std from source if modified #127974

Merged
merged 2 commits into from
Aug 5, 2024

Conversation

onur-ozkan
Copy link
Member

This allows the standard library to be compiled even with download-rustc enabled. Which means it's no longer a requirement to compile rustc in order to compile std.

Ref. #127322 (comment).
Blocker for #122709.

@onur-ozkan
Copy link
Member Author

r? bootstrap

@rustbot rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jul 19, 2024
@rust-log-analyzer

This comment has been minimized.

@onur-ozkan onur-ozkan force-pushed the force-std-builds branch 3 times, most recently from 2e97e9f to fb5ce7f Compare July 20, 2024 07:43
@onur-ozkan onur-ozkan added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 23, 2024
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

It seems a bit unintuitive to me that x build library uses this logic, but x build doesn't. Couldn't this use the same logic as rustdoc, which checks if the library isn't modified?

@onur-ozkan
Copy link
Member Author

It seems a bit unintuitive to me that x build library uses this logic, but x build doesn't. Couldn't this use the same logic as rustdoc, which checks if the library isn't modified?

We already compile library if it's changed. This is for compiling library only, while using rustc from CI. Doing this by default with x build would kill the purpose of download-rustc.

@Kobzol
Copy link
Contributor

Kobzol commented Jul 23, 2024

I tested it locally, seems to work fine. We should however somehow document (?) that this only makes sense for rust.download-rustc = true, with if-unchanged it still rebuilds the whole sysroot starting from stage 0.

@rustbot
Copy link
Collaborator

rustbot commented Jul 23, 2024

This PR modifies config.example.toml.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

src/bootstrap/src/core/builder/tests.rs Outdated Show resolved Hide resolved
@@ -136,7 +142,8 @@ impl Step for Std {
compiler: run.builder.compiler(run.builder.top_stage, run.build_triple()),
target: run.target,
crates,
force_recompile: false,
// Force compiling from source as it was explicitly called with `x build library (or std/sysroot)`.
force_recompile: !run.builder.paths.is_empty(),
Copy link
Member

Choose a reason for hiding this comment

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

It seems like this will be recompilation even if e.g. x.py build compiler is passed? I'm not sure that gating on just any path is the right call. Shouldn't this use something like "is std modified?" perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems like this will be recompilation even if e.g. x.py build compiler is passed? I'm not sure that gating on just any path is the right call.

No, that shouldn't happen.

Shouldn't this use something like "is std modified?" perhaps?

We can do that but it's not necessary in my opinion. If user explicitly asks for library build, we can do it right away without checking if "is std modified?".

Copy link
Member

Choose a reason for hiding this comment

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

I think it's pretty confusing -- certainly against my expectations -- that the same source tree can produce different results depending on the command (x.py build vs. x.py build library, for example). I'd expect that to always yield the same results, though the path-less (x.py build) would perhaps build more.

If we choose whether to rebuild std based on the paths provided, that violates this basic assumption that I think we have so far upheld.

Copy link
Member Author

@onur-ozkan onur-ozkan Jul 30, 2024

Choose a reason for hiding this comment

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

Changed into if modified logic.

@onur-ozkan onur-ozkan force-pushed the force-std-builds branch 4 times, most recently from db181e0 to dc87814 Compare July 30, 2024 13:58
This allows the standard library to be compiled even with `download-rustc` enabled.
Which means it's no longer a requirement to compile `rustc` in order to compile `std`.

Signed-off-by: onur-ozkan <[email protected]>
@onur-ozkan onur-ozkan changed the title force compiling std from source if explicitly called force compiling std from source if modified Jul 30, 2024
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 4, 2024

📌 Commit 6fcc630 has been approved by Mark-Simulacrum

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 Aug 4, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 4, 2024
…k-Simulacrum

force compiling std from source if modified

This allows the standard library to be compiled even with `download-rustc` enabled. Which means it's no longer a requirement to compile `rustc` in order to compile `std`.

Ref. rust-lang#127322 (comment).
Blocker for rust-lang#122709.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 4, 2024
…k-Simulacrum

force compiling std from source if modified

This allows the standard library to be compiled even with `download-rustc` enabled. Which means it's no longer a requirement to compile `rustc` in order to compile `std`.

Ref. rust-lang#127322 (comment).
Blocker for rust-lang#122709.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 4, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#127655 (turn `invalid_type_param_default` into a `FutureReleaseErrorReportInDeps`)
 - rust-lang#127974 (force compiling std from source if modified)
 - rust-lang#128026 (std::thread: available_parallelism implementation for vxWorks proposal.)
 - rust-lang#128362 (add test for symbol visibility of `#[naked]` functions)
 - rust-lang#128500 (Add test for updating enum discriminant through pointer)
 - rust-lang#128630 (docs(resolve): more explain about `target`)
 - rust-lang#128638 (run-make: enable msvc for `link-dedup`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 4, 2024
…k-Simulacrum

force compiling std from source if modified

This allows the standard library to be compiled even with `download-rustc` enabled. Which means it's no longer a requirement to compile `rustc` in order to compile `std`.

Ref. rust-lang#127322 (comment).
Blocker for rust-lang#122709.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 4, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#127655 (turn `invalid_type_param_default` into a `FutureReleaseErrorReportInDeps`)
 - rust-lang#127974 (force compiling std from source if modified)
 - rust-lang#128026 (std::thread: available_parallelism implementation for vxWorks proposal.)
 - rust-lang#128309 (Implement cursors for `BTreeSet`)
 - rust-lang#128500 (Add test for updating enum discriminant through pointer)
 - rust-lang#128630 (docs(resolve): more explain about `target`)
 - rust-lang#128631 (handle crates when they are not specified for std docs)
 - rust-lang#128638 (run-make: enable msvc for `link-dedup`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 4, 2024
…k-Simulacrum

force compiling std from source if modified

This allows the standard library to be compiled even with `download-rustc` enabled. Which means it's no longer a requirement to compile `rustc` in order to compile `std`.

Ref. rust-lang#127322 (comment).
Blocker for rust-lang#122709.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 4, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#127655 (turn `invalid_type_param_default` into a `FutureReleaseErrorReportInDeps`)
 - rust-lang#127974 (force compiling std from source if modified)
 - rust-lang#128026 (std::thread: available_parallelism implementation for vxWorks proposal.)
 - rust-lang#128309 (Implement cursors for `BTreeSet`)
 - rust-lang#128500 (Add test for updating enum discriminant through pointer)
 - rust-lang#128630 (docs(resolve): more explain about `target`)
 - rust-lang#128638 (run-make: enable msvc for `link-dedup`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 5, 2024
…k-Simulacrum

force compiling std from source if modified

This allows the standard library to be compiled even with `download-rustc` enabled. Which means it's no longer a requirement to compile `rustc` in order to compile `std`.

Ref. rust-lang#127322 (comment).
Blocker for rust-lang#122709.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 5, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#127655 (turn `invalid_type_param_default` into a `FutureReleaseErrorReportInDeps`)
 - rust-lang#127907 (built-in derive: remove BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE hack and lint)
 - rust-lang#127974 (force compiling std from source if modified)
 - rust-lang#128309 (Implement cursors for `BTreeSet`)
 - rust-lang#128500 (Add test for updating enum discriminant through pointer)
 - rust-lang#128623 (Do not fire unhandled attribute assertion on multi-segment `AttributeType::Normal` attributes with builtin attribute as first segment)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f6c8b7a into rust-lang:master Aug 5, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 5, 2024
Rollup merge of rust-lang#127974 - onur-ozkan:force-std-builds, r=Mark-Simulacrum

force compiling std from source if modified

This allows the standard library to be compiled even with `download-rustc` enabled. Which means it's no longer a requirement to compile `rustc` in order to compile `std`.

Ref. rust-lang#127322 (comment).
Blocker for rust-lang#122709.
@onur-ozkan onur-ozkan deleted the force-std-builds branch August 5, 2024 08:18
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