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 --keep-stage-std to x.py for keeping only standard library artifacts #77120

Merged
merged 3 commits into from
Sep 25, 2020

Conversation

ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Sep 23, 2020

Unlike --keep-stage 0, --keep-stage-std 0 will allow the stage 0 compiler artifacts (i.e., stage1/bin/rustc) to be rebuilt if it has changed. This allows contributors to iterate on later stages of the compiler in tandem with the standard library without needing to to rebuild the entire compiler. I often run into this when working on const-checking, since I may need to add a feature gate or make a small tweak to the standard library.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 23, 2020
@Mark-Simulacrum
Copy link
Member

I have been unable to come up with a use case for keep-staging the compiler, but I do think it makes sense to not do so. Maybe we should:

  • Change the message when the compiler gets keep staged to say "Please file a github issue describing your use case if you're seeing this"
  • Go with --keep-stage-std for now, as you suggest.

This keeps only the `std` artifacts compiled by the given stage, not the
compiler. This is useful when working on the latter stages of the
compiler in tandem with the standard library, since you don't have to
rebuild the *entire* compiler when the standard library changes.
@ecstatic-morse ecstatic-morse changed the title Ignore --keep-stage when building the compiler Add --keep-stage-std keeping only standard library artifacts Sep 23, 2020
@ecstatic-morse
Copy link
Contributor Author

Done. The new warning for --keep-stage is printed regardless of whether the compiler has changed, so I tried to phrase it accordingly. WDYT?

@ecstatic-morse ecstatic-morse changed the title Add --keep-stage-std keeping only standard library artifacts Add --keep-stage-std to x.py for keeping only standard library artifacts Sep 23, 2020
@Mark-Simulacrum
Copy link
Member

I think it's good. We just recently landed a changelog for bootstrap, and I think adding this there makes sense (though without incrementing the version). Could you add to it? It's in https://github.com/rust-lang/rust/blob/master/src/bootstrap/CHANGELOG.md.

Something quick like "Deprecated keep-stage in favor of keep-stage-std" and a link to this PR seems sufficient.

r=me with that entry added

@ehuss
Copy link
Contributor

ehuss commented Sep 23, 2020

I don't know if you want feedback, but there are some (rare) use cases where I find --keep-stage is useful. For example, documentation changes: x.py doc --keep-stage=0 --stage=0 src/tools/error_index_generator can avoid rebuilding the compiler. Similarly x.py doc --keep-stage=0 --stage=1 src/doc/rustc.

I would also think if anyone is working on a ignore-stage1 test, that it would still be useful?

@Mark-Simulacrum
Copy link
Member

Interesting cases! I am definitely interested in hearing about them :)

I think documentation changes are indeed a great example -- I think basically all of those fall under "I've modified the compiler in a way that does not affect the binary, but just separate outputs" which I didn't realize we had, but of course we definitely do. In that case @ecstatic-morse let's avoid the "deprecated" language and just document the addition of keep-stage-std. I think suggesting its use for keep-stage users still makes sense, though, if it's possible.

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 23, 2020

📌 Commit c0ddaed has been approved by Mark-Simulacrum

@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 Sep 23, 2020
@ecstatic-morse
Copy link
Contributor Author

Should be good to go now.

@Mark-Simulacrum
Copy link
Member

Oh I had forgotten myself :)

@bors r+

@bors
Copy link
Contributor

bors commented Sep 23, 2020

📌 Commit 16769eb has been approved by Mark-Simulacrum

@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Sep 23, 2020
RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 25, 2020
…rk-Simulacrum

Add `--keep-stage-std` to `x.py` for keeping only standard library artifacts

Unlike `--keep-stage 0`, `--keep-stage-std 0` will allow the stage 0 compiler artifacts (i.e., stage1/bin/rustc) to be rebuilt if it has changed. This allows contributors to iterate on later stages of the compiler in tandem with the standard library without needing to to rebuild the entire compiler. I often run into this when working on const-checking, since I may need to add a feature gate or make a small tweak to the standard library.
RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 25, 2020
…rk-Simulacrum

Add `--keep-stage-std` to `x.py` for keeping only standard library artifacts

Unlike `--keep-stage 0`, `--keep-stage-std 0` will allow the stage 0 compiler artifacts (i.e., stage1/bin/rustc) to be rebuilt if it has changed. This allows contributors to iterate on later stages of the compiler in tandem with the standard library without needing to to rebuild the entire compiler. I often run into this when working on const-checking, since I may need to add a feature gate or make a small tweak to the standard library.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 25, 2020
…as-schievink

Rollup of 15 pull requests

Successful merges:

 - rust-lang#76932 (Relax promises about condition variable.)
 - rust-lang#76973 (Unstably allow assume intrinsic in const contexts)
 - rust-lang#77005 (BtreeMap: refactoring around edges)
 - rust-lang#77066 (Fix dest prop miscompilation around references)
 - rust-lang#77073 (dead_code: look at trait impls even if they don't contain items)
 - rust-lang#77086 (Include libunwind in the rust-src component.)
 - rust-lang#77097 (Make [].as_[mut_]ptr_range() (unstably) const.)
 - rust-lang#77106 (clarify that `changelog-seen = 1` goes to the beginning of config.toml)
 - rust-lang#77120 (Add `--keep-stage-std` to `x.py` for keeping only standard library artifacts)
 - rust-lang#77126 (Invalidate local LLVM cache less often)
 - rust-lang#77146 (Install std for non-host targets)
 - rust-lang#77155 (remove enum name from ImplSource variants)
 - rust-lang#77176 (Removing erroneous semicolon in transmute documentation)
 - rust-lang#77183 (Allow multiple allow_internal_unstable attributes)
 - rust-lang#77189 (Remove extra space from vec drawing)

Failed merges:

r? `@ghost`
@bors bors merged commit 0a3cf02 into rust-lang:master Sep 25, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 25, 2020
@ecstatic-morse ecstatic-morse deleted the keep-stage-std branch October 6, 2020 01:42
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) 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.

7 participants