search

mesonbuild / meson-python

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

Fix editable install of meson-python for pip 23.2.1 and python3.11 #478

Closed robtaylor closed 10 months ago

robtaylor commented 10 months ago

With python 3.11 and pip 23.2.1, an editable install of meson-python itself fails with: pip._internal.exceptions.InstallationSubprocessError: Checking if build backend supports build_editable exited with 1

This is due to lacking an implementation of PEP660's prepare_metadata_for_build_editable.

robtaylor commented 10 months ago

Still getting an issue in my environment that _meson_python_editable_loader.py is created with the ninja execitable set to the one installed in the temporary build enviroment created by pip. Not sure why I'm hitting this but the tests aren't!

dnicolodi commented 10 months ago

How did you determine that the lack of the prepare_metadata_for_build_editable() hook is the source of your problems?

Everything works as expected for me:

$ python --version
Python 3.11.4
$ python -m pip --version
pip 23.2.1 from /Users/daniele/src/meson-python/venv/lib/python3.11/site-packages/pip (python 3.11)
$ python -m pip install --no-build-isolation -e .
Obtaining file:///Users/daniele/src/meson-python
  Checking if build backend supports build_editable ... done
  Preparing editable metadata (pyproject.toml) ... done
Requirement already satisfied: meson>=0.63.3 in ./venv/lib/python3.11/site-packages (from meson-python==0.14.0.dev0) (1.2.1)
Requirement already satisfied: pyproject-metadata>=0.7.1 in ./venv/lib/python3.11/site-packages (from meson-python==0.14.0.dev0) (0.7.1)
Requirement already satisfied: packaging>=19.0 in ./venv/lib/python3.11/site-packages (from pyproject-metadata>=0.7.1->meson-python==0.14.0.dev0) (23.1)
Building wheels for collected packages: meson-python
  Building editable for meson-python (pyproject.toml) ... done
  Created wheel for meson-python: filename=meson_python-0.14.0.dev0-py3-none-any.whl size=17632 sha256=43e997401297472d0d9edb03577b91f7f40637b2c3e100f2bb9f475020a4e599
  Stored in directory: /private/var/folders/6f/4l4d5n9x1y71cxyrprf_x35c0000gn/T/pip-ephem-wheel-cache-s0rqdxog/wheels/91/8a/31/237de59d176b9ce63c1dcd55038958a1238f2fd5f1c0cd7506
Successfully built meson-python
Installing collected packages: meson-python
  Attempting uninstall: meson-python
    Found existing installation: meson-python 0.14.0.dev0
    Uninstalling meson-python-0.14.0.dev0:
      Successfully uninstalled meson-python-0.14.0.dev0
Successfully installed meson-python-0.14.0.dev0
dnicolodi commented 10 months ago

Still getting an issue in my environment that _meson_python_editable_loader.py is created with the ninja execitable set to the one installed in the temporary build enviroment created by pip. Not sure why I'm hitting this but the tests aren't!

As I already answered here https://github.com/mesonbuild/meson-python/discussions/425#discussioncomment-6897683, you are installing in editable mode without disabling build isolation, and you don't have a suitable ninja installed.

robtaylor commented 10 months ago

How did you determine that the lack of the prepare_metadata_for_build_editable() hook is the source of your problems?

Everything works as expected for me:

$ python --version
Python 3.11.4
$ python -m pip --version
pip 23.2.1 from /Users/daniele/src/meson-python/venv/lib/python3.11/site-packages/pip (python 3.11)
$ python -m pip install --no-build-isolation -e .
Obtaining file:///Users/daniele/src/meson-python
  Checking if build backend supports build_editable ... done
  Preparing editable metadata (pyproject.toml) ... done
Requirement already satisfied: meson>=0.63.3 in ./venv/lib/python3.11/site-packages (from meson-python==0.14.0.dev0) (1.2.1)
Requirement already satisfied: pyproject-metadata>=0.7.1 in ./venv/lib/python3.11/site-packages (from meson-python==0.14.0.dev0) (0.7.1)
Requirement already satisfied: packaging>=19.0 in ./venv/lib/python3.11/site-packages (from pyproject-metadata>=0.7.1->meson-python==0.14.0.dev0) (23.1)
Building wheels for collected packages: meson-python
  Building editable for meson-python (pyproject.toml) ... done
  Created wheel for meson-python: filename=meson_python-0.14.0.dev0-py3-none-any.whl size=17632 sha256=43e997401297472d0d9edb03577b91f7f40637b2c3e100f2bb9f475020a4e599
  Stored in directory: /private/var/folders/6f/4l4d5n9x1y71cxyrprf_x35c0000gn/T/pip-ephem-wheel-cache-s0rqdxog/wheels/91/8a/31/237de59d176b9ce63c1dcd55038958a1238f2fd5f1c0cd7506
Successfully built meson-python
Installing collected packages: meson-python
  Attempting uninstall: meson-python
    Found existing installation: meson-python 0.14.0.dev0
    Uninstalling meson-python-0.14.0.dev0:
      Successfully uninstalled meson-python-0.14.0.dev0
Successfully installed meson-python-0.14.0.dev0

using pip install -vvv

If you try running the test changes in this patchset without the fixes, you'll see the issues...

