Don't download/sync llvm-project submodule if download-ci-llvm is set #76864
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvm-project takes > 1GB storage space and a long time to download. It's better to not download it unless needed.
rust-highfive
added
the
S-waiting-on-review
| if self.get_toml('llvm-config') and self.get_toml('lld') != 'true': | ||
| continue | ||
| if self.get_toml('llvm-config') or self.get_toml('download-ci-llvm') == 'true': | ||
| if self.get_toml('lld') != 'true': |
There was a problem hiding this comment.
Hm I don't know that we actually support lld with download-ci-llvm. I guess I didn't test :)
But seems okay for now until we can check.
|
@bors r+ |
|
📌 Commit f05b47c has been approved by |
bors
added
S-waiting-on-bors
jyn514
added
A-contributor-roadblock
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 19, 2020
…ces, r=Mark-Simulacrum Don't download/sync llvm-project submodule if download-ci-llvm is set llvm-project takes > 1GB storage space and a long time to download. It's better to not download it unless needed.
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 19, 2020
…ces, r=Mark-Simulacrum Don't download/sync llvm-project submodule if download-ci-llvm is set llvm-project takes > 1GB storage space and a long time to download. It's better to not download it unless needed.
This was referenced Sep 19, 2020
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 19, 2020
…ces, r=Mark-Simulacrum Don't download/sync llvm-project submodule if download-ci-llvm is set llvm-project takes > 1GB storage space and a long time to download. It's better to not download it unless needed.
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Sep 22, 2020
…ces, r=Mark-Simulacrum Don't download/sync llvm-project submodule if download-ci-llvm is set llvm-project takes > 1GB storage space and a long time to download. It's better to not download it unless needed.
|
☀️ Test successful - checks-actions, checks-azure |
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Oct 22, 2020
…=Mark-Simulacrum Sync LLVM submodule if it has been initialized Since having enabled the download-ci-llvm option, and having rebased on top of rust-lang#76864, I've noticed that I had to update the llvm-project submodule manually if it was checked out. Orignally, the submodule update logic was introduced to reduce the friction for contributors to manage the submodules, or in other words, to prevent getting PRs that have unwanted submodule rollbacks because the contributors didn't run git submodule update. This commit adds logic to ensure there is no inadvertent LLVM submodule rollback in a PR if download-ci-llvm (or llvm-config) is enabled. It will detect whether the llvm-project submodule is initialized, and if so, update it in any case. If it is not initialized, behaviour is kept to not do any update/initialization. An alternative to the chosen implementation would be to not pass the --init command line arg to `git submodule update` for the src/llvm-project submodule. This would show a confusing error message however on all builds with an uninitialized repo. We could pass the --silent param, but we still want it to print something if it is initialized and has to update something. So we just do a manual check for whether the submodule is initialized.
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this pull request
Oct 22, 2020
…=Mark-Simulacrum Sync LLVM submodule if it has been initialized Since having enabled the download-ci-llvm option, and having rebased on top of rust-lang#76864, I've noticed that I had to update the llvm-project submodule manually if it was checked out. Orignally, the submodule update logic was introduced to reduce the friction for contributors to manage the submodules, or in other words, to prevent getting PRs that have unwanted submodule rollbacks because the contributors didn't run git submodule update. This commit adds logic to ensure there is no inadvertent LLVM submodule rollback in a PR if download-ci-llvm (or llvm-config) is enabled. It will detect whether the llvm-project submodule is initialized, and if so, update it in any case. If it is not initialized, behaviour is kept to not do any update/initialization. An alternative to the chosen implementation would be to not pass the --init command line arg to `git submodule update` for the src/llvm-project submodule. This would show a confusing error message however on all builds with an uninitialized repo. We could pass the --silent param, but we still want it to print something if it is initialized and has to update something. So we just do a manual check for whether the submodule is initialized.
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Oct 23, 2020
…=Mark-Simulacrum Sync LLVM submodule if it has been initialized Since having enabled the download-ci-llvm option, and having rebased on top of rust-lang#76864, I've noticed that I had to update the llvm-project submodule manually if it was checked out. Orignally, the submodule update logic was introduced to reduce the friction for contributors to manage the submodules, or in other words, to prevent getting PRs that have unwanted submodule rollbacks because the contributors didn't run git submodule update. This commit adds logic to ensure there is no inadvertent LLVM submodule rollback in a PR if download-ci-llvm (or llvm-config) is enabled. It will detect whether the llvm-project submodule is initialized, and if so, update it in any case. If it is not initialized, behaviour is kept to not do any update/initialization. An alternative to the chosen implementation would be to not pass the --init command line arg to `git submodule update` for the src/llvm-project submodule. This would show a confusing error message however on all builds with an uninitialized repo. We could pass the --silent param, but we still want it to print something if it is initialized and has to update something. So we just do a manual check for whether the submodule is initialized.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
llvm-project takes > 1GB storage space and a long time to download.
It's better to not download it unless needed.