- 
                Notifications
    
You must be signed in to change notification settings  - Fork 15.1k
 
cmake: Allow CLANG_RESOURCE_DIR to be absolute. #145996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Merged
      
      
            pcc
  merged 2 commits into
  main
from
users/pcc/spr/cmake-allow-clang_resource_dir-to-be-absolute
  
      
      
   
  Jun 27, 2025 
      
    
                
     Merged
            
            cmake: Allow CLANG_RESOURCE_DIR to be absolute. #145996
                    pcc
  merged 2 commits into
  main
from
users/pcc/spr/cmake-allow-clang_resource_dir-to-be-absolute
  
      
      
   
  Jun 27, 2025 
              
            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
  
| 
               | 
          ||
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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}) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, fixed.
Created using spr 1.3.6-beta.1
              
                    petrhosek
  
              
              approved these changes
              
                  
                    Jun 27, 2025 
                  
              
              
            
            
    
  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.
    
  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.