-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Sync LLVM submodule if it has been initialized #78153
Merged
Merged
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
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
label
Oct 20, 2020
est31
force-pushed
the
downloaded_llvm_maybe_sync
branch
from
October 20, 2020 17:06
b7dfab4
to
bb70719
Compare
est31
force-pushed
the
downloaded_llvm_maybe_sync
branch
from
October 20, 2020 17:14
bb70719
to
65faa73
Compare
It seems like we should sync the submodule with llvm-config specified too, I don't see why the two options should be different here. |
Since having enabled the download-ci-llvm option, and having rebased on top of f05b47c, 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.
est31
force-pushed
the
downloaded_llvm_maybe_sync
branch
from
October 20, 2020 18:25
65faa73
to
5948e62
Compare
I think CI failure here is unrelated so going to go ahead and r+, but if not we'll want to fix that. @bors r+ |
📌 Commit 5948e62 has been approved by |
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
Oct 20, 2020
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.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 23, 2020
Rollup of 17 pull requests Successful merges: - rust-lang#77268 (Link to "Contributing to Rust" rather than "Getting Started".) - rust-lang#77339 (Implement TryFrom between NonZero types.) - rust-lang#77488 (Mark `repr128` as `incomplete_features`) - rust-lang#77890 (Fixing escaping to ensure generation of welformed json.) - rust-lang#77918 (Cleanup network tests) - rust-lang#77920 (Avoid extraneous space between visibility kw and ident for statics) - rust-lang#77969 (Doc formating consistency between slice sort and sort_unstable, and big O notation consistency) - rust-lang#78098 (Clean up and improve some docs) - rust-lang#78116 (Make inline const work in range patterns) - rust-lang#78153 (Sync LLVM submodule if it has been initialized) - rust-lang#78163 (Clean up lib docs) - rust-lang#78169 (Update cargo) - rust-lang#78231 (Make closures inherit the parent function's target features) - rust-lang#78235 (Explain where the closure return type was inferred) - rust-lang#78255 (Reduce diagram mess in 'match arms have incompatible types' error) - rust-lang#78263 (Add regression test of issue-77668) - rust-lang#78265 (Add some inference-related regression tests about incorrect diagnostics) Failed merges: r? `@ghost`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
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.
Since having enabled the download-ci-llvm option,
and having rebased on top of #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-projectsubmodule. 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.