From 76ac553f8b81a597910dc55b4433b3ae32ab804b Mon Sep 17 00:00:00 2001 From: AP Ljungquist Date: Thu, 26 May 2022 16:14:33 +0200 Subject: [PATCH 1/3] Allow wildcard argument for `--extra` The goal is to make it easy to compile version locks for all runtime dependencies in a way that is robust to human error. By using the wildcard extra the risk of forgetting to list any extra is removed. Since the expanded extras are needed when we look for the wildcard it is moved to before the loop. This is OK because it does not depend on the loop and nothing in the loop depends on its previous value. --- piptools/scripts/compile.py | 12 ++++++++++-- tests/test_cli_compile.py | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index 9424968ac..d805c8ce4 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -401,6 +401,8 @@ def cli( # Parsing/collecting initial requirements ### + extras = tuple(itertools.chain.from_iterable(ex.split(",") for ex in extras)) + constraints: List[InstallRequirement] = [] setup_file_found = False for src_file in src_files: @@ -442,6 +444,14 @@ def cli( for req in metadata.get_all("Requires-Dist") or [] ] ) + # Since "*" is not a valid extra there is no risk confusing an actual extra + # for the wildcard. + # https://peps.python.org/pep-0508/#grammar + if "*" in extras: + if len(extras) != 1: + msg = "--extra=* only makes sense if it is the only --extra" + raise click.BadParameter(msg) + extras = tuple(metadata.get_all("Provides-Extra")) else: constraints.extend( parse_requirements( @@ -452,8 +462,6 @@ def cli( ) ) - extras = tuple(itertools.chain.from_iterable(ex.split(",") for ex in extras)) - if extras and not setup_file_found: msg = "--extra has effect only with setup.py and PEP-517 input formats" raise click.BadParameter(msg) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 72acd52fe..7b3e00b01 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -2190,6 +2190,44 @@ def test_multiple_extras(fake_dists, runner, make_module, fname, content, extra_ assert "extra ==" not in out.stderr +@pytest.mark.network +@pytest.mark.parametrize(("fname", "content"), METADATA_TEST_CASES) +def test_wildcard_extras(fake_dists, runner, make_module, fname, content): + """ + Test passing wildcard `--extra` includes all applicable extras. + """ + meta_path = make_module(fname=fname, content=content) + out = runner.invoke( + cli, + [ + "-n", + "--extra", + "*", + "--find-links", + fake_dists, + "--no-annotate", + "--no-emit-options", + "--no-header", + meta_path, + ], + ) + assert out.exit_code == 0, out.stderr + assert ( + dedent( + """\ + small-fake-a==0.1 + small-fake-b==0.2 + small-fake-c==0.3 + small-fake-d==0.4 + small-fake-e==0.5 + small-fake-f==0.6 + Dry-run, so nothing updated. + """ + ) + == out.stderr + ) + + def test_extras_fail_with_requirements_in(runner, tmpdir): """ Test that passing `--extra` with `requirements.in` input file fails. From b7dde7d35ddfbb7c7b7197cc5f9f3fb533f276e8 Mon Sep 17 00:00:00 2001 From: AP Ljungquist Date: Sat, 1 Oct 2022 14:04:00 +0200 Subject: [PATCH 2/3] Replace extra wildcard with all-extras flag ... to not give users the misleading impression that pattern matching, with regex, fnmatch or some other system, works. --- piptools/scripts/compile.py | 20 ++++++++++++-------- tests/test_cli_compile.py | 34 ++++++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index d805c8ce4..18a3b312f 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -79,6 +79,12 @@ def _get_default_option(option_name: str) -> Any: multiple=True, help="Name of an extras_require group to install; may be used more than once", ) +@click.option( + "--all-extras", + is_flag=True, + default=False, + help="Install all extras_require groups", +) @click.option( "-f", "--find-links", @@ -259,6 +265,7 @@ def cli( pre: bool, rebuild: bool, extras: Tuple[str, ...], + all_extras: bool, find_links: Tuple[str, ...], index_url: str, extra_index_url: Tuple[str, ...], @@ -401,8 +408,6 @@ def cli( # Parsing/collecting initial requirements ### - extras = tuple(itertools.chain.from_iterable(ex.split(",") for ex in extras)) - constraints: List[InstallRequirement] = [] setup_file_found = False for src_file in src_files: @@ -444,12 +449,9 @@ def cli( for req in metadata.get_all("Requires-Dist") or [] ] ) - # Since "*" is not a valid extra there is no risk confusing an actual extra - # for the wildcard. - # https://peps.python.org/pep-0508/#grammar - if "*" in extras: - if len(extras) != 1: - msg = "--extra=* only makes sense if it is the only --extra" + if all_extras: + if extras: + msg = "--extra has no effect when used with --all-extras" raise click.BadParameter(msg) extras = tuple(metadata.get_all("Provides-Extra")) else: @@ -462,6 +464,8 @@ def cli( ) ) + extras = tuple(itertools.chain.from_iterable(ex.split(",") for ex in extras)) + if extras and not setup_file_found: msg = "--extra has effect only with setup.py and PEP-517 input formats" raise click.BadParameter(msg) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 7b3e00b01..0fab785c6 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -2192,17 +2192,16 @@ def test_multiple_extras(fake_dists, runner, make_module, fname, content, extra_ @pytest.mark.network @pytest.mark.parametrize(("fname", "content"), METADATA_TEST_CASES) -def test_wildcard_extras(fake_dists, runner, make_module, fname, content): +def test_all_extras(fake_dists, runner, make_module, fname, content): """ - Test passing wildcard `--extra` includes all applicable extras. + Test passing `--all-extras` includes all applicable extras. """ meta_path = make_module(fname=fname, content=content) out = runner.invoke( cli, [ "-n", - "--extra", - "*", + "--all-extras", "--find-links", fake_dists, "--no-annotate", @@ -2228,6 +2227,33 @@ def test_wildcard_extras(fake_dists, runner, make_module, fname, content): ) +# This should not depend on the metadata format so testing all cases is wasteful +@pytest.mark.parametrize(("fname", "content"), METADATA_TEST_CASES[:1]) +def test_all_extras_fail_with_extra(fake_dists, runner, make_module, fname, content): + """ + Test that passing `--all-extras` and `--extra` fails. + """ + meta_path = make_module(fname=fname, content=content) + out = runner.invoke( + cli, + [ + "-n", + "--all-extras", + "--extra", + "dev", + "--find-links", + fake_dists, + "--no-annotate", + "--no-emit-options", + "--no-header", + meta_path, + ], + ) + assert out.exit_code == 2 + exp = "--extra has no effect when used with --all-extras" + assert exp in out.stderr + + def test_extras_fail_with_requirements_in(runner, tmpdir): """ Test that passing `--extra` with `requirements.in` input file fails. From d8fe47d5276090eb0ea6d7b97726919a9f122994 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Wed, 5 Oct 2022 12:47:22 +0100 Subject: [PATCH 3/3] Update tests/test_cli_compile.py Co-authored-by: Sviatoslav Sydorenko --- tests/test_cli_compile.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 0fab785c6..5b9d57f03 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -2214,14 +2214,14 @@ def test_all_extras(fake_dists, runner, make_module, fname, content): assert ( dedent( """\ - small-fake-a==0.1 - small-fake-b==0.2 - small-fake-c==0.3 - small-fake-d==0.4 - small-fake-e==0.5 - small-fake-f==0.6 - Dry-run, so nothing updated. - """ + small-fake-a==0.1 + small-fake-b==0.2 + small-fake-c==0.3 + small-fake-d==0.4 + small-fake-e==0.5 + small-fake-f==0.6 + Dry-run, so nothing updated. + """ ) == out.stderr )