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

BUG: install_subdir() in meson.build does not honor exlude_files/directories #317

Closed
dalcinl opened this issue Feb 20, 2023 · 8 comments · Fixed by #364
Closed

BUG: install_subdir() in meson.build does not honor exlude_files/directories #317

dalcinl opened this issue Feb 20, 2023 · 8 comments · Fixed by #364
Labels
bug Something isn't working dependency-bug A bug experienced by users of meson-python caused by a dependency, rather than in code in this repo

Comments

@dalcinl
Copy link

dalcinl commented Feb 20, 2023

I'm trying to use install_subdir() to simplify the inclusion of Python source files and other data files in the final wheel. However, the install_subdir() function is not honoring the exlude_files/directories options.

Core Meson is installing things properly and exclusions are honored. However, when mesonpy takes over, it ends up copying the subtree from the source directory, and not the install directory as I was expecting. That's the reason the exclusions are not honored: things are being copied from the wrong location. As Meson is all about out-of-tree builds, I would argue this is a bug.

The following trivial patch fixes the problem:

diff --git a/mesonpy/__init__.py b/mesonpy/__init__.py
index 40b9645..fe5055d 100644
--- a/mesonpy/__init__.py
+++ b/mesonpy/__init__.py
@@ -486,7 +486,7 @@ class _WheelBuilder():
                 )
                 if install_details:
                     scheme, destination = install_details
