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?

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

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

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?