cleanup and delete unnecessary tests

aignas · aignas · commit b9efaab4c7ac · 2024-07-15T21:24:57.000+09:00
diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl
@@ -27,6 +27,7 @@ load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_r
 load(":parse_whl_name.bzl", "parse_whl_name")
 load(":pip_repository_attrs.bzl", "ATTRS")
 load(":render_pkg_aliases.bzl", "whl_alias")
+load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
 load(":simpleapi_download.bzl", "simpleapi_download")
 load(":whl_library.bzl", "whl_library")
 load(":whl_repo_name.bzl", "whl_repo_name")
@@ -184,12 +185,16 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s
 
     requirements_by_platform = parse_requirements(
         module_ctx,
-        requirements_by_platform = pip_attr.requirements_by_platform,
-        requirements_linux = pip_attr.requirements_linux,
-        requirements_lock = pip_attr.requirements_lock,
-        requirements_osx = pip_attr.requirements_darwin,
-        requirements_windows = pip_attr.requirements_windows,
-        extra_pip_args = pip_attr.extra_pip_args,
+        requirements_by_platform = requirements_files_by_platform(
+            requirements_by_platform = pip_attr.requirements_by_platform,
+            requirements_linux = pip_attr.requirements_linux,
+            requirements_lock = pip_attr.requirements_lock,
+            requirements_osx = pip_attr.requirements_darwin,
+            requirements_windows = pip_attr.requirements_windows,
+            extra_pip_args = pip_attr.extra_pip_args,
+            python_version = major_minor,
+            logger = logger,
+        ),
         get_index_urls = get_index_urls,
         python_version = major_minor,
         evaluate_markers = lambda requirements: evaluate_markers(
@@ -305,7 +310,7 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s
 
         requirement = select_requirement(
             requirements,
-            platform = repository_platform,
+            platform = None if pip_attr.download_only else repository_platform,
         )
         if not requirement:
             # Sometimes the package is not present for host platform if there
diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl
@@ -29,7 +29,6 @@ behavior.
 load("//python/private:normalize_name.bzl", "normalize_name")
 load(":index_sources.bzl", "index_sources")
 load(":parse_requirements_txt.bzl", "parse_requirements_txt")
-load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
 load(":whl_target_platforms.bzl", "select_whls")
 
 # This includes the vendored _translate_cpu and _translate_os from
@@ -85,10 +84,6 @@ def parse_requirements(
         ctx,
         *,
         requirements_by_platform = {},
-        requirements_osx = None,
-        requirements_linux = None,
-        requirements_lock = None,
-        requirements_windows = None,
         extra_pip_args = [],
         get_index_urls = None,
         python_version = None,
@@ -102,10 +97,6 @@ def parse_requirements(
         requirements_by_platform (label_keyed_string_dict): a way to have
             different package versions (or different packages) for different
             os, arch combinations.
-        requirements_osx (label): The requirements file for the osx OS.
-        requirements_linux (label): The requirements file for the linux OS.
-        requirements_lock (label): The requirements file for all OSes, or used as a fallback.
-        requirements_windows (label): The requirements file for windows OS.
         extra_pip_args (string list): Extra pip arguments to perform extra validations and to
             be joined with args fined in files.
         get_index_urls: Callable[[ctx, list[str]], dict], a callable to get all
@@ -135,17 +126,6 @@ def parse_requirements(
 
         The second element is extra_pip_args should be passed to `whl_library`.
     """
-    requirements_by_platform, download_only = requirements_files_by_platform(
-        requirements_by_platform = requirements_by_platform,
-        requirements_osx = requirements_osx,
-        requirements_linux = requirements_linux,
-        requirements_lock = requirements_lock,
-        requirements_windows = requirements_windows,
-        extra_pip_args = extra_pip_args,
-        python_version = python_version,
-        logger = logger,
-    )
-
     options = {}
     requirements = {}
     for file, plats in requirements_by_platform.items():
@@ -209,7 +189,6 @@ def parse_requirements(
                     requirement_line = requirement_line,
                     target_platforms = [],
                     extra_pip_args = extra_pip_args,
-                    download = download_only,
                 ),
             )
             for_req.target_platforms.append(target_platform)
@@ -276,7 +255,6 @@ def parse_requirements(
                     requirement_line = r.requirement_line,
                     target_platforms = sorted(target_platforms),
                     extra_pip_args = r.extra_pip_args,
-                    download = r.download,
                     whls = whls,
                     sdist = sdist,
                     is_exposed = is_exposed,
@@ -294,8 +272,9 @@ def select_requirement(requirements, *, platform):
     Args:
         requirements (list[struct]): The list of requirements as returned by
             the `parse_requirements` function above.
-        platform (str): The host platform. Usually an output of the
-        `host_platform` function.
+        platform (str or None): The host platform. Usually an output of the
+            `host_platform` function. If None, then this function will return
+            the first requirement it finds.
 
     Returns:
         None if not found or a struct returned as one of the values in the
@@ -305,7 +284,7 @@ def select_requirement(requirements, *, platform):
     maybe_requirement = [
         req
         for req in requirements
-        if [p for p in req.target_platforms if p.endswith(platform)] or req.download
+        if not platform or [p for p in req.target_platforms if p.endswith(platform)]
     ]
     if not maybe_requirement:
         # Sometimes the package is not present for host platform if there
diff --git a/python/private/pypi/pip_repository.bzl b/python/private/pypi/pip_repository.bzl
@@ -21,6 +21,7 @@ load("//python/private:text_util.bzl", "render")
 load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
 load(":pip_repository_attrs.bzl", "ATTRS")
 load(":render_pkg_aliases.bzl", "render_pkg_aliases", "whl_alias")
+load(":requirements_files_by_platform.bzl", "requirements_files_by_platform")
 
 def _get_python_interpreter_attr(rctx):
     """A helper function for getting the `python_interpreter` attribute or it's default
@@ -71,11 +72,14 @@ exports_files(["requirements.bzl"])
 def _pip_repository_impl(rctx):
     requirements_by_platform = parse_requirements(
         rctx,
-        requirements_by_platform = rctx.attr.requirements_by_platform,
-        requirements_linux = rctx.attr.requirements_linux,
-        requirements_lock = rctx.attr.requirements_lock,
-        requirements_osx = rctx.attr.requirements_darwin,
-        requirements_windows = rctx.attr.requirements_windows,
+        requirements_by_platform = requirements_files_by_platform(
+            requirements_by_platform = rctx.attr.requirements_by_platform,
+            requirements_linux = rctx.attr.requirements_linux,
+            requirements_lock = rctx.attr.requirements_lock,
+            requirements_osx = rctx.attr.requirements_darwin,
+            requirements_windows = rctx.attr.requirements_windows,
+            extra_pip_args = rctx.attr.extra_pip_args,
+        ),
         extra_pip_args = rctx.attr.extra_pip_args,
     )
     selected_requirements = {}
@@ -85,6 +89,7 @@ def _pip_repository_impl(rctx):
         r = select_requirement(
             requirements,
             platform = repository_platform,
+            platform = None if rctx.attr.download_only else repository_platform,
         )
         if not r:
             continue
diff --git a/python/private/pypi/requirements_files_by_platform.bzl b/python/private/pypi/requirements_files_by_platform.bzl
@@ -128,10 +128,8 @@ def requirements_files_by_platform(
         fail_fn (Callable[[str], None]): A failure function used in testing failure cases.
 
     Returns:
-        A tuple where the first item is a dict with keys as the platform names
-        and values as requirement labels. The second item in the tuple is a
-        boolean to signify if we are set to only download wheels for a target
-        platform.
+        A dict with keys as the labels to the files and values as lists of
+        platforms that the files support.
     """
     if not (
         requirements_lock or
@@ -145,7 +143,7 @@ def requirements_files_by_platform(
             "via 'requirements_by_platform' or an os-specific lockfiles must be specified " +
             "via 'requirements_*' attributes",
         )
-        return None, None
+        return None
 
     platforms = _platforms_from_args(extra_pip_args)
     if logger:
@@ -168,7 +166,7 @@ def requirements_files_by_platform(
             # a single `requirements_lock` file instead of the OS specific ones as that is
             # the only correct way to use the API.
             fail_fn("only a single 'requirements_lock' file can be used when using '--platform' pip argument, consider specifying it via 'requirements_lock' attribute")
-            return None, None
+            return None
 
         files_by_platform = [
             (lock_files[0], platforms),
@@ -215,7 +213,7 @@ def requirements_files_by_platform(
                             file,
                         ),
                     )
-                    return None, None
+                    return None
 
                 configured_platforms[p] = file
         else:
@@ -246,7 +244,7 @@ def requirements_files_by_platform(
                     file,
                     p,
                 ))
-                return None, None
+                return None
             requirements[p] = file
 
     # Now return a dict that is similar to requirements_by_platform - where we
@@ -257,4 +255,4 @@ def requirements_files_by_platform(
     for plat, file in requirements.items():
         ret.setdefault(file, []).append(plat)
 
-    return ret, len(platforms) > 0
+    return ret
diff --git a/tests/pypi/parse_requirements/parse_requirements_tests.bzl b/tests/pypi/parse_requirements/parse_requirements_tests.bzl
diff --git a/tests/pypi/requirements_files_by_platform/requirements_files_by_platform_tests.bzl b/tests/pypi/requirements_files_by_platform/requirements_files_by_platform_tests.bzl