-                    wheel_files[scheme].append((destination, file))
+                    wheel_files[scheme].append((destination, copy_files[file]))
                     continue
                 # not found
                 warnings.warn(

However, this patch makes some tests in tests/test_wheel.py break.
I'm not familiar enough with Meson and meson-python to triage these test failures. Is anyone able to help?

@dnicolodi
Copy link
Member

I think this is a bug in Meson: we should be able to map from source code and build artifacts to installation paths via the intro-install_plan.json introspection data only but the introspection data contains the wrong source path.

@FFY00
Copy link
Member

FFY00 commented Feb 20, 2023

Considering that, I'd say it's a meson-python bug, Meson just does not provide a way for us to fix it.

@dnicolodi
Copy link
Member

dnicolodi commented Feb 20, 2023

Well, this is not completely true. If we keep running meson install we can simply pick the subdir content from the installed tree instead than from the source tree. However, I would really like to do not have to run meson install and #280 and #279 move in that direction.

@dalcinl
Copy link
Author

dalcinl commented Feb 21, 2023

@dnicolodi Sorry if this has been discussed elsewhere, but could you please elaborate in the rationale for avoiding the install step? Is is only about supporting editable installs? Messon favors and out-of-source approach, and if you try to workaround that, at some point some logics will break. This is exactly what happened when I tried to install_subdir() with exclude options 😞. Editable installs could be easily supported by copying over built extensions from the install directory back to the source tree, pretty much as setuptools does it.

PS: To be fully open and fair, I'm using install_subdir() to workaround the fact that Meson/meson-python does not allow (out of the box) use of wildcards to include py sources and data files. I would love seeing meson-python taking a more pragmatic approach (assuming core Meson would allow for it).

@dnicolodi
Copy link
Member

I think you are confusing the build directory and the install prefix in some of your comments. Anyhow, I'll reply trying do disentangle this confusion. The reason for avoiding the install step for editable support has been discussed at length in #279.

Editable installs could be easily supported by copying over built extensions from the install directory back to the source tree, pretty much as setuptools does it.

As you state above, Meson enforces an out-of-tree approach, therefore copying build artifacts into the source tree is not desirable. More fundamentally, the layout of the source tree does not necessarily need to match the layout of the installed tree, therefore there may not be a place where to copy the build artifacts or a Python package that can be imported.

Messon favors and out-of-source approach, and if you try to workaround that, at some point some logics will break. This is exactly what happened when I tried to install_subdir() with exclude options

I'm not trying to work around the split between the source tree and the build tree. What I want is to avoid to have an installed tree in addition to the build and source trees and this is exactly to because I want to avoid to have two set of rules to map files to their location in the Python wheel: one mapping from source directory and build directory to the wheel location via the intro-install_plan.json Meson introspection data and one from the install tree to the wheel location based (mostly) in introspecting the Python interpreter. The second mapping in particular is not trivial to implement for all supported version of Python, thus I would be very happy to do not have to duplicate it in meson-python too.

The problem with the exclude_files argument to install_subdir() is due to no one thinking about supporting it yet and resolving it is complicated by the fact that Meson lacks a bit of introspection metadata. Adding the metadata to Meson and then adding support to meson-python for skipping the excluded files when building the wheel is straightforward.

Please note that having support for exclude_files in install_subdir() conflicts with your idea for implementing editable wheels copying build artifacts into the source directory: what should meson-python do when assembling and editable install and a file in the source directory is supposed to do not appear in the installed directory? Delete it from the source tree?

PS: To be fully open and fair, I'm using install_subdir() to workaround the fact that Meson/meson-python does not allow (out of the box) use of wildcards to include py sources and data files. I would love seeing meson-python taking a more pragmatic approach (assuming core Meson would allow for it).

I don't find having to list all files that comprise a package that much of a burden. It takes a few minutes to generate the lists when writing the meson.build the first time and I don't add or remove files from packages that often to find it annoying to have to add a line in meson.build when I do.

Anyhow, meson-python cannot alter the syntax allowed in Meson projects definitions. Thus it cannot take any pragmatic approach that would relieve you from having to list all source files or using install_subdir() (which I would not call a workaround but the supported way of installing the content of a project subdirectory). If you think a facility for installing a collections of files from a subdirectory with a semantic different that install_subdir() is needed, you should propose it to the Meson developers, possibly with a concrete plan for how it would work.

@rgommers
Copy link
Contributor

Editable installs could be easily supported by copying over built extensions from the install directory back to the source tree, pretty much as setuptools does it.

As you state above, Meson enforces an out-of-tree approach, therefore copying build artifacts into the source tree is not desirable. More fundamentally, the layout of the source tree does not necessarily need to match the layout of the installed tree, therefore there may not be a place where to copy the build artifacts or a Python package that can be imported.

I'd also say that seeing it as just a limitation is not quite right. It yields a number of desirable features, like the ability to have multiple parallel builds (e.g., a debug, a release, and a different compiler) in the same repo. When you come from setuptools, you probably only see the differences that are an immediate problem, but I'd say that 100% out of tree builds are a significant improvement. That's why Meson enforces them, and they're often recommended for CMake too.

A very similar comment was made on #47 (comment). I think it'd be helpful to add an explanation page on this topic in the docs. WDYT?

@dnicolodi dnicolodi added the dependency-bug A bug experienced by users of meson-python caused by a dependency, rather than in code in this repo label Feb 25, 2023
@dnicolodi
Copy link
Member

The required introspection data has been added to Meson mesonbuild/meson#11432. I'll add support for exclude_files and exclude_directories to meson-python as soon as #280 is merged (or at least reviewed) as it touches the same code.

@rgommers
Copy link
Contributor

rgommers commented Sep 1, 2023

gh-364 was merged without behavior changes, so reopening. this should be easier to address now though.

dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Sep 1, 2023
Install files in the right location in case of nested directory
structures and implement handling of exclude_directories and
exclude_files.

Fixes mesonbuild#317.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Sep 1, 2023
Install files in the right location in case of nested directory
structures and implement handling of exclude_directories and
exclude_files.

Fixes mesonbuild#317.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Sep 1, 2023
Install files in the right location in case of nested directory
structures, and implement handling of exclude_directories and
exclude_files. The latter requires Meson 1.1.0 or later.

Fixes mesonbuild#317.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Sep 1, 2023
Install files in the right location in case of nested directory
structures, and implement handling of exclude_directories and
exclude_files. The latter requires Meson 1.1.0 or later.

Fixes mesonbuild#317.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Sep 2, 2023
Install files in the right location in case of nested directory
structures, and implement handling of exclude_directories and
exclude_files. The latter requires Meson 1.1.0 or later.

Fixes mesonbuild#317.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependency-bug A bug experienced by users of meson-python caused by a dependency, rather than in code in this repo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants