cmake: Allow CLANG_RESOURCE_DIR to be absolute. #145996
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Created using spr 1.3.6-beta.1
pcc
added a commit
to pcc/nixpkgs
that referenced
this pull request
Jun 27, 2025
When Clang is statically linked against other programs they are unable to find the headers in Clang's resource directory. Typically the resource directory is found by searching a path relative to argv[0] but this would only really work for Clang itself due to each binary having a separate prefix (and not in Nix because of the full resource directory being split between multiple derivations and assembled in the wrapper). Because users of Clang as a library typically only need include in order for parsing to succeed, let's set that as the resource directory. The LLVM patch to make this work was sent upstream in llvm/llvm-project#145996, I intend to land it upstream and drop it from this PR.
13 tasks
| @@ -10,7 +10,7 @@ function(get_clang_resource_dir out_var) | |||
| cmake_parse_arguments(ARG "" "PREFIX;SUBDIR" "" ${ARGN}) | |||
|
|
|||
| if(DEFINED CLANG_RESOURCE_DIR AND NOT CLANG_RESOURCE_DIR STREQUAL "") | |||
| set(ret_dir bin/${CLANG_RESOURCE_DIR}) | |||
| cmake_path(APPEND bin ${CLANG_RESOURCE_DIR} OUTPUT_VARIABLE ret_dir) | |||
There was a problem hiding this comment.
cmake_path(APPEND <path-var> [<input>...] [OUTPUT_VARIABLE <out-var>])
IIUC here bin should be replaced by a variable name:
Suggested change
| cmake_path(APPEND bin ${CLANG_RESOURCE_DIR} OUTPUT_VARIABLE ret_dir) | |
| set(ret_dir bin) | |
| cmake_path(APPEND ret_dir ${CLANG_RESOURCE_DIR}) |
Created using spr 1.3.6-beta.1
petrhosek
approved these changes
Jun 27, 2025
pcc
deleted the
users/pcc/spr/cmake-allow-clang_resource_dir-to-be-absolute
branch
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jun 27, 2025
Currently an absolute CLANG_RESOURCE_DIR is treated as being relative to bin. Fix that by using cmake_path(APPEND) to append the path to bin. The prepending of PREFIX is left as-is because callers passing PREFIX are usually forming a path within the build directory and would not want build products to be written to an absolute resource directory that is likely to be an installation prefix. One exception is the caller in lldb/cmake/modules/LLDBStandalone.cmake; for now it is not possible to build LLDB with an absolute resource directory until the users are disambiguated. Reviewers: petrhosek Reviewed By: petrhosek Pull Request: llvm/llvm-project#145996
pcc
added a commit
to pcc/nixpkgs
that referenced
this pull request
Jul 1, 2025
When Clang is statically linked against other programs they are unable to find the headers in Clang's resource directory. Typically the resource directory is found by searching a path relative to argv[0] but this would only really work for Clang itself due to each binary having a separate prefix (and not in Nix because of the full resource directory being split between multiple derivations and assembled in the wrapper). Because users of Clang as a library typically only need include in order for parsing to succeed, let's set that as the resource directory. The LLVM patch to make this work was sent upstream in llvm/llvm-project#145996, I intend to land it upstream and drop it from this PR.
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
Currently an absolute CLANG_RESOURCE_DIR is treated as being relative to bin. Fix that by using cmake_path(APPEND) to append the path to bin. The prepending of PREFIX is left as-is because callers passing PREFIX are usually forming a path within the build directory and would not want build products to be written to an absolute resource directory that is likely to be an installation prefix. One exception is the caller in lldb/cmake/modules/LLDBStandalone.cmake; for now it is not possible to build LLDB with an absolute resource directory until the users are disambiguated. Reviewers: petrhosek Reviewed By: petrhosek Pull Request: llvm#145996
pcc
added a commit
that referenced
this pull request
Jul 2, 2025
After #145996 CLANG_RESOURCE_DIR can be an absolute path so we need to handle it correctly in the driver. llvm::sys::path::append does not append absolute paths in the way that I expected (or consistent with other similar APIs such as C++17 std::filesystem::path::append or Python os.path.join); instead, it effectively discards the leading / and appends the resulting relative path (e.g. append(P, "/bar") with P = "/foo" sets P to "/foo/bar"). Many tests start failing if I try to align llvm::sys::path::append with the other APIs because of callers that expect the existing behavior, so for now let's add a special case here for absolute resource paths, and document the behavior in Path.h. Reviewers: MaskRay Reviewed By: MaskRay Pull Request: #146449
alyssais
pushed a commit
to NixOS/nixpkgs
that referenced
this pull request
Jul 2, 2025
When Clang is statically linked against other programs they are unable to find the headers in Clang's resource directory. Typically the resource directory is found by searching a path relative to argv[0] but this would only really work for Clang itself due to each binary having a separate prefix (and not in Nix because of the full resource directory being split between multiple derivations and assembled in the wrapper). Because users of Clang as a library typically only need include in order for parsing to succeed, let's set that as the resource directory. The LLVM patch to make this work was sent upstream in llvm/llvm-project#145996, I intend to land it upstream and drop it from this PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently an absolute CLANG_RESOURCE_DIR is treated as being relative
to bin. Fix that by using cmake_path(APPEND) to append the path to bin.
The prepending of PREFIX is left as-is because callers passing PREFIX
are usually forming a path within the build directory and would not
want build products to be written to an absolute resource directory
that is likely to be an installation prefix. One exception is the caller
in lldb/cmake/modules/LLDBStandalone.cmake; for now it is not possible
to build LLDB with an absolute resource directory until the users are
disambiguated.