feat: Adding variable support for distribution in py_wheel

[ns-tkonduri]

This allows the distribution attribute to expand workspace status keys, just as the
version attribute can.

This allows, for example, the VCS's branch name (e.g test, release, etc) to be part of the
distribution name without having to modify the BUILD file. Having distinct distribution
names is necessary because tools like pip-compile, which determine version compatibility
and replacements, and having the same distribution name would mean the different builds
are seen as equivalent.

Closes #1250

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[chrislovecnm]

@ns-tkonduri can you sign the CLA?

[ns-tkonduri]

@chrislovecnm I just signed it. Thank you.
Can you please review the PR and suggest changes for my feature request: #1250

[chrislovecnm]

You need to run the doc update target please.

[ns-tkonduri]

@chrislovecnm @rickeylev I have updated the PR with my changes and the github checks are passing, Can you please review the changes? Thanks.

[rickeylev]

FYI I'm not too familiar with this code, that said:

First, can you explain a bit more about your use case and goal? What from the make vars or status keys do you want to put in your distribution name and why?

The original intent seems to have been to support make variables, but that looks to have been removed? The Starlark code that did makevars expansion is gone. Instead, it's now supporting workspace status key expansion. Which is more appropriate depends on what you're trying to do. Something worth noting is volatile status keys will be "wrong" on subsequent/incremental builds.

Otherwise, can you add some tests? The original change to just do makevar expansion was simple enough I don't think it necessitated tests, but now the Python tool is modified with if-else statements, so testing is more important.

Please also update the PR description to:

• Better answer the "why" for this feature addition. Note that a reason like "makes the distribution attribute expand variables so that you can use variables in the distribute attribute" doesn't really answer "why"
• Be a more appropriate commit message (the pr description will become the commit message)

[rickeylev]

Great explanation for the PR and FR, thanks! I've shorted the PR description to make for a better commit message.

Just needs some sort of test to verify the expansion is happening, otherwise LGTM.

[ns-tkonduri]

@rickeylev I prepared these unit tests wheelmaker_test.py (file is in the PR) which I tested locally and it's working but I found no framework for hooking this into the builds, and also I looked for some sort of end-to-end test that I can piggyback on to enhance this logic, but could not find that either. So this seems like more of an effort than simply adding unit tests, for this, I will require to add a framework that exceeds my expertise as a new contributor like me to the project.

[rickeylev]

Ah, sorry -- I should have pointed you more explicitly to what I had in mind. Thanks for creating wheelmaker_test.py, but you can delete that -- apparently we don't have any unit tests of that code; I'm not going require you to have to establish that from scratch.

Instead:

Most of py_wheel is tested using examples/py_wheel/BUILD.bazel. The basic structure is a py_wheel is defined, added as a data dependency to :wheel_test, and then wheel_test.py has code to load the file and assert on the contents.

For an example, see :minimal_with_py_library_with_stamp, and its corresponding test test_rule_sets_stamped_version_in_wheel_metadata. This is the test for the expansion of the version attribute.

What I would suggest is either modifying the :minimal_with_py_library_with_stamp target and test to set e.g. distribution="example_minimal_library.user_{BUILD_USER}" and just check that the output matches "user_XXX" where XXX isn't {BUILD_USER} (we need to test that the expansion occurred, but the value isn't stable).

[ns-tkonduri]

@rickeylev Thanks for pointing out the way you test.
I updated the test case, but looks like there is some unrelated check that failed, can you please have a look and re-trigger it from your side?
Test the Gazelle extension using minimum supported Bazel version on :ubuntu: 20.04 LTS (OpenJDK 11, gcc 9.4.0)

[rickeylev]

Just needs the test renamed, otherwise LGTM.

[rickeylev]

Also: yes, that test is flaky; i retried and it passed.

[ns-tkonduri]

@rickeylev Thank you, I updated the test name, and all the checks are green now.
Can you please help merge the PR as I don't have write access to this repository?