Allow using `download-ci-llvm = true` outside the git checkout #98526

jyn514 · 2022-06-26T09:42:01Z

@bjorn3 noticed that this is already allowed today when download-llvm is disabled, but breaks with it enabled:

$ ./rust2/x.py build
fatal: not a git repository (or any of the parent directories): .git
thread 'main' panicked at 'command did not execute successfully: "git" "rev-list" "--author=bors@rust-lang.org" "-n1" "--first-parent" "HEAD" "--" "/home/jnelson/rust-lang/rust2/src/llvm-project" "/home/jnelson/rust-lang/rust2/src/bootstrap/download-ci-llvm-stamp" "/home/jnelson/rust-lang/rust2/src/version"
expected success, got: exit status: 128', src/bootstrap/native.rs:134:20
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Support it too for consistency. It's unclear to me when anyone would need to use this, but @bjorn3
feels we should support it, and it's not much additional effort to get it working.

rust-highfive · 2022-06-26T09:42:02Z

⚠️ Warning ⚠️

These commits modify submodules.

Mark-Simulacrum · 2022-06-26T16:43:31Z

Can you look through the other git invocations in bootstrap to check those for similar problems? I think download-rustc's invocation here has the same problem, at least, and there's maybe a few more -- it'd be great to solve all of them rather than just one case.

I'm not sure the submodule checkouts work properly either.

jyn514 · 2022-06-26T19:25:23Z

I think download-rustc's invocation here has the same problem, at least, and there's maybe a few more -- it'd be great to solve all of them rather than just one case.

Good catch, thanks. Fixed that and a bunch of ohter issues.

I haven't run a full build with these changes, but x.py check works from outside the repo - is that enough or would you like me to leave this running a full build for a few hours? I don't think we'll get a proper test of all this unless we change CI to build from outside the current directory, since there's lots of stuff like toolstate that's hard to run locally ...

I'm not sure the submodule checkouts work properly either.

they do :)

