Skip to content

Commit

Permalink
feat: Support multiple constraints files
Browse files Browse the repository at this point in the history
Fromager now supports multiple constraints files. The option
`fromager --constraints-file` can be supplied multiple times. The option
no longer accepts URLs. Remote constraints files are now specified with
`--constraints-url`. The split was necessary to correctly parse
environment variables with multiple entries and white spaces in local
file names. Paths are split at `:`, URLs are split at ` ` (white space).

There can only be one constraint per package.

Internally, remote constraints from `https://` URLs are no longer
retrieved twice. Instead all constraints are merged and the final set is
dumped into a single constraints file.

Fixes: python-wheel-build#539
Signed-off-by: Christian Heimes <cheimes@redhat.com>
  • Loading branch information
tiran committed Jan 24, 2025
1 parent d7f5395 commit 481ab95
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 37 deletions.
12 changes: 8 additions & 4 deletions e2e/test_bootstrap_prerelease.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,20 @@ fi

$pass

CONSTRAINTS_FILE="$OUTDIR/flit_core_constraints.txt"
echo "flit_core==2.0rc3" > $CONSTRAINTS_FILE
CONSTRAINTS_FILE_1="$OUTDIR/flit_core_constraints.txt"
echo "flit_core==2.0rc3" > $CONSTRAINTS_FILE_1

CONSTRAINTS_FILE_2="$OUTDIR/misc_constraints.txt"
echo "setuptools>=75.8.0" > $CONSTRAINTS_FILE_2

DEBUG_RESOLVER=true fromager \
--verbose \
--log-file="$OUTDIR/bootstrap.log" \
--sdists-repo="$OUTDIR/sdists-repo" \
--wheels-repo="$OUTDIR/wheels-repo" \
--work-dir="$OUTDIR/work-dir" \
--constraints-file="$CONSTRAINTS_FILE" \
--constraints-file="$CONSTRAINTS_FILE_1" \
--constraints-file="$CONSTRAINTS_FILE_2" \
bootstrap $DIST || true


Expand All @@ -46,4 +50,4 @@ if ! grep -q "flit_core: new toplevel dependency flit_core<2.0.1 resolves to 2.0
pass=false
fi

$pass
$pass
28 changes: 24 additions & 4 deletions src/fromager/__main__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env python3

import logging
import os
import pathlib
import sys

