Properly respect DESTDIR in the build env. #240
Conversation
The method is to inject _CATKIN_SETUP_DIR into the generated build_env.sh environment wrapper, allowing the build to proceed even when the installspace at build-time is not in its final runtime location.
|
Tried a few other build configurations, corrected an issue with building against isolated develspaces. |
| # Intentionally not using os.path.join in this instance, as it's expected that | ||
| # when DESTDIR is specified, the install space is an absolute path, so a straight | ||
| # concatenation is more suitable than os.path.join. | ||
| space_path = context.destdir + context.install_space_abs |
There was a problem hiding this comment.
A better explanation (in my opinion) is that since the install space is absolute (and it always is, not just when used in conjunction with DESTDIR), os.path.join will not return the concatenation but rather just the install space. It's not that this is more suitable than os.path.join (which to me implies os.path.join would also work) but instead that os.path.join doesn't work at all and therefore this is necessary, e.g.:
>>> import os
>>> a = '/tmp/build/'
>>> b = '/usr'
>>> os.path.join(a, b)
'/usr'
>>> a = '/tmp/build'
>>> os.path.join(a, b)
'/usr'
There was a problem hiding this comment.
Yeah, that was the reason, and you're right it should say so. I just didn't want it to become a novel. I can try again, or switch it to do what catkin does. What's your preference?
There was a problem hiding this comment.
I think this is fine. I might update the comment on merge.
There was a problem hiding this comment.
I updated it in 72c5f3e, let me know if you think that's incorrect.
|
Other than a comment about a comment (so meta) this lgtm. It seems to line up well as the equivalent to ros/catkin@7282a4b which made |
|
Since you're using |
Properly respect DESTDIR in the build env.
|
Sweet, thanks for the quick review. |
The method is to explicitly inject _CATKIN_SETUP_DIR into the generated build_env.sh environment wrapper, allowing the build to proceed even when the installspace at build-time is not in its final runtime location.
I'm open to suggestions for what tests could validate the behaviour here— the main scenario where it comes into play is when building two packages, one of which
find_packages the other. With DESTDIR specified, the build_env of the second package will haveCMAKE_PREFIX_PATHset incorrectly, and thefind_packagecall will fail. Perhaps there's room for a new type of test to be added which covers interactions between multiple packages in a workspace?This is a follow-on to #238, addressing #235.