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
Fix installation of shared library symlink #535
Conversation
And to save ppl some time digging through this again. These targets were originally added in eb3d977 to split out the library itself (which is required at runtime) and the namelink (which is required at compile/link time only) into different components. (This commit also introduced a typo that caused the gnueabi library to be missing the link unfortunately...) There appears to be some compatibility issues with old cmake versions (#417) as noted and fixed in #419 though I couldn't really find the reference for the quote in that PR and a brief looking at the document for pre-cmake 3.12/3.14 doesn't make it immediately obvious to me why the dummy default DESTINATION is required. The commit message for the removal of the dummy target doesn't have anything useful in it but based on the comments in #531 (comment) and #510 I'm guessing it's only about the installation to "dummy" without realizing it's also removing the namelink needed for linking so I assume adding back the target itself shouldn't be a problem. |
Hi! Thanks a lot for looking into that and explaining in details. I cannot see what the mentioned typo was from your patch? Can you point me to it? I don't think there was a particular use for the The justification for removing the Otherwise change sounds good to me, will test locally. |
eb3d977#diff-888e45d79814842a399e05bff360c2598a83fdcdffd2c3fcb66f8ebf3d4dd86dR1097 The target name here was wrong. Probably from copy and pasting from a few lines above in the same file… |
The Since the project promises 3.18 support now, |
This fixes the regression from 29391cc. The dummy target that was removed in 29391cc was responsible for installing the unversioned symlink for the shared libraries. The only part of this that was causing issue appears to be the dummy destination that was only added as an workaround for old cmake version (pre 3.14-ish). With the cmake version requirement bumped to 3.18 this should not be an issue anymore, and we could also use the new NAMELINK_COMPONENT option to avoid using two install statements. This also fixes a bug from eb3d977 causing the libsleefgnuabi library to be missing the symlink...
Done. |
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.
I can't verify right now if these are all the instances, but otherwise LGTM.
Looks good to me too. I believe these are all the targets. Is it what is expected when installing each components? If so then I'll merge.
and
I don't think a GNU ABI header is required. |
Yep, the runtime component should really only have the real library and the soname, everything else goes in the dev component. Think |
And should we expect the symlink to show in red in the dev component?
All show in red. |
I don't remember what a red name means with |
The red means that the link target isn't there, which is expected since you didn't install the runtime library in the same directory. At the end of the day when the dev component is installed (as a separate package) it must still be installed to the same directory as the runtime library and the symlink would work. |
This looks right! |
I also tried using this as a patch with the |
Ah, I didn't realize that there was an issue for this. I just looked at the open PR's but not the issues. #532 was basically the typo that I mentioned above. |
Ok, that's what I was suspecting. Thanks for clarifying! |
This partially reverts 29391cc.
The dummy target that was removed in 29391cc was responsible for installing the unversioned symlink for the shared libraries. The only part of this that was causing issue appears to be the dummy destination that was only added as an workaround for old cmake version (pre 3.14-ish). With the cmake version requirement bumped to 3.18 this should not be an issue anymore.
This also fixes a typo from eb3d977 causing the libsleefgnuabi library to be missing the symlink...
Fixes #532