Skip to content
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

Check for pip configuration when an externally managed environment is detected. #979

Merged
merged 24 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
696be7b
Raise an error when an externally managed environment is detected.
nuclearsandwich Aug 8, 2024
c121579
Fix typos and punctuation.
nuclearsandwich Aug 8, 2024
e9ad440
Use tuple rather than list literal.
nuclearsandwich Aug 8, 2024
8ce4a75
Consolidate version check.
nuclearsandwich Aug 9, 2024
2c76686
Pass necessary environment variable via sudo.
nuclearsandwich Aug 9, 2024
29ec176
Update test to expect sudo --preserve-env for pip.
nuclearsandwich Aug 9, 2024
48b9da2
flake8 cleanup
nuclearsandwich Aug 9, 2024
a5e10e7
Run pip tests with PIP_BREAK_SYSTEM_PACKAGES=1.
nuclearsandwich Aug 9, 2024
f042758
Add documentation for pip configuration.
nuclearsandwich Aug 9, 2024
a2adfe8
Add doc link to error output.
nuclearsandwich Aug 9, 2024
5b2cc64
Use inline monospace font to refer to rosdep the cli tool.
nuclearsandwich Aug 9, 2024
73ed28a
Briefly note that sudo configuration could prevent this from working.
nuclearsandwich Aug 9, 2024
f1dffc6
Recommend a specific config file to use and format user config as a w…
nuclearsandwich Aug 9, 2024
f685769
Fix errors in config checker.
nuclearsandwich Aug 9, 2024
65c7152
Change formatting of rosdep.
nuclearsandwich Aug 9, 2024
f8472d2
Complete a sentence I stopped writing.
nuclearsandwich Aug 9, 2024
70775f4
Add period to end of sentence.
nuclearsandwich Aug 9, 2024
25ac0e0
Add period to end of sentence.
nuclearsandwich Aug 9, 2024
8c57dba
Invert conditional for an earlier return.
nuclearsandwich Aug 9, 2024
56a745a
Edit text for clarity and typos.
nuclearsandwich Aug 9, 2024
a2e9a1e
Reflow conditional for easier reading.
nuclearsandwich Aug 10, 2024
f14c6de
Fix control flow after inverting the conditional.
nuclearsandwich Aug 10, 2024
8b48f13
Add test to confirm that get_install_command handles externally manag…
nuclearsandwich Aug 10, 2024
8e94ef1
Use ConfigParser.getboolean to check config value.
nuclearsandwich Aug 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/contents.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ Contents
sources_list
developers_guide
rosdep2_api
pip_and_pep_668
74 changes: 74 additions & 0 deletions doc/pip_and_pep_668.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
Pip installation after PEP 668
==============================

`PEP-668`_ introduced `externally managed environments <externally-managed-environments>`_ to Python packaging.

rosdep is designed to use pip as an alternative system package manager, rosdep installation of pip packages requires installing packages globally as root.
Starting with Python 3.11, `PEP-668`_ compliance requires you to allow pip to install alongside externally managed packages using the ``break-system-packages`` option.

There are multiple ways to configure pip so that rosdep will succeed.


Configure using environment variable
------------------------------------

This is the way that we recommend configuring pip for rosdep usage.
We recommend configuring pip using the system environment.
Setting environment variables in your login profile, ``PIP_BREAK_SYSTEM_PACKAGES`` in your environment.
The value of the environment variable can be any of ``1``, ``yes``, or ``true``.
The string values are not case sensitive.

rosdep is designed to use ``sudo`` in order to gain root privileges for installation when not run as root.
If your system's sudo configuration prohibits the passing of environment variables use the :ref:`pip.conf <configure-using-pip.conf>` method below.


.. _configure-using-pip.conf:

Configure using pip.conf
------------------------

`Pip configuration files <pip-configuration>`_ can be used to set the desired behavior.
Pip checks for global configuration files in ``XDG_CONFIG_DIRS``, as well as ``/etc/pip.conf``.
For details on ``XDG_CONFIG_DIRS`` refer to the `XDG base directories specification <xdg-base-dirs>`_.
If you're unsure which configuration file is in use by your system, ``/etc/pip.conf`` seems like the most generic.

.. code-block:: ini

[install]
break-system-packages = true


.. warning:: Creating a pip.conf in your user account's ``XDG_CONFIG_HOME`` (e.g. ``~/.config/pip/pip.conf``) does not appear to be sufficent when installing packages globally.


Configuring for CI setup
------------------------

Either environment variables or configuration files can be used with your CI system.
Which one you choose will depend on how your CI environment is configured.
Perhaps the most straightforward will be to set the environent variable in the shell or script execution context before invoking ``rosdep``.

.. code-block:: bash

sudo rosdep init
rosdep update
PIP_BREAK_SYSTEM_PACKAGES=1 rosdep install -r rolling --from-paths src/

If ``rosdep`` is invoked by internal processes in your CI and you need to set the configuration without having direct control over how ``rosdep install`` is run, setting the environment variable globally would also work.

.. code-block:: bash

export PIP_BREAK_SYSTEM_PACKAGES=1
./path/to/ci-script.sh


If you cannot set environment variables but you can create configuration files, you can set ``/etc/pip.conf`` with the necessary configuration.

.. code-block:: bash

printf "[install]\nbreak-system-packages = true\n" | sudo tee -a /etc/pip.conf

.. _PEP-668: https://peps.python.org/pep-0668/
.. _pip-configuration: https://pip.pypa.io/en/stable/topics/configuration/
.. _externally-managed-environments: https://packaging.python.org/en/latest/specifications/externally-managed-environments/
.. _xdg-base-dirs: https://specifications.freedesktop.org/basedir-spec/latest/
59 changes: 59 additions & 0 deletions src/rosdep2/platforms/pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
import subprocess
import sys

from configparser import ConfigParser
from pathlib import Path

try:
import importlib.metadata as importlib_metadata
except ImportError:
Expand All @@ -43,6 +46,17 @@
# pip package manager key
PIP_INSTALLER = 'pip'

EXTERNALLY_MANAGED_EXPLAINER = """
rosdep installation of pip packages requires installing packages globally as root.
When using Python >= 3.11, PEP 668 compliance requires you to allow pip to install alongside
externally managed packages using the 'break-system-packages' option.
The recommeded way to set this option when using rosdep is to set the environment variable
PIP_BREAK_SYSTEM_PACKAGES=1
in your environment.

For more information refer to http://docs.ros.org/en/independent/api/rosdep/html/pip_and_pep_668.html
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this documentation won't actually be live until the PR is merged and docs rebuild the next day. This will only affect those using rosdep from mainline or this branch. I'll wait until the docs rebuild to make a new rosdep release with this change.

"""


def register_installers(context):
context.set_installer(PIP_INSTALLER, PipInstaller())
Expand Down Expand Up @@ -70,6 +84,45 @@ def get_pip_command():
return None


def externally_managed_installable():
"""
PEP 668 enacted in Python 3.11 blocks pip from working in "externally
managed" environments such as operating systems with included package
managers. If we're on Python 3.11 or greater, we need to check that pip
is configured to allow installing system-wide packages with the
flagrantly named "break system packages" config option or environment
variable.
"""

# This doesn't affect Python versions before 3.11
if sys.version_info < (3, 11):
return True

if (
'PIP_BREAK_SYSTEM_PACKAGES' in os.environ and
os.environ['PIP_BREAK_SYSTEM_PACKAGES'].lower() in ('yes', '1', 'true')
):
return True

# Check the same configuration directories as pip does per
# https://pip.pypa.io/en/stable/topics/configuration/
pip_config = ConfigParser()
if 'XDG_CONFIG_DIRS' in os.environ:
for xdg_dir in os.environ['XDG_CONFIG_DIRS'].split(':'):
pip_config_file = Path(xdg_dir) / 'pip' / 'pip.conf'
pip_config.read(pip_config_file)
if pip_config.getboolean('install', 'break-system-packages', fallback=False):
return True

fallback_config = Path('/etc/pip.conf')
pip_config.read(fallback_config)
if pip_config.getboolean('install', 'break-system-packages', fallback=False):
return True
# On Python 3.11 and later, when no explicit configuration is present,
# global pip installation will not work.
return False


def is_cmd_available(cmd):
try:
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
Expand Down Expand Up @@ -131,6 +184,10 @@ class PipInstaller(PackageManagerInstaller):
def __init__(self):
super(PipInstaller, self).__init__(pip_detect, supports_depends=True)

# Pass necessary environment for pip functionality via sudo
if self.as_root and self.sudo_command != '':
self.sudo_command += ' --preserve-env=PIP_BREAK_SYSTEM_PACKAGES'

def get_version_strings(self):
pip_version = importlib_metadata.version('pip')
# keeping the name "setuptools" for backward compatibility
Expand All @@ -145,6 +202,8 @@ def get_install_command(self, resolved, interactive=True, reinstall=False, quiet
pip_cmd = get_pip_command()
if not pip_cmd:
raise InstallFailed((PIP_INSTALLER, 'pip is not installed'))
if not externally_managed_installable():
raise InstallFailed((PIP_INSTALLER, EXTERNALLY_MANAGED_EXPLAINER))
packages = self.get_packages_to_install(resolved, reinstall=reinstall)
if not packages:
return []
Expand Down
19 changes: 18 additions & 1 deletion test/test_rosdep_pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,23 @@ def test_PipInstaller_get_depends():
assert ['foo'] == installer.get_depends(dict(depends=['foo']))


@patch('rosdep2.platforms.pip.externally_managed_installable')
def test_PipInstaller_handles_externally_managed_environment(externally_managed_installable):
from rosdep2 import InstallFailed
from rosdep2.platforms.pip import EXTERNALLY_MANAGED_EXPLAINER, PipInstaller
Comment on lines +72 to +73
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not super up on why imports are done per test function here but I am propagating the convention until I understand it better.


externally_managed_installable.return_value = False
installer = PipInstaller()
try:
installer.get_install_command(['whatever'])
assert False, 'should have raised'
except InstallFailed as e:
assert e.failures == [('pip', EXTERNALLY_MANAGED_EXPLAINER)]
Comment on lines +77 to +81
Copy link
Member

Choose a reason for hiding this comment

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

If you want, a more Pythonic way could be:

Suggested change
try:
installer.get_install_command(['whatever'])
assert False, 'should have raised'
except InstallFailed as e:
assert e.failures == [('pip', EXTERNALLY_MANAGED_EXPLAINER)]
with pytest.raises(InstallFailed) as exception_info:
installer.get_install_command(['whatever'])
assert exception_info.value.failures == [('pip', EXTERNALLY_MANAGED_EXPLAINER)]

and make sure to import pytest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. I'm actually going to keep this as is for now, because I don't believe the infrastructure packages ever explicitly require pytest. I think pytest is supposed to be compatible with stdlib unittest as well and utilizing that may be the preferred approach.

externally_managed_installable.return_value = True
assert installer.get_install_command(['whatever'], interactive=False)


@patch.dict(os.environ, {'PIP_BREAK_SYSTEM_PACKAGES': '1'})
nuclearsandwich marked this conversation as resolved.
Show resolved Hide resolved
def test_PipInstaller():
from rosdep2 import InstallFailed
from rosdep2.platforms.pip import PipInstaller
Expand Down Expand Up @@ -104,7 +121,7 @@ def test(expected_prefix, mock_method, mock_get_pip_command):
try:
if hasattr(os, 'geteuid'):
with patch('rosdep2.installers.os.geteuid', return_value=1):
test(['sudo', '-H'])
test(['sudo', '-H', '--preserve-env=PIP_BREAK_SYSTEM_PACKAGES'])
with patch('rosdep2.installers.os.geteuid', return_value=0):
test([])
else:
Expand Down
Loading