-
-
Notifications
You must be signed in to change notification settings - Fork 97
Fix install dirs wrt binary relocatability #1624
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4517122
to
7b315be
Compare
dominiklohmann
commented
May 4, 2021
9d0c3ec
to
cf7c101
Compare
90aa01a
to
7f3d60d
Compare
tobim
requested changes
May 6, 2021
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 like the installdirs
abstraction 👍
Just two small things that I think can be improved.
This commit solves a multitude of issues we've had in one fell swoop: - Data, configuration and library dirs interpreted relative to the VAST binary file are no longer specified as fixed paths. E.g., we now support running integration tests and relocatable binaries on systems where the libary install directory is `lib64` instead of `lib`. - Non-relocatable VAST binaries no longer look for configuration, schemas, and plugins in directories relative to the binary location. - Relocatable VAST binaries no longer look for configuration, schemas, and plugins in their original install directory, and instead always use paths relative to their binary location. - Non-relocatable VAST binaries on macOS were broken for a while. Since there is no use case for them we now always build relocatable binaries on macOS. - With `--bare-mdoe` enabled, VAST now unloads static plugins that are not explicitly specified in an explicitly specified configuration file. This allows for running VAST's test suites for static VAST binaries.
The CMAKE_INSTALL_PREFIX can be overriden for installations, but we rely on the paths we know at build time for non-relocatable builds. This adds a custom installation step that detects such mismatches and fails the installation.
7f3d60d
to
96205ed
Compare
96205ed
to
b167102
Compare
tobim
approved these changes
May 7, 2021
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📔 Description
This PR solves a multitude of issues we've had in one fell swoop:
Data, configuration and library dirs interpreted relative to the VAST binary file are no longer specified as fixed paths. E.g., we now support running integration tests and relocateable binaries on systems where the libary install directory is
lib64
instead oflib
.Non-relocatable VAST binaries no longer look for configuration, schemas, and plugins in directories relative to the binary location.
Relocatable VAST binaries no longer look for configuration, schemas, and plugins in their original install directory, and instead always use paths relative to their binary location.
Non-relocatable VAST binaries on macOS were broken for a while. Since there is no use case for them we now always build relocatable binaries on macOS.
With
--bare-mdoe
enabled, VAST now unloads static plugins that are not explicitly specified in an explicitly specified configuration file. This allows for running VAST's test suites for static VAST binaries.📝 Checklist
🎯 Review Instructions
FIle-by-file. Test carefully.
The plugin loading change can be tested by building the static binary with the PCAP plugin enabled, which fails the
vast dump
integration test without this change.Note that this change is not of very high priority, so I don't mind this sitting a bit until we have time to test it carefully.