Skip to content

release/18.x: [llvm-shlib] Fix the version naming style of libLLVM for Windows (#85710) #85746

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
merged 3 commits into from
Mar 19, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Mar 19, 2024

Backport ec2b752 f849805 cb2ca23

Requested by: @mstorsjo

@llvmbot llvmbot added this to the LLVM 18.X Release milestone Mar 19, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Mar 19, 2024

@nikic @mstorsjo @tstellar What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested review from mstorsjo, nikic and tstellar March 19, 2024 07:43
@llvmbot llvmbot added the cmake Build system in general and CMake in particular label Mar 19, 2024
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit unfortunate to have these changes post-release, but I think we do need to make them.

@mstorsjo
Copy link
Member

A bit unfortunate to have these changes post-release, but I think we do need to make them.

Yes, in particular as the earlier changes were backported. If the whole series would have been left out from 18.x, there wouldn't have been any hurry in backporting.

tstellar and others added 3 commits March 19, 2024 12:48
This is a partial revert of 10c48a7
with a fix for the symlink target name on MacOS

See llvm#84637

(cherry picked from commit ec2b752)
The TARGET_SONAME_FILE_NAME generator expression is not available on dll
target platforms.

(cherry picked from commit f849805)
…m#85710)

This reverts the changes from 91a3846
for Windows targets. The changes in that commit don't work as expected
for Windows targets (those parts of llvm_add_library don't quite behave
the same for Windows), while the previous status quo (producing a
library named "libLLVM-<major>.dll") is the defacto standard way of
doing versioned library names there, contrary to on Unix.

After that commit, the library always ended up named "libLLVM.dll",
executables linking against it would reference "libLLVM.dll", and
"libLLVM-<major>.dll" was provided as a symlink.

Thus revert this bit back to as it were, so that executables actually
link against a versioned libLLVM, and no separate symlink is needed.

The only thing that might be improved compared to the status quo as it
was before these changes, is that the import library is named
"lib/libLLVM-<major>.dll.a", while the common style would be to name it
plainly "lib/libLLVM.dll.a" (even while it produces references to
"libLLVM-<major>.dll", but none of these had that effect for Windows
targets.

(As a side note, the llvm-shlib library can be built for MinGW, but not
currently in MSVC configurations.)

(cherry picked from commit cb2ca23)
@tstellar tstellar merged commit c09f6f6 into llvm:release/18.x Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular
Projects
Development

Successfully merging this pull request may close these issues.

4 participants