meson
meson copied to clipboard
modules/python: Deprecate find_installation without positional arg
When the name_or_path argument is omitted, the returned Python installation will be the one used to run Meson. But that is really an implementation detail that should not be useful to projects other than Meson itself. There can also be Meson implementation in other languages where returning anything does not make sense.
Moreover, some people started relying on various expectations like that:
- Meson uses Python 3.5 or newer
- Python used by Meson is guaranteed to provide both 'xml' and 'pkg_resources' modules
What is more, Meson can use a different Python interpreter than the one available in PATH so if the project needs some extra Python modules, it gets complicated. Using minimal bootstrapping Python for Meson, and cross-compilation are just two common use cases, where this can occur.
Applications should instead explicitly list their requirements by passing python3 as positional arg and modules as kwarg.
Fixes: https://github.com/mesonbuild/meson/issues/7415
Additionally, this PR makes python3 always fall back on Meson’s Python installation when everything else fails (partially fixing https://github.com/mesonbuild/meson/issues/4608), and adds support for python3 and python2 keys in native file as mentioned in https://github.com/mesonbuild/meson/commit/71a5f990d09f04d8eb8d636abf7e2b446fdf826a.
Codecov Report
Attention: Patch coverage is 61.90476% with 8 lines in your changes missing coverage. Please review.
Project coverage is 67.07%. Comparing base (
19684d2) to head (0d2f8bc). Report is 2719 commits behind head on master.
| Files | Patch % | Lines |
|---|---|---|
| mesonbuild/modules/python.py | 61.90% | 5 Missing and 3 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #9904 +/- ##
==========================================
- Coverage 67.78% 67.07% -0.71%
==========================================
Files 402 402
Lines 85522 85530 +8
Branches 18811 18815 +4
==========================================
- Hits 57969 57368 -601
- Misses 23059 23685 +626
+ Partials 4494 4477 -17
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Moreover, some people started relying on various expectations like that:
- Python used by Meson is guaranteed to provide both 'xml' and 'pkg_resources' modules
The former is true, the latter was rewritten to not rely on something that is technically deprecated in the sense that the authors consider using it to be a bad idea except in niche cases like setuptools itself.
Point being this is hardly a textbook example of why you'd want to check for modules. And in the case where you don't want to check for modules, you really don't care what version of python you find.
Anyway, I would rather somehow deprecate using module-less python as a command rather than an object with methods for building and installing things to site-packages, but I'm not sure what that would look like.
If you just want control of which python gets found, you can override python in a machine file already, without adding new machine file entries even. (Though I guess those couldn't hurt? idk)
that is really an implementation detail that should not be useful to projects other than Meson itself
It is an implementation detail, but find_installation() is the only way to have a default path to a Python interpreter without trying all of python, python3, python3.[6789], etc.
IMO find_installation without a positional arg should remain, and if anything it's the case with a positional arg that's yucky. However:
-
the documentation should mention that, if the positional arg is absent, Meson will first look for a
pythonkey in the host machine file. This is already the case but it's undocumented. -
there should be support for a
nativekeyword argument that causes Meson to look for apythonkey in the build machine file instead of the object file. When cross-compiling,native: falseshould not fall back tomesonlib.python_command. -
Meson should warn, or even error out, if
find_installation()falls back tomesonlib.python_commandand the basename (possibly minus.exe) does not match^python3?\b. This would reject RHEL's/usr/libexec/platform-pythonexecutable. RHEL would then drop a machine file in /usr/share/meson/native that points to/usr/bin/python3.
All this, especially the last point, needs to be very well documented.
I personally have been recommending the exact opposite, omitting the argument should be the sane default unless you have specific python requirements. Most of the time find_installation() is used to run python scripts, and using whatever meson is using is the right thing to do.
@bonzini
It is an implementation detail, but
find_installation()is the only way to have a default path to a Python interpreter without trying all ofpython,python3,python3.[6789], etc.
As I understand it, python3 is a special value meaning any Python 3 interpreter (though it currently does not look for python3.[6789]). https://github.com/mesonbuild/meson/commit/98cd045a68c9f6cf754e4b4763477259ef444f42 already changes it to use the Meson’s interpreter as a fallback.
- the documentation should mention that, if the positional arg is absent, Meson will first look for a
pythonkey in the host machine file. This is already the case but it's undocumented.
It currently always looks at the machine file as the first thing. I agree that I should document this, along with the fact that we now support python3 and python2 native keys.
- there should be support for a
nativekeyword argument that causes Meson to look for apythonkey in the build machine file instead of the object file. When cross-compiling,native: falseshould not fall back tomesonlib.python_command.
That sounds reasonable but I am not sure how to implement it in a way that modules kwarg would be also supported.
- Meson should warn, or even error out, if
find_installation()falls back tomesonlib.python_commandand the basename (possibly minus.exe) does not match^python3?\b. This would reject RHEL's/usr/libexec/platform-pythonexecutable. RHEL would then drop a machine file in /usr/share/meson/native that points to/usr/bin/python3.
That sounds reasonable for Unix systems but not sure about Windows (I would not be surprised if could be py.exe there).
@xclaesse
I personally have been recommending the exact opposite, omitting the argument should be the sane default unless you have specific python requirements.
Well, you almost always have specific python requirements – most scripts cannot run on Python 2. And whatever Meson is using does not really make sense for Meson implementations not written in Python.
Most of the time find_installation() is used to run python scripts, and using whatever meson is using is the right thing to do.
Using whatever Meson is using is definitely not the right thing to do – for example, on Nix that will be a completely bare Python without any modules. And requiring everyone to use machine file just to point python to the one on PATH is not very ergonomic.
If you think that not specifying any Python requirements is valuable we could drop the deprecation from https://github.com/mesonbuild/meson/commit/6f44ac0671aac9023269d908b2d0cbc0bbe0e1fc, which would change the order for find_installation without posarg to the following:
python3entry in machine filepythonentry in the machine filepython3onPATH- (On Windows) result of
_get_win_pythonpathonPATH. pythononPATHmesonlib.python_command
Instead of the current:
pythonentry in the machine filemesonlib.python_command
I remain unconvinced by this PR and by the linked ticket for several reasons, but I just want to highlight this one point in particular:
And whatever Meson is using does not really make sense for Meson implementations not written in Python.
Dear G-d what is this supposed to mean?
Meson doesn't have a contract that find_installation returns the python that Meson is using. No one relies on it using the python that Meson is using. The Meson DSL isn't violated or debased by corruption of intentions because muon returns an error if it doesn't find an external python, and cannot fall back to the one it is(n't) implemented in.
It is a fallback. A final quirk mode for attempting to figure out where a valid python installation is. Meson doesn't guarantee that "if a valid python isn't found, we'll return one anyways and this is a DSL contract", Meson guarantees that meson.py probably has a valid python and will check that too in its quest to find a valid python.
That is the only promise the docs give here. I utterly reject the premise that this PR is being done in defense and validation of muon's right to declare itself a compatible Meson DSL executor.
How, exactly, does removing support for find_installation without a posarg make muon "more correct because it isn't documented to return the implementation version of python"?
It's a total red herring.
On the order of lookup, I was under the impression that "python from the machine file" was actually "python from standard find_program style lookup, first from machine files and second from PATH", which implies the real issue is that you don't ship a python command together with python3.
I'm already completely unsympathetic to BSD doing that, so it's no great effort for me to expand my lack of sympathy to Nix too.
There's a PEP for this. Stop breaking python.
Well, you almost always have specific python requirements – most scripts cannot run on Python 2.
Meson requires >=3.5 so I'm guaranteed to have at least that. It's never going to give python2.
And whatever Meson is using does not really make sense for Meson implementations not written in Python.
Well... that's their problem to find a sane default then.
Using whatever Meson is using is definitely not the right thing to do – for example, on Nix that will be a completely bare Python without any modules.
And how is that a problem? What are the chances that you have a python installation with lots of modules and you're using another bare installation just for running meson?
And requiring everyone to use machine file just to point
pythonto the one onPATHis not very ergonomic.
You don't, that's the whole point. User already has to make sure they run Meson with the proper interpreter by executing python meson or python3 meson, so you can just use that.
@xclaesse the fact that /usr/bin/meson's shebang interpreter for python uses a custom PYTHONPATH that only includes meson itself, and thus differs from /usr/bin/python, is a legitimate use case and a real issue for Nix, which is not (and/or should not be) solved by invoking python /usr/bin/meson.
I happen to think it's an irrelevant distinction because it should be solved by making sure that python actually exists, but that doesn't mean the mesonlib.python_command() should be expected to have rando modules from outside the stdlib on Nix, just because it's system-dependent on other distros and may in fact actually have said rando modules.
I like the logic in https://github.com/mesonbuild/meson/pull/9904#issuecomment-1030083440, except that I would not want both python3 and python in the machine file. python should be enough.
On top of this, however, the same logic should be used for Meson's builtin support for the magic #!/usr/bin/env python3, so it should be in mesonlib rather than in the Python module. We might also consider extending the magic shebang to #!/usr/bin/env python.
What are the chances that you have a python installation with lots of modules and you're using another bare installation just for running meson?
That's actually the case for RHEL and derivatives (and for good reasons, see here for the issues that RHEL's solution eschews); so I am sympathetic to Nix on this front. However, it should be fixed at the distro level with machine files. It doesn't make mesonlib.python_command a bad fallback for everybody else.
@eli-schwartz
And whatever Meson is using does not really make sense for Meson implementations not written in Python.
Dear G-d what is this supposed to mean?
The docs literally say “if [the argument is] not provided then the returned python installation will be the one used to run Meson.”
That is the only promise the docs give here. I utterly reject the premise that this PR is being done in defense and validation of muon's right to declare itself a compatible Meson DSL executor.
The motivation is really the current unsuitable search order when find_installation is called without positional argument. The mention of alternative implementation was only given as an additional argument for this change given the statement in the current documentation.
On the order of lookup, I was under the impression that "
pythonfrom the machine file" was actually "pythonfrom standard find_program style lookup, first from machine files and second from PATH", which implies the real issue is that you don't ship apythoncommand together withpython3.I'm already completely unsympathetic to BSD doing that, so it's no great effort for me to expand my lack of sympathy to Nix too.
There's a PEP for this. Stop breaking python.
If you are referring to the PEP that specifies that python program should be Python 3, Nixpkgs does in fact respect that:
$ python --version
Python 3.9.9
We only pass a machine file to meson when cross-compiling and I would still expect Python from PATH to be picked up when a machine file is not used. Otherwise it will fail for users building Meson projects outside the package manager.
We only pass a machine file to meson when cross-compiling and I would still expect Python from PATH to be picked up when a machine file is not used. Otherwise it will fail for users building Meson projects outside the package manager.
You can drop a machine file in /usr/share/meson/native that describes the build machine, and it should work outside the package manager.
But yeah, they are not read by default. Perhaps there should be another directory (e.g. /usr/share/meson/native.d) that should be read by default...
Codecov Report
Attention: Patch coverage is 61.90476% with 8 lines in your changes missing coverage. Please review.
Project coverage is 67.07%. Comparing base (
19684d2) to head (0d2f8bc). Report is 2871 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| mesonbuild/modules/python.py | 61.90% | 5 Missing and 3 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #9904 +/- ##
==========================================
- Coverage 67.78% 67.07% -0.71%
==========================================
Files 402 402
Lines 85522 85530 +8
Branches 18811 18815 +4
==========================================
- Hits 57969 57368 -601
- Misses 23059 23685 +626
+ Partials 4494 4477 -17
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.