Skip to content
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

release/18.x: [cmake] Add minor version to library SONAME (#79376) #82409

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Feb 20, 2024

Backport 91a3846

Requested by: @tstellar

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

llvmbot commented Feb 20, 2024

@jyknight What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from jyknight February 20, 2024 19:52
@llvmbot llvmbot added the cmake Build system in general and CMake in particular label Feb 20, 2024
We need to do this now that we are bumping the minor release number when
we create the release branch.

This also results in a slight change to the library names for LLVM. The
main library now has a more convential library name:
'libLLVM.so.$major.$minor'. The old library name: libLLVM-$major.so is
now a symlink that points to the new library. However, the symlink is
not present in the build directory. It is only present in the install
directory.

The library name was changed because it helped to keep the CMake changes
more simple.

Fixes llvm#76273

(cherry picked from commit 91a3846)
This was broken by 91a3846.

(cherry picked from commit ff4d6c6)
@tstellar tstellar merged commit 6c90f8d into llvm:release/18.x Feb 21, 2024
6 of 8 checks passed
@RalfJung
Copy link
Contributor

RalfJung commented Mar 4, 2024

Making such a fundamental change to the .so file so late in the release was probably a bad idea... a bunch of downstream projects will have to adjust the way they link with LLVM and that's not always easy.

@tstellar
Copy link
Collaborator

tstellar commented Mar 4, 2024

@RalfJung The intention was that the change would be backwards compatible. Is there a specific problem you see? We can still fix this in the release/18.x branch if necessary.

@RalfJung
Copy link
Contributor

RalfJung commented Mar 4, 2024

Yeah, I linked to it: rust-lang/rust#121889.

I'm not a linker expert, but it seems that now symlinks are required to link against the LLVM .so file, but we can't ship symlinks in our artifacts because of Windows. The old approach we used of just shipping the .so file and pointing the linker at it doesn't seem to work any more with the new sonames. I don't claim to understand why, I am involved solely because the tool I work on (which links against rustc directly, and thus against LLVM) no longer builds after rustc updated to LLVM 18 rc4.

A solution is in the works though so we may be fine soon. But who knows which other setups get broken by this.

@jyknight
Copy link
Member

jyknight commented Mar 4, 2024

So the problem Rust sees isn't that a ".1" was added to the version, but rather that the name was changed from "libLLVM-18-rust-1.78.0-nightly.so" to "libLLVM.so.18.1-rust-1.78.0-nightly". (that is: all the version info previously went into the library name which comes before ".so", and now goes into the library version which comes after ".so").

And issues with symlinks on Windows.

@tstellar
Copy link
Collaborator

tstellar commented Mar 4, 2024

@RalfJung Are you configuring LLVM with -DLLVM_VERSION_SUFFIX=-rust-1.78.0-nightly ?

@RalfJung
Copy link
Contributor

RalfJung commented Mar 4, 2024

So the problem Rust sees isn't that a ".1" was added to the version, but rather that the name was changed from "libLLVM-18-rust-1.78.0-nightly.so" to "libLLVM.so.18.1-rust-1.78.0-nightly". (that is: all the version info previously went into the library name which comes before ".so", and now goes into the library version which comes after ".so").

Ah, I didn't catch the shifted .so -- sounds right.

Are you configuring LLVM with -DLLVM_VERSION_SUFFIX=-rust-1.78.0-nightly ?

I have no idea, as I said I am working on a tool that fails to link now but have no direct contact with our LLVM stuff otherwise.^^ @nikic would know more.

At this point we seem to be on-track for fixing this so a revert would probably be extra work for us. But other users that tested rc3 may still be surprised by this I guess.

@nikic
Copy link
Contributor

nikic commented Mar 4, 2024

@RalfJung Are you configuring LLVM with -DLLVM_VERSION_SUFFIX=-rust-1.78.0-nightly ?

Yes.

So the problem Rust sees isn't that a ".1" was added to the version, but rather that the name was changed from "libLLVM-18-rust-1.78.0-nightly.so" to "libLLVM.so.18.1-rust-1.78.0-nightly". (that is: all the version info previously went into the library name which comes before ".so", and now goes into the library version which comes after ".so").

Right. The new scheme requires a symlink for linking, and rustup components currently do not support symlinks. But thankfully we can use a linker script instead of a symlink to sidestep the issue.

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.

5 participants