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

bootstrap: Remove commit hash from LLVM version suffix to avoid rebuilds #69397

Merged
merged 1 commit into from
Mar 1, 2020

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Feb 23, 2020

The custom LLVM version suffix was introduced to avoid unintentional
library names conflicts. By default it included the LLVM submodule
commit hash. Changing the version suffix requires the complete LLVM
rebuild, and since then every change to the submodules required it as
well.

Remove the commit hash from version suffix to avoid complete rebuilds,
while leaving the rust string, the release number and release channel
to disambiguate the library name.

Context: version suffix was introduced by #59173 as solution to #59034.

Resolves #68715.

The custom LLVM version suffix was introduced to avoid unintentional
library names conflicts. By default it included the LLVM submodule
commit hash. Changing the version suffix requires the complete LLVM
rebuild, and since then every change to the submodules required it as
well.

Remove the commit hash from version suffix to avoid complete rebuilds,
while leaving the `rust` string, the release number and release channel
to disambiguate the library name.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 23, 2020
@nagisa
Copy link
Member

nagisa commented Feb 23, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Feb 23, 2020

📌 Commit 598b187 has been approved by nagisa

@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 23, 2020
@nikomatsakis
Copy link
Contributor

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned nikomatsakis Feb 25, 2020
bors added a commit that referenced this pull request Mar 1, 2020
Rollup of 7 pull requests

Successful merges:

 - #69397 (bootstrap: Remove commit hash from LLVM version suffix to avoid rebuilds)
 - #69549 (Improve MinGW detection when cross compiling )
 - #69562 (Don't `bug` when taking discriminant of generator during dataflow)
 - #69579 (parser: Remove `Parser::prev_span`)
 - #69580 (use .copied() instead of .map(|x| *x) on iterators)
 - #69583 (Do not ICE on invalid type node after parse recovery)
 - #69605 (Use `opt_def_id()` over `def_id()`)

Failed merges:

r? @ghost
@bors bors merged commit 1e25878 into rust-lang:master Mar 1, 2020
@tmiasko tmiasko deleted the llvm-version-suffix branch March 1, 2020 20:12
@RalfJung
Copy link
Member

RalfJung commented Apr 5, 2020

Remove the commit hash from version suffix to avoid complete rebuilds,
while leaving the rust string, the release number and release channel
to disambiguate the library name.

I am still seeing full rebuilds. Looks like this did not help?

@petrochenkov
Copy link
Contributor

It did help to me, e.g. the recent LLVM update (#70582), caused a very limited rebuild.

@tmiasko
Copy link
Contributor Author

tmiasko commented Apr 5, 2020

With changes here, there should be a rebuild whenever release number changes. This still could be problematic when working with multiple branches. We could get rid of version completely on dev channel (@RalfJung you could try configuring it manually in config.toml and see if this is the issue).

@RalfJung
Copy link
Member

RalfJung commented Apr 5, 2020

I just got a full rebuild from a pull that just skipped a few days (so, probably #70582).

Not sure what I should configure manually?

@tmiasko
Copy link
Contributor Author

tmiasko commented Apr 5, 2020

version-suffix in llvm section, but it needs to match the last one if you want to avoid yet another full rebuild next time LLVM submodule is updated.

@RalfJung
Copy link
Member

RalfJung commented Apr 9, 2020

On the next LLVM bump I am now seeing a partial rebuild, so the previous one I saw was probably a one-off hickup. Thanks!

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustbuild: LLVM is not partially rebuilt anymore
7 participants