Klipper3d / klipper

Klipper is a 3d-printer firmware
GNU General Public License v3.0
8.95k stars 5.16k forks source link

Add setuptools to klippy requirements for python 3.12 #6550

Closed nelsongraca closed 2 months ago

nelsongraca commented 2 months ago

In Python 3.12 setuptools is no longer installed by default on virtual environment creation (see https://docs.python.org/3/whatsnew/3.12.html and https://github.com/python/cpython/issues/95299) This is breaking Klipper when connecting through CAN due to missing dependencies.

This PR adds setuptools conditionally like greenlet already is.

Klipper error as well to properly show the issue:

Unhandled exception during connect
Traceback (most recent call last):
  File "/opt/klipper/klippy/klippy.py", line 176, in _connect
    self.send_event("klippy:mcu_identify")
  File "/opt/klipper/klippy/klippy.py", line 263, in send_event
    return [cb(*params) for cb in self.event_handlers.get(event, [])]
            ^^^^^^^^^^^
  File "/opt/klipper/klippy/mcu.py", line 763, in _mcu_identify
    self._serial.connect_canbus(self._serialport, nodeid,
  File "/opt/klipper/klippy/serialhdl.py", line 112, in connect_canbus
    import can # XXX
    ^^^^^^^^^^
  File "/opt/venv/lib/python3.12/site-packages/can/__init__.py", line 31, in <module>
    from .io import Logger, Printer, LogReader, MessageSync
  File "/opt/venv/lib/python3.12/site-packages/can/io/__init__.py", line 11, in <module>
    from .logger import Logger
  File "/opt/venv/lib/python3.12/site-packages/can/io/logger.py", line 13, in <module>
    from .asc import ASCWriter
  File "/opt/venv/lib/python3.12/site-packages/can/io/asc.py", line 19, in <module>
    from ..util import channel2int
  File "/opt/venv/lib/python3.12/site-packages/can/util.py", line 23, in <module>
    from can.interfaces import VALID_INTERFACES
  File "/opt/venv/lib/python3.12/site-packages/can/interfaces/__init__.py", line 8, in <module>
    from pkg_resources import iter_entry_points
ModuleNotFoundError: No module named 'pkg_resources'
flowerysong commented 2 months ago

My initial reaction was negative: this is a python-can dependency, not a Klipper dependency. However, Klipper is pinned to an old version of python-can, and the current version does not have this import. The best way to resolve this error would be to move to a newer version of python-can, but that's a lot more work and would entail dropping support for Python < 3.7 (which is a good idea, but not a trivial decision.)

I think adding it as a dependency here is an acceptable workaround, but there should be a comment in the file about why this dependency is there. It also shouldn't be a strict version or marked as dependent on the version of Python, since it's always a dependency even if there was previously a non-dependency mechanism that installed it in some circumstances.

Arksine commented 2 months ago

If newer versions of python-can do not include this import then its version could be pinned conditionally, presuming there are no changes that break the API.

nelsongraca commented 2 months ago

@Arksine I do like your suggestion, I have updated python-can to 4.3.1 communication is working on my printer and currently printing.

The question that remains is, should I change this PR or just drop it and create a new one.

nelsongraca commented 2 months ago

Dropped, please see: https://github.com/Klipper3d/klipper/pull/6557