fix: forward well known attributes to macro expanded targets

[reltuk]

As per #369, trying to put a test in an oci_image target by making it testonly = True is currently broken.

This PR just adds the common rule attributetestonly to the oci_image and oci_tarball macros and forwards its value along to all the rules they instantiate.

[thesayyn]

I feel like we shouldn't have to do this one by one. I'll check if there's a macro that could return well-known/common kwargs so that we can forward them to all targets.

[alexeagle]

#350 is an example

[reltuk]

#350 is an example

Is the proposal to make a propagate_common_rule_attributes(kwargs) and then make all the rule invocations inside the macros take ..., **propagate_common_rule_attributes(kwargs) as their last parameter argument?

Is it typical / idiomatic / safe to just forward all unknown **kwargs to all rule invocations, and make every non-common-rule-attribute which the macros is actually concerned with a real parameter, not relying on the **kwargs glob for those specific attributes at all?

[thesayyn]

Is the proposal to make a propagate_common_rule_attributes(kwargs) and then make all the rule invocations inside the macros take ..., **propagate_common_rule_attributes(kwargs) as their last parameter argument?

exactly, but I believe this should go into bazel-lib. https://github.com/aspect-build/bazel-lib/tree/main

[reltuk]

Thanks for the feedback. I took a pass at something which may be helpful here: bazel-contrib/bazel-lib#553.

The changes here end up looking something like:

@@ -21,7 +22,7 @@ oci_image_rule = _oci_image
 oci_image_index = _oci_image_index
 oci_push_rule = _oci_push
 
-def oci_image(name, labels = None, annotations = None, env = None, cmd = None, entrypoint = None, tags = [], testonly = False, **kwargs):
+def oci_image(name, labels = None, annotations = None, env = None, cmd = None, entrypoint = None, tags = [], **kwargs):
     """Macro wrapper around [oci_image_rule](#oci_image_rule).
 
     Allows labels and annotations to be provided as a dictionary, in addition to a text file.
@@ -45,6 +46,9 @@ def oci_image(name, labels = None, annotations = None, env = None, cmd = None, e
         tags: Tags to propagate to targets declared by this macro.
         **kwargs: other named arguments to [oci_image_rule](#oci_image_rule)
     """
+    forwarded_kwargs = propagate_common_rule_attributes(kwargs)
+    forwarded_kwargs.pop("tags", None)
+
     if types.is_dict(annotations):
         annotations_label = "_{}_write_annotations".format(name)
         write_file(
@@ -52,7 +56,7 @@ def oci_image(name, labels = None, annotations = None, env = None, cmd = None, e
             out = "_{}.annotations.txt".format(name),
             content = ["{}={}".format(key, value) for (key, value) in annotations.items()],
             tags = tags,
-            testonly = testonly,
+            **forwarded_kwargs,
         )
         annotations = annotations_label
 

[alexeagle]

@reltuk we released bazel-lib yesterday including your change, so this should be unblocked now ;)

[reltuk]

@alexeagle Thanks for the ping and the quick response! I took a pass at the dep bump + the change to the macros. Happy to split it up into separate PRs as well, if that's preferred.

[reltuk]

Took a pass at removing tags and also added forwarding common attributes for oci_push. I originally omitted it, since testonly didn't seem to be directly relevant, but there are use cases where it might be applicable.

[reltuk]

bazel run //docs:update for me is a no-op on this branch. Is there something I should do to get this test passing?

Thanks for your help :).

[thesayyn]

bazel run //docs:update for me is a no-op on this branch. Is there something I should do to get this test passing?

Thanks for your help :).

I think you need to rebase and then run the command.

[reltuk]

Unfortunately, none of the following produced any updates:

1. Rebasing against github.com/bazel-contrib/rules_oci:main

2. Running on Linux instead of macOS.

3. Running with 6.2.0 instead of 6.3.2.

I may get a chance to poke at it further in a bit :).

[adzenith]

looks like there's a typo in "attributes" (here and in four other locations in the PR)

[reltuk]

Thanks for catching that. Just took a pass at fixing it.