fix(packaging): Format `METADATA` correctly if given empty `requires_file` by FrankPortman · Pull Request #2771 · bazel-contrib/rules_python

I am not sure I like using regex to solve this

I don't really like it either. If you look at the commit history you will see I attempted to reconstruct the METADATA file line by line, but that broke a bunch of tests. I am guessing the metadata.replace(....) calls were doing more than just "fixing" one line at a time, which felt confusing to me.

Do you know where the Requires-Dist appears from in the first place? Maybe we should error if the requirements are empty/not-specified? I see that the metadata file is read from a file, so I assume this could be templated out somewhere else in our code.

It comes in via this file as far as I can tell:

if ctx.attr.python_requires:

Personally, I would prefer the empty file case to be handled seamlessly (like in this PR), than fail. I believe this is what rules_python used to do, because our code had been working until we upgraded. But I didn't do a thorough bisect search so I could be wrong (maybe we changed something else at the same time).

Right now, we wrap py_wheel in a macro, and it would be nice to not have to push the "is empty requires" to the analysis phase. But, we can always patch this internally if you prefer that the default behavior in rules_python is to error. Let me know how you want to proceed.

Extra things that this PR will need:

  • The packaging rule should have :::versionchanged::: when we agree on the behaviour change.
  • The CHANGELOG entry should be there mentioning the fix briefly.

Adding now.