Skip to content

Commit

Permalink
Move check for --extra and --all-extras outside loop
Browse files Browse the repository at this point in the history
Previously, an error would always be thrown when running `compile` with
`--all-extras` on multiple source files containing extras. The reason was
that the first iteration over the first source file would fill the
`extras` variable, which in the second iteration would trigger an error
since both `all_extras` and `extras` would be set.

This change moves the check outside the loop and early in the script to
avoid unnecessary processing before throwing the error.
  • Loading branch information
dragly committed Nov 10, 2023
1 parent f20d6ae commit 4ae48d6
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 5 deletions.
9 changes: 5 additions & 4 deletions piptools/scripts/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ def cli(
).format(", ".join(DEFAULT_REQUIREMENTS_FILES))
)

if all_extras and extras:
msg = "--extra has no effect when used with --all-extras"
raise click.BadParameter(msg)

if not output_file:
# An output file must be provided for stdin
if src_files == ("-",):
Expand Down Expand Up @@ -359,10 +363,7 @@ def cli(
if not only_build_deps:
constraints.extend(metadata.requirements)
if all_extras:
if extras:
msg = "--extra has no effect when used with --all-extras"
raise click.BadParameter(msg)
extras = metadata.extras
extras += metadata.extras
if build_deps_targets:
constraints.extend(metadata.build_requirements)
else:
Expand Down
33 changes: 32 additions & 1 deletion tests/test_cli_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -3001,6 +3001,38 @@ def test_cli_compile_strip_extras(runner, make_package, make_sdist, tmpdir):
assert "[more]" not in out.stderr


def test_cli_compile_all_extras_with_multiple_packages(
runner, make_package, make_sdist, tmpdir
):
"""
Assures that ``--all-extras`` works when multiple sources are specified.
"""
test_package_1 = make_package(
"test_package_1",
version="0.1",
extras_require={"more": []},
)
test_package_2 = make_package(
"test_package_2",
version="0.1",
extras_require={"more": []},
)
dists_dir = tmpdir / "dists"

out = runner.invoke(
cli,
[
"--all-extras",
"--output-file",
"requirements.txt",
str(test_package_1 / "setup.py"),
str(test_package_2 / "setup.py"),
],
)

assert out.exit_code == 0, out


@pytest.mark.parametrize(
("package_specs", "constraints", "existing_reqs", "expected_reqs"),
(
Expand Down Expand Up @@ -3133,7 +3165,6 @@ def test_resolver_drops_existing_conflicting_constraint(
req_txt_content = req_txt.read()
assert expected_requirements.issubset(req_txt_content.splitlines())


def test_resolution_failure(runner):
"""Test resolution impossible for unknown package."""
with open("requirements.in", "w") as reqs_out:
Expand Down

0 comments on commit 4ae48d6

Please sign in to comment.