$ ./rust2/x.py b
Updating only changed submodules
Updating submodule src/tools/rust-installer
Submodule 'src/rust-installer' (https://github.com/rust-lang/rust-installer.git) registered for path 'src/tools/rust-installer'
Submodule path 'src/tools/rust-installer': checked out '5254dbfd25d5284728ab624dca1969d61427a0db'

Mark-Simulacrum · 2022-07-02T02:26:49Z

I'm not too worried about us validating it, but it seems like moving one of the CI builders to a non-in-tree directory might be a good way to test this without all that much hassle, in theory. It's probably a good bit of work to convince CI to behave if we do that though, so maybe not the best use of time for such a niche feature; I'm happy with x.py check as testing for now.

Mark-Simulacrum · 2022-07-02T02:27:46Z

r=me if you don't want to spend the cycles adding that builder here.

jyn514 · 2022-07-03T00:00:20Z

Yeah, I think I'd prefer to avoid spending time on this until / unless someone complains.

@bors r=Mark-Simulacrum rollup

bors · 2022-07-03T00:00:22Z

📌 Commit 0e58792 has been approved by Mark-Simulacrum

…ut, r=Mark-Simulacrum Allow using `download-ci-llvm = true` outside the git checkout `@bjorn3` noticed that this is already allowed today when download-llvm is disabled, but breaks with it enabled: ``` $ ./rust2/x.py build fatal: not a git repository (or any of the parent directories): .git thread 'main' panicked at 'command did not execute successfully: "git" "rev-list" "--author=bors@rust-lang.org" "-n1" "--first-parent" "HEAD" "--" "/home/jnelson/rust-lang/rust2/src/llvm-project" "/home/jnelson/rust-lang/rust2/src/bootstrap/download-ci-llvm-stamp" "/home/jnelson/rust-lang/rust2/src/version" expected success, got: exit status: 128', src/bootstrap/native.rs:134:20 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` Support it too for consistency. It's unclear to me when anyone would need to use this, but `@bjorn3` feels we should support it, and it's not much additional effort to get it working.

…ut, r=Mark-Simulacrum Allow using `download-ci-llvm = true` outside the git checkout ``@bjorn3`` noticed that this is already allowed today when download-llvm is disabled, but breaks with it enabled: ``` $ ./rust2/x.py build fatal: not a git repository (or any of the parent directories): .git thread 'main' panicked at 'command did not execute successfully: "git" "rev-list" "--author=bors@rust-lang.org" "-n1" "--first-parent" "HEAD" "--" "/home/jnelson/rust-lang/rust2/src/llvm-project" "/home/jnelson/rust-lang/rust2/src/bootstrap/download-ci-llvm-stamp" "/home/jnelson/rust-lang/rust2/src/version" expected success, got: exit status: 128', src/bootstrap/native.rs:134:20 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` Support it too for consistency. It's unclear to me when anyone would need to use this, but ``@bjorn3`` feels we should support it, and it's not much additional effort to get it working.

…ut, r=Mark-Simulacrum Allow using `download-ci-llvm = true` outside the git checkout ```@bjorn3``` noticed that this is already allowed today when download-llvm is disabled, but breaks with it enabled: ``` $ ./rust2/x.py build fatal: not a git repository (or any of the parent directories): .git thread 'main' panicked at 'command did not execute successfully: "git" "rev-list" "--author=bors@rust-lang.org" "-n1" "--first-parent" "HEAD" "--" "/home/jnelson/rust-lang/rust2/src/llvm-project" "/home/jnelson/rust-lang/rust2/src/bootstrap/download-ci-llvm-stamp" "/home/jnelson/rust-lang/rust2/src/version" expected success, got: exit status: 128', src/bootstrap/native.rs:134:20 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` Support it too for consistency. It's unclear to me when anyone would need to use this, but ```@bjorn3``` feels we should support it, and it's not much additional effort to get it working.

RalfJung · 2022-07-03T20:40:06Z

I think this broke two rollups
#98853
#98859
@bors r- rollup=never

@bjorn3

@bjorn3 noticed that this is already allowed today when download-llvm is disabled, but breaks with it enabled: ``` $ ./rust2/x.py build fatal: not a git repository (or any of the parent directories): .git thread 'main' panicked at 'command did not execute successfully: "git" "rev-list" "--author=bors@rust-lang.org" "-n1" "--first-parent" "HEAD" "--" "/home/jnelson/rust-lang/rust2/src/llvm-project" "/home/jnelson/rust-lang/rust2/src/bootstrap/download-ci-llvm-stamp" "/home/jnelson/rust-lang/rust2/src/version" expected success, got: exit status: 128', src/bootstrap/native.rs:134:20 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ``` Support it too for consistency. It's unclear to me when anyone would need to use this, but @bjorn3 feels we should support it, and it's not much additional effort to get it working. This also updates a bunch of other git commands that were similarly depending on the current directory.

jyn514 · 2022-07-10T21:35:22Z

I've discarded all the toolstate changes. It's extremely rare to run toolstate outside of CI, and also extremely rare to run x.py from outside the checkout; I don't think we need to support both at once.

@bors r=Mark-Simulacrum rollup=iffy

bors · 2022-07-10T21:35:23Z

📌 Commit 56e42b8 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

bors · 2022-07-11T01:15:51Z

⌛ Testing commit 56e42b8 with merge adaddb5...

bors · 2022-07-11T03:56:31Z

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing adaddb5 to master...

rust-timer · 2022-07-11T06:19:45Z

Finished benchmarking commit (adaddb5): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

Primary benchmarks: mixed results
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	2.9%	3.2%	2
Regressions 😿 (secondary)	2.8%	3.5%	2
Improvements 🎉 (primary)	-4.6%	-4.6%	1
Improvements 🎉 (secondary)	-1.3%	-1.3%	1
All 😿🎉 (primary)	0.4%	-4.6%	3

Cycles

Results

Primary benchmarks: 🎉 relevant improvement found
Secondary benchmarks: 🎉 relevant improvement found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-3.3%	-3.3%	1
Improvements 🎉 (secondary)	-4.7%	-4.7%	1
All 😿🎉 (primary)	-3.3%	-3.3%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jun 26, 2022

jyn514 assigned Mark-Simulacrum Jun 26, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 26, 2022

jyn514 force-pushed the download-llvm-outside-checkout branch from 3ce8ed5 to 071dd98 Compare June 26, 2022 09:42

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 26, 2022

jyn514 force-pushed the download-llvm-outside-checkout branch from 071dd98 to 0e58792 Compare June 26, 2022 19:24

jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 1, 2022

Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 2, 2022

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 3, 2022

RalfJung mentioned this pull request Jul 3, 2022

Rollup of 3 pull requests #98853

Closed

matthiaskrgr mentioned this pull request Jul 3, 2022

Rollup of 5 pull requests #98858

Closed

RalfJung mentioned this pull request Jul 3, 2022

Rollup of 4 pull requests #98859

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 3, 2022

jyn514 force-pushed the download-llvm-outside-checkout branch from 0e58792 to 56e42b8 Compare July 10, 2022 21:34

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 11, 2022

bors merged commit adaddb5 into rust-lang:master Jul 11, 2022

rustbot added this to the 1.64.0 milestone Jul 11, 2022

jyn514 deleted the download-llvm-outside-checkout branch July 11, 2022 04:23

jyn514 mentioned this pull request Aug 19, 2022

build: vendor = true seems to not work anymore #100364

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow using `download-ci-llvm = true` outside the git checkout #98526

Allow using `download-ci-llvm = true` outside the git checkout #98526

jyn514 commented Jun 26, 2022

rust-highfive commented Jun 26, 2022

Mark-Simulacrum commented Jun 26, 2022

jyn514 commented Jun 26, 2022 •

edited

Loading

Mark-Simulacrum commented Jul 2, 2022

Mark-Simulacrum commented Jul 2, 2022

jyn514 commented Jul 3, 2022

bors commented Jul 3, 2022

RalfJung commented Jul 3, 2022

jyn514 commented Jul 10, 2022

bors commented Jul 10, 2022

bors commented Jul 11, 2022

bors commented Jul 11, 2022

rust-timer commented Jul 11, 2022

Allow using download-ci-llvm = true outside the git checkout #98526

Allow using download-ci-llvm = true outside the git checkout #98526

Conversation

jyn514 commented Jun 26, 2022

rust-highfive commented Jun 26, 2022

Mark-Simulacrum commented Jun 26, 2022

jyn514 commented Jun 26, 2022 • edited Loading

Mark-Simulacrum commented Jul 2, 2022

Mark-Simulacrum commented Jul 2, 2022

jyn514 commented Jul 3, 2022

bors commented Jul 3, 2022

RalfJung commented Jul 3, 2022

jyn514 commented Jul 10, 2022

bors commented Jul 10, 2022

bors commented Jul 11, 2022

bors commented Jul 11, 2022

rust-timer commented Jul 11, 2022

Instruction count

Max RSS (memory usage)

Cycles

Footnotes

Allow using `download-ci-llvm = true` outside the git checkout #98526

Allow using `download-ci-llvm = true` outside the git checkout #98526

jyn514 commented Jun 26, 2022 •

edited

Loading