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.

Sign up for GitHub

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

Jump to bottom

Rerun bootstrap's build script when RUSTC changes #93723

Merged
merged 1 commit into from
Feb 7, 2022
Merged

Rerun bootstrap's build script when RUSTC changes #93723

merged 1 commit into from
Feb 7, 2022

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Feb 7, 2022

Previously, rustbuild would give strange errors if you tried to reuse the same build directory under two names:

$ mkdir tmp && cd tmp
$ ../x.py check
Building rustbuild
    Finished dev [unoptimized] target(s) in 35.27s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
^C
$ cd ..
$ mv tmp/build build
$ ./x.py check
Building rustbuild
   Compiling bootstrap v0.0.0 (/home/jnelson/rust-lang/rust/src/bootstrap)
    Finished dev [unoptimized] target(s) in 11.18s

failed to execute command: "/home/jnelson/rust-lang/rust/tmp/build/x86_64-unknown-linux-gnu/stage0/bin/rustc" "--target" "x86_64-unknown-linux-gnu" "--print" "target-libdir"
error: No such file or directory (os error 2)

This fixes the error. Reusing the same build directory is useful if you want to test path-related things in
bootstrap itself, without having to recompile it each time.

For good measure, this also reruns the build script when PATH changes.

@jyn514
Rerun bootstrap's build script when RUSTC changes
b5b2150 
Previously, rustbuild would give strange errors if you tried to reuse the same build directory under two names:

```
$ mkdir tmp && cd tmp
$ ../x.py check
Building rustbuild
    Finished dev [unoptimized] target(s) in 35.27s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
^C
$ cd ..
$ mv tmp/build build
$ ./x.py check
Building rustbuild
   Compiling bootstrap v0.0.0 (/home/jnelson/rust-lang/rust/src/bootstrap)
    Finished dev [unoptimized] target(s) in 11.18s

failed to execute command: "/home/jnelson/rust-lang/rust/tmp/build/x86_64-unknown-linux-gnu/stage0/bin/rustc" "--target" "x86_64-unknown-linux-gnu" "--print" "target-libdir"
error: No such file or directory (os error 2)
```

This fixes the error. Reusing the same build directory is useful if you want to test path-things in
bootstrap itself, without having to recompile it each time.

For good measure, this also reruns the build script when PATH changes.
@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 Feb 7, 2022
Mark-Simulacrum
Mark-Simulacrum reviewed Feb 7, 2022
View reviewed changes
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

Did you confirm that this fixes things? I would've guessed that the problem here is rather that Cargo doesn't view the build directory movement as cause for re-running build.rs (probably in some sense this is intentional/good), and neither of the env-changed keys would actually work to prevent that... but maybe I am wrong?

src/bootstrap/build.rs
@@ -3,6 +3,8 @@ use std::path::PathBuf;

