[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

mgorny added a comment. This is the final version that was pushed, correct? (the commits don't link the revision) I've raised some concerns about the resulting API in https://reviews.llvm.org/D156270#4557902. I'd appreciate if you could take a look. Repository: rG LLVM Github Monorepo

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

tstellar added a comment. In D141907#4409973 , @MaskRay wrote: > In D141907#4094748 , @MaskRay wrote: > >> [...] >> edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable >>

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

MaskRay added a comment. In D141907#4094748 , @MaskRay wrote: > [...] > edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable > `CLANG_RESOURCE_DIR` but did not explain why. > In the long term, the CMake variable `CLANG_RESOURCE_DIR`

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

sebastian-ne added inline comments. Comment at: cmake/Modules/GetClangResourceDir.cmake:15 + else() +string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR ${PACKAGE_VERSION}) +set(ret_dir lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION_MAJOR}) This fails in

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice added a comment. In D141907#4398196 , @protze.joachim wrote: > The review should not be closed, since the patch was reverted and should be > recommitted (this time possibly with a reference to this review?) This is recommitted as

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

protze.joachim reopened this revision. protze.joachim added a comment. This revision is now accepted and ready to land. The review should not be closed, since the patch was reverted and should be recommitted (this time possibly with a reference to this review?) Repository: rG LLVM Github

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice added a comment. In D141907#4361594 , @tstellar wrote: > In D141907#4355229 , @paperchalice > wrote: > >> In D141907#4355228 , @tstellar >> wrote: >> >>>

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

tstellar added a comment. In D141907#4355229 , @paperchalice wrote: > In D141907#4355228 , @tstellar > wrote: > >> @paperchalice Do you need someone to commit this for you? > > Sure, I don't have the commit

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice added a comment. In D141907#4355228 , @tstellar wrote: > @paperchalice Do you need someone to commit this for you? Sure, I don't have the commit access. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

tstellar added a comment. @paperchalice Do you need someone to commit this for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141907/new/ https://reviews.llvm.org/D141907 ___ cfe-commits mailing

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

tstellar accepted this revision. tstellar added a comment. This revision is now accepted and ready to land. Thanks for the patch. I've tested this and it works for my build configuration. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice added inline comments. Comment at: cmake/Modules/GetClangResourceDir.cmake:13 + if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "") +set(ret_dir bin/${CLANG_RESOURCE_DIR}) + else() tstellar wrote: > tstellar wrote: > >

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

tstellar added inline comments. Comment at: cmake/Modules/GetClangResourceDir.cmake:13 + if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "") +set(ret_dir bin/${CLANG_RESOURCE_DIR}) + else() tstellar wrote: > tstellar wrote: > >

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

tstellar added inline comments. Comment at: cmake/Modules/GetClangResourceDir.cmake:13 + if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "") +set(ret_dir bin/${CLANG_RESOURCE_DIR}) + else() tstellar wrote: > paperchalice wrote: > >

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

tstellar added inline comments. Comment at: cmake/Modules/GetClangResourceDir.cmake:13 + if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "") +set(ret_dir bin/${CLANG_RESOURCE_DIR}) + else() paperchalice wrote: > tstellar wrote: > > Why is

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice added inline comments. Comment at: cmake/Modules/GetClangResourceDir.cmake:13 + if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "") +set(ret_dir bin/${CLANG_RESOURCE_DIR}) + else() tstellar wrote: > Why is the bin prefix

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

tstellar added inline comments. Herald added subscribers: ekilmer, jplehr. Comment at: cmake/Modules/GetClangResourceDir.cmake:13 + if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "") +set(ret_dir bin/${CLANG_RESOURCE_DIR}) + else() Why

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice added a comment. Ping. Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp:56 + const std::string clang_resource_path = + clang::driver::Driver::GetResourcesPath("bin/lldb", CLANG_RESOURCE_DIR); Should be

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice updated this revision to Diff 494223. paperchalice added a comment. - Use `clang::driver::Driver::GetResourcesPath` - Use `ARG` as prefix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141907/new/ https://reviews.llvm.org/D141907

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

phosek added inline comments. Comment at: cmake/Modules/GetClangResourceDir.cmake:17 + + set(oneValueArgs PREFIX SUBDIR) + cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN}) This variable is only used once on the following line, I'd just

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice added a comment. In D141907#4094748 , @MaskRay wrote: > The CMake part of this patch improves the matter. Manually constructed > resource dir (many duplicates) string is replace with a library function call. > > However, the introduced

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

MaskRay added a comment. edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable `CLANG_RESOURCE_DIR` but did not explain why. The patch introduced conditions in C++ code like std::string path_to_clang_dir = std::string(CLANG_RESOURCE_DIR).empty()

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

mgorny added reviewers: MaskRay, thesamesam, tstellar. mgorny added a comment. Ok, this turned out to be surprisingly painless, at least within our packaging. However, I'd prefer if someone more experienced looked at the CMake changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

mgorny added a comment. While the general direction seems sound, I suspect this may be a real PITA given that for us: -DCLANG_RESOURCE_DIR="../../../../lib/clang/${LLVM_MAJOR}" I'm going to try and see later throughout the week if it's possible to keep that working with this patch.

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice updated this revision to Diff 492684. paperchalice added a comment. Use `LLVM_LIBRARY_OUTPUT_INTDIR`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141907/new/ https://reviews.llvm.org/D141907 Files:

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

paperchalice created this revision. paperchalice added reviewers: phosek, Ericson2314. Herald added a subscriber: Enna1. Herald added a project: All. paperchalice requested review of this revision. Herald added projects: clang, Sanitizers, LLDB, OpenMP, LLVM. Herald added subscribers: