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

Move llvm submodule updates to rustbuild #81601

Merged
merged 2 commits into from
May 23, 2021
Merged

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jan 31, 2021

This enables better caching, since LLVM is only updated when needed, not
whenever x.py is run. Before, bootstrap.py had to use heuristics to
guess if LLVM would be needed, and updated the module more often than
necessary as a result.

This syncs the LLVM submodule only just before building the compiler, so
people working on the standard library never have to worry about it.
Example output:

Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Updating submodule src/llvm-project
Submodule 'src/llvm-project' (https://github.com/rust-lang/llvm-project.git) registered for path 'src/llvm-project'
Submodule path 'src/llvm-project': checked out 'f9a8d70b6e0365ac2172ca6b7f1de0341297458d'

Implements #76653 (comment). This could be easily extended to other submodules, like rust-by-example and rustc-dev-guide, which aren't needed for cargo's workspace resolution.

@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Jan 31, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 31, 2021
@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member Author

jyn514 commented Jan 31, 2021

I can't reproduce the CI failure locally :/

Suite("src/test/run-make") not skipped for "bootstrap::test::RunMake" -- not in ["src/tools/tidy"]
Skipping Set({"src/tools/tidy"}) because it is excluded
Suite("src/test/ui") not skipped for "bootstrap::test::Ui" -- not in ["src/tools/tidy"]
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

@sanxiyn
Copy link
Member

sanxiyn commented Feb 2, 2021

CI job x86_64-gnu-llvm-9 runs with llvm-config key set in config.toml. The old code explicitly handles this key, while I don't see anything handling it in the new code.

@sanxiyn sanxiyn 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 Feb 2, 2021
@rust-log-analyzer

This comment has been minimized.

src/bootstrap/native.rs Outdated Show resolved Hide resolved
@jyn514 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 Feb 2, 2021
@sanxiyn
Copy link
Member

sanxiyn commented Feb 3, 2021

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Feb 3, 2021

📌 Commit f688d0eeb1eb566fef74dcbe76ff384a05852384 has been approved by sanxiyn

@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 3, 2021
@bors
Copy link
Contributor

bors commented Feb 3, 2021

⌛ Testing commit f688d0eeb1eb566fef74dcbe76ff384a05852384 with merge 96ba9faf2f8839c911cfe4cd777ebc03dd0e9af1...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 3, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 3, 2021
@jyn514
Copy link
Member Author

jyn514 commented Feb 3, 2021

@bors rollup=iffy

@jyn514
Copy link
Member Author

jyn514 commented Feb 3, 2021

Does anyone know how to test cross-compiling? I tried > python3 ./x.py dist --host mips-unknown-linux-gnu --target mips-unknown-linux-gnu and it error-ed out with couldn't find required command: "mips-linux-gnu-gcc". I installed g++-mips-linux-gnu and now LLVM is being built appropriately. I'll try setting llvm-config and see if it helps.

@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member Author

jyn514 commented Feb 3, 2021

I still can't replicate the CI failure :/ I guess I can change try builds to cross-compile to mips and see if I can get a backtrace from that? I have the following config.toml:

[target.mips-unknown-linux-gnu]
llvm-config = "/usr/lib/llvm-11/bin/llvm-config"

and I'm running python3 ./x.py dist --host mips-unknown-linux-gnu --target mips-unknown-linux-gnu with an empty src/llvm-project (I ran git submodule deinit src/llvm-project first).

@Mark-Simulacrum
Copy link
Member

IIRC, CI has custom code to download submodules that's different from what is built in to x.py, but I may be misremembering.

@jyn514
Copy link
Member Author

jyn514 commented Feb 3, 2021

Ok, that is indeed the issue: src/ci/init-repo.sh runs touch src/llvm-project/.git which leaves the submodule in an invalid state. Do you know why it does that? It seems really strange, I don't know why it would be necessary.

@rust-log-analyzer

This comment has been minimized.

If something goes wrong here, showing `fn output` is unhelpful. Show
where the command is being run instead.
@jyn514 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 May 20, 2021
@Mark-Simulacrum
Copy link
Member

r=me with #81601 (comment) resolved

@Mark-Simulacrum 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 May 22, 2021
@jyn514
Copy link
Member Author

jyn514 commented May 22, 2021

@bors r=Mark-Simulacrum rollup=never

@bors
Copy link
Contributor

bors commented May 22, 2021

📌 Commit 7b9ddf421ce11efbaaffa033cd6507aef2c7bb36 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 22, 2021
@rust-log-analyzer

This comment has been minimized.

This enables better caching, since LLVM is only updated when needed, not
whenever x.py is run. Before, bootstrap.py had to use heuristics to
guess if LLVM would be needed, and updated the module more often than
necessary as a result.

This syncs the LLVM submodule only just before building the compiler, so
people working on the standard library never have to worry about it.
Example output:

```
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Updating submodule src/llvm-project
Submodule 'src/llvm-project' (https://github.com/rust-lang/llvm-project.git) registered for path 'src/llvm-project'
Submodule path 'src/llvm-project': checked out 'f9a8d70b6e0365ac2172ca6b7f1de0341297458d'
```

- Don't try to update the LLVM submodule when using system LLVM

  Previously, this would try to update LLVM unconditionally. Now the
  submodule is only initialized if `llvm-config` is not set.

- Don't update LLVM submodule in dry runs

  This prevents the following test failures:

  ```
  running 17 tests
  fatal: invalid gitfile format: /checkout/src/llvm-project/.git
  test builder::tests::defaults::build_cross_compile ... FAILED

  ---- builder::tests::defaults::build_default stdout ----
  thread 'main' panicked at 'command did not execute successfully: "git" "rev-parse" "HEAD"
  expected success, got: exit code: 128', src/build_helper/lib.rs:139:9
  ```

- Try running git without --progress if it fails the first time

  This avoids having to do version detection to see if --progress is
  supported or not.

- Don't try to update submodules when the source repository isn't managed by git

- Update LLVM submodules that have already been checked out

- Only check for whether the submodule should be updated in lib.rs; update
it unconditionally in native.rs
@jyn514
Copy link
Member Author

jyn514 commented May 22, 2021

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented May 22, 2021

📌 Commit 0be4046 has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented May 23, 2021

⌛ Testing commit 0be4046 with merge 9c3a2a5...

@bors
Copy link
Contributor

bors commented May 23, 2021

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 23, 2021
@bors bors merged commit 9c3a2a5 into rust-lang:master May 23, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 23, 2021
@jyn514 jyn514 deleted the llvm-on-demand branch May 23, 2021 03:24
JohnTitor added a commit to JohnTitor/rust that referenced this pull request May 25, 2021
…nd, r=jyn514

Revert "Move llvm submodule updates to rustbuild"

Reverts rust-lang#81601

This updates LLVM a bit too eagerly -- and particularly on Windows, this can be slow. See discussion on [Zulip].

[Zulip]: https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/x.2Epy.20always.20updates.20LLVM.20submodule
JohnTitor added a commit to JohnTitor/rust that referenced this pull request May 26, 2021
…nd, r=jyn514

Revert "Move llvm submodule updates to rustbuild"

Reverts rust-lang#81601

This updates LLVM a bit too eagerly -- and particularly on Windows, this can be slow. See discussion on [Zulip].

[Zulip]: https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/x.2Epy.20always.20updates.20LLVM.20submodule
JohnTitor added a commit to JohnTitor/rust that referenced this pull request May 26, 2021
…nd, r=jyn514

Revert "Move llvm submodule updates to rustbuild"

Reverts rust-lang#81601

This updates LLVM a bit too eagerly -- and particularly on Windows, this can be slow. See discussion on [Zulip].

[Zulip]: https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/x.2Epy.20always.20updates.20LLVM.20submodule
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself merged-by-bors This PR was explicitly merged by bors. 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.

10 participants