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: add support for DESTDIR install relocation #5152

Merged
merged 3 commits into from
Oct 12, 2021

Conversation

fmoessbauer
Copy link
Contributor

CMake might change the value of CMAKE_INSTALL_PREFIX on the fly at
install time in case the install is relocated via DESTDIR.

This is only supported in case the paths of the install target are
encoded as relative path.
These paths are anyways located relative to the CMAKE_INSTALL_PREFIX,
so manually specifying the prefix there is not required.

This patch enables a user to install multiple DynamoRIO versions
simultaneously and to switch between them by using tools like xstow.
It also helps as a preparation to package DynamoRIO for Linux
distributions like Debian.

Signed-off-by: Felix Moessbauer felix.moessbauer@siemens.com

Copy link
Contributor

@derekbruening derekbruening left a comment

Choose a reason for hiding this comment

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

Thank you for the patch. I had a few comments/requests/questions.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
api/samples/CMakeLists.txt Outdated Show resolved Hide resolved
@derekbruening
Copy link
Contributor

add to whitelist

@derekbruening
Copy link
Contributor

run arm tests

CMake might change the value of CMAKE_INSTALL_PREFIX on the fly at
install time in case the install is relocated via DESTDIR.

This is only supported in case the paths of the install target are
encoded as relative path.
These paths are anyways located relative to the CMAKE_INSTALL_PREFIX,
so manually specifying the prefix there is not required.

This patch enables a user to install multiple DynamoRIO versions
simultaneously and to switch between them by using tools like xstow.
It also helps as a preparation to package DynamoRIO for Linux
distributions like Debian.

Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
This patch replaces the install(CODE) commands that generate
files on the fly.
Instead, the files are generated at configuration time and just copied
at install time. By that, install relocation via various strategies like
DESTDIR and CPack works correctly. Further, this gets rid of a lot of
complex escaping and makes the results more predictable.

When appending to cmake-generated files, we also ensure that the file
is cleared on a re-configuration run to avoid duplicates in the file.

Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
@fmoessbauer
Copy link
Contributor Author

The initial strategy did not work under e.g. CPack.
The new strategy gets rid of the install(CODE file(...)) commands and generates the files at configuration time.
By that, the standard CMake install(FILES) can be used which is able to handle all the possible install relocations correctly.

In general, the install(CODE) approach should ONLY be used for trivial and idempotent code, like displaying messages, etc....

One thing I stumbles upon, but let it as it is:

DR_target_install(CODE "file(APPEND \"${PROJECT_BINARY_DIR}/CMakeFiles/Export/cmake/${exported_targets_name}.cmake\" \"${exported_targets_append}\")")

There, files in the PROJECT_BINARY_DIR are modified. This just works because the command will be executed prior to the target install (can be checked by reading cmake_install.cmake. Note, that this relies on CMake internals.

@derekbruening : Could you please trigger the pipeline, so I can compare the generated packages?

CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@derekbruening derekbruening merged commit 09ad4c0 into DynamoRIO:master Oct 12, 2021
@derekbruening
Copy link
Contributor

Thank you for the contribution.

derekbruening pushed a commit that referenced this pull request Oct 12, 2021
…5152)

This patch replaces the install(CODE) commands that generate
files on the fly.
Instead, the files are generated at configuration time and just copied
at install time. By that, install relocation via various strategies like
DESTDIR and CPack works correctly. Further, this gets rid of a lot of
complex escaping and makes the results more predictable.

When appending to cmake-generated files, we also ensure that the file
is cleared on a re-configuration run to avoid duplicates in the file.

This patch enables a user to install multiple DynamoRIO versions
simultaneously and to switch between them by using tools like xstow.
It also helps as a preparation to package DynamoRIO for Linux
distributions like Debian.

Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>

Issue: #5153
@fmoessbauer fmoessbauer deleted the chore-destdir branch June 3, 2023 04:22
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