Skip to content
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

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

Closed
agriyakhetarpal opened this issue Apr 17, 2024 · 10 comments
Labels
invalid This doesn't seem right

Comments

@agriyakhetarpal
Copy link

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: PyWavelets/pywt#702

@eli-schwartz
Copy link
Member

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

  • The filename for the “editable” wheel needs to be PEP 427 compliant too. It does not need to use the same tags as build_wheel but it must be tagged as compatible with the system.
  • An “editable” wheel uses the wheel format not for distribution but as ephemeral communication between the build system and the front end. This avoids having the build backend install anything directly. This wheel must not be exposed to end users, nor cached, nor distributed.

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
Copy link
Member

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
Copy link
Author

agriyakhetarpal commented Apr 17, 2024

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
Copy link
Author

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

@agriyakhetarpal agriyakhetarpal changed the title Non-conformant with PEP 660, does not append editable keyword before ABI tag(s) with --editable flag meson-python does not append editable keyword before ABI tag(s) with --editable flag Apr 17, 2024
@eli-schwartz
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

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.

@dnicolodi dnicolodi added the invalid This doesn't seem right label Apr 17, 2024
@dnicolodi dnicolodi closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2024
@agriyakhetarpal
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

3 participants