Create symlinks via llvm_add_tool_symlink/llvm_install_symlink#738
Merged
quic-seaswara merged 1 commit intoqualcomm:mainfrom Feb 2, 2026
Merged
Create symlinks via llvm_add_tool_symlink/llvm_install_symlink#738quic-seaswara merged 1 commit intoqualcomm:mainfrom
llvm_add_tool_symlink/llvm_install_symlink#738quic-seaswara merged 1 commit intoqualcomm:mainfrom
Conversation
jonathonpenix
requested review from
partaror,
quic-akaryaki,
quic-areg,
quic-k and
quic-seaswara
Contributor
Author
|
Is there a Windows builder that I can test this on/check that things look as expected? Local Linux builds look ok, but I don't have a local setup for Windows builds |
jonathonpenix
force-pushed
the
llvm_symlink
branch
from
1d3e504 to
9da3271
Compare
jonathonpenix
added a commit
to jonathonpenix/eld
that referenced
this pull request
Jan 25, 2026
ELDExpected.test looks for ELDExpectedUsage in an `%llvmobjroot`- relative dir (which IIUC boils down to a `LLVM_BINARY_DIR`-relative dir). ELDExpectedUsage is currently installed in a `CMAKE_BINARY_DIR`-relative dir. `LLVM_BINARY_DIR` and `CMAKE_BINARY_DIR` aren't always the same--they can be different when llvm is added as a subdirectory. When this is the case, we see test failures due to the mismatched location--see [1] for details. To fix this, just place ELDExpectedUsage in a `LLVM_BINARY_DIR`-relative location as that is what the tests expect and it matches the behavior when `LLVM_BINARY_DIR == CMAKE_BINARY_DIR`. This partially fixes [1], the symlink/wrapper part is handled in [2]. [1] qualcomm#710 [2] qualcomm#738 Signed-off-by: Jonathon Penix <jpenix@quicinc.com>
eld's symlink creation currently has a few issues/quirks in how it interacts with CPack and where symlinks are placed in builds [1][2]. I think the easiest way to fix these issues is just to leverage LLVM's cmake utilities `llvm_add_tool_symlink` and `llvm_install_symlink`. Also, I think there's a few other advantages: - The end result (created symlinks or copied/renamed binaries) should be the same as before while allowing us to simplify eld's cmake. We are already tightly coupled to LLVM/LLVM's cmake anyway - eld should behave a bit more consistently to other LLVM tools now--for example, if someone does want to force symlinks on Windows in LLVM, eld will do the same The one (maybe negative, intended) functional change here is that the symlinks now won't have their own targets. But: - It isn't clear to me how helpful it is to have per-symlink targets (any/all symlinks will be built/installed with ld.eld) - This should be similar to how ex: lld's symlinks work - Worst case, I think we can add them back without reverting any of this if needed So, I omitted them for now. [1] qualcomm#737 [2] qualcomm#710 Signed-off-by: Jonathon Penix <jpenix@quicinc.com>
jonathonpenix
force-pushed
the
llvm_symlink
branch
from
9da3271 to
238c4c7
Compare
This was referenced Jan 26, 2026
Contributor
Author
|
FWIW, we have been testing cpullvm with this (and the other open cmake-y patches) for a little bit now. If you want, you can check out a builds with these applied here: https://github.com/qualcomm/cpullvm-toolchain/actions/runs/21575041496 |
quic-seaswara
approved these changes
Feb 2, 2026
quic-seaswara
pushed a commit
that referenced
this pull request
Feb 2, 2026
ELDExpected.test looks for ELDExpectedUsage in an `%llvmobjroot`- relative dir (which IIUC boils down to a `LLVM_BINARY_DIR`-relative dir). ELDExpectedUsage is currently installed in a `CMAKE_BINARY_DIR`-relative dir. `LLVM_BINARY_DIR` and `CMAKE_BINARY_DIR` aren't always the same--they can be different when llvm is added as a subdirectory. When this is the case, we see test failures due to the mismatched location--see [1] for details. To fix this, just place ELDExpectedUsage in a `LLVM_BINARY_DIR`-relative location as that is what the tests expect and it matches the behavior when `LLVM_BINARY_DIR == CMAKE_BINARY_DIR`. This partially fixes [1], the symlink/wrapper part is handled in [2]. [1] #710 [2] #738 Signed-off-by: Jonathon Penix <jpenix@quicinc.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
eld's symlink creation currently has a few issues/quirks in how it interacts with CPack and where symlinks are placed in builds [1][2].
I think the easiest way to fix these issues is just to leverage LLVM's cmake utilities
llvm_add_tool_symlinkandllvm_install_symlink. Also, I think there's a few other advantages:The one (maybe negative, intended) functional change here is that the symlinks now won't have their own targets. But:
[1] #737
[2] #710