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

metadata: remove Distribution._local #354

Merged
merged 7 commits into from
Dec 20, 2021

Conversation

FFY00
Copy link
Member

@FFY00 FFY00 commented Oct 30, 2021

Importing an external module is anti-pattern and very unexpected
behavior. Furthermore, the specific builder implementation we are using
will provision an isolated virtual environmnent and perform the build
there, which is unwanted in various scenarious.

Perhaps there was a time this helper was needed, but we can now
remove this in favor of build.util.project_wheel_metadata[1].

[1] https://github.com/pypa/build/blob/82051d509a87124a46f3766134c11fc8aee9b86a/src/build/util.py#L27

Signed-off-by: Filipe Laíns lains@riseup.net

Importing an external module is anti-pattern and very unexpected
behavior. Furthermore, the specific builder implementation we are using
will provision an isolated virtual environmnent and perform the build
there, which is unwanted in various scenarious.

Perhaps there was a time this helper was needed, but we can now
remove this in favor of build.util.project_wheel_metadata[1].

[1] https://github.com/pypa/build/blob/82051d509a87124a46f3766134c11fc8aee9b86a/src/build/util.py#L27

Signed-off-by: Filipe Laíns <lains@riseup.net>
@jaraco
Copy link
Member

jaraco commented Nov 14, 2021

Unfortunately, it doesn't look like build.util.project_wheel_metadata obviates this behavior. For example, the 'metadata' returned only includes the field-based metadata. It doesn't include other files in the metadata such as entry points. It's a lot more difficult to construct the full metadata because it requires multiple files, and to achieve that, pep517 constructs an in-memory zipfile.

Also a bit of a nitpick, but I don't see how "wheel" is relevant to all of these functions. A wheel isn't generated; only the project metadata. I'd like to see a function that doesn't involve irrelevant words like util and wheel to get the metadata.

@FFY00
Copy link
Member Author

FFY00 commented Nov 14, 2021

Unfortunately, it doesn't look like build.util.project_wheel_metadata obviates this behavior. For example, the 'metadata' returned only includes the field-based metadata. It doesn't include other files in the metadata such as entry points. It's a lot more difficult to construct the full metadata because it requires multiple files, and to achieve that, pep517 constructs an in-memory zipfile.

We can introduce further helpers to make that possible.

Also a bit of a nitpick, but I don't see how "wheel" is relevant to all of these functions. A wheel isn't generated; only the project metadata. I'd like to see a function that doesn't involve irrelevant words like util and wheel to get the metadata.

It is the metadata that is generated for the wheel. The name comes from PEP 517.

https://www.python.org/dev/peps/pep-0517/#prepare-metadata-for-build-wheel

A wheel is actually generated as a fallback if the backend does not implement the metadata hook, as recommended.

sdists also contain metadata, which may be different from the wheel, and there is no guarantee that further binary formats that may be introduced in the future must have the same metadata as the wheel.
One way I can imagine this happening is introducing a new format that also takes care of binary dependencies for eg., where we would most likely have different metadata under certain scenarios.

I know I am being a bit conservative about the naming and origin of the data, but I didn't feel like it containing "wheel" in the name really hurt anything. Maybe I am an outlier, but my first questing after seeing a function named "project_metadata" would be "which metadata? from where?", as there are multiple metadata sources.

@jaraco
Copy link
Member

jaraco commented Dec 20, 2021

Oh! I see that pep517 already provides this same functionality. Let's get rid of it here.

@jaraco jaraco merged commit 9491ef9 into python:main Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants