Use deb_system scheme when available to avoid 'local' in path

[sloretz]

Fixes #503

This PR makes colcon use deb_system scheme when getting python install paths if available. This works around the default python_local scheme on Ubuntu Jammy which was added in a Debian package patch. It does this with a new API get_python_install_path containing the logic for selecting the scheme.

I also added PythonScriptsPathEnvironment to get the python script install path. This should be bin on Linux if the scheme stuff is correct, but given that CPython packagers may mess with the schemes this gets it explicitly should that assumption not hold in the future.

[cottsay]

I believe this is the right fix - thanks for the investigation.

Please fix this in ament as well: https://github.com/ament/ament_cmake/blob/2bf27ce5ad01af340e1f4efd44232c1067d0dbda/ament_cmake_python/ament_cmake_python-extras.cmake#L49

[codecov]

Codecov Report

Merging #504 (54b34d6) into master (37cf8aa) will increase coverage by 0.01%.
The diff coverage is 94.28%.

@@            Coverage Diff             @@
##           master     #504      +/-   ##
==========================================
+ Coverage   81.77%   81.79%   +0.01%     
==========================================
  Files          59       60       +1     
  Lines        3540     3553      +13     
  Branches      676      680       +4     
==========================================
+ Hits         2895     2906      +11     
- Misses        599      600       +1     
- Partials       46       47       +1     

Impacted Files	Coverage Δ
colcon_core/python_install_path.py	75.00% <75.00%> (ø)
colcon_core/environment/path.py	100.00% <100.00%> (ø)
colcon_core/environment/pythonpath.py	100.00% <100.00%> (ø)
colcon_core/task/python/build.py	35.82% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37cf8aa...54b34d6. Read the comment docs.

[cottsay]

I verified this behavior on Jammy, Fedora, and RHEL 8, and the behavior is consistent after this patch.

Interestingly, I could only replicate the bug on Jammy, even though Fedora 35 also uses Python 3.10.

[sloretz]

Interestingly, I could only replicate the bug on Jammy, even though Fedora 35 also uses Python 3.10.

Looks like there's a Debian patch adding posix_local

https://salsa.debian.org/cpython-team/python3/-/blob/master/debian/patches/sysconfig-debian-schemes.diff#L19

[sloretz]

@cottsay I'm having some trouble with the bootstrap process on Jammy in an apptainer container. Would you be willing to check the bootstrap commands to see if I'm doing something wrong?

$ apptainer build --fakeroot  bad.sif repro.def
[...]
+ eval export PATH="/root/colcon-from-source/install/colcon-core/bin:$PATH"
+ export PATH=/root/colcon-from-source/install/colcon-core/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
+ unset _colcon_ordered_commands
+ unset _colcon_prefix_sh_source_script
+ unset _colcon_prefix_sh_COLCON_CURRENT_PREFIX
+ colcon build
Traceback (most recent call last):
  File "/root/colcon-from-source/install/colcon-core/bin/colcon", line 33, in <module>
    sys.exit(load_entry_point('colcon-core==0.8.1', 'console_scripts', 'colcon')())
  File "/root/colcon-from-source/install/colcon-core/bin/colcon", line 22, in importlib_load_entry_point
    for entry_point in distribution(dist_name).entry_points
  File "/usr/lib/python3.10/importlib/metadata/__init__.py", line 957, in distribution
    return Distribution.from_name(distribution_name)
  File "/usr/lib/python3.10/importlib/metadata/__init__.py", line 548, in from_name
    raise PackageNotFoundError(name)
importlib.metadata.PackageNotFoundError: No package metadata was found for colcon-core
FATAL:   While performing build: while running engine: exit status 1

repro.def

Bootstrap: docker
From: ubuntu:22.04

