Skip to content

Support Debian-specific install dir for ament_cmake_python#431

Merged
cottsay merged 2 commits intoament:rollingfrom
roehling:python-install-dir
Mar 28, 2023
Merged

Support Debian-specific install dir for ament_cmake_python#431
cottsay merged 2 commits intoament:rollingfrom
roehling:python-install-dir

Conversation

@roehling
Copy link
Copy Markdown
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.

Signed-off-by: Timo Röhling <roehling@debian.org>
@roehling roehling requested a review from mjeronimo as a code owner March 1, 2023 10:49
@clalancette
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Signed-off-by: Timo Röhling <roehling@debian.org>
@cottsay
Copy link
Copy Markdown
Contributor

cottsay commented Mar 28, 2023

Thanks!

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

Copy link
Copy Markdown
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