fn main() {
println!("cargo:rerun-if-changed=build.rs");
println!("cargo:rerun-if-env-changed=RUSTC");
println!("cargo:rerun-if-env-changed=PATH");
Copy link
Member

Choose a reason for hiding this comment

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

PATH seems correct, but I am surprised that RUSTC is needed: cargo docs claim that those variables set by Cargo are already taken care of by Cargo: "...it is not necessary to use this for environment variables like TARGET that Cargo sets."

Maybe RUSTC is missing there? It would be great if you could file an issue upstream, since presumably that's the cause?

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 would be great if you could file an issue upstream, since presumably that's the cause?

ok, but I suspect it will be a while before it's fixed

Copy link
Member

Choose a reason for hiding this comment

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

yeah, happy to r+ this before then, just wanted something tracking things. I don't think the fix is that important.

jyn514
jyn514 commented Feb 7, 2022
View reviewed changes
Copy link
Member Author

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Did you confirm that this fixes things?

Yes, I confirmed that this fixes things.

src/bootstrap/build.rs
@@ -3,6 +3,8 @@ use std::path::PathBuf;

fn main() {
println!("cargo:rerun-if-changed=build.rs");
println!("cargo:rerun-if-env-changed=RUSTC");
println!("cargo:rerun-if-env-changed=PATH");
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 would be great if you could file an issue upstream, since presumably that's the cause?

ok, but I suspect it will be a while before it's fixed

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 7, 2022

📌 Commit b5b2150 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 Feb 7, 2022
@jyn514 jyn514 mentioned this pull request Feb 7, 2022
Open
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 7, 2022
@matthiaskrgr
Rollup merge of rust-lang#93723 - jyn514:rerun-if-changed, r=Mark-Sim…
ae6a34c 
…ulacrum

Rerun bootstrap's build script when RUSTC changes

Previously, rustbuild would give strange errors if you tried to reuse the same build directory under two names:

```
$ mkdir tmp && cd tmp
$ ../x.py check
Building rustbuild
    Finished dev [unoptimized] target(s) in 35.27s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
^C
$ cd ..
$ mv tmp/build build
$ ./x.py check
Building rustbuild
   Compiling bootstrap v0.0.0 (/home/jnelson/rust-lang/rust/src/bootstrap)
    Finished dev [unoptimized] target(s) in 11.18s

failed to execute command: "/home/jnelson/rust-lang/rust/tmp/build/x86_64-unknown-linux-gnu/stage0/bin/rustc" "--target" "x86_64-unknown-linux-gnu" "--print" "target-libdir"
error: No such file or directory (os error 2)
```

This fixes the error. Reusing the same build directory is useful if you want to test path-related things in
bootstrap itself, without having to recompile it each time.

For good measure, this also reruns the build script when PATH changes.
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 7, 2022
Closed
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Feb 7, 2022
@m-ou-se
Rollup merge of rust-lang#93723 - jyn514:rerun-if-changed, r=Mark-Sim…
14ffdd2 
…ulacrum

Rerun bootstrap's build script when RUSTC changes

Previously, rustbuild would give strange errors if you tried to reuse the same build directory under two names:

```
$ mkdir tmp && cd tmp
$ ../x.py check
Building rustbuild
    Finished dev [unoptimized] target(s) in 35.27s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
^C
$ cd ..
$ mv tmp/build build
$ ./x.py check
Building rustbuild
   Compiling bootstrap v0.0.0 (/home/jnelson/rust-lang/rust/src/bootstrap)
    Finished dev [unoptimized] target(s) in 11.18s

failed to execute command: "/home/jnelson/rust-lang/rust/tmp/build/x86_64-unknown-linux-gnu/stage0/bin/rustc" "--target" "x86_64-unknown-linux-gnu" "--print" "target-libdir"
error: No such file or directory (os error 2)
```

This fixes the error. Reusing the same build directory is useful if you want to test path-related things in
bootstrap itself, without having to recompile it each time.

For good measure, this also reruns the build script when PATH changes.
@m-ou-se m-ou-se mentioned this pull request Feb 7, 2022
Closed
@m-ou-se m-ou-se mentioned this pull request Feb 7, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2022
@bors
Auto merge of rust-lang#93738 - m-ou-se:rollup-zjyd2et, r=m-ou-se
f52c318 
Rollup of 13 pull requests

Successful merges:

 - rust-lang#88313 (Make the pre-commit script pre-push instead)
 - rust-lang#91530 (Suggest 1-tuple parentheses on exprs without existing parens)
 - rust-lang#92724 (Cleanup c_str.rs)
 - rust-lang#93208 (Impl {Add,Sub,Mul,Div,Rem,BitXor,BitOr,BitAnd}Assign<$t> for Wrapping<$t> for rust 1.60.0)
 - rust-lang#93394 (Don't allow {} to refer to implicit captures in format_args.)
 - rust-lang#93416 (remove `allow_fail` test flag)
 - rust-lang#93487 (Fix linking stage1 toolchain in `./x.py setup`)
 - rust-lang#93673 (Linkify sidebar headings for sibling items)
 - rust-lang#93680 (Drop json::from_reader)
 - rust-lang#93682 (Update tracking issue for `const_fn_trait_bound`)
 - rust-lang#93722 (Use shallow clones for submodules managed by rustbuild, not just bootstrap.py)
 - rust-lang#93723 (Rerun bootstrap's build script when RUSTC changes)
 - rust-lang#93737 (bootstrap: prefer using '--config' over 'RUST_BOOTSTRAP_CONFIG')

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0f9d3b4 into rust-lang:master Feb 7, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 7, 2022
@jyn514 jyn514 deleted the rerun-if-changed branch February 25, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.60.0
Development

Successfully merging this pull request may close these issues.
5 participants
@jyn514 @rust-highfive @Mark-Simulacrum @bors @rustbot