fix(pypi): drop unsupported args in the whl_library macro

news/4077.fixed.md

@@ -0,0 +1,2 @@
+(pypi) Fixed the handling of optional args for the {obj}`pip_archive` and {obj}`whl_archive`
+repository rules within the {obj}`whl_library`. From now on we are dropping unsupported args.

python/private/pypi/pip_archive.bzl

@@ -338,7 +338,13 @@ def _pip_archive_impl(rctx):
     patch_and_extract_whl(rctx, whl_path = whl_path, logger = logger, sdist_filename = sdist_filename)
 
 # NOTE @aignas 2024-03-21: The usage of dict({}, **common) ensures that all args to `dict` are unique
-_attrs = whl_archive_attrs | {
+pip_archive_attrs = {
+    k: v
+    for k, v in whl_archive_attrs.items()
+    # Only the whl_file parameter is unusable in the pip_archive rule. the rest can be
+    # reused.
+    if k != "whl_file"
+} | {
     k: ATTRS[k]
     for k in [
         # used for pulling deps with pip
@@ -352,31 +358,30 @@ _attrs = whl_archive_attrs | {
         "quiet",
         "timeout",
     ]
-} | {
-    "_python_path_entries": attr.label_list(
-        # Get the root directory of these rules and keep them as a default attribute
-        # in order to avoid unnecessary repository fetching restarts.
-        #
-        # This is very similar to what was done in https://github.com/bazelbuild/rules_go/pull/3478
-        default = [
-            Label("//:BUILD.bazel"),
-        ] + [
-            # Includes all the external dependencies from repositories.bzl
-            Label("@" + repo + "//:BUILD.bazel")
-            for repo in all_repo_names
-        ],
-    ),
-    "_python_srcs": attr.label_list(
-        # Used as a default value in a rule to ensure we fetch the dependencies.
-        default = [
-            Label("//python/private/pypi/whl_installer:wheel_installer.py"),
-            Label("//python/private/pypi/whl_installer:arguments.py"),
-        ] + record_files.values(),
-    ),
 }
 
 pip_archive = repository_rule(
-    attrs = _attrs | {
+    attrs = pip_archive_attrs | {
+        "_python_path_entries": attr.label_list(
+            # Get the root directory of these rules and keep them as a default attribute
+            # in order to avoid unnecessary repository fetching restarts.
+            #
+            # This is very similar to what was done in https://github.com/bazelbuild/rules_go/pull/3478
+            default = [
+                Label("//:BUILD.bazel"),
+            ] + [
+                # Includes all the external dependencies from repositories.bzl
+                Label("@" + repo + "//:BUILD.bazel")
+                for repo in all_repo_names
+            ],
+        ),
+        "_python_srcs": attr.label_list(
+            # Used as a default value in a rule to ensure we fetch the dependencies.
+            default = [
+                Label("//python/private/pypi/whl_installer:wheel_installer.py"),
+                Label("//python/private/pypi/whl_installer:arguments.py"),
+            ] + record_files.values(),
+        ),
         "_rule_name": attr.string(default = "pip_archive"),
     },
     doc = """

python/private/pypi/whl_archive.bzl

@@ -88,6 +88,10 @@ The expected checksum of the downloaded whl in Subresource Integrity format
 The list of urls of the whl to be downloaded using bazel downloader. Using this
 attr makes `extra_pip_args` and `download_only` ignored.""",
     ),
+    # attributes only relevant to this rule and not reusable outside
+    "whl_file": attr.label(
+        doc = "The whl file that should be used instead of downloading or building the whl.",
+    ),
     "whl_patches": attr.label_keyed_string_dict(
         doc = """
 A label-keyed-string dict with patch files as keys and json-strings as values.
@@ -125,10 +129,6 @@ way to define whl_library and move whl patching to a separate place. INTERNAL US
 
 whl_archive = repository_rule(
     attrs = whl_archive_attrs | {
-        # attributes only relevant to this rule and not reusable outside
-        "whl_file": attr.label(
-            doc = "The whl file that should be used instead of downloading or building the whl.",
-        ),
         "_rule_name": attr.string(default = "whl_archive"),
     },
     doc = """

python/private/pypi/whl_library.bzl

@@ -14,8 +14,22 @@
 
 ""
 
-load(":pip_archive.bzl", "pip_archive")
-load(":whl_archive.bzl", "whl_archive")
+load(":pip_archive.bzl", "pip_archive", "pip_archive_attrs")
+load(":whl_archive.bzl", "whl_archive", "whl_archive_attrs")
+
+def _filter(kwargs, subset, debug = False):
+    dropped = {}
+    filtered = {}
+    for k, v in kwargs.items():
+        if k in subset:
+            filtered[k] = v
+        else:
+            dropped[k] = v
+
+    if debug:
+        print("Ignored args: {}".format(dropped))  # buildifier: disable=print
+
+    return filtered
 
 def whl_library(name, repo = None, **kwargs):
     """Create a whl_library.
@@ -42,6 +56,6 @@ def whl_library(name, repo = None, **kwargs):
     kwargs.setdefault("dep_template", "@{}{{name}}//:{{target}}".format(kwargs.pop("repo_prefix", "")))
 
     if whl_file or (urls and filename and filename.endswith(".whl")):
-        whl_archive(name = name, **kwargs)
+        whl_archive(name = name, **_filter(kwargs, whl_archive_attrs))
     else:
-        pip_archive(name = name, **kwargs)
+        pip_archive(name = name, **_filter(kwargs, pip_archive_attrs))