mesonbuild / meson-python

Meson PEP 517 Python build backend
https://mesonbuild.com/meson-python/
MIT License
118 stars 59 forks source link

`meson-python` does not append `editable` keyword before ABI tag(s) with `--editable` flag #620

Closed agriyakhetarpal closed 2 months ago

agriyakhetarpal commented 2 months ago

Description

The meson-python build backend does not seem to conform entirely with the specification noted by PEP 660, and currently builds Python packages in editable mode but without the editable keyword.

The wheel built after the compilation proceeds is named as follows:

Created wheel for PyWavelets: filename=pywavelets-1.7.0.dev0-cp312-cp312-linux_x86_64.whl size=<...> sha256=<...>

MWE / steps to reproduce

For the purpose of this bug report, a reproducer is as follows:

git clone https://github.com/PyWavelets/pywt
pip install "numpy>=2.0.0b1" "meson-python>=0.16.0" "Cython>=3.0.4"
sudo apt-get install ninja-build  # or pip install ninja
pip install -e . --no-build-isolation

Any other Python packages using meson as the underlying build system should also produce the same output.

Expected behaviour

The build backend should append the editable keyword at the get_requires_for_build_editable stage when the wheel gets created, therefore renaming the wheel to have this keyword before the ABI tag and after the version of the package, i.e., the wheel should be named as follows:

Created wheel for PyWavelets: filename=pywavelets-1.7.0.dev0.editable-cp312-cp312-linux_x86_64.whl size=<...> sha256=<...>

Other available and popular build backends – such as setuptools for compiled extensions and hatchling for pure Python projects correctly add the editable keyword.

Relevant logs or additional context

I noticed this when trying to test out the recent editable installations improvements (#569) that came out with the new 0.16.0 release. xref: https://github.com/PyWavelets/pywt/pull/702

eli-schwartz commented 2 months ago

I re-read the PEP and I don't see where it makes any statement about the wheel filename other than:

If you can see the filename to tell that it uses a different style than setuptools, that implies the wheel has somehow been exposed to you as an end user, which may be a pip bug.

dnicolodi commented 2 months ago

Can you please point me to where PEP660 specifies that editable must be appended to the version tag in the wheel filename? I re-read it quickly now and I don't find it. I actually see adding the editable local version identifier explicitly listed as a rejected idea https://peps.python.org/pep-0660/#editable-local-version-identifier

agriyakhetarpal commented 2 months ago

I offer my apologies – both of you are right (what I assumed here was that the PEP 660 specification is what is controlling how get_requires_for_build_editable is to be implemented such that it is standardised across all build backends, but I don't think the PEP offers a comment on whether get_requires_for_build_editable gets to control how the wheel is named based on the mode of installation being performed). Something different is going on; I haven't dug deeper yet as to how setuptools and other backends are right now performing the behaviour I suggested, so even if this isn't coming from PEP 660, I still feel that this is a valid report of a discrepancy in behaviour across backends where meson-python does not comply. I'll edit the issue title to remove this note.

I actually see adding the editable local version identifier explicitly listed as a rejected idea

The local version identifier is slightly different, however, it talks about a local version that is appended with a + symbol after the version of the package in the wheel filename (similar to, say, how JAX labels its CUDA-specific wheels). So, having the editable keyword in an editable distribution/wheel is not the same thing as these identifiers (since it was rejected in the PEP indeed), it is more like an indication that the wheel being built is an editable one and the + symbol is not appended at the time of writing for other build backends.

If you can see the filename to tell that it uses a different style than setuptools, that implies the wheel has somehow been exposed to you as an end user, which may be a pip bug.

I don't think pip is controlling what the build system does (but I can be wrong, of course, coming from just intermediate packaging experience), it just invokes these PEP 660 hooks at the time of installing the package that are conformant with PEP 517 (though that had listed these hooks as optional).

agriyakhetarpal commented 2 months ago

I'll try to find some resources on the editable keyword and its history, plus some implementation details for it in the source code.

eli-schwartz commented 2 months ago

I still don't understand what the issue is here.

The PEP says you are not permitted to use the wheel file, it's an internal implementation detail of a frontend. The only reason wheel files are used at all is because the python ecosystem has no replacement for python setup.py install -- i.e. no standards process for installing built software, only a process for creating package archives.

In Makefile terms, you can't do make install, only make rpm.

The rpm, sorry, python wheel, is used as a bootstrap shim.

build_wheel returns a filename string for a public wheel that can be saved in a cache, uploaded to PyPI, added to a GitHub Releases tag, or emailed to a friend.

build_editable returns a filename string for an internal bootstrap shim that the frontend (pip) must internally install, and then must delete the bootstrap shim so that it never hits a cache or is available to users.

Why does it matter what the filename of the bootstrap shim is? Where did you encounter the inconsistency, and what difficulties did you encounter as a result of the inconsistency?

agriyakhetarpal commented 2 months ago

Thanks for the comment and the extra information, @eli-schwartz.

Why does it matter what the filename of the bootstrap shim is? Where did you encounter the inconsistency, and what difficulties did you encounter as a result of the inconsistency?

I think the editable installations are working as they are intended to, so the issue is just really about not having the editable keyword in the filename of the wheel built from build_editable and is therefore quite minor (but I had thought it should be okay to file based on the reasons I had described above).

It does not affect the installation and does not give rise to further issues at package runtime, of course, so it can be considered something that's quite low-priority in terms of getting a fix out for it – but I would be interested in contributing if other people too think that this is in scope and if I could receive some pointers :). There are no difficulties that I am currently having because of the inconsistency.

eli-schwartz commented 2 months ago

If it doesn't give rise to any issues then perhaps the filename does not matter and there isn't anything that needs fixing?

agriyakhetarpal commented 2 months ago

Yes, the filename does not matter at all, but it could be misleading to other users (just as it was to me). It let me to think that I pursued the non-editable installation pathway (pip install .) when I glanced at the wheel's filename, but that wasn't the case – it was indeed an editable installation (pip install -e . --no-build-isolation).

N.B., the size of the wheel is correctly displayed, though, as is the case with editable wheels, they are smaller in comparison to normal wheels since the compiled files are not copied or bundled into an editable wheel.

dnicolodi commented 2 months ago

Wheel filenames do not contain keywords. Wheel filenames are defined to be {distribution}-{version}(-{build tag})?-{python tag}-{abi tag}-{platform tag}.whl. https://packaging.python.org/en/latest/specifications/binary-distribution-format/#file-name-convention

Therefore what other build backends to is to append .editable to the package version. In the example you report, this results in a invalid version string:

>>> import packaging.utils
>>> packaging.utils.parse_wheel_filename('pywavelets-1.7.0.dev0.editable-cp312-cp312-linux_x86_64.whl')
Traceback (most recent call last):
...
packaging.version.InvalidVersion: Invalid version: '1.7.0.dev0.editable'

therefore, if there is anything that is violating Python packaging standards or user expectation, they are other build backends, not meson-python.

agriyakhetarpal commented 2 months ago

Thanks, @dnicolodi for the response and the resolution. I did feel that packaging would raise an error here since the filename is obviously not valid. I think I should ask about this on the issue trackers for other build backends, like you mentioned.