pep621 is not optional but the dependency isn't declared

[rgommers]

gh-33 talks about replacing pep621 but really seems to mean "rename" rather than replace. There seems to be a more urgent issue here: pep621 is imported here: https://github.com/FFY00/meson-python/blob/main/mesonpy/__init__.py#L552-L553 but is not declared as a dependency. So I'm seeing:

ModuleNotFoundError: No module named 'pep621'

in the CI job that tests building via sdist for SciPy (log. Why is a non-optional dependency not declared? The comment even explicitly discusses raising an exception - it should not be necessary for meson-python users like SciPy to list pep621 as a direct dependency. It should be either vendored, declared as non-optional, or made actually optional.

[FFY00]

pep621 is only needed when using a pyproject.toml, so we return it in get_requires_for_build_{sdist,wheel}. How is the environment where the build is running being provisioned? If it's manual, you need to add pep621 to the requirements, if it's a tool, it's a bug of that tool for not including the dependencies returned by get_requires_for_build_{sdist,wheel}.

[rgommers]

pep621 is only needed when using a pyproject.toml,

I don't understand this. Don't you always use pyproject.toml? That's where the build hook lives.

How is the environment where the build is running being provisioned? If it's manual, you need to add pep621 to the requirements

It's in the job that explicitly tests building via an sdist:

$ python -m build --sdist
$ pip install dist/scipy-1.9.0.dev0.tar.gz  --no-build-isolation

If it's manual, you need to add pep621 to the requirements

If by manual you mean --no-build-isolation (= the default/recommended method in many dev setups), then I disagree. It makes sense to tell users that they need:

1. SciPy-specific build requirements (openblas, pkg-config, numpy`)
2. meson-python as the build system hook

It does not make sense for everyone to have to know about dependencies of meson-python, and especially not very niche ones like pep621 that are meaningless to those users.

[FFY00]

I don't understand this. Don't you always use pyproject.toml? That's where the build hook lives.

Sorry, I meant PEP 621 metadata.

If by manual you mean --no-build-isolation (= the default/recommended method in many dev setups), then I disagree. It makes sense to tell users that they need:

IMO this is a bug in pip, it should be erroring out or at least warning users if the required dependencies are not present when using --no-build-isolation.

It does not make sense for everyone to have to know about dependencies of meson-python, and especially not very niche ones like pep621 that are meaningless to those users.

And they don't have to, people generally shouldn't being provisioning the build environment themselves.

To solve your issue, I'd probably recommend that you drop pip and just use pypa/build and pypa/installer:

$ python -m build
$ python -m installer dist/*.whl

If you want to provision the build environment yourself, you can pass --no-isolation to pypa/build. It will error out if any of the build dependencies are missing.

[rgommers]

And they don't have to, people generally shouldn't being provisioning the build environment themselves.

This really is not true for complex compiled packages. It is only true when you're working in a virtualenv. Half of the scientific community works in conda envs, HPC folks work in spack, all packagers (Linux/Homebrew/Nix/...) need to use their own libraries. --no-build-isolation is extremely important, and it should work with as little friction as possible.

To solve your issue, I'd probably recommend that you drop pip and just use pypa/build and pypa/installer:

I must use build with --skip-dependency-check anyway, for multiple reasons (numpy== pins, non-wheel-installed dependencies), so that doesn't make a difference.

Sorry, I meant PEP 621 metadata.

What's the difference though - we always use that with meson-python? The main actions we need meson-python for are (1) build an sdist, and (2) build a wheel. For both we need pep621. Hence it should be a required dependency. I don't see a problem with that. Leaving it out on the other hand is a bad user experience.

[rgommers]

It makes sense to tell users that they need:

1. SciPy-specific build requirements (`openblas`, pkg-config`, `numpy`)
2. `meson-python` as the build system hook

Let me restate this a bit more generally: For every project using Meson and meson-python, the general way that the consumers of those projects must prepare their build envs is:

1. Set up all dependencies that are not available on PyPI or must come from another packaging ecosystem for some reason (e.g., binary compatibility). This also includes compilers.
2. Install all the build dependencies listed in the project's pyproject.toml.

It should not include "3. "check each build dependency and hunt down its dependencies".

[FFY00]

I think it's a design drawback from the PEP 517, but it's not unprecedented. setuptools does the same with the wheel dependency. I don't think we are doing anything wrong here, but I can understand your issue.

We could possibly provide a full extra which pulls all these dependencies.

What's the difference though - we always use that with meson-python? The main actions we need meson-python for are (1) build an sdist, and (2) build a wheel. For both we need pep621. Hence it should be a required dependency. I don't see a problem with that. Leaving it out on the other hand is a bad user experience.

No. We can operate fully without it, which is useful for hacking around. We could make it a dependency of meson-python itself, but that's not really accurate.

https://meson-python.readthedocs.io/en/latest/usage/start.html#automatic-metadata

[rgommers]

We could possibly provide a full extra which pulls all these dependencies.

I don't like that much.

We could make it a dependency of meson-python itself, but that's not really accurate.

There is nothing inaccurate about this - whether you make something required or optional if it's needed of a subset of a dependency is always a choice, there is no right or wrong. Can you please just explain what the downside is of including pep517 as a dependency? Because I don't see any, and the upside is major.

https://meson-python.readthedocs.io/en/latest/usage/start.html#automatic-metadata

That comes with a warning that it's not recommended ....

[rgommers]

I now understood what https://github.com/FFY00/meson-python/blob/5f83a8d817833385f6f8a42e9715dd666302daed/mesonpy/__init__.py#L54-L58 is doing. That whole _depstr should be removed I believe, it's just a source of unnecessary complexity that also breaks --no-build-isolation. I would have discovered gh-52 way earlier for example if patchelf-wrapper was listed as a dependency on Linux in pyproject.toml. And it avoids exposing what are internal implementation details. Imagine for example gh-33 happens, and pep621 gets renamed and meson-python updates for it. Then all users of --no-build-isolation will see breakage and need to manually switch.

The design requirement here is: all dependencies for build-sdist and build-wheel should be declared as runtime dependencies of meson-python, so that users of meson-python do not have to care about any transitive dependencies when adding meson-python as a build-time dependency.

[rgommers]

That whole _depstr should be removed I believe, it's just a source of unnecessary complexity that also breaks

Ah wait, if the purpose is to remove dependencies that are not needed, so that creating an isolated build env, then that makes sense. Adding new dependencies is never right. Some comments would have helped me understand more easily - if I find some time tonight, I can make a PR.

[rgommers]

Ah wait, if the purpose is to remove dependencies that are not needed,

I had to read the code closely and re-read PEP 517 - this was wrong, my first take was right. _depstr was used in the optional hooks to add extra dependencies for sdist and wheel build.

[rgommers]

After discussion:

• pep621 will be a hard dependency
• patchelf could later use a "detect if present on system, otherwise install" approach - this would be a valid use of _depstr and the optional hooks (perhaps ninja too, TBD).
• @FFY00 wants to keep the _depstr hooks