dnicolodi commented 10 months ago

I just tried python -m pip install -vvv -e . in the meson-python source directory and everything still works as expected, just with more output. Can you please be more specific about how that fails for you?

robtaylor commented 10 months ago

Still getting an issue in my environment that _meson_python_editable_loader.py is created with the ninja execitable set to the one installed in the temporary build enviroment created by pip. Not sure why I'm hitting this but the tests aren't!

As I already answered here #425 (reply in thread), you are installing in editable mode without disabling build isolation, and you don't have a suitable ninja installed.

Ahh, ok I had definitely missed how-to-guides-editable-installs. The patch here seems to fix the need for this in projects using meson-python - at least that's what the test set shows, maybe there's a corner case i'm missing?

Still need --no-build-isolation for meson-python itself, as the pip environment results in the installed ninja failing to work (the stub fails on from ninja import ninja). I wonder if we can do something so folk don't get caught out, and add a note in contributing?

dnicolodi commented 10 months ago

The tests work just because ninja is provided by the system on the test runners. It would fail the same with or without these patches if it would not be present, as yourself pointed out happens on your system. How to install an editable wheel for a project using meson-python is documented. I don't think we need to repeat that. Also, an editable (or otherwise) installation of meson-python is not required to work, debug, or test meson-python.

dnicolodi commented 10 months ago

Can you please describe what this PR achieves and why we need it? PEP 660 states that the prepare_metadata_for_build_editable is optional and the front end should fall back to build_editable if it is missing. In the case of meson-python, providing accurate metadata requires building the project, thus there is no advantage in providing prepare_metadata_for_build_editable over build_editable as the former would need to do all the work the latter does, except packaging the build artifacts in a zip file, which is negligible work compared to the rest.

rgommers commented 10 months ago

I'll note that we do have an issue for this optional hook: gh-236. Implementing it is fine and a small improvement over the status quo - it may avoid a build step in certain circumstances (fully static metadata, and pip decides that this package version is not the right one).

Agreed that it shouldn't be directly related to editable builds.

dnicolodi commented 10 months ago

I was actually going to close that issue as wontfix over the weekend. To produce metadata we need to know if the wheel is pure or not (ie if it included platform or python version specific content) and for doing that we need to configure, and in some occasions, actually build the project.

PEP 660 is not clear on what prepare_metadata_for_build_editable is used for by the front-end. I fear that some front-ends may execute prepare_metadata_for_build_editable before build_editable, just in case. This would result in meson-python doing work twice for no good reason. Lacking an use case requiring prepare_metadata_for_build_editable (or prepare_metadata_for_build_wheel as requested in #236), I much prefer not to implement these methods.

robtaylor commented 10 months ago

The tests work just because ninja is provided by the system on the test runners. It would fail the same with or without these patches if it would not be present, as yourself pointed out happens on your system. How to install an editable wheel for a project using meson-python is documented. I don't think we need to repeat that. Also, an editable (or otherwise) installation of meson-python is not required to work, debug, or test meson-python.

To check this, i've added the test set on top of main: https://github.com/robtaylor/meson-python/actions/runs/6074445627

CF: test set with the two other commits: https://github.com/robtaylor/meson-python/actions/runs/6071489381

Definitly a difference, though I seem to have broken something on windows here. If there is no interest, I shall leave this as is, but if there is interest I'll debug the windows issue,

robtaylor commented 10 months ago

So, some sleep deprivation definitely in play. Looks

Can you please describe what this PR achieves and why we need it? PEP 660 states that the prepare_metadata_for_build_editable is optional and the front end should fall back to build_editable if it is missing. In the case of meson-python, providing accurate metadata requires building the project, thus there is no advantage in providing prepare_metadata_for_build_editable over build_editable as the former would need to do all the work the latter does, except packaging the build artefacts in a zip file, which is negligible work compared to the rest.

Ok, definitely sleep deprivation on my side!

The patches that are useful here are the patch for breakage on 3.7/3.8 and the test set for meson-python editable install

I'll close this and submit those as separate issues. I'd also like to look into ways we could warn folk if they make the same mistake, not sure if its possible....

dnicolodi commented 10 months ago

No problem. I'm happy that we can agree that there is nothing to fix.

However, for the future, I recommend you to be mindful of the fact that the project maintainers have most likely spend much more time than you thinking about an issue and almost certainly have a better understanding of the context in which a project is developed. Providing a concise and punctual description of the issue you have encountered and of the reason why you are proposing a patch. Having to argue the position of the maintainers, especially when the opposing position is not well defined, is very tiring. This talk does a very good job at describing the issue https://www.youtube.com/watch?v=o_4EX4dPppA

dnicolodi commented 10 months ago

The patches that are useful here are the patch for breakage on 3.7/3.8 and the test set for meson-python editable install

We have tests for the editable wheels functionality, for example https://github.com/mesonbuild/meson-python/blob/main/tests/test_editable.py#L170 They run also on Python 3.7 and 3.8 thus I doubt there is anything to fix there.

rgommers commented 10 months ago

Thanks @robtaylor, glad there was no problem in the end.

I was actually going to close that issue as wontfix over the weekend. [...]

Thanks for those thoughts @dnicolodi - I'll follow up on that issue now with a reply to your concerns.