Skip to content

Commit

Permalink
Merge pull request #459 from tiran/api-kw-only
Browse files Browse the repository at this point in the history
Use keyword-only args for API functions
  • Loading branch information
mergify[bot] authored Oct 2, 2024
2 parents 52ed2c3 + bee333b commit db8129d
Show file tree
Hide file tree
Showing 13 changed files with 103 additions and 39 deletions.
17 changes: 13 additions & 4 deletions src/fromager/build_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -197,6 +199,7 @@ def _createenv(self) -> None:


def prepare_build_environment(
*,
ctx: context.WorkContext,
req: Requirement,
sdist_root_dir: pathlib.Path,
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down
10 changes: 8 additions & 2 deletions src/fromager/commands/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
18 changes: 13 additions & 5 deletions src/fromager/commands/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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, "")
Expand Down
10 changes: 8 additions & 2 deletions src/fromager/commands/download_sequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")

Expand Down
12 changes: 9 additions & 3 deletions src/fromager/commands/step.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down Expand Up @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions src/fromager/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@


def get_build_system_dependencies(
*,
ctx: context.WorkContext,
req: Requirement,
sdist_root_dir: pathlib.Path,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/fromager/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@


def resolve(
*,
ctx: context.WorkContext,
req: Requirement,
sdist_server_url: str,
Expand Down
15 changes: 10 additions & 5 deletions src/fromager/sdist.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -427,7 +430,9 @@ def _resolve_prebuilt_with_history(
)
else:
servers = wheels.get_wheel_server_urls(ctx, req)
wheel_url, resolved_version = wheels.resolve_prebuilt_wheel(ctx, req, servers)
wheel_url, resolved_version = wheels.resolve_prebuilt_wheel(
ctx=ctx, req=req, wheel_server_urls=servers
)
return (wheel_url, resolved_version)


Expand All @@ -446,7 +451,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))

Expand Down Expand Up @@ -483,7 +488,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))

Expand Down Expand Up @@ -520,7 +525,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))

Expand Down
9 changes: 8 additions & 1 deletion src/fromager/sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -67,6 +71,7 @@ def download_source(


def resolve_source(
*,
ctx: context.WorkContext,
req: Requirement,
sdist_server_url: str,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -429,6 +435,7 @@ def prepare_new_source(


def build_sdist(
*,
ctx: context.WorkContext,
req: Requirement,
version: Version,
Expand Down
9 changes: 6 additions & 3 deletions src/fromager/wheels.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ def default_add_extra_metadata_to_wheels(


def add_extra_metadata_to_wheels(
*,
ctx: context.WorkContext,
req: Requirement,
version: Version,
Expand Down Expand Up @@ -235,6 +236,7 @@ def add_extra_metadata_to_wheels(


def build_wheel(
*,
ctx: context.WorkContext,
req: Requirement,
sdist_root_dir: pathlib.Path,
Expand Down Expand Up @@ -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],
Expand All @@ -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,
)
Expand Down
2 changes: 1 addition & 1 deletion tests/test_build_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
)
Expand Down
30 changes: 18 additions & 12 deletions tests/test_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand All @@ -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")])

Expand All @@ -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()
Expand All @@ -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")])

Expand All @@ -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()
Expand All @@ -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")])
Loading

0 comments on commit db8129d

Please sign in to comment.