feature(py_wheel) Resolve wheel props from user defined properties file

[mgosk]

Resolves #1804

Adds new custom_properties param to py_wheel. This feature allow to inject version generated by other target.

Tested with integration tests: bazel build examples/wheel:version_from_other_target.dist

[aignas]

I personally liked the suggested version_file attribute in the rule. I am wondering what other problems would this more generic templating rule solve?

Since expand_template rules already exist in bazel-skylib or aspect's bazel lib, I am not sure if rules_python should be doing templating here.

What do you think?

[mgosk]

ad1.

My initial idea also was to use simpler approach with :version_file with version inside. Unfortunately name of output wheel file is dynamically generated (contains version ) so I couldn't do it in build phase. In this situation I moved implementation to run phase next to stamping.

Right now I don't see any other use cases for custom_properties than passing version. I decided to implement feature this way to be consistent with stamping implementation.

ad2.

skylib expand_template works only on files in build phase. Current templating is done during run phase in wheelmaker python script.

Right now I see three potential solutions, please advise which is most suitable for you :

1. Keep implementation from this PR
2. Pass just :version_file param instead of custom_properites and use dedicated templating for this variable
3. Refactor py_wheel rule and use static wheelfile name. It breaks the py_wheel API but looks like more "bazelish" solution

[mgosk]

Hi @aignas any advices how I can improve this PR to get it merged

[aignas]

My initial thoughts are that having a bigger API surface may add extra maintenance to the rule set. Having the version taken from a file sounds like something that has precedent, e.g. rules_oci is doing this - it can only support a file for remote tags when pushing an artifact.

The bazel-skylib expand_template does not support templating based on the stamp value, but aspect's library does, similarly to how it is documented in rules_oci documentation here. A similar thing can be done with a simple genrule (although that if you don't want to depend on aspect's bazel lib. This is how one might do that with the genrule - it is one of the first search results, so please ignore the py_library code in the example.

Does this change your thoughts about how this should be implemented?

[github-actions[bot]]

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days. Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

[aignas]

I got hit by the dynamic output filename (depends on the version) as well and I think the best way would be to use a dir artifact. I think that would be the cleanest approach.