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

Adding support for $MESON or another way to point at a customized Meson #458

Closed
rgommers opened this issue Aug 23, 2023 · 14 comments
Closed
Labels
enhancement New feature or request

Comments

@rgommers
Copy link
Contributor

In NumPy we had to vendor Meson in order to include a major new feature (for SIMD support) that is not yet available in Meson itself. That is fine, that's how it's supposed to work, since Meson isn't extensible by design - the point is to keep things simple and upstream your needed features/changes.

However, using that vendored Meson also forced us to vendor meson-python. The only reason for that was to be able to point at the correct meson executable. So ideally meson-python would have a way for a package to point at that custom Meson instead. This could be done via a MESON environment variable (similar to the NINJA one), and/or in some other way (a setting in pyproject.toml perhaps.

Part of what's needed here is being able to point at a meson.py (as shipped with Meson itself), and have meson-python translate that to [sys.executable, 'path/to/meson.py']. This has to do with it being a problem on Windows to ship an executable file meson and have it recognized. See numpy/numpy#24379 (comment) and following comments for details.

This idea was also touched upon in gh-323. It's related but not the same, so I thought I'd open a separate issue instead. The implementation is probably quite simply, the main thing to decide is what the UX for this should look like.

@rgommers rgommers added the enhancement New feature or request label Aug 23, 2023
@dnicolodi
Copy link
Member

dnicolodi commented Aug 23, 2023

On the surface, adding support for looking up the meson executable via a$MESON environment variable seems a good idea. However, I wonder how this would work if it is used to point meson-python to a meson that has features that are not included in any distributed version of meson.

In the Python packaging ecosystem there is the assumption that binary distributions can be compiled from source distributions with the tools specified in the package definition. There is even a proposed PEP that looks into extending this beyond Python packages and things that can be installed from PyPI. @rgommers I'm sure you are familiar with it 😃

This stops working if something not specified in pyproject.toml in the [build-system] is required to compile a package.

A solution I used a while back for a package of mine when there were still bugs to iron out in the Meson Python support, is something like:

[build-system]
build-backend = 'mesonpy'
requires = [
  'meson-python',
  'meson @ https://github.com/dnicolodi/meson.git',
]

See this commit dnicolodi/python-siphash24@f18ab73 This ensures that the specific version of Meson is installed in the build environment, and the $PATH should be set up properly to have it take precedence over the system one.

Why doesn't something like this work for NumPy? I haven't read the linked issue in detail, but I have the impression that vendoring Meson into NumPy made the problem harder to solve because the vendored Meson cannot be (easily, at least) installed before compiling NumPy. Am I missing something?

@dnicolodi
Copy link
Member

The solution proposed above sucks for distributions where the build tools required to build a distribution package need to be packaged themselves. In this case, the distribution would need to merge the patches from the "friendly fork" into their Meson package, or they would need to have an additional numpy-meson package on which the numpy package build-depends upon. For simplicity, the numpy-meson package would need to not be co-installable with the regular meson package.

I admit that this is not ideal, but it looks better than the alternatives, IMHO. Producing the numpy-meson package should be straightforward if a meson package already exits.

@rgommers
Copy link
Contributor Author

This stops working if something not specified in pyproject.toml in the [build-system] is required to compile a package.

Only if it's not part of your own package. It's one or the other:

  1. you either vendor it inside your own source tree, or
  2. you have to distribute it as a separate package and depend on it.

There's trade-offs involved, but there's good reasons (explained below) for why I went with (1) instead of (2).

A solution I used a while back for a package of mine when there were still bugs to iron out in the Meson Python support, is something like:

Indeed, pointing at a fork in a git repo is very convenient while developing a feature or bug fix (and is exactly what we did for numpy). It clearly cannot work for distribution a release though, you have to change to one of the two methods listed above.

haven't read the linked issue in detail, but I have the impression that vendoring Meson into NumPy made the problem harder to solve because the vendored Meson cannot be (easily, at least) installed before compiling NumPy.

Yes, the vendored Meson cannot be installed if you vendor it - it must be run straight from the source tree of your package.

or they would need to have an additional numpy-meson package on which the numpy package build-depends upon. For simplicity, the numpy-meson package would need to not be co-installable with the regular meson package.

This is the alternative solution, but is exactly what I'm trying to avoid. The impact of this is pretty bad:

  • I'd have to publish a numpy-meson package (easy),
  • The packagers of the numpy release that need it would need to repackage numpy-meson for each distro, and it would need to remain available for as long as that numpy release is supported (a ton of work for dozen of people),
  • numpy-meson and meson would conflict, since they both want to install, e.g., /usr/bin/meson. That's not okay in distro land, so now who must patch this? (hard)
    • And if it's patched by renaming the executable installed by numpy-meson, we still need support for $MESON in meson-python.

In contrast, with the meson vendored in https://github.com/numpy/numpy/tree/main/vendored-meson, no one also but me as the person who did the vendoring has to do much/anything here. The one thing that was a problem is that I also had to vendor meson-python; still works, but I'd like to be able to fix that and unvendor meson-python from numpy, because we don't need any changes beyond pointing at vendored-meson/meson/meson.py.

@dnicolodi
Copy link
Member

I was going to suggest that a possible solution is to use the path to the vendored meson in build-system.requires:

[build-system]
build-backend = 'mesonpy'
requires = [
  'meson-python',
  'meson @ file://_vendored/meson',
]

but pip does not accept a relative path for file: URIs. I am not sure I understand why this is the case.

@dnicolodi
Copy link
Member

dnicolodi commented Aug 24, 2023

I'm not sure this checks against the PEP specification, but it works:

[build-system]
build-backend = 'mesonpy'
requires = [
  'meson-python',
  '_vendored/meson',
]

@rgommers
Copy link
Contributor Author

That does seem to work for me on Linux with python -m build --wheel. I'm not quite sure if it's guaranteed to work by a standard, or if the / is going to work in a cross-platform fashion. Either way, one key downside is that it only works for pip/build, and not for Linux distros, conda-forge, etc. that don't use pyproject.toml or disable build isolation.

I'm also still a little confused about why we wouldn't implement $MESON support? It's the most robust option and I think the only one that's not problematic for distro packagers. It's also what was suggested by @eli-schwartz in the linked issue.

@dnicolodi
Copy link
Member

dnicolodi commented Aug 24, 2023

that don't use pyproject.toml or disable build isolation.

If they don't use pyproject.toml they are not using meson-python, so of course that would not work. If they disable build isolation, they need to install the build dependencies by themselves, and that includes also the vendored copy of meson. I don't think that this would be problematic.

I'm also still a little confused about why we wouldn't implement $MESON support?

I don't have anything against implementing $MESON support. What I haven't understood is that how you plan to use that for pointing meson-python to a vendored copy of meson with functionality not implemented upstream. Who is responsible for setting the environment variable if I try to build the package from an sdist I downloaded from PyPI? In this case there isn't even a file I can point $MESON to because the source distribution has not been unpacked yet. I think I'm missing something.

@eli-schwartz
Copy link
Member

I'm not quite sure if it's guaranteed to work by a standard, or if the / is going to work in a cross-platform fashion.

It will probably work in a cross platform fashion because Windows usually just accepts forward slashes or backslashes just fine.

However, it's guaranteed to not work by the spec, since PEP 518 mandates that it is only permissible to list PEP 508 dependency specifiers. The fact that it works anyway is a build frontend bug -- and for the record, the bug is that build will simply pass all arguments to pip as is with no validation. But if you build with isolation disabled then it does check that all dependencies are installed, and this fatally errors out as you'd expect.

The inconsistency is bad, and it's explicitly spelled out in a comment in build that it's "undefined behavior", but I'm fixing that to make it well defined as an error.

@rgommers
Copy link
Contributor Author

If they disable build isolation, they need to install the build dependencies by themselves, and that includes also the vendored copy of meson. I don't think that this would be problematic.

It is problematic imho, we're then back to the "impact of this is pretty bad" of #458 (comment). I really want to avoid creating any work or problems for distro packagers.

Disabling build isolation is standard practice for pretty much all distros, they're by design not going to download stuff from PyPI.

@rgommers
Copy link
Contributor Author

In this case there isn't even a file I can point $MESON to because the source distribution has not been unpacked yet. I think I'm missing something.

You're correct there. In my first comment I said "the main thing to decide is what the UX for this should look like." I think it requires something like a pyproject.toml field with a path pointing to the executable, that works better than an environment variable (which we also want, maybe, but not necessarily right now). We had the same problem with spin, and that's what solved it: scientific-python/spin#97.

@eli-schwartz
Copy link
Member

The inconsistency is bad, and it's explicitly spelled out in a comment in build that it's "undefined behavior", but I'm fixing that to make it well defined as an error.

PR submitted as pypa/build#665

@dnicolodi
Copy link
Member

Disabling build isolation is standard practice for pretty much all distros, they're by design not going to download stuff from PyPI.

The hack I proposed (which is indeed just a hack) does not download anything from PyPI. It installs the vendored Meson copy. But I understand that it is still weird to have to install the vendored build tool. I was not really proposing this as a solution for building distribution packages.

@dnicolodi
Copy link
Member

PR submitted as pypa/build#665

Thank you @eli-schwartz for killing my hack immediately 🤣

@rgommers
Copy link
Contributor Author

rgommers commented Sep 1, 2023

I'll try and open a PR with my preferred solution for this. It should be pretty minimal. That will hopefully allow unvendoring meson-python from the numpy repo before the next numpy release in a couple of weeks from now.

rgommers added a commit to rgommers/meson-python that referenced this issue Sep 10, 2023
Allow using a MESON environment variable, analogous to the NINJA one,
or a string to an executable or Python script in the
`[tool.meson-python]` section of pyproject.toml

Closes mesonbuildgh-458
rgommers added a commit to rgommers/meson-python that referenced this issue Sep 10, 2023
Allow using a MESON environment variable, analogous to the NINJA one,
or a string to an executable or Python script in the
`[tool.meson-python]` section of pyproject.toml

Closes mesonbuildgh-458
rgommers added a commit to rgommers/meson-python that referenced this issue Sep 10, 2023
Allow using a MESON environment variable, analogous to the NINJA one,
or a string to an executable or Python script in the
`[tool.meson-python]` section of pyproject.toml

Closes mesonbuildgh-458
rgommers added a commit to rgommers/meson-python that referenced this issue Sep 10, 2023
Allow using a MESON environment variable, analogous to the NINJA one,
or a string to an executable or Python script in the
`[tool.meson-python]` section of pyproject.toml

Closes mesonbuildgh-458
rgommers added a commit to rgommers/meson-python that referenced this issue Sep 10, 2023
Allow using a MESON environment variable, analogous to the NINJA one,
or a string to an executable or Python script in the
`[tool.meson-python]` section of pyproject.toml

Closes mesonbuildgh-458
rgommers added a commit to rgommers/meson-python that referenced this issue Sep 12, 2023
Allow using a MESON environment variable, analogous to the NINJA one,
or a string to an executable or Python script in the
`[tool.meson-python]` section of pyproject.toml

Closes mesonbuildgh-458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants