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

Add to BUILD_PATH #606

Merged
merged 1 commit into from
Aug 4, 2023

Conversation

pnoltes
Copy link
Contributor

@pnoltes pnoltes commented Aug 3, 2023

This hotfix adds the $ORIGIN (loader_path) to the RPATH for bundle activator libraries during build.

Note that previously RPATH was set to $ORIGIN with target property BUILD_WITH_INSTALL_RPATH (see https://github.com/apache/celix/blob/rel/celix-2.3.0/cmake/cmake_celix/BundlePackaging.cmake#L341). But this brings issues while building and testing celix.

WIth BUILD_WITH_INSTALL_RPATH removed and because the bundle activator libs in Apache Celix bundles are never install (they are added into zip during build) the current used RPATH contains build paths and no $ORIGIN path.
If $ORIGIN in not part of the RPATH, libraries inside the bundles cannot be successfully loaded on targets, but this is not apparent during build (because the build paths set in RPATH do exist).

Adding $ORIGIN to the build RPATH solve the bundle loading issues, but is not an ideal solution because the build specific paths are also still in the RPATH for installed bundles.

Copy link
Contributor

@PengZheng PengZheng left a comment

Choose a reason for hiding this comment

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

This can work as a stop gap. An issue (#607) is opened for this.

Given that the issue is caused by the fact that libs in bundles are never installed, we can actually install them to a temporary location, then use install(SCRIPT <file> [...]) to compress the bundle at install time from installed binaries and do some cleanup (removing these temporary installed libraries). By installing libs to some temporary place, we utilize CMake's internal RPATH rewrite capability. We need install(SCRIPT <file> [...]) because sometimes simply copy things into the install area isn't enough. This is indeed the case for a complex custom target like Celix Bundle.

It seems that a refactor of install_celix_bundle should be enough. But I have not used install(SCRIPT <file> [...]) before.
The devil is always in the details.

I volunteered to send a fix for this issue introduced by myself.
I'll merge this right now so that the development workflow won't be interrupted, and will not delete this branch until a fix is submitted and merged.

@PengZheng PengZheng merged commit 8695bd7 into master Aug 4, 2023
22 checks passed
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