extensions: pre/append_dir: support rpath tokens by not using eval #3128
Conversation
This file contains 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
4 tasks
This commit changes the prepend_dir and append_dir functions to not use eval and to not check whether the directory exists when the path contains tokens. This makes it possible using rpath tokens in the LD_LIBRARY_PATH, so that it can contain paths which support multi-arch binaries.
|
@sergiusens I don't know why travis keeps failing on the katkin plugin tests. I restarted it about three times now, but I still get the same result. Do you know what's going on here? |
|
Thanks for the commit, the catkin failure seems to be just a 404 on the ROS repo, so probably an archive update (in case you were unaware, the error can be unfolded and inspected, but in any case I have no expectation of you having to know what this unrelated error is all about) |
cjp256
approved these changes
May 26, 2020
sergiusens
added
bug
|
lgtm!! I built this, then used this snapcraft to build photoscape (which uses sommelier-core that implements the rtokens) and all looked fine to me. |
sergiusens
approved these changes
May 28, 2020
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.
./runtests.sh static?./runtests.sh tests/unit?This commit changes the
prepend_dirandappend_dirfunctions to not useevaland to not check whether the directory exists when the path contains tokens. This makes it possible to use rpath tokens in theLD_LIBRARY_PATH. These tokens are useful to create paths which support multi-arch apps.More info about rpath tokens: ld.so man page, Using LD_PRELOAD mixed 64bit/32bit environment in Linux.
These versions of
pre/append_dirand rpath tokens are already used in the sommelier-core script to support Wine, which uses both 32-bit and 64-bit libraries when it runs 32-bit windows apps. The PhotoScape snap is built using sommelier-core.Note about compatibility
This PR makes sure we don't break things when earlier scripts in the command-chain use rpath tokens in
LD_LIBRARY_PATH.However, we can't yet use rpath tokens in
LD_LIBRARY_PATHindesktop-helpersitself, because later scripts in the command-chain might still use the oldpre/append_dirfunctions. At least one snap on GitHub uses the old functions together with a gnome extension.Using rpath tokens in desktop-helpers will thus break backwards compatibility; we can only use it for new extensions and/or new bases.
Note: This PR is another small step towards having better tooling to support 32-bit desktop apps in snapcraft.