-
Notifications
You must be signed in to change notification settings - Fork 29
Initial changes exploring python3.14 support #118
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
base: master
Are you sure you want to change the base?
Conversation
makes sense |
| def install(self, requirements, extra_pip_args): | ||
| """Purge the cache first before installing.""" # (KLAD) testing to debug an issue on build farm | ||
| command = [self._venv_bin("python"), "-m", "pip", "cache", "purge"] | ||
| command = [self._venv_bin(self.python), "-m", "pip", "cache", "purge"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is weird, because AFAIK, whatever python interpreter created the python, will be symlinked into venv/bin/python, and so used by self._venv_bin("python")
# python3.14 -m venv venv
# ll venv/bin/python
lrwxrwxrwx 1 pbovbel pbovbel 10 Nov 12 11:57 venv/bin/python -> python3.14*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a number of cases that might have different system expressions, including the fresh build, re-build, builds fail, or calling install, lock, etc. independently.
I might have been doing something wrong, but I was running into cases that had pythonreferring to 3.10 and not the version we're trying to target with PYTHON_INTERPRETER.
Being explicit is probably good here anyways, given just how complex this all gets and how brittle Python versioning management feels. One less thing to reason about when stuff breaks.
Problem 1
The first problem I ran into when trying to run
rospywith Python 3.14 is that it wouldn't build. I chased down errors and found that it was trying to utilize a different version of python to do the building than the version we wanted to actually run with. I think this just generally creates a lot of potential for compatibility issues.In fact, my version of Ubuntu 22.04 doesn't even have
pythonavailable. I dunno if that was a bug or is part of Python2 being discontinued, but it caused the changed code, when reached, to error. When I manually set it topython3.10I got new kinds of errors related to pip and all that. I think it generally is a bad idea to use Python 3.10 to construct a virtualenv and gather all the dependencies for a Python3.14 project.Instead of calling
pythonby string name, we use the target version of Python, which is passed in fromCMakeLists.txtviaargparse. To do this in a number of functions, we movepythonto the constructor args and make it an instance member.Problem 2
Once I started using this further, I found that there is a further problem with
setuptoolsmissing. This came when trying to depend onlocus_py. I found thatsetuptoolsis no-longer pre-installed in venvs (Changed in version 3.12: setuptools is no longer a core venv dependency.)I hacked away at
venv.pyto installsetuptoolsas part ofpreinstall. Also went to a newer version ofpipandpip-tools. Pretty sure this breaks Python2 support.Problem 3
When depending on
tycho_pyI ran into install failure issues withnumpy. Meson complains:../meson.build:41:12: ERROR: Python dependency not foundI tried updating
numpyintycho_pyrequirements.txtto the latest version:2.3.4and get a version resolution error:It looks like the issue here is due to the design of
catkin_virtualenv, which usesrequirements.txtfound in every dependency that also usescatkin_virtualenv. Consider if a maintainer has pinnedrequirements.in->requirements.txtbecause when that package's scripts are run, it wants to usepython3.10, sonumpywas pinned to2.2.6. Great! But if another package wants to use that package's python modules withpython3.14, trying to usenumpy=2.2.6breaks because that's not valid.This is why lockfiles (eg.
npm'spackage-lock.json) are generated for the target package, containing the entire tree of dependencies, and do this from the non-lockedpackage.json. If a dependency maintainer has opinions on dependency versions, they define that inpackage.jsonwith, ideally, the widest ranges possible. I think this should be inrequirements.in.I'm wondering if
catkin_virtualenvshould:requirements.txtin favour ofrequirements.infor all dependenciesrequirements.txtto include the entire tree of dependenciesrequirements.txtif it exists for all dependencies, not even looking at dependency requirements files.requirements.txtno-longer satifies therequirements.intreeI'm also wondering how much wheel we're re-inventing when it comes to resolving dependency trees and lockfiles. As long as we're carefully respecting both the semantics of
requirements.inandrequirements.txtas well as the core intent, this still makes sense?Drafted Changes
Python Interpreter Use
All scripts now use the
PYTHON_INTERPRETERif defined. Otherwisepython3. Some scripts don't actually care about Python version, but having to reason about whenVirtualenvdoes and does not care is not a great design.setuptools
Changed some of the pip preinstall dependency versions and added
setuptoolsto be installed.Handling the lockfile issues
I'm trying to add an
EXPERIMENTAL_GENERATE_FULL_LOCKFILEflag to play with the idea of fundamentally changing how catkin_virtualenv generates and uses lockfiles. The idea is that the package being built is responsible for defining the whole lockfile for the entire dependency tree used for populating the venv. It stops readingrequirements.txtand starts readingrequirements.in.This will require making
requirements.inavailable in dependency projects. I'm not sure how we'd want to approach that. For now I'm hacking at this to prove the concept.However, it may also be as simple as changing the semantics of
pip_requirements, expecting it to point torequirements.inand not the lockfilerequirements.txt.Testing
You can test this with this repo: https://github.com/ablakey/rospypypi/tree/main Though you may have to add additionally complex dependencies to figure out what else is breaking where.
Other Thoughts
How was
catkin_virtualenvsupposed to work if it required every dependency to require the exact same version of a dependency from the lockfile, rather than finding a version that satisfies all of therequirements.infiles?