Skip to content

Move LLVM submodule updates back to native.rs - #86015

Merged
bors merged 4 commits into
rust-lang:masterfrom
jyn514:revert-revert
Jun 26, 2021
Merged

Move LLVM submodule updates back to native.rs#86015
bors merged 4 commits into
rust-lang:masterfrom
jyn514:revert-revert

Conversation

@jyn514

@jyn514jyn514 commented Jun 5, 2021•
edited
Loading

Copy link
Copy Markdown
Member

Time to find more bugs!

The first commit is a straight revert of #85647, the second is a fix for https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/x.2Epy.20always.20updates.20LLVM.20submodule/near/240113320 and #82653 (comment). I haven't been able to replicate #82653 (comment).

@jyn514jyn514 added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 5, 2021
@rust-highfive

rust-highfive commented Jun 5, 2021

Copy link
Copy Markdown
Contributor

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 5, 2021
@jyn514jyn514 mentioned this pull request Jun 5, 2021
Merged
@rust-log-analyzer

This comment has been minimized.

Sign in to view
LingMan
LingMan reviewed Jun 5, 2021
View reviewed changes
Comment threadsrc/bootstrap/native.rs Outdated
jyn514 added 2 commits June 5, 2021 12:47
@jyn514
Simplify commit check
2fbe2ca
@jyn514
Inline maybe_update_submodule
73a40ac
It was a trivial function only used once.
@Mark-Simulacrum

Mark-Simulacrum commented Jun 20, 2021

Copy link
Copy Markdown
Member

Seems OK -- r=me, but I would prefer to land this when you have some cycles to either revert it or fix bugs that may pop up. Obviously, you know better than I when that is :)

@Mark-SimulacrumMark-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 20, 2021
@jyn514

jyn514 commented Jun 20, 2021

Copy link
Copy Markdown
MemberAuthor

Thanks :) I should have time this Friday afternoon

@jyn514

jyn514 commented Jun 25, 2021

Copy link
Copy Markdown
MemberAuthor

@bors r=Mark-Simulacrum

@bors

bors commented Jun 25, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit 73a40ac has been approved by Mark-Simulacrum

@borsbors 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 Jun 25, 2021
@bors

bors commented Jun 25, 2021

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 73a40ac with merge dd1525a...

@bors

bors commented Jun 26, 2021

Copy link
Copy Markdown
Collaborator

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

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Jun 26, 2021
@bors
bors merged commit dd1525a into rust-lang:masterJun 26, 2021
@rustbotrustbot added this to the 1.55.0 milestone Jun 26, 2021
@jyn514
jyn514 deleted the revert-revert branch June 26, 2021 02:29
@durin42

durin42 commented Jul 7, 2021

Copy link
Copy Markdown
Contributor

FWIW, this appears to have the same behavior mentioned in #85647 that it unconditionally updates LLVM even if [build] submodules = false is specified. I'm not sure if that's intentional, but it'd be nice to have a way to have LLVM not get updated when doing builds. Should I file a bug?

@jyn514

jyn514 commented Jul 7, 2021

Copy link
Copy Markdown
MemberAuthor

FWIW, this appears to have the same behavior mentioned in #85647 that it unconditionally updates LLVM even if [build] submodules = false is specified. I'm not sure if that's intentional, but it'd be nice to have a way to have LLVM not get updated when doing builds. Should I file a bug?

Yes, please do. Thanks!

@durin42durin42 mentioned this pull request Jul 7, 2021
Closed
@durin42

durin42 commented Jul 7, 2021

Copy link
Copy Markdown
Contributor

Filed #86954, thanks!

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 21, 2021
@bors
Auto merge ofrust-lang#82653- jyn514:submodules-on-demand, r=Mark-S…
ac575b6
…imulacrum
Update all submodules that rustbuild doesn't depend on lazily
This only updates the submodules the first time they're needed, instead
of unconditionally the first time you run x.py.
Ideally, this would move *all* submodules to rustbuild and not exclude some tools and
backtrace. Unfortunately, cargo requires all `Cargo.toml` files in the
whole workspace to be present to build any crate.
On my machine, this takes the time for an initial submodule clone (for
`x.py --help`) from 55.70 to 15.87 seconds.
Helps with rust-lang#76653. Builds on rust-lang#86015 and should not be merged before (only the last commit is relevant).
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@LingManLingManLingMan left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@Mark-SimulacrumMark-Simulacrum

Labels

A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Milestone

1.55.0

Development

Successfully merging this pull request may close these issues.

8 participants

@jyn514@rust-highfive@rust-log-analyzer@Mark-Simulacrum@bors@durin42@LingMan@rustbot