Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Support multiple constraints files #540

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: tuple[pathlib.Path],
constraints_url: tuple[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] = list(constraints_url)
constraints.extend(os.fspath(p) for p in constraints_file)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies that constraints at URLs take precedence over local files. I don't think that ordering is wrong, but I can definitely imagine wanting a local file to have a value that replaces a value in some global file on a web server.


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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we log the constraints as we parse the input? I know we log matching constraints as we process individual packages. It may also be useful to log the contents of this file to help with debugging.


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
Loading