Skip to content

Support Debian-specific install dir for ament_cmake_python #431

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

Merged
merged 2 commits into from
Mar 28, 2023

Conversation

roehling
Copy link
Contributor

@roehling roehling commented Mar 1, 2023

Debian recently added a new default scheme posix_local for Python installations, which hardcodes /usr/local and ignores the base variable, thereby breaking the PYTHON_INSTALL_DIR discovery in ament_cmake_python.

This PR will explicitly use the deb_system scheme if available (which matches the old behavior on Debian and Ubuntu) and fall back to the default scheme for other OS.

@roehling roehling requested a review from mjeronimo as a code owner March 1, 2023 10:49
@clalancette
Copy link
Contributor

@cottsay @sloretz Could you take a quick look at this and see whether this is what we want to do? This may or may not be related to #386 .

@cottsay
Copy link
Contributor

cottsay commented Mar 13, 2023

We attempted to address this in #386 but it has continually fallen off our radar. I'll find some time to look at this PR this week.

@cottsay cottsay self-assigned this Mar 23, 2023
@cottsay
Copy link
Contributor

cottsay commented Mar 28, 2023

"this week" came and went, but I'm here now.

Can we update this to take a similar approach to colcon? In particular, it would be preferable not to rely on private API in sysconfig: https://github.com/colcon/colcon-core/blob/master/colcon_core/python_install_path.py

@cottsay
Copy link
Contributor

cottsay commented Mar 28, 2023

Thanks!

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Copy link
Contributor

@cottsay cottsay left a comment

Choose a reason for hiding this comment

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

Thanks!

Closes #386

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.

4 participants