Page MenuHomePhabricator

Set the path to the shared cmake modules based on the llvm directory
ClosedPublic

Authored by mstorsjo on Dec 31 2021, 11:06 PM.

Details

Summary

It’s still possible to build parts of the main llvm build (lld, clang etc) by symlinking them into llvm/tools.

Diff Detail

Event Timeline

Ericson2314 created this revision.Dec 31 2021, 11:06 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Ericson2314 requested review of this revision.Dec 31 2021, 11:07 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptDec 31 2021, 11:07 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added subscribers: Restricted Project, jdoerfert. · View Herald Transcript

@mstorsjo I hope this fixes the issue you had in https://reviews.llvm.org/D115568#3215445. I just whipped this up coding blind. If you could test it to ensure it solves your issue, that would be much appreciated!

@mstorsjo I hope this fixes the issue you had in https://reviews.llvm.org/D115568#3215445. I just whipped this up coding blind. If you could test it to ensure it solves your issue, that would be much appreciated!

Thanks!

We shouldn’t need this for the runtimes; they have their own story for how they’re built (which is complicated enough in other ways), and I don’t think anyone is symlinking then around.

Secondly, we’d need to set this in llvm/CMakeLists.txt too, to make it work automatically.

I can try to fix the patch and commandeer it.

mstorsjo commandeered this revision.Jan 1 2022, 1:40 AM
mstorsjo edited reviewers, added: Ericson2314; removed: mstorsjo.
mstorsjo updated this revision to Diff 396850.Jan 1 2022, 1:46 AM

Removed the changes to runtimes; the ways they are built is a separate story - afaik we mandate the monorepo layout there (and the symlinking setup isn't used there afaik).

Added a default setting in llvm/CMakeLists.txt, renamed to match LLVM_THIRD_PARTY_DIR. Made it a plain internal variable - it's not supposed to user settable in general (I think we're moving to mandate the monorepo layout overall).

Herald added a project: Restricted Project. · View Herald TranscriptJan 1 2022, 1:46 AM
mstorsjo removed reviewers: Restricted Project, Restricted Project, Restricted Project.Jan 1 2022, 1:48 AM
mstorsjo removed a subscriber: libcxx-commits.

Removing reviewers/subscribers that shouldn’t be affected by this patch any longer.

mstorsjo retitled this revision from Allow overriding path to shared CMake utilities to Set the path to the shared cmake modules based on the llvm directory.Jan 1 2022, 1:52 AM
mstorsjo edited the summary of this revision. (Show Details)
mstorsjo removed projects: Restricted Project, Restricted Project, Restricted Project, Restricted Project.
Ericson2314 accepted this revision.Jan 1 2022, 9:49 AM
This revision is now accepted and ready to land.Jan 1 2022, 9:49 AM
This revision was landed with ongoing or failed builds.Jan 1 2022, 9:59 AM
This revision was automatically updated to reflect the committed changes.