Use keyword-only args for API functions

Import API functions no longer accept positional arguments and only support keyword arguments. This makes the code more reliable, readable, and prepares the functions for the `timeit` decorator. Related: #408 Related: #445 Signed-off-by: Christian Heimes <[email protected]>
python-wheel-build · Oct 2, 2024 · 75c011c · 75c011c
1 parent 16df958
commit 75c011c
Show file tree

Hide file tree

Showing 13 changed files with 100 additions and 38 deletions.
diff --git a/src/fromager/build_environment.py b/src/fromager/build_environment.py
@@ -52,7 +52,9 @@ def __init__(
         resolutions = []
         for r in all_reqs:
             try:
-                _, version = resolver.resolve(ctx, r, resolver.PYPI_SERVER_URL)
+                _, version = resolver.resolve(
+                    ctx=ctx, req=r, sdist_server_url=resolver.PYPI_SERVER_URL
+                )
             except Exception as err:
                 resolutions.append(f"{r} -> {err}")
             else:
@@ -197,6 +199,7 @@ def _createenv(self) -> None:
 
 
 def prepare_build_environment(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     sdist_root_dir: pathlib.Path,
@@ -205,7 +208,9 @@ def prepare_build_environment(
 
     next_req_type = RequirementType.BUILD_SYSTEM
     build_system_dependencies = dependencies.get_build_system_dependencies(
-        ctx, req, sdist_root_dir
+        ctx=ctx,
+        req=req,
+        sdist_root_dir=sdist_root_dir,
     )
 
     for dep in build_system_dependencies:
@@ -227,7 +232,9 @@ def prepare_build_environment(
 
     next_req_type = RequirementType.BUILD_BACKEND
     build_backend_dependencies = dependencies.get_build_backend_dependencies(
-        ctx, req, sdist_root_dir
+        ctx=ctx,
+        req=req,
+        sdist_root_dir=sdist_root_dir,
     )
 
     for dep in build_backend_dependencies:
@@ -249,7 +256,9 @@ def prepare_build_environment(
 
     next_req_type = RequirementType.BUILD_SDIST
     build_sdist_dependencies = dependencies.get_build_sdist_dependencies(
-        ctx, req, sdist_root_dir
+        ctx=ctx,
+        req=req,
+        sdist_root_dir=sdist_root_dir,
     )
 
     for dep in build_sdist_dependencies:

diff --git a/src/fromager/commands/bootstrap.py b/src/fromager/commands/bootstrap.py
@@ -110,10 +110,16 @@ def bootstrap(
         pbi = wkctx.package_build_info(req)
         if pbi.pre_built:
             servers = wheels.get_wheel_server_urls(wkctx, req)
-            source_url, version = wheels.resolve_prebuilt_wheel(wkctx, req, servers)
+            source_url, version = wheels.resolve_prebuilt_wheel(
+                ctx=wkctx,
+                req=req,
+                wheel_server_urls=servers,
+            )
         else:
             source_url, version = sources.resolve_source(
-                wkctx, req, resolver.PYPI_SERVER_URL
+                ctx=wkctx,
+                req=req,
+                sdist_server_url=resolver.PYPI_SERVER_URL,
             )
         logger.info("%s resolves to %s", req, version)
         wkctx.dependency_graph.add_dependency(

diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py
@@ -84,7 +84,9 @@ def build(
     server.start_wheel_server(wkctx)
     req = Requirement(f"{dist_name}=={dist_version}")
     source_url, version = sources.resolve_source(
-        ctx=wkctx, req=req, sdist_server_url=sdist_server_url
+        ctx=wkctx,
+        req=req,
+        sdist_server_url=sdist_server_url,
     )
     wheel_filename = _build(wkctx, version, req, source_url)
     print(wheel_filename)
@@ -163,7 +165,9 @@ def build_sequence(
                     resolved_version,
                 )
                 wheel_filename = wheels.download_wheel(
-                    req, source_download_url, wkctx.wheels_build
+                    req=req,
+                    wheel_url=source_download_url,
+                    output_directory=wkctx.wheels_build,
                 )
             else:
                 logger.info(
@@ -308,11 +312,13 @@ def _build(
 
     # Prepare source
     source_root_dir = sources.prepare_source(
-        wkctx, req, source_filename, resolved_version
+        ctx=wkctx, req=req, source_filename=source_filename, version=resolved_version
     )
 
     # Build environment
-    build_environment.prepare_build_environment(wkctx, req, source_root_dir)
+    build_environment.prepare_build_environment(
+        ctx=wkctx, req=req, sdist_root_dir=source_root_dir
+    )
     build_env = build_environment.BuildEnvironment(wkctx, source_root_dir.parent, None)
 
     # Make a new source distribution, in case we patched the code.
@@ -358,7 +364,9 @@ def _is_wheel_built(
 
     try:
         logger.info(f"{req.name}: checking if {req} was already built")
-        url, _ = wheels.resolve_prebuilt_wheel(wkctx, req, [wkctx.wheel_server_url])
+        url, _ = wheels.resolve_prebuilt_wheel(
+            ctx=wkctx, req=req, wheel_server_urls=[wkctx.wheel_server_url]
+        )
         pbi = wkctx.package_build_info(req)
         build_tag_from_settings = pbi.build_tag(resolved_version)
         build_tag = build_tag_from_settings if build_tag_from_settings else (0, "")

diff --git a/src/fromager/commands/download_sequence.py b/src/fromager/commands/download_sequence.py
@@ -89,8 +89,14 @@ def download_one(entry: dict[str, typing.Any]):
 
         if include_wheels:
             try:
-                wheel_url, _ = wheels.resolve_prebuilt_wheel(wkctx, req, wheel_servers)
-                wheels.download_wheel(req, wheel_url, wkctx.wheels_downloads)
+                wheel_url, _ = wheels.resolve_prebuilt_wheel(
+                    ctx=wkctx, req=req, wheel_server_urls=wheel_servers
+                )
+                wheels.download_wheel(
+                    req=req,
+                    wheel_url=wheel_url,
+                    output_directory=wkctx.wheels_downloads,
+                )
             except Exception as err:
                 logger.error(f"failed to download wheel for {req}: {err}")
 

diff --git a/src/fromager/commands/step.py b/src/fromager/commands/step.py
@@ -85,8 +85,12 @@ def prepare_source(
         raise RuntimeError(
             f"Cannot find sdist for {req.name} version {dist_version} in {sdists_downloads} among {dir_contents}"
         )
-    # FIXME: Does the version need to be a Version instead of str?
-    source_root_dir = sources.prepare_source(wkctx, req, source_filename, dist_version)
+    source_root_dir = sources.prepare_source(
+        ctx=wkctx,
+        req=req,
+        source_filename=source_filename,
+        version=dist_version,
+    )
     print(source_root_dir)
 
 
@@ -156,7 +160,9 @@ def prepare_build(
     server.start_wheel_server(wkctx)
     req = Requirement(f"{dist_name}=={dist_version}")
     source_root_dir = _find_source_root_dir(wkctx, wkctx.work_dir, req, dist_version)
-    build_environment.prepare_build_environment(wkctx, req, source_root_dir)
+    build_environment.prepare_build_environment(
+        ctx=wkctx, req=req, sdist_root_dir=source_root_dir
+    )
 
 
 @step.command()

diff --git a/src/fromager/dependencies.py b/src/fromager/dependencies.py
@@ -20,6 +20,7 @@
 
 
 def get_build_system_dependencies(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     sdist_root_dir: pathlib.Path,
@@ -84,6 +85,7 @@ def default_get_build_system_dependencies(
 
 
 def get_build_backend_dependencies(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     sdist_root_dir: pathlib.Path,
@@ -139,6 +141,7 @@ def default_get_build_backend_dependencies(
 
 
 def get_build_sdist_dependencies(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     sdist_root_dir: pathlib.Path,

diff --git a/src/fromager/resolver.py b/src/fromager/resolver.py
@@ -46,6 +46,7 @@
 
 
 def resolve(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     sdist_server_url: str,

diff --git a/src/fromager/sdist.py b/src/fromager/sdist.py
@@ -141,7 +141,10 @@ def handle_requirement(
 
     if not pre_built:
         sdist_root_dir = sources.prepare_source(
-            ctx, req, source_filename, resolved_version
+            ctx=ctx,
+            req=req,
+            source_filename=source_filename,
+            version=resolved_version,
         )
         unpack_dir = sdist_root_dir.parent
 
@@ -446,7 +449,7 @@ def _handle_build_system_requirements(
     prev_graph: DependencyGraph | None,
 ) -> set[Requirement]:
     build_system_dependencies = dependencies.get_build_system_dependencies(
-        ctx, req, sdist_root_dir
+        ctx=ctx, req=req, sdist_root_dir=sdist_root_dir
     )
     progressbar.update_total(len(build_system_dependencies))
 
@@ -483,7 +486,7 @@ def _handle_build_backend_requirements(
     prev_graph: DependencyGraph | None,
 ) -> set[Requirement]:
     build_backend_dependencies = dependencies.get_build_backend_dependencies(
-        ctx, req, sdist_root_dir
+        ctx=ctx, req=req, sdist_root_dir=sdist_root_dir
     )
     progressbar.update_total(len(build_backend_dependencies))
 
@@ -520,7 +523,7 @@ def _handle_build_sdist_requirements(
     prev_graph: DependencyGraph | None,
 ) -> set[Requirement]:
     build_sdist_dependencies = dependencies.get_build_sdist_dependencies(
-        ctx, req, sdist_root_dir
+        ctx=ctx, req=req, sdist_root_dir=sdist_root_dir
     )
     progressbar.update_total(len(build_sdist_dependencies))
 

diff --git a/src/fromager/sources.py b/src/fromager/sources.py
@@ -46,7 +46,11 @@ def get_source_type(ctx: context.WorkContext, req: Requirement) -> str:
 
 
 def download_source(
-    ctx: context.WorkContext, req: Requirement, version: Version, download_url: str
+    *,
+    ctx: context.WorkContext,
+    req: Requirement,
+    version: Version,
+    download_url: str,
 ) -> pathlib.Path:
     source_path = overrides.find_and_invoke(
         req.name,
@@ -67,6 +71,7 @@ def download_source(
 
 
 def resolve_source(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     sdist_server_url: str,
@@ -350,6 +355,7 @@ def read_build_meta(unpack_dir: pathlib.Path) -> dict:
 
 
 def prepare_source(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     source_filename: pathlib.Path,
@@ -429,6 +435,7 @@ def prepare_new_source(
 
 
 def build_sdist(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     version: Version,

diff --git a/src/fromager/wheels.py b/src/fromager/wheels.py
@@ -116,6 +116,7 @@ def default_add_extra_metadata_to_wheels(
 
 
 def add_extra_metadata_to_wheels(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     version: Version,
@@ -235,6 +236,7 @@ def add_extra_metadata_to_wheels(
 
 
 def build_wheel(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     sdist_root_dir: pathlib.Path,
@@ -379,6 +381,7 @@ def get_wheel_server_urls(ctx: context.WorkContext, req: Requirement) -> list[st
 
 
 def resolve_prebuilt_wheel(
+    *,
     ctx: context.WorkContext,
     req: Requirement,
     wheel_server_urls: list[str],
@@ -387,9 +390,9 @@ def resolve_prebuilt_wheel(
     for url in wheel_server_urls:
         try:
             wheel_url, resolved_version = resolver.resolve(
-                ctx,
-                req,
-                url,
+                ctx=ctx,
+                req=req,
+                sdist_server_url=url,
                 include_sdists=False,
                 include_wheels=True,
             )

diff --git a/tests/test_build_environment.py b/tests/test_build_environment.py
@@ -18,7 +18,7 @@ def test_missing_dependency_format(
         "flit_core": "3.9.0",
         "setuptools": "69.5.1",
     }
-    resolve_dist.side_effect = lambda ctx, req, url: (
+    resolve_dist.side_effect = lambda ctx, req, sdist_server_url: (
         "",
         Version(resolutions[req.name]),
     )

diff --git a/tests/test_dependencies.py b/tests/test_dependencies.py
@@ -70,9 +70,9 @@ def _with_cleanup(*args, **kwds):
 @_clean_build_artifacts
 def test_get_build_system_dependencies(_: Mock, tmp_context: context.WorkContext):
     results = dependencies.get_build_system_dependencies(
-        tmp_context,
-        Requirement("fromager"),
-        _fromager_root,
+        ctx=tmp_context,
+        req=Requirement("fromager"),
+        sdist_root_dir=_fromager_root,
     )
     names = set(r.name for r in results)
     assert names == set(["setuptools", "setuptools_scm"])
@@ -87,7 +87,9 @@ def test_get_build_system_dependencies_cached(
     req_file = tmp_path / "build-system-requirements.txt"
     req_file.write_text("foo==1.0")
     results = dependencies.get_build_system_dependencies(
-        tmp_context, Requirement("fromager"), sdist_root_dir
+        ctx=tmp_context,
+        req=Requirement("fromager"),
+        sdist_root_dir=sdist_root_dir,
     )
     assert results == set([Requirement("foo==1.0")])
 
@@ -96,9 +98,9 @@ def test_get_build_system_dependencies_cached(
 @_clean_build_artifacts
 def test_get_build_backend_dependencies(_: Mock, tmp_context: context.WorkContext):
     results = dependencies.get_build_backend_dependencies(
-        tmp_context,
-        Requirement("fromager"),
-        _fromager_root,
+        ctx=tmp_context,
+        req=Requirement("fromager"),
+        sdist_root_dir=_fromager_root,
     )
     names = set(r.name for r in results)
     assert names == set()
@@ -113,7 +115,9 @@ def test_get_build_backend_dependencies_cached(
     req_file = tmp_path / "build-backend-requirements.txt"
     req_file.write_text("foo==1.0")
     results = dependencies.get_build_backend_dependencies(
-        tmp_context, Requirement("fromager"), sdist_root_dir
+        ctx=tmp_context,
+        req=Requirement("fromager"),
+        sdist_root_dir=sdist_root_dir,
     )
     assert results == set([Requirement("foo==1.0")])
 
@@ -122,9 +126,9 @@ def test_get_build_backend_dependencies_cached(
 @_clean_build_artifacts
 def test_get_build_sdist_dependencies(_: Mock, tmp_context: context.WorkContext):
     results = dependencies.get_build_sdist_dependencies(
-        tmp_context,
-        Requirement("fromager"),
-        _fromager_root,
+        ctx=tmp_context,
+        req=Requirement("fromager"),
+        sdist_root_dir=_fromager_root,
     )
     names = set(r.name for r in results)
     assert names == set()
@@ -139,6 +143,8 @@ def test_get_build_sdist_dependencies_cached(
     req_file = tmp_path / "build-sdist-requirements.txt"
     req_file.write_text("foo==1.0")
     results = dependencies.get_build_sdist_dependencies(
-        tmp_context, Requirement("fromager"), sdist_root_dir
+        ctx=tmp_context,
+        req=Requirement("fromager"),
+        sdist_root_dir=sdist_root_dir,
     )
     assert results == set([Requirement("foo==1.0")])