This is an archive of the discontinued LLVM Phabricator instance.

CMake: Fix stand-alone clang builds since r353268
ClosedPublic

Authored by tstellar on Feb 13 2019, 1:11 PM.

Details

Summary

Handle the case where LLVM_MAIN_SRC_DIR is not set and also use
LLVM_CMAKE_DIR for locating installed cmake files rather than
LLVM_CMAKE_PATH.

Diff Detail

Repository
rL LLVM

Event Timeline

tstellar created this revision.Feb 13 2019, 1:11 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 13 2019, 1:11 PM
phosek accepted this revision.Feb 13 2019, 1:15 PM

LGTM

This revision is now accepted and ready to land.Feb 13 2019, 1:15 PM
smeenai added inline comments.Feb 13 2019, 1:16 PM
clang/lib/Basic/CMakeLists.txt
13 ↗(On Diff #186726)

Will this work when using llvm-config instead of LLVM's CMake package?

andrewrk accepted this revision.Feb 13 2019, 1:23 PM

I confirm this solves the issue.

llvm/cmake/modules/AddLLVM.cmake
1739 ↗(On Diff #186726)

I think ${path} should still be quoted here for best practice, as well as ${GIT_EXECUTABLE} above.

1745 ↗(On Diff #186726)

these variables should be quoted too

tstellar marked an inline comment as done.Feb 13 2019, 1:28 PM
tstellar added inline comments.
clang/lib/Basic/CMakeLists.txt
13 ↗(On Diff #186726)

I will test this and a few other configurations to make sure.

This could also be improved by teaching clang to use LLVM_BUILD_MAIN_SRC_DIR when that's available.

tstellar updated this revision to Diff 186769.Feb 13 2019, 4:14 PM

Updated patch to fix non-standalone builds. There is more refactoring
that could be done, but this at least restores functionality to before
r353268.

I have tested non-standalone builds and standalone builds with and
without llvm-config.

smeenai accepted this revision.Feb 13 2019, 5:31 PM

LGTM. There's more clean up that could be done here, but this addresses the immediate issue. Thanks!

This revision was automatically updated to reflect the committed changes.