feat(bzlmod): Implementing wheel annotations via whl_mods

[chrislovecnm]

This commit implements a bzlmod extension that allows users to create "annotations" for wheel builds. The wheel_builder.py accepts a JSON file via a parameter called annotations; this extension creates those JSON files. The pip extension accepts a Label -> String dict argument of the JSON files.

This feature is renamed to whl_mods because the JSON files are handled differently
and the name "annotations" is uninformative. This modifies the creation of the BUILD
files and their content, and is much more than just adding some notes about a whl.

The whl_mod extension wheel names and the wheel names in pip must match.

Closes: #1213

[rickeylev]

I'm still kinda meh about using an extension to do this, but this does provide a relatively straight forward way to port a workspace build to bzlmod, so that's a decent win.

Any thought to "auto registering" the modifications?
i.e. by doing this:

pip = use_extension(...)
pip.parse(
  hub_name = "pip", ...
)
pip.whl_mod(
  pip_hub = "pip"
  whl_name = "requests,
)

The "requests" wheel will automatically get modified? No need to manually pass the whl_modifications= arg.

Under the hood, all we need to do is generate e.g. hub_name = pip_name + "_mods"

[rickeylev]

The hub and spoke part isn't very relevant to users (they can't control it, nor do they really see it. The available attributes are self-documenting in the attrs structure, plus we don't support all the same attributes (e.g incompatible_generate_aliases), so pointing users to that is a bit misleading.

Suggested change

	This tag class is used to create a pip hub and all of the spokes that are part of that hub.
	This tag class reuses the pip attributes that are found in
	@rules_python//python/pip_install:pip_repository.bzl
	This tag class resolves requirements.txt into dependencies usable by Bazel.

[chrislovecnm]

The attributes are not self-documenting because we cannot use stardoc to generate the docs for extensions yet. We have an issue open. As a new user, digging through the code to find the docs is a pain. Do you want me to link the docs for pip or leave them as is?

[rickeylev]

I think referring to the pip repo attrs is ok, but saying/implying it reuses them or supports all of them isn't (it just isn't accurate/true). i.e. "most of the pip attrs are supported" but not "all the pip attrs are supported"

[chrislovecnm]

I understand. I will figure out how to document it better.

[rickeylev]

I think string attributes default to None? So the second expression will always be true.

[chrislovecnm]

mods.additive_build_content_file is a label. Please explain more. I wish we had unit tests for this darn stuff.

[rickeylev]

Sorry, I meant the additive_build_content != "" part of the expression.

[chrislovecnm]

See https://bazel.build/rules/lib/toplevel/attr#string. It defaults to an empty string if I am reading the docs correctly.

[chrislovecnm]

Any thought to "auto registering" the modifications?

@rickeylev I have thought about it, but we have a use case that I could see could cause us a problem.

I'm curious about what you think. Here is my use case.

We have a foo wheel that differs between Python 3.10 and 3.11, and I need the BUILD files to differ between Python 3.10 and 3.11. If we turn on auto registration, we would need to be able to turn it off as well because I would need to use different hub_names for 3.10 vs. 3.11.

So that would be more code to maintain. Also, it may make it more difficult to debug this because I cannot, as a user, easily change the label.

We could autoregister by a hub_name. Create a whl_mods_hub_name attribute, and then autoregister off of that.

As a user with tricky stuff like this, I like fine-grain control; for instance, I can remove an element from a dict rather than modify the entire whl_mod.

I think the number of people that use this is low, so I would rather have less code and less "magic" happening in the background. We need to document it well.

[chrislovecnm]

@rickeylev, if when we merge, if the repo is not set up to do a squash on the commits; I need to open another PR on a different branch. Somehow my commits got unmanageable, and doing a rebase is driving me up the wall.

[rickeylev]

re: squash on commits: squash on commits is enabled. The PR description and title become the commit message.

re: whl mods varying by python version: We could just have python_versions attribute of the whl_mod call that says what versions it applies to.

But, all this said, auto-registration is a nice to have, not a requirement. I'm kinda meh about using an extension for this already, and like the idea of having pip.parse have an arg to specify modifications (as we have now). This lets users create the config themselves if they want, and we can always allow specifying a bzl file, patch file, or w/e in the future to handle some of the other ideas posted in the issue.

[rickeylev]

The core impl here is fine, so I'm going to give approval before I head out on vacation.

[groodt]

At Canva, we just provide an optional patches argument, which is semantically similar to the patches in http_file and other built-ins. We apply the patches to the unpacked wheels. I think this approach has benefits over providing a bespoke and less flexible implementation with similar behaviour.

We can look into upstreaming our implementation and deprecating annotations and whl_mods if you are interested?

It looks quite clumsy having to build a DSL that supports a weaker and less flexible form of patching. You've got provide arguments, teach people about new functionality, serialize data that represents the transformations to apply that are then deserialized and applied later. With patches, it would look similar I believe, but it wouldn't require a custom DSL or logic, we would just leverage rctx.patch.