bootstrap: don't resolve symlinks for initial_cargo#121341
bootstrap: don't resolve symlinks for initial_cargo#121341bors merged 1 commit intorust-lang:masterfrom
Conversation
|
r? @clubby789 rustbot has assigned @clubby789. Use r? to explicitly pick a reviewer |
|
This PR modifies If appropriate, please update |
There was a problem hiding this comment.
Stopping bootstrapping on the invalid configuration is actually quite useful (so people doesn't waste time on building when configuration is invalid). I would rather change this validation to be running a simple cargo command and checking if it's working or not. With this approach, your usecase should be working just fine and we can keep validating the initial cargo.
|
r? onur-ozkan |
|
@rustbot review |
|
☔ The latest upstream changes (presumably #121400) made this pull request unmergeable. Please resolve the merge conflicts. |
bdaf186 to
94d6b72
Compare
| } | ||
| PathBuf::from(cargo) | ||
| } else { | ||
| config.download_beta_toolchain(); |
There was a problem hiding this comment.
Oh, this fixes a bug. Thanks :)
|
I will take this to the merge queue once it's been rebased and the commits have been squashed/unified. Having the first commit no longer makes sense. |
+ Do not resolve symlinks (as this may break rustup) + Check version not just for rustc, but also for cargo. + Check that the program's self-reported name matches the expected name (such as "rustc" or "cargo").
94d6b72 to
c42057f
Compare
|
@bors r+ |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (71ff1c6): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 651.286s -> 650.172s (-0.17%) |
|
Thank you a lot for the review and the feedback! Really appreciate it ❤️ |
I have put the following in my
config.toml:I have rustup installed from Arch's repos, which has all of the above paths be symlinks to
/usr/bin/rustup. This works just fine with theargv[0]trick that rustup uses.However,
bootstrapresolves symlinks to check whethercargoexists and then uses the resolved path, so it ends up callingrustupdirectly expecting it to behave likecargo. Which it doesn't.This PR removes the canonicalization step, in turn fixing the issue, but sacrificing a pretty error message. However, this exact thing is checked by
x.pyin advance, so I hope it is not a big deal?