build meta wheel does not respect metadata_directory per PEP-517

[gaborbernat]

From https://www.python.org/dev/peps/pep-0517/#build-wheel:

If the build frontend has previously called prepare_metadata_for_build_wheel and depends on the wheel resulting from this call to have metadata matching this earlier call, then it should provide the path to the created .dist-info directory as the metadata_directory argument.

https://github.com/pypa/setuptools/blob/master/setuptools/build_meta.py#L208 though ignores the passed in metadata_directory argument and goes to regenerate this information in the root of the project.

    def build_wheel(self, wheel_directory, config_settings=None,
                    metadata_directory=None):
        return self._build_with_temp_dir(['bdist_wheel'], '.whl',
                                         wheel_directory, config_settings)

[dholth]

IMO the passed directory is not necessary. It will certainly match the recently generated one.

[gaborbernat]

IMO the passed directory is not necessary. It will certainly match the recently generated one.

First and foremost it's mandated per PEP-517, so you MUST conform to that expectation.

Second, you can't really make the assumption that it will match. For example, the user might have invoked manually (aka via python setup.py, and then the metadata directory defaults to the default location (in-place of the source tree at the moment); while frontends may want to move the metadata directory out of the source tree into a frontend specific folder (such as .tox). So I'm not sure how you can make that certainly assumption, without causing headaches and subtle bugs.

[dholth]

I think the current backends always rebuild dist-info as part of the wheel build. The newly rebuilt dist-info is very likely to match the one built moments earlier by the other hook

[gaborbernat]

The newly rebuilt dist-info is very likely to match the one built moments earlier by the other hook

I'm not sure I follow how this is related to the fact that this hook should take and use the metadata_directory, maybe the frontend wants to use the content of the folder afterward, not?

[pradyunsg]

Updated OP's post to a permalink:

setuptools/setuptools/build_meta.py

Line 214 in 2878bad

def build_wheel(self, wheel_directory, config_settings=None,

[uranusjr]

This is also discussed in pypa/build#217. Although setuptools is free to rebuild dist-info in build_wheel (instead of using the content of metadata_directory), it should at least verify the built wheel indeed contains identical metadata to those provided in metadata_directory. #2529 is a concrete example of setuptools failing expectations due to this nonconformance.

This has also been raised to flit (pypa/flit#389) and poetry (python-poetry/poetry#1078).

[dholth]

Would it be easier to remove this requirement from the specification since no backend found it worthwhile

…

On Sat, Jan 30, 2021, at 8:17 AM, Tzu-ping Chung wrote: This is also discussed in pypa/build#217 <pypa/build#217>. Although setuptools is free to rebuild dist-info in `build_wheel` (instead of using the content of `metadata_directory`), it should at least *verify* the built wheel indeed contains identical metadata to those provided in `metadata_directory`. #2529 <#2529> is a concrete example of setuptools failing expectations due to this nonconformance. This has also been raised to flit (pypa/flit#389 <pypa/flit#389>) and poetry (python-poetry/poetry#1078 <python-poetry/poetry#1078>). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1825 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABSZERQ3PENSZTRR5ZWKN3S4QBGZANCNFSM4IM4JSKQ>.

[uranusjr]

But then what happens if a backend return different things from build_wheel and prepare_metadata_for_build_wheel? (Which setuptools is guity of doing right now.) It is certainly possible to remove the requirement for backend to make use of the metadata_directory argument, but the backend should still somehow return consistent things from the two hooks. Otherwise the frontend would be stuck and can’t proceed. The discussion needs to be moved to discuss.python.org if you want to do that so frontend authors can join the conversation.

[wimglenn]

setuptools-ext is a thin wrapper around setuptools.build_meta allowing to specify some more metadata fields such as Requires-External, which aren't otherwise supported by setuptools. Passing around the metadata_directory (ref) would have been really useful if it worked!

If setuptools were to diligently follow PEP 517 (i.e. does not just ignore the metadata_directory), we could simply call prepare_metadata_for_build_wheel before dropping in some additional files, or modify existing ones, under .dist-info subdir, and then call build_wheel passing in the metadata dir.

However since that didn't work, instead setuptools-ext has to call build_wheel, and then completely rewrite the result, which is a lot more complicated (considering that modifying the METADATA file also invalidates the RECORD file!)

This "tweaking" of a build backend by wrapping the hooks is apparently an expected usage, because it's documented as a way for an in-tree build backend to augment the build dependencies (by @abravalheri here). It's unfortunate that one can't reliably use a similar technique for prepare_metadata_for_build_wheel + build_wheel.

[pelson]

I just encountered exactly the same thing as seen by setuptools-ext - modifying the metadata in prepare_metadata_for_build_wheel would be super useful (and PEP-517 spec compliant).

I'm guessing it isn't already easy to build using an existing dist_info directory with setuptools, hence the reason for not having implemented this already?

[abravalheri]

I'm guessing it isn't already easy to build using an existing dist_info directory with setuptools, hence the reason for not having implemented this already?

Precisely. Things are complicated because of the division between the setuptools and wheel packages.

Right now the METADATA file should be similar to the PKG-INFO considering that there is no other packaging wrapping setuptools. So setuptools/wheel kind of try¹ to comply with the PEP requirements² as long as there is no other "man-in-the-middle" modifying the files. I believe other backends also adopt the approach of re-generating the file.

Footnotes

1. Tries but probably fails in some edge cases, because it is a difficult problem. ↩

2. The PEP does not necessarily excludes the possibility of re-generating the files, but it requires them to a "non-modified" version of each other... ↩

[pelson]

Curiosity got the better of me, and I opened an issue in pypa/wheel#611 and prototyped in pypa/wheel#612. With proposed PR to wheel, and the following change in setuptools, build_wheel would be PEP-517 compliant on this topic (taking the approach of using the dist-info metadata, rather than re-computing and then subsequently validating):

diff --git a/setuptools/build_meta.py b/setuptools/build_meta.py
index 2decd2d21..58b50cfc4 100644
--- a/setuptools/build_meta.py
+++ b/setuptools/build_meta.py
@@ -406,9 +406,12 @@ class _BuildMetaBackend(_ConfigSettingsTranslator):
     def build_wheel(
         self, wheel_directory, config_settings=None, metadata_directory=None
     ):
+        cmd = ['bdist_wheel']
+        if metadata_directory:
+            cmd.extend(['--dist-info-dir', metadata_directory])
         with suppress_known_deprecation():
             return self._build_with_temp_dir(
-                ['bdist_wheel'],
+                cmd,
                 '.whl',
                 wheel_directory,
                 config_settings,