-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[llvm-config-removal] Instead of maintaining our own LLVM_LIBRARY_DIR… #3133
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
[llvm-config-removal] Instead of maintaining our own LLVM_LIBRARY_DIR… #3133
Conversation
@swift-ci Please test |
76e0441
to
efae3ba
Compare
@swift-ci Please test |
efae3ba
to
ec228c7
Compare
@swift-ci Please smoke test |
The first commit is actually in #3134. I am just trying to make forward progress. |
ec228c7
to
7e53ab9
Compare
I decided to just hit all 3 variables |
@swift-ci Please test |
@gribozavr What do you think? |
7122da9
to
5c1bbf0
Compare
@swift-ci Please smoke test |
5c1bbf0
to
838bf0d
Compare
@swift-ci Please smoke test |
|
||
function(precondition var) | ||
if (NOT ${var}) | ||
message(FATAL_ERROR "Error! Variable ${var} is not set!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can actually be set to false
, 0
or any other string that CMake treats as false.
The linux failure looks unrelated. I am going to fix the things that Dmitri suggested and then merge. |
… such as precondition.
…lues instead of re-setting it later ot the same value. Before this patch, we would always set ${product}_PATH_TO_LLVM_{SOURCE,BUILD} to a value taken from llvm-config. But, turns out we had generated llvm-config using these values. Thus llvm-config is just giving us back values we already have. This commit changes these variables to be precondition variables and doesn't cause them to be reset. rdar://26154980
…bles. Previously in order to support cross-compiling, we were setting these variables in build-script. Now that we are using LLVMConfig.cmake, passing in these values are no longer necessary. As an added benefit, Swift when not cross compiling, does not need to set these variables anyways. rdar://26154980
838bf0d
to
e24e1f7
Compare
Made the changes, merging. |
What's in this pull request?
Resolved bug number: (SR-)
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.
…, use LLVM_BUILD_LIBRARY_DIR from LLVMConfig.cmake.
LLVM_LIBRARY_DIR is a variable that was added to enable build-script to find the
location of llvm's library dir even if
The reason why LLVM_LIBRARY_DIR variable was passed in from the command line
historically was b/c we did not use LLVMConfig.cmake and instead relied on
llvm-config. This meant when cross-compiling llvm, we could not get the llvm
build information via llvm-config.
LLVM_LIBRARY_DIR is meant to point to the the llvm library directory for the
LLVM that we just compiled. The only reason why it is set as an option when cross compiling is b/c a cross compiled
rdar://26154980