[llvm-lib] Don't rewrite paths for members in non-thin archives #123416
Conversation
This file contains 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
This matches what MS lib.exe does (and llvm-ar too); MS lib.exe doesn't produce relative paths when producing regular static archives. (lib.exe doesn't support producing thin archives; that's an LLVM extension; see the thin-relative.test testcase.) It is unclear why 451c2ef chose to extend this behaviour to non-thin archives in the case of llvm-lib (and even mentioning it in a comment), while not doing that for llvm-ar; the code review at https://reviews.llvm.org/D57842 or the linked bug report at https://crbug.com/41440160 doesn't touch upon that. Therefore, assume that this only was done out of convenience, and change llvm-lib to not adjust the paths for non-thin archives. Normally, the actual member names doesn't matter for non-thin archives; however for short import libraries, where each member is named e.g. "foo.dll", the names do matter. If using llvm-lib to merge two import libraries (as a non-thin library), preserve the original names rather than making the member names relative.
zmodem
approved these changes
Jan 20, 2025
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.
This matches what MS lib.exe does (and llvm-ar too); when adding files to an archive, MS lib.exe stores the file name as it was given on the command line, whereas llvm-lib rewrote it into a relative path name, relative to the archive location. Such a rewrite makes sense for thin archives, but not for regular archives. (MS lib.exe doesn't support producing thin archives; that's an LLVM extension - see the thin-relative.test testcase.)
The behaviour to rewrite these paths was added in 451c2ef; it is unclear why it was chosen to do the rewriting for non-thin archives as well. This quirk is even pointed out in a code comment - but neither the code review at https://reviews.llvm.org/D57842 nor the linked bug report at https://crbug.com/41440160 mentions why this is done for all archives, not only thin ones.
Therefore, assume that this only was done out of convenience, and change llvm-lib to not adjust the paths for non-thin archives.
Normally, the actual member names doesn't matter for non-thin archives; however for short import libraries, where each member is named e.g. "foo.dll", the names do matter. If using llvm-lib to merge two import libraries (as a non-thin library), preserve the original names rather than making the member names relative.