Repository navigation
bootstrap: always build llvm's dylib when linking to it - #160597
Conversation
|
This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. |
|
r? @clubby789 rustbot has assigned @clubby789. Use Why was this reviewer chosen?The reviewer was selected based on:
|
I wonder how does that show up? Because if LLVM was already built locally, and you modify the LLVM bootstrap settings, CMake won't be involved at all, because bootstrap currently caches the whole build and doesn't examine whether the settings changed or not. |
That's right. LLVM configuration can be rerun by removing .llvm-stamp. See the following quote.
|
|
Yeah, that's what I was wondering about, if you trigger this by manually removing the stamp. This seems reasonable, and could help making LLVM rebuilds more robust (which is important especially if we manage to re-land #158380). |
This comment has been minimized.
This comment has been minimized.
|
Sorry for missing this; if you want to rebase, I think this looks reasonable. @rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
The bootstrap example says that LLVM's dynamic library will be built when llvm.link-shared is true. It actually only set the flag that enables linking against the library (LLVM_LINK_LLVM_DYLIB). It should also set LLVM_BUILD_LLVM_DYLIB. On clean builds, LLVM's configuration helpfully sets the BUILD flag to true if the LINK flag is true. If a dev toggles link-shared from false to true after an initial build, LINK will be "ON" (given by bootstrap) and BUILD will be "OFF" (from the cmake cache). The library to link will not be built, causing the build to fail at link time. With this change, the library is always built when it's to be linked. Conversely, it is not built if it is not to be linked.
228d0d8 to
a6bcbeb
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@rustbot ready |
|
Could you please also add a comment on top of the env. var. setting, which explains that we should ideally always set these variables? To avoid someone moving it back inside an if condition in the future. |
|
Agree with adding a comment; r=me after that |
|
@clubby789 Comment added. BTW I'm not sure what |
|
@bors r=clubby789 It's a command for another bot, bors, but you couldn't actually use it 😅 |
…d, r=clubby789 bootstrap: always build llvm's dylib when linking to it The bootstrap example says that LLVM's dynamic library will be built when llvm.link-shared is true. It actually only set the flag that enables linking against the library (LLVM_LINK_LLVM_DYLIB). It should also set LLVM_BUILD_LLVM_DYLIB. On clean builds, LLVM's configuration helpfully sets the BUILD flag to true if the LINK flag is true. If a dev toggles link-shared from false to true after an initial build, LINK will be "ON" (given by bootstrap) and BUILD will be "OFF" (from the cmake cache). The library to link will not be built, causing the build to fail at link time. With this change, the library is always built when it's to be linked. Conversely, it is not built if it is not to be linked.
Rollup of 6 pull requests Successful merges: - #160597 (bootstrap: always build llvm's dylib when linking to it) - #162810 (vectorized vec::dedup pre-scan) - #162931 (directly deallocates the nodes instead of traversing every element when BTreemap dropping) - #162027 (std: add `fs::rename_noreplace`) - #164146 (Nuke `Unique` (and `ptr_internals`)) - #164151 (std: add doctest to parent_id)
Rollup of 7 pull requests Successful merges: - #159744 (Allow unary operand types to be inferred later) - #160597 (bootstrap: always build llvm's dylib when linking to it) - #162810 (vectorized vec::dedup pre-scan) - #162931 (directly deallocates the nodes instead of traversing every element when BTreemap dropping) - #162027 (std: add `fs::rename_noreplace`) - #163883 (Add `oneshot` `Sender::is_closed`) - #164146 (Nuke `Unique` (and `ptr_internals`))
Rollup merge of #160597 - xSetech:bootstrap/build_llvm_shared, r=clubby789 bootstrap: always build llvm's dylib when linking to it The bootstrap example says that LLVM's dynamic library will be built when llvm.link-shared is true. It actually only set the flag that enables linking against the library (LLVM_LINK_LLVM_DYLIB). It should also set LLVM_BUILD_LLVM_DYLIB. On clean builds, LLVM's configuration helpfully sets the BUILD flag to true if the LINK flag is true. If a dev toggles link-shared from false to true after an initial build, LINK will be "ON" (given by bootstrap) and BUILD will be "OFF" (from the cmake cache). The library to link will not be built, causing the build to fail at link time. With this change, the library is always built when it's to be linked. Conversely, it is not built if it is not to be linked.
The bootstrap example says that LLVM's dynamic library will be built when llvm.link-shared is true. It actually only set the flag that enables linking against the library (LLVM_LINK_LLVM_DYLIB). It should also set LLVM_BUILD_LLVM_DYLIB.
On clean builds, LLVM's configuration helpfully sets the BUILD flag to true if the LINK flag is true. If a dev toggles link-shared from false to true after an initial build, LINK will be "ON" (given by bootstrap) and BUILD will be "OFF" (from the cmake cache). The library to link will not be built, causing the build to fail at link time.
With this change, the library is always built when it's to be linked. Conversely, it is not built if it is not to be linked.