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

Allow proper runtime linking with pkgin-installed libraries on NetBSD #695

Merged
merged 5 commits into from Jul 9, 2022
Merged

Allow proper runtime linking with pkgin-installed libraries on NetBSD #695

merged 5 commits into from Jul 9, 2022

Conversation

devfonks
Copy link
Contributor

@devfonks devfonks commented Jul 9, 2022

The NetBSD runtime linker appears to look in the same places as the dynamic linker on other systems but does not search LD_LIBRARY_PATH like it does on others, in what appears to be a deliberate design decision. Whatever the case, in order to prevent a pkgin user needing to symlink tons of libraries from the /usr/X11R7 and /usr/pkg trees into /usr/local/lib, Psi should tell the compiler to include parameters for the runtime linker to find the necessary libraries on that system. To aid in portability, I have created some conditions in CMakeLists.txt to allow for setting the CFLAGS/CXXFLAGS variables with the appropriate parameters.

I have done minimal testing on NetBSD-9.1 amd64, and it builds successfully and runs correctly with these flags set (where it would build successfully but fail to link at runtime before). I have not thoroughly tested the program's functionality but it does start and show the UI.

Though it shouldn't matter much, I should note that I only build Psi+ (I haven't used base Psi in quite a few years now). If anyone else wishes to test regular Psi builds your input would be appreciated.

Adds a linking location to let the compiler know about where the
libSM.*, libICE.*, etc. shared objects live on a base NetBSD
system.
The earlier commit on this branch incorrectly assumed GCC would get
the hint from the -L option that /usr/X11R7/lib might need
inclusion at runtime linking. This is not the case, rather it needs
to see a -Wl option to pass the parameters directly to the runtime
linker.

Signed-off-by: devfonks <nkeck72@netzero.net>
/usr/pkg/lib is the tree under which packaged libraries can be linked
from at either runtime or at buildtime, potentially to isolate libs
from a non-working system. This should prevent the user having to
duct-tape symlinks all over the tree after the build.

Signed-off-by: devfonks <nkeck72@netzero.net>
NetBSD seems to have subtrees for large frameworks (like qt5) under
the /usr/pkg/*/lib umbrella. This one allows for Qt to be seen by Psi.

Signed-off-by: devfonks <nkeck72@netzero.net>
A typo was causing NetBSD options to be applied on all BSD systems.
This commit fixes that typo.

Signed-off-by: devfonks <nkeck72@netzero.net>
@Ri0n Ri0n merged commit 84d767c into psi-im:master Jul 9, 2022
@Ri0n
Copy link
Member

Ri0n commented Jul 9, 2022

Thanks @devfonks.
Unfortunately don't have NetBSD to test. But the PR looks good to me.
Even so I'm a little surprised it's necessary to do such stuff for every single package.

@devfonks
Copy link
Contributor Author

devfonks commented Jul 9, 2022

I am too, but it seems this is a deliberate decision for pkgin, mainly to keep binaries from polluting a custom tree, perhaps? I don't know if pkgsrc does the same thing but this should at least help the user who is wanting to build in that case.

@Ri0n
Copy link
Member

Ri0n commented Jul 10, 2022

as @Vitozz noticed on the chat, there is an interesting variable https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_RPATH.html#variable:CMAKE_INSTALL_RPATH seems to be solving the same problem.

@Vitozz
Copy link
Contributor

Vitozz commented Jul 11, 2022

Yep, much proper way is to use cmake functions. For example:

# Detect NetBSD and handle library path accordingly
STRING (REGEX MATCH "NetBSD" PROJECT_OS_NETBSD ${CMAKE_SYSTEM_NAME})
if(PROJECT_OS_NETBSD)
set( CMAKE_BUILD_WITH_INSTALL_RPATH FALSE )
list(APPEND CMAKE_INSTALL_RPATH
"/usr/X11R7/lib"
"/usr/pkg/lib"
"/usr/pkg/qt5/lib"
)
endif()

this can be added before executable creation in src/CMakeLists.txt
I can't test this code because I don't have NetBSD

@devfonks
Copy link
Contributor Author

Thanks for pointing that out, I didn't know about that. I will see about testing that fix tomorrow when I have free time. If it works then I will open another PR to revert this change and implement the proper fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants