This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Passthrough CMAKE_SYSROOT to external projects
ClosedPublic

Authored by phosek on Oct 17 2017, 6:04 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek created this revision.Oct 17 2017, 6:04 PM
compnerd accepted this revision.Oct 30 2017, 7:58 PM
This revision is now accepted and ready to land.Oct 30 2017, 7:58 PM
beanz edited edge metadata.Nov 7 2017, 3:32 PM

Sorry I've been bad at staying on top of your patches.

I think this shouldn't be an unconditional passthrough. I can imagine a situation where at least for the runtimes you might want different values set for different targets.

You've done most of the work in that area. What do you think?

phosek added a comment.Nov 7 2017, 5:46 PM

Sorry I've been bad at staying on top of your patches.

I think this shouldn't be an unconditional passthrough. I can imagine a situation where at least for the runtimes you might want different values set for different targets.

You've done most of the work in that area. What do you think?

We already use CMAKE_SYSROOT in build for our targets (here https://github.com/llvm-mirror/clang/blob/master/cmake/caches/Fuchsia-stage2.cmake#L45). The way this works is that the -DCMAKE_SYSROOT=... value we set in our cache file will be eventually passed to this function in ${ARG_CMAKE_ARGS} variable and since that one comes after the one I've added, CMake will pick that value as the final one. I admit it's not the prettiest solution though. I could change it to only set -DCMAKE_SYSROOT if it's defined and it's not already set in ${ARG_CMAKE_ARGS}, would you prefer that solution?

beanz accepted this revision.Nov 8 2017, 10:11 AM

Sounds reasonable to me. The added complexity of not passing it doesn't seem worth it.

This revision was automatically updated to reflect the committed changes.