%post
  export DEBIAN_FRONTEND=noninteractive

  apt update
  apt install locales tzdata
  locale-gen en_US en_US.UTF-8
  update-locale LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8

  echo 'Etc/UTC' > /etc/timezone
  ln -sf /usr/share/zoneinfo/Etc/UTC /etc/localtime

  apt install -y --quiet --no-install-recommends \
    build-essential \
    cmake \
    git \
    python3-flake8 \
    python3-flake8-blind-except \
    python3-flake8-builtins \
    python3-flake8-class-newline \
    python3-flake8-comprehensions \
    python3-flake8-deprecated \
    python3-flake8-docstrings \
    python3-flake8-import-order \
    python3-flake8-quotes \
    python3-pip \
    python3-pytest \
    python3-pytest-cov \
    python3-pytest-repeat\
    python3-pytest-rerunfailures \
    python3-setuptools \
    wget \
    curl

  python3 -m pip install -U vcstool pip setuptools

  # Broken when installed from pip
  # python3 -m pip install -U colcon-common-extensions

  # Broken when bootstrapped from source
  mkdir ~/colcon-from-source && cd ~/colcon-from-source
  curl --output colcon.repos https://raw.githubusercontent.com/colcon/colcon.readthedocs.org/main/colcon.repos
  mkdir src
  vcs import src < colcon.repos

  # Not broken when bootstrapped with scheme fix
  cd src/colcon-core
  git checkout sloretz__sysconfig__scheme
  cd -

  curl --output requirements.txt https://raw.githubusercontent.com/colcon/colcon.readthedocs.org/main/requirements.txt
  pip install -r requirements.txt
  ./src/colcon-core/bin/colcon build --paths src/*
  . install/local_setup.sh
  colcon build

  mkdir -p ~/ros2_rolling/src
  cd ~/ros2_rolling
  wget https://raw.githubusercontent.com/ros2/ros2/master/ros2.repos
  vcs import src < ros2.repos

%environment
  export LANG=en_US.UTF-8
  export ROS_PYTHON_VERSION=3

[cottsay]

Looks like there's a Debian patch adding posix_local

I always figured this problem was setuptools' fault, but you're right, it looks like that patch is what's causing all this nonsense.

[cottsay]

I'm having some trouble with the bootstrap process on Jammy

This PR is missing an update to this use of sysconfig.get_path:

colcon-core/colcon_core/task/python/build.py

Lines 298 to 300 in ac294df

	def _get_python_lib(self, args):
	path = sysconfig.get_path('purelib', vars={'base': args.install_base})
	return os.path.relpath(path, start=args.install_base)

That seemed to resolve the problem for me.

[sloretz]

This PR is missing an update to this use of sysconfig.get_path:

That brings it a little closer, but there's still al local path install/colcon-core/local/bin/colcon here preventing me from bootstrapping because colcon isn't on PATH after sourcing the setup script.

[clalancette]

The latest change here (along with ament/ament_cmake#386) fixed the original problem for me. So I would say we are making progress.

[cottsay]

This PR is missing an update to this use of sysconfig.get_path:

That brings it a little closer, but there's still al local path install/colcon-core/local/bin/colcon here preventing me from bootstrapping because colcon isn't on PATH after sourcing the setup script.

I just tried from scratch, and I can no longer reproduce your error after you pushed the latest fix.

Here's the spot that needs to be updated to resolve the test failure:

colcon-core/test/test_environment_pythonpath.py

Lines 25 to 30 in d288238

	# Python path exists
	python_path = Path(
	sysconfig.get_path('purelib', vars={'base': prefix_path}))
	python_path.mkdir(parents=True)
	hooks = extension.create_environment_hooks(prefix_path, 'pkg_name')
	assert len(hooks) == 2

[cottsay]

Your new PythonScriptsPathEnvironment shouldn't be necessary. sysconfig.get_path('scripts', ...) shouldn't be returning anything different from <prefix>/bin, so the normal PathEnvironment should be finding the scripts and appending to PATH.

I can't reproduce your PATH problem. Please give me repro steps.

[sloretz]

I can't reproduce your PATH problem. Please give me repro steps.

Here they are. Maybe it's a newer version of setuptools using sysconfig.get_path internally? If so, maybe embracing the local is the way to go.

repro_binprob.def

Bootstrap: docker
From: ubuntu:22.04

%post
  export DEBIAN_FRONTEND=noninteractive

  apt update
  apt install locales tzdata
  locale-gen en_US en_US.UTF-8
  update-locale LC_ALL=en_US.UTF-8 LANG=en_US.UTF-8

  echo 'Etc/UTC' > /etc/timezone
  ln -sf /usr/share/zoneinfo/Etc/UTC /etc/localtime

  apt install -y --quiet --no-install-recommends \
    build-essential \
    cmake \
    git \
    python3-flake8 \
    python3-flake8-blind-except \
    python3-flake8-builtins \
    python3-flake8-class-newline \
    python3-flake8-comprehensions \
    python3-flake8-deprecated \
    python3-flake8-docstrings \
    python3-flake8-import-order \
    python3-flake8-quotes \
    python3-pip \
    python3-pytest \
    python3-pytest-cov \
    python3-pytest-repeat\
    python3-pytest-rerunfailures \
    python3-setuptools \
    wget \
    curl

  python3 -m pip install -U vcstool pip setuptools

  # Broken when installed from pip
  # python3 -m pip install -U colcon-common-extensions

  # Broken when bootstrapped from source
  mkdir /colcon-from-source && cd /colcon-from-source
  curl --output colcon.repos https://raw.githubusercontent.com/colcon/colcon.readthedocs.org/main/colcon.repos
  mkdir src
  vcs import src < colcon.repos

  # Not broken when bootstrapped with scheme fix
  cd src/colcon-core
  git checkout sloretz__sysconfig__scheme
  cd -

  curl --output requirements.txt https://raw.githubusercontent.com/colcon/colcon.readthedocs.org/main/requirements.txt
  pip install -r requirements.txt
  ./src/colcon-core/bin/colcon build --paths src/*
  . install/local_setup.sh
  echo $PYTHONPATH
  colcon build

  # Problem!
  # colcon is installed to `<prefix>/local/bin/colcon`

%environment
  export LANG=en_US.UTF-8
  export ROS_PYTHON_VERSION=3

Problem is colcon is installed to local/bin/colcon instead of bin/colcon

$ apptainer build --fakeroot binprob.sif repro_binprob.def
[...]
+ colcon build
/.post.script: 54: colcon: not found

[cottsay]

Here they are.

Thank you. I see the issue now.

Is there a corresponding patch to the python3-setuptools deb that gets overridden when we use setuptools from pip? It would be nice to understand why the behavior keeps flip-flopping like this.

maybe embracing the local is the way to go.

Yeah, that might be a good plan, in which case your PythonScriptsPathEnvironment will be needed. We'd also need to add an ament hook for non-colcon builds.

It's going to be strange having the executables split between bin and local/bin on Jammy like this. There will probably be fallout from code making assumptions.

[sloretz]

Is there a corresponding patch to the python3-setuptools deb that gets overridden when we use setuptools from pip? It would be nice to understand why the behavior keeps flip-flopping like this.

This patch in Jammy setuptools seems relevant (I couldn't find a web link)

combined with this code

colcon-core/colcon_core/task/python/build.py

Lines 302 to 304 in 37cf8aa

	def _append_install_layout(self, args, cmd):
	if 'dist-packages' in self._get_python_lib(args):
	cmd += ['--install-layout', 'deb']

install-layout.diff

--- a/setuptools/command/easy_install.py
+++ b/setuptools/command/easy_install.py
@@ -138,6 +138,8 @@ class easy_install(Command):
         ('local-snapshots-ok', 'l',
          "allow building eggs from local checkouts"),
         ('version', None, "print version information and exit"),
+        ('install-layout=', None, "installation layout to choose (known values: deb)"),
+        ('force-installation-into-system-dir', '0', "force installation into /usr"),
         ('no-find-links', None,
          "Don't load find-links defined in packages being installed"),
         ('user', None, "install in user site-package '%s'" % site.USER_SITE)
@@ -145,7 +147,7 @@ class easy_install(Command):
     boolean_options = [
         'zip-ok', 'multi-version', 'exclude-scripts', 'upgrade', 'always-copy',
         'editable',
-        'no-deps', 'local-snapshots-ok', 'version',
+        'no-deps', 'local-snapshots-ok', 'version', 'force-installation-into-system-dir'
         'user'
     ]
 
@@ -188,6 +190,10 @@ class easy_install(Command):
         self.pth_file = self.always_copy_from = None
         self.site_dirs = None
         self.installed_projects = {}
+        # enable custom installation, known values: deb
+        self.install_layout = None
+        self.force_installation_into_system_dir = None
+
         # Always read easy_install options, even if we are subclassed, or have
         # an independent instance created.  This ensures that defaults will
         # always come from the standard configuration file(s)' "easy_install"
@@ -259,6 +265,11 @@ class easy_install(Command):
         self.expand_basedirs()
         self.expand_dirs()
 
+        if self.install_layout:
+            if not self.install_layout.lower() in ['deb']:
+                raise DistutilsOptionError("unknown value for --install-layout")
+            self.install_layout = self.install_layout.lower()
+
         self._expand(
             'install_dir', 'script_dir', 'build_directory',
             'site_dirs',
@@ -285,6 +296,15 @@ class easy_install(Command):
         if self.user and self.install_purelib:
             self.install_dir = self.install_purelib
             self.script_dir = self.install_scripts
+
+        if self.prefix == '/usr' and not self.force_installation_into_system_dir:
+            raise DistutilsOptionError("""installation into /usr
+
+Trying to install into the system managed parts of the file system. Please
+consider to install to another location, or use the option
+--force-installation-into-system-dir to overwrite this warning.
+""")
+
         # default --record from the install command
         self.set_undefined_options('install', ('record', 'record'))
         # Should this be moved to the if statement below? It's not used
@@ -1322,11 +1342,28 @@ class easy_install(Command):
                 self.debug_print("os.makedirs('%s', 0o700)" % path)
                 os.makedirs(path, 0o700)
 
+    if sys.version[:3] in ('2.3', '2.4', '2.5') or 'real_prefix' in sys.__dict__:
+        sitedir_name = 'site-packages'
+    else:
+        sitedir_name = 'dist-packages'
+
     INSTALL_SCHEMES = dict(
         posix=dict(
             install_dir='$base/lib/python$py_version_short/site-packages',
             script_dir='$base/bin',
         ),
+        unix_local = dict(
+            install_dir = '$base/local/lib/python$py_version_short/%s' % sitedir_name,
+            script_dir  = '$base/local/bin',
+        ),
+        posix_local = dict(
+            install_dir = '$base/local/lib/python$py_version_short/%s' % sitedir_name,
+            script_dir  = '$base/local/bin',
+        ),
+        deb_system = dict(
+            install_dir = '$base/lib/python3/%s' % sitedir_name,
+            script_dir  = '$base/bin',
+        ),
     )
 
     DEFAULT_SCHEME = dict(
@@ -1337,11 +1374,18 @@ class easy_install(Command):
     def _expand(self, *attrs):
         config_vars = self.get_finalized_command('install').config_vars
 
-        if self.prefix:
+        if self.prefix or self.install_layout:
+            if self.install_layout and self.install_layout in ['deb']:
+                    scheme_name = "deb_system"
+                    self.prefix = '/usr'
+            elif self.prefix or 'real_prefix' in sys.__dict__:
+                scheme_name = os.name
+            else:
+                scheme_name = "posix_local"
             # Set default install_dir/scripts from --prefix
             config_vars = config_vars.copy()
             config_vars['base'] = self.prefix
-            scheme = self.INSTALL_SCHEMES.get(os.name, self.DEFAULT_SCHEME)
+            scheme = self.INSTALL_SCHEMES.get(scheme_name,self.DEFAULT_SCHEME)
             for attr, val in scheme.items():
                 if getattr(self, attr, None) is None:
                     setattr(self, attr, val)
@@ -1385,9 +1429,15 @@ def get_site_dirs():
             sitedirs.extend([
                 os.path.join(
                     prefix,
+                    "local/lib",
+                    "python" + sys.version[:3],
+                    "dist-packages",
+                ),
+                os.path.join(
+                    prefix,
                     "lib",
                     "python{}.{}".format(*sys.version_info),
-                    "site-packages",
+                    "dist-packages",
                 ),
                 os.path.join(prefix, "lib", "site-python"),
             ])
--- a/setuptools/command/install_egg_info.py
+++ b/setuptools/command/install_egg_info.py
@@ -1,5 +1,5 @@
 from distutils import log, dir_util
-import os
+import os, sys
 
 from setuptools import Command
 from setuptools import namespaces
@@ -18,14 +18,31 @@ class install_egg_info(namespaces.Instal
 
     def initialize_options(self):
         self.install_dir = None
+        self.install_layout = None
+        self.prefix_option = None
 
     def finalize_options(self):
         self.set_undefined_options('install_lib',
                                    ('install_dir', 'install_dir'))
+        self.set_undefined_options('install',('install_layout','install_layout'))
+        if sys.hexversion > 0x2060000:
+            self.set_undefined_options('install',('prefix_option','prefix_option'))
         ei_cmd = self.get_finalized_command("egg_info")
         basename = pkg_resources.Distribution(
             None, None, ei_cmd.egg_name, ei_cmd.egg_version
         ).egg_name() + '.egg-info'
+
+        if self.install_layout:
+            if not self.install_layout.lower() in ['deb']:
+                raise DistutilsOptionError("unknown value for --install-layout")
+            self.install_layout = self.install_layout.lower()
+            basename = basename.replace('-py%s' % pkg_resources.PY_MAJOR, '')
+        elif self.prefix_option or 'real_prefix' in sys.__dict__:
+            # don't modify for virtualenv
+            pass
+        else:
+            basename = basename.replace('-py%s' % pkg_resources.PY_MAJOR, '')
+
         self.source = ei_cmd.egg_info
         self.target = os.path.join(self.install_dir, basename)
         self.outputs = []

I'm going to grab lunch, but now looking at that setuptools patch I'm thinking another option is to use the scheme deb_system if it exists (instead of posix_prefix). Or maybe another option is to not change the --install-layout in the Python build task.

Feedback addressed

[cottsay]

• Linux
• Linux-aarch64
• Linux-RHEL
• Windows

[cottsay]

Thanks for iterating with me on this - I'm really happy with how this change turned out. The separated PythonScriptsPathEnvironment, though probably not necessary after changing to use deb_system opportunistically, should help reduce the scope of issues like this in the future, and it removed the explicit workaround for Windows.