From 43a9c065092988b4e126719cf14d88adcea02f70 Mon Sep 17 00:00:00 2001 From: Shubh Bapna Date: Tue, 12 Nov 2024 15:34:43 -0500 Subject: [PATCH] allow use of url for req, constraint and graph files users can now pass a url instead of a path to a local file to commands that need req files, constraints file or graph files. pip allows using url for req and constraints file so this is a pip compatible change Signed-off-by: Shubh Bapna --- src/fromager/__main__.py | 4 ++-- src/fromager/commands/bootstrap.py | 12 ++++------ src/fromager/commands/build.py | 3 ++- src/fromager/commands/download_sequence.py | 4 ++-- src/fromager/commands/graph.py | 16 ++++++------- src/fromager/constraints.py | 13 ++++------ src/fromager/context.py | 28 +++++++++++++--------- src/fromager/dependency_graph.py | 5 ++-- src/fromager/read.py | 26 ++++++++++++++++++++ src/fromager/requirements_file.py | 6 +++-- tests/test_bootstrap.py | 2 +- tests/test_context.py | 2 +- tests/test_read.py | 24 +++++++++++++++++++ 13 files changed, 98 insertions(+), 47 deletions(-) create mode 100644 src/fromager/read.py create mode 100644 tests/test_read.py diff --git a/src/fromager/__main__.py b/src/fromager/__main__.py index 42661a11..8666e987 100644 --- a/src/fromager/__main__.py +++ b/src/fromager/__main__.py @@ -107,7 +107,7 @@ @click.option( "-c", "--constraints-file", - type=clickext.ClickPath(), + type=str, help="location of the constraints file", ) @click.option( @@ -142,7 +142,7 @@ def main( patches_dir: pathlib.Path, settings_file: pathlib.Path, settings_dir: pathlib.Path, - constraints_file: pathlib.Path, + constraints_file: str, cleanup: bool, variant: str, jobs: int | None, diff --git a/src/fromager/commands/bootstrap.py b/src/fromager/commands/bootstrap.py index 87c1f2f1..4352a1bf 100644 --- a/src/fromager/commands/bootstrap.py +++ b/src/fromager/commands/bootstrap.py @@ -1,5 +1,4 @@ import logging -import pathlib import typing import click @@ -7,7 +6,6 @@ from .. import ( bootstrapper, - clickext, context, dependency_graph, progress, @@ -27,7 +25,7 @@ def _get_requirements_from_args( toplevel: typing.Iterable[str], - req_files: typing.Iterable[pathlib.Path], + req_files: typing.Iterable[str], ) -> list[Requirement]: parsed_req: list[str] = [] parsed_req.extend(toplevel) @@ -58,14 +56,14 @@ def _get_requirements_from_args( "--requirements-file", "requirements_files", multiple=True, - type=clickext.ClickPath(), + type=str, help="pip requirements file", ) @click.option( "-p", "--previous-bootstrap-file", "previous_bootstrap_file", - type=clickext.ClickPath(), + type=str, help="graph file produced from a previous bootstrap", ) @click.option( @@ -78,8 +76,8 @@ def _get_requirements_from_args( @click.pass_obj def bootstrap( wkctx: context.WorkContext, - requirements_files: list[pathlib.Path], - previous_bootstrap_file: pathlib.Path | None, + requirements_files: list[str], + previous_bootstrap_file: str | None, cache_wheel_server_url: str | None, toplevel: list[str], ) -> None: diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py index f504096a..3ce92ef4 100644 --- a/src/fromager/commands/build.py +++ b/src/fromager/commands/build.py @@ -24,6 +24,7 @@ hooks, overrides, progress, + read, server, sources, wheels, @@ -136,7 +137,7 @@ def build_sequence( entries: list[BuildSequenceEntry] = [] logger.info("reading build order from %s", build_order_file) - with open(build_order_file, "r") as f: + with read.open_file_or_url(build_order_file) as f: for entry in progress.progress(json.load(f)): dist_name = entry["dist"] resolved_version = Version(entry["version"]) diff --git a/src/fromager/commands/download_sequence.py b/src/fromager/commands/download_sequence.py index b7bfcb56..fda64673 100644 --- a/src/fromager/commands/download_sequence.py +++ b/src/fromager/commands/download_sequence.py @@ -8,7 +8,7 @@ from packaging.requirements import Requirement from packaging.version import Version -from .. import context, progress, sources, wheels +from .. import context, progress, read, sources, wheels logger = logging.getLogger(__name__) @@ -57,7 +57,7 @@ def download_sequence( wheel_servers = [sdist_server_url] logger.info("reading build order from %s", build_order_file) - with open(build_order_file, "r") as f: + with read.open_file_or_url(build_order_file) as f: build_order = json.load(f) def download_one(entry: dict[str, typing.Any]): diff --git a/src/fromager/commands/graph.py b/src/fromager/commands/graph.py index c82821a2..48ea0c9d 100644 --- a/src/fromager/commands/graph.py +++ b/src/fromager/commands/graph.py @@ -36,12 +36,10 @@ def graph(): ) @click.argument( "graph-file", - type=clickext.ClickPath(), + type=str, ) @click.pass_obj -def to_constraints( - wkctx: context.WorkContext, graph_file: pathlib.Path, output: pathlib.Path -): +def to_constraints(wkctx: context.WorkContext, graph_file: str, output: pathlib.Path): "Convert a graph file to a constraints file." graph = DependencyGraph.from_file(graph_file) if output: @@ -59,10 +57,10 @@ def to_constraints( ) @click.argument( "graph-file", - type=clickext.ClickPath(), + type=str, ) @click.pass_obj -def to_dot(wkctx, graph_file: pathlib.Path, output: pathlib.Path): +def to_dot(wkctx: context.WorkContext, graph_file: str, output: pathlib.Path): "Convert a graph file to a DOT file suitable to pass to graphviz." graph = DependencyGraph.from_file(graph_file) if output: @@ -114,7 +112,7 @@ def get_node_id(node): @graph.command() @click.argument( "graph-file", - type=clickext.ClickPath(), + type=str, ) @click.pass_obj def explain_duplicates(wkctx, graph_file): @@ -183,13 +181,13 @@ def explain_duplicates(wkctx, graph_file): ) @click.argument( "graph-file", - type=clickext.ClickPath(), + type=str, ) @click.argument("package-name", type=str) @click.pass_obj def why( wkctx: context.WorkContext, - graph_file: pathlib.Path, + graph_file: str, package_name: str, version: list[Version], depth: int, diff --git a/src/fromager/constraints.py b/src/fromager/constraints.py index 844a6595..488afb75 100644 --- a/src/fromager/constraints.py +++ b/src/fromager/constraints.py @@ -40,14 +40,9 @@ def _parse(content: typing.Iterable[str]) -> Constraints: return Constraints(constraints) -def load(filename: pathlib.Path | None) -> Constraints: - if not filename: +def load(constraints_file: str | pathlib.Path | None) -> Constraints: + if not constraints_file: return Constraints({}) - filepath = pathlib.Path(filename) - if not filepath.exists(): - raise FileNotFoundError( - f"constraints file {filepath.absolute()} does not exist, ignoring" - ) - logger.info("loading constraints from %s", filepath.absolute()) - parsed_req_file = requirements_file.parse_requirements_file(filename) + logger.info("loading constraints from %s", constraints_file) + parsed_req_file = requirements_file.parse_requirements_file(constraints_file) return _parse(parsed_req_file) diff --git a/src/fromager/context.py b/src/fromager/context.py index c739ac6c..1106b72b 100644 --- a/src/fromager/context.py +++ b/src/fromager/context.py @@ -7,11 +7,7 @@ from packaging.utils import NormalizedName, canonicalize_name from packaging.version import Version -from . import ( - constraints, - dependency_graph, - packagesettings, -) +from . import constraints, dependency_graph, packagesettings, request_session logger = logging.getLogger(__name__) @@ -24,7 +20,7 @@ class WorkContext: def __init__( self, active_settings: packagesettings.Settings | None, - constraints_file: pathlib.Path | None, + constraints_file: str | None, patches_dir: pathlib.Path, sdists_repo: pathlib.Path, wheels_repo: pathlib.Path, @@ -44,12 +40,12 @@ def __init__( max_jobs=max_jobs, ) self.settings = active_settings - self.input_constraints_file: pathlib.Path | None + self.input_constraints_uri: str | None if constraints_file is not None: - self.input_constraints_file = constraints_file.absolute() + self.input_constraints_uri = constraints_file self.constraints = constraints.load(constraints_file) else: - self.input_constraints_file = None + self.input_constraints_uri = None self.constraints = constraints.Constraints({}) self.sdists_repo = pathlib.Path(sdists_repo).absolute() self.sdists_downloads = self.sdists_repo / "downloads" @@ -81,9 +77,19 @@ def pip_wheel_server_args(self) -> list[str]: @property def pip_constraint_args(self) -> list[str]: - if not self.input_constraints_file: + if not self.input_constraints_uri: return [] - return ["--constraint", os.fspath(self.input_constraints_file)] + + 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)] def write_to_graph_to_file(self): with open(self.work_dir / "graph.json", "w") as f: diff --git a/src/fromager/dependency_graph.py b/src/fromager/dependency_graph.py index bf10e6e5..bb9d36b9 100644 --- a/src/fromager/dependency_graph.py +++ b/src/fromager/dependency_graph.py @@ -7,6 +7,7 @@ from packaging.utils import NormalizedName, canonicalize_name from packaging.version import Version +from .read import open_file_or_url from .requirements_file import RequirementType logger = logging.getLogger(__name__) @@ -122,9 +123,9 @@ def __init__(self) -> None: @classmethod def from_file( cls, - graph_file: pathlib.Path, + graph_file: pathlib.Path | str, ) -> "DependencyGraph": - with open(graph_file) as f: + with open_file_or_url(graph_file) as f: # TODO: add JSON validation to ensure it is a parsable graph json raw_graph = typing.cast(dict[str, dict], json.load(f)) return cls.from_dict(raw_graph) diff --git a/src/fromager/read.py b/src/fromager/read.py new file mode 100644 index 00000000..4fa65ee0 --- /dev/null +++ b/src/fromager/read.py @@ -0,0 +1,26 @@ +import io +import pathlib +import typing +from contextlib import contextmanager +from urllib.parse import urlparse + +from .request_session import session + + +@contextmanager +def open_file_or_url( + path_or_url: str | pathlib.Path, +) -> typing.Generator[io.TextIOWrapper, typing.Any, None]: + location = str(path_or_url) + if location.startswith("file://"): + location = urlparse(location).path + + if location.startswith(("https://", "http://")): + response = session.get(location) + yield io.StringIO(response.text) + else: + f = open(location, "r") + try: + yield f + finally: + f.close() diff --git a/src/fromager/requirements_file.py b/src/fromager/requirements_file.py index 46241c68..e8661ca7 100644 --- a/src/fromager/requirements_file.py +++ b/src/fromager/requirements_file.py @@ -6,6 +6,8 @@ from packaging import markers from packaging.requirements import Requirement +from .read import open_file_or_url + logger = logging.getLogger(__name__) @@ -40,11 +42,11 @@ class SourceType(StrEnum): def parse_requirements_file( - req_file: pathlib.Path, + req_file: str | pathlib.Path, ) -> typing.Iterable[str]: logger.debug("reading requirements file %s", req_file) lines = [] - with open(req_file, "r") as f: + with open_file_or_url(req_file) as f: for line in f: useful, _, _ = line.partition("#") useful = useful.strip() diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index a2a3a2ac..58a71eee 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -22,7 +22,7 @@ def test_get_requirements_args_and_file(tmp_path: pathlib.Path): requirements_file = tmp_path / "requirements.txt" requirements_file.write_text("c\n") requirements = bootstrap._get_requirements_from_args( - ["a", "b"], [requirements_file] + ["a", "b"], [str(requirements_file)] ) assert [ Requirement("a"), diff --git a/tests/test_context.py b/tests/test_context.py index 917e685f..6b9f1cdb 100644 --- a/tests/test_context.py +++ b/tests/test_context.py @@ -8,7 +8,7 @@ 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=constraints_file, + constraints_file=str(constraints_file), patches_dir=tmp_path / "overrides/patches", sdists_repo=tmp_path / "sdists-repo", wheels_repo=tmp_path / "wheels-repo", diff --git a/tests/test_read.py b/tests/test_read.py new file mode 100644 index 00000000..41599c82 --- /dev/null +++ b/tests/test_read.py @@ -0,0 +1,24 @@ +import pathlib + +import requests_mock + +from fromager import read + + +def test_read_from_file(tmp_path: pathlib.Path): + file = tmp_path / "test" + text = ["hello", "world"] + file.write_text("\n".join(text)) + with read.open_file_or_url(file) as f: + for index, line in enumerate(f): + assert line.strip() == text[index] + + +def test_read_from_url(): + url = "https://someurl.com" + text = ["hello", "world"] + with requests_mock.Mocker() as r: + r.get(url, text="\n".join(text)) + with read.open_file_or_url(url) as f: + for index, line in enumerate(f): + assert line.strip() == text[index]