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

cmake: set RPATH of installed mavsdk_server #2441

Merged
merged 1 commit into from
Nov 19, 2024
Merged

Conversation

julianoes
Copy link
Collaborator

This allows to install the mavsdk_server binary in any install prefix and run it.

Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

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

Won't this always inject an rpath, even when the intent is actually to use system dependencies?

Also does it inject relative paths or absolute paths? It feels like CMAKE_INSTALL_PREFIX may be absolute, right?

@julianoes
Copy link
Collaborator Author

I don't think you ever want to use system dependencies with mavsdk_server, because you build mavsdk, mavsdk_server, and mavsdk_server_bin at the same time and install it together, locally or system wide.

Yes, CMAKE_INSTALL_PREFIX is absolute, but it does work, and without it, it doesn't work.

This allows to install the mavsdk_server binary in any install prefix
and run it.
Copy link

sonarcloud bot commented Nov 15, 2024

@JonasVautherin
Copy link
Collaborator

and without it, it doesn't work.

Yeah without it you probably have to set LD_LIBRARY_PATH, which is inconvenient. I was just concerned that this may inject paths into some system packages people may distribute.

@julianoes
Copy link
Collaborator Author

Let's merge it and check the release once done.

@julianoes julianoes merged commit e840b3a into main Nov 19, 2024
39 checks passed
@julianoes julianoes deleted the pr-mavsdk-server-install branch November 19, 2024 07:53
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.

2 participants