Expand Down Expand Up @@ -107,8 +108,20 @@
@click.option(
"-c",
"--constraints-file",
# for click.Path, click splits env vars on os.pathsep (':')
type=clickext.ClickPath(exists=True, file_okay=True, dir_okay=False),
multiple=True,
default=[],
help="location of constraints files (multiple use)",
)
@click.option(
"--constraints-url",
# for str, click splits env vars on space
type=str,
help="location of the constraints file",
multiple=True,
default=[],
callback=clickext.verify_url_callback,
help="remote location of constraints file as URL (multiple use)",
)
@click.option(
"--cleanup/--no-cleanup",
Expand Down Expand Up @@ -142,7 +155,8 @@ def main(
patches_dir: pathlib.Path,
settings_file: pathlib.Path,
settings_dir: pathlib.Path,
constraints_file: str,
constraints_file: list[pathlib.Path],
constraints_url: list[str],
cleanup: bool,
variant: str,
jobs: int | None,
Expand Down Expand Up @@ -195,13 +209,19 @@ def main(
logger.info(f"variant: {variant}")
logger.info(f"patches dir: {patches_dir}")
logger.info(f"maximum concurrent jobs: {jobs}")
logger.info(f"constraints file: {constraints_file}")
logger.info(
f"constraints files: {os.pathsep.join(str(p) for p in constraints_file)}"
)
logger.info(f"constraints URLs: {' '.join(constraints_url)}")
logger.info(f"network isolation: {network_isolation}")
overrides.log_overrides()

if network_isolation and not SUPPORTS_NETWORK_ISOLATION:
ctx.fail(f"network isolation is not available: {NETWORK_ISOLATION_ERROR}")

constraints: list[str] = constraints_url[:]
constraints.extend(os.fspath(p) for p in constraints_file)

wkctx = context.WorkContext(
active_settings=packagesettings.Settings.from_files(
settings_file=settings_file,
Expand All @@ -210,7 +230,7 @@ def main(
variant=variant,
max_jobs=jobs,
),
constraints_file=constraints_file,
constraints_files=constraints,
patches_dir=patches_dir,
sdists_repo=sdists_repo,
wheels_repo=wheels_repo,
Expand Down
13 changes: 13 additions & 0 deletions src/fromager/clickext.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@
from . import requirements_file


def verify_url_callback(
ctx: click.Context, param: click.Parameter, value: tuple[str]
) -> tuple[str]:
for item in value:
if not item.startswith(("https://", "http://", "file://")):
raise click.BadParameter(
f"value must be a http, https, or file URL, got {item}",
ctx=ctx,
param=param,
)
return value


class ClickPath(click.Path):
"""ClickPath that returns pathlib.Path"""

Expand Down
11 changes: 10 additions & 1 deletion src/fromager/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ def __init__(self) -> None:
def __iter__(self) -> typing.Iterable[NormalizedName]:
yield from self._data

def __bool__(self) -> bool:
return bool(self._data)

def add_constraint(self, unparsed: str) -> None:
"""Add new constraint, must not conflict with any existing constraints"""
req = Requirement(unparsed)
Expand All @@ -34,12 +37,18 @@ def add_constraint(self, unparsed: str) -> None:
self._data[canon_name] = req

def load_constraints_file(self, constraints_file: str | pathlib.Path) -> None:
"""Load constraints from a constraints file"""
"""Load constraints from a constraints file or URI"""
logger.info("loading constraints from %s", constraints_file)
content = requirements_file.parse_requirements_file(constraints_file)
for line in content:
self.add_constraint(line)

def dump_constraints_file(self, constraints_file: pathlib.Path) -> None:
"""Dump all constraints into a file"""
with constraints_file.open("w", encoding="utf-8") as f:
for req in self._data.values():
f.write(f"{req}\n")

def get_constraint(self, name: str) -> Requirement | None:
return self._data.get(canonicalize_name(name))

Expand Down
36 changes: 13 additions & 23 deletions src/fromager/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from packaging.utils import NormalizedName, canonicalize_name
from packaging.version import Version

from . import constraints, dependency_graph, packagesettings, request_session
from . import constraints, dependency_graph, packagesettings

logger = logging.getLogger(__name__)

Expand All @@ -21,7 +21,7 @@ class WorkContext:
def __init__(
self,
active_settings: packagesettings.Settings | None,
constraints_file: str | None,
constraints_files: list[str],
patches_dir: pathlib.Path,
sdists_repo: pathlib.Path,
wheels_repo: pathlib.Path,
Expand All @@ -41,13 +41,6 @@ def __init__(
max_jobs=max_jobs,
)
self.settings = active_settings
self.input_constraints_uri: str | None
self.constraints = constraints.Constraints()
if constraints_file is not None:
self.input_constraints_uri = constraints_file
self.constraints.load_constraints_file(constraints_file)
else:
self.input_constraints_uri = None
self.sdists_repo = pathlib.Path(sdists_repo).absolute()
self.sdists_downloads = self.sdists_repo / "downloads"
self.sdists_builds = self.sdists_repo / "builds"
Expand All @@ -64,10 +57,16 @@ def __init__(
self.network_isolation = network_isolation
self.settings_dir = settings_dir

self._constraints_filename = self.work_dir / "constraints.txt"

self.dependency_graph = dependency_graph.DependencyGraph()

self.constraints = constraints.Constraints()
self._constraints_filename: pathlib.Path | None = None
if constraints_files:
# local or remote (HTTPS) files
for cf in constraints_files:
self.constraints.load_constraints_file(cf)
self._constraints_filename = self.work_dir / "combined-constraints.txt"

# storing metrics
self.time_store: dict[str, dict[str, float]] = collections.defaultdict(
dict[str, float]
Expand All @@ -84,19 +83,10 @@ def pip_wheel_server_args(self) -> list[str]:

@property
def pip_constraint_args(self) -> list[str]:
if not self.input_constraints_uri:
if self._constraints_filename is None:
return []

if self.input_constraints_uri.startswith(("https://", "http://", "file://")):
path_to_constraints_file = self.work_dir / "input-constraints.txt"
if not path_to_constraints_file.exists():
response = request_session.session.get(self.input_constraints_uri)
path_to_constraints_file.write_text(response.text)
else:
path_to_constraints_file = pathlib.Path(self.input_constraints_uri)

path_to_constraints_file = path_to_constraints_file.absolute()
return ["--constraint", os.fspath(path_to_constraints_file)]
self.constraints.dump_constraints_file(self._constraints_filename)
return ["--constraint", os.fspath(self._constraints_filename)]

def write_to_graph_to_file(self):
with open(self.work_dir / "graph.json", "w") as f:
Expand Down
4 changes: 2 additions & 2 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def tmp_context(tmp_path: pathlib.Path) -> context.WorkContext:
variant = "cpu"
ctx = context.WorkContext(
active_settings=None,
constraints_file=None,
constraints_files=[],
patches_dir=patches_dir,
sdists_repo=tmp_path / "sdists-repo",
wheels_repo=tmp_path / "wheels-repo",
Expand All @@ -46,7 +46,7 @@ def testdata_context(
variant=variant,
max_jobs=None,
),
constraints_file=None,
constraints_files=[],
patches_dir=overrides / "patches",
sdists_repo=tmp_path / "sdists-repo",
wheels_repo=tmp_path / "wheels-repo",
Expand Down
19 changes: 19 additions & 0 deletions tests/test_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,29 @@ def test_load_non_existant_constraints_file(tmp_path: pathlib.Path):
c.load_constraints_file(non_existant_file)


def test_magic_methods():
c = constraints.Constraints()
assert not c
assert list(c) == []
c.add_constraint("flit_core==2.0rc3")
assert c
assert list(c) == ["flit-core"]


def test_load_constraints_file(tmp_path: pathlib.Path):
constraint_file = tmp_path / "constraint.txt"
constraint_file.write_text("egg\ntorch==3.1.0 # comment\n")
c = constraints.Constraints()
c.load_constraints_file(constraint_file)
assert list(c) == ["egg", "torch"] # type: ignore
assert c.get_constraint("torch") == Requirement("torch==3.1.0")


def test_dump_constraints_file(tmp_path: pathlib.Path):
c = constraints.Constraints()
c.add_constraint("foo<=1.1")
c.add_constraint("bar>=2.0")

constraint_file = tmp_path / "constraint.txt"
c.dump_constraints_file(constraint_file)
assert constraint_file.read_text().split("\n") == ["foo<=1.1", "bar>=2.0", ""]
9 changes: 6 additions & 3 deletions tests/test_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,21 @@ def test_pip_constraints_args(tmp_path):
constraints_file.write_text("\n") # the file has to exist
ctx = context.WorkContext(
active_settings=None,
constraints_file=str(constraints_file),
constraints_files=[constraints_file],
patches_dir=tmp_path / "overrides/patches",
sdists_repo=tmp_path / "sdists-repo",
wheels_repo=tmp_path / "wheels-repo",
work_dir=tmp_path / "work-dir",
)
ctx.setup()
assert ["--constraint", os.fspath(constraints_file)] == ctx.pip_constraint_args
assert ctx.pip_constraint_args == [
"--constraint",
os.fspath(ctx.work_dir / "combined-constraints.txt"),
]

ctx = context.WorkContext(
active_settings=None,
constraints_file=None,
constraints_files=[],
patches_dir=tmp_path / "overrides/patches",
sdists_repo=tmp_path / "sdists-repo",
wheels_repo=tmp_path / "wheels-repo",
Expand Down

0 comments on commit 481ab95

Please sign in to comment.