python-wheel-build / fromager

Build your own wheels
https://pypi.org/project/fromager/
Apache License 2.0
3 stars 9 forks source link

Pass dist_info_dir to `add_extra_metadata_to_wheels` override #339

Closed tiran closed 2 weeks ago

tiran commented 3 weeks ago

The add_extra_metadata_to_wheels override method should have access to the unpacked wheel files. Let's run the method after unpacking the wheel and pass the dist_info_dir to the method, too. The caller can assume that the parent directory of dist_info_dir is the root directory of the unpacked wheel.

dhellmann commented 3 weeks ago

I think that method is designed with the idea that it returns data, and something else writes it to the wheel. @shubhbapna can confirm.

shubhbapna commented 3 weeks ago

Yes, currently it is designed to only return data which we will write into the fromager-build-settings file. Will giving access to dist_info_dir to the plugin cause the following issues:

dhellmann commented 3 weeks ago

We give plugins write information to lots of other aspects of the build, so if they choose to do something that undoes fromager's work I think I'd just say that's up to them and they get what they get as a result. We could also deal with that by writing the core metadata after the plugin is called, so we would be overwriting any files they create with our names.

The bigger question for me is, do we want plugins to write files or do we want them to give us data and let us write files? Letting them write files is more flexible, since the plugin can create whatever types of files it wants/needs. Having them return data to us means we control the format and location of the data, which is more consistent.

shubhbapna commented 3 weeks ago

I prefer to have control over the format and location of data:

tiran commented 3 weeks ago

I don't have any intent to write any files from add_extra_metadata_to_wheels. I would like to give the plugin method access to the unpacked files, so it can read and analyze the content of the wheel file.

shubhbapna commented 3 weeks ago

Make sense we can add it