This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [CMake] Convert paths to the right form in standalone builds on Windows
ClosedPublic

Authored by mstorsjo on Jun 20 2018, 2:58 AM.

Details

Summary

The paths output from llvm-config --cmakedir and from clang --print-libgcc-file-name can contain backslashes, while CMake can't handle the paths in this form.

This matches what compiler-rt already does (since SVN r203789 and r293195).

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.Jun 20 2018, 2:58 AM
smeenai accepted this revision.Jun 20 2018, 12:31 PM
smeenai added a subscriber: smeenai.

Looks good with the quotes added.

cmake/Modules/HandleCompilerRT.cmake
17 ↗(On Diff #152047)

Nit: cmake recommends always putting quotes around the path argument to ensure it's treated as a single argument, i.e.

file(TO_CMAKE_PATH "${LIBRARY_FILE}" LIBRARY_FILE)

That needs to happen in all other uses as well.

cmake/Modules/HandleOutOfTreeLLVM.cmake
52 ↗(On Diff #152047)

Is there ever a chance LLVM_BINARY_DIR will have backslashes, or are you just being extra careful (or going for consistency)?

This revision is now accepted and ready to land.Jun 20 2018, 12:31 PM
mstorsjo added inline comments.Jun 20 2018, 12:46 PM
cmake/Modules/HandleCompilerRT.cmake
17 ↗(On Diff #152047)

Ok, will do.

cmake/Modules/HandleOutOfTreeLLVM.cmake
52 ↗(On Diff #152047)

I'm mostly just going for consistency and copy+paste; this path identically matches a part from compiler-rt/cmake/Modules/CompilerRTUtils.cmake - and that part actually seems to have been added intentionally to fix some case: https://reviews.llvm.org/rL203789

smeenai added inline comments.Jun 20 2018, 12:47 PM
cmake/Modules/HandleOutOfTreeLLVM.cmake
52 ↗(On Diff #152047)

Fair enough; sounds good to me.

This revision was automatically updated to reflect the committed changes.