Skip to content

Use DESTDIR on ament_python_install_package()#323

Merged
clalancette merged 2 commits intomasterfrom
hidmic/use-destdir
Feb 26, 2021
Merged

Use DESTDIR on ament_python_install_package()#323
clalancette merged 2 commits intomasterfrom
hidmic/use-destdir

Conversation

@hidmic
Copy link
Copy Markdown

@hidmic hidmic commented Feb 25, 2021

Follow up after #316. This patch should solve the regression introduced in debian builds.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic added the bug Something isn't working label Feb 25, 2021
@hidmic hidmic requested a review from clalancette February 25, 2021 23:05
Copy link
Copy Markdown
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

Hooray for Rolling deb jobs 🎉

This is much better than catkin catching it right before the freeze date :)

\"${PYTHON_EXECUTABLE}\" setup.py install
--single-version-externally-managed
--prefix \"${install_dir}\"
--prefix \"\$ENV{DESTDIR}${CMAKE_INSTALL_PREFIX}\"
Copy link
Copy Markdown
Contributor

@sloretz sloretz Feb 25, 2021

Choose a reason for hiding this comment

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

Catkin uses the --root option to pass DESTDIR, and --prefix to pass CMAKE_INSTALL_PREFIX - but I don't know why. Catkin is stable enough that I would recommend blindly copying that though 🙃

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good idea. See dbbd94f.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Copy Markdown
Author

hidmic commented Feb 26, 2021

Full CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@hidmic hidmic requested a review from sloretz February 26, 2021 14:11
@hidmic
Copy link
Copy Markdown
Author

hidmic commented Feb 26, 2021

Ready to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants