prusa3d / Prusa-Firmware

Firmware for Original Prusa i3 3D printer by PrusaResearch
GNU General Public License v3.0
1.99k stars 1.05k forks source link

bootstrap.py: add option '--break-system-packages' to pip3 arguments to mitigate PEP 668 #4578

Closed ilka-schulz closed 4 months ago

ilka-schulz commented 5 months ago

See PEP 668 for the detailed specification.

This PR mitigates the issue that pip3 under Debian 12 will refuse to install packages and thus breaks bootstrap.py:

$ ./utils/bootstrap.py 
Downloading ninja-1.10.2
Extracting ninja-1.10.2
Downloading cmake-3.22.5
Extracting cmake-3.22.5
Downloading avr-gcc-7.3.0
Extracting avr-gcc-7.3.0
Downloading prusa3dboards-1.0.6
Extracting prusa3dboards-1.0.6
Installing Python package pyelftools
error: externally-managed-environment

× This environment is externally managed
╰─> To install Python packages system-wide, try apt install
    python3-xyz, where xyz is the package you are trying to
    install.

    If you wish to install a non-Debian-packaged Python package,
    create a virtual environment using python3 -m venv path/to/venv.
    Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make
    sure you have python3-full installed.

    If you wish to install a non-Debian packaged Python application,
    it may be easiest to use pipx install xyz, which will manage a
    virtual environment for you. Make sure you have pipx installed.

    See /usr/share/doc/python3.11/README.venv for more information.

note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
hint: See PEP 668 for the detailed specification.
Traceback (most recent call last):
  File "/home/user/Prusa-Firmware/./utils/bootstrap.py", line 202, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/user/Prusa-Firmware/./utils/bootstrap.py", line 195, in main
    run(sys.executable, '-m', 'pip', 'install', package,
  File "/home/user/Prusa-Firmware/./utils/bootstrap.py", line 103, in run
    process = subprocess.run([str(a) for a in cmd],
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/usr/bin/python3', '-m', 'pip', 'install', 'pyelftools', '--disable-pip-version-check']' returned non-zero exit status 1.

running again with the change of this PR:

$ ./utils/bootstrap.py 
Installing Python package pyelftools
Installing Python package polib
Installing Python package regex
github-actions[bot] commented 5 months ago

All values in bytes. Δ Delta to base

Target ΔFlash ΔSRAM Used Flash Used SRAM Free Flash Free SRAM
MK3S_MULTILANG 0 0 246786 5657 7166 2535
MK3_MULTILANG 0 0 246068 5664 7884 2528
sarusani commented 5 months ago

This is a brute force solution. It will break some users system packages for sure. (Hence the name)

You should use a virtual environment instead.

wavexx commented 5 months ago

Me too. I hate pip, but the suggestion it writes is correct: use the system package if available, or use a virtualenv. --break-system-packages can work in some scenarios, but for most users it's a sure-fire way to mess up python.

ilka-schulz commented 5 months ago

Ah, I understand the critic. I am a Qubes user so I never need to bother about broken packages, thus I overlooked the regression this PR would introduce. Maybe in that case, the script should offer some option to use --break-system-packages anyways? Using virtual environments sounds unnecessarily convoluted to me...

vintagepc commented 5 months ago

I would not do that. Keep in mind that users of this script aren't necessarily advanced users that understand the implications of what they are doing and so passively enabling them to pick the single wrong thing out of three possibilities isn't good.

We should either set up a virtualenv and use it, or offer them guidance on which packages to install via the system package manager in this scenario. Unfortunately it's tricky to automate the latter because not all distros use the same naming or package management scripts.

3d-gussner commented 5 months ago

@ilka-schulz After all the comments I added a won't fix label as we will not merge is PR as it is. Thanks for the PR anyway always appreciated. Maybe we get another solution for the issue because of it.

3d-gussner commented 4 months ago

Closing as we won't use this fix