From b43ce0c6c44d2916fbbec3abb74372070e9ab5ed Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Sat, 8 Jul 2023 15:39:03 +0200 Subject: [PATCH 01/11] Fix revealed default config in header if requirements in suboflder --- piptools/scripts/compile.py | 5 +++-- piptools/scripts/sync.py | 5 +++-- tests/test_utils.py | 23 +++++++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index f444271af..6be74604b 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -251,7 +251,9 @@ def _determine_linesep( default=10, help="Maximum number of rounds before resolving the requirements aborts.", ) -@click.argument("src_files", nargs=-1, type=click.Path(exists=True, allow_dash=True)) +@click.argument( + "src_files", nargs=-1, type=click.Path(exists=True, allow_dash=True), is_eager=True +) @click.option( "--build-isolation/--no-build-isolation", is_flag=True, @@ -316,7 +318,6 @@ def _determine_linesep( ), help=f"Read configuration from TOML file. By default, looks for a {CONFIG_FILE_NAME} or " "pyproject.toml.", - is_eager=True, callback=override_defaults_from_config_file, ) @click.option( diff --git a/piptools/scripts/sync.py b/piptools/scripts/sync.py index e162feac6..5352fb14d 100755 --- a/piptools/scripts/sync.py +++ b/piptools/scripts/sync.py @@ -86,7 +86,9 @@ help="Path to SSL client certificate, a single file containing " "the private key and the certificate in PEM format.", ) -@click.argument("src_files", required=False, type=click.Path(exists=True), nargs=-1) +@click.argument( + "src_files", required=False, type=click.Path(exists=True), nargs=-1, is_eager=True +) @click.option("--pip-args", help="Arguments to pass directly to pip install.") @click.option( "--config", @@ -100,7 +102,6 @@ ), help=f"Read configuration from TOML file. By default, looks for a {CONFIG_FILE_NAME} or " "pyproject.toml.", - is_eager=True, callback=override_defaults_from_config_file, ) @click.option( diff --git a/tests/test_utils.py b/tests/test_utils.py index 4820492d6..51fe987d4 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -410,6 +410,29 @@ def test_get_compile_command_with_config(tmpdir_cwd, config_file, expected_comma assert get_compile_command(ctx) == expected_command +def test_get_compile_command_does_not_include_default_config_if_reqs_file_in_subdir( + tmpdir_cwd, +): + """ + Test that ``get_compile_command`` does not include default config file + if requirements file is in a subdirectory. + Regression test for issue GH-1903. + """ + default_config_file = Path("pyproject.toml") + default_config_file.touch() + + (tmpdir_cwd / "subdir").mkdir() + req_file = Path("subdir/requirements.in") + req_file.touch() + + with open(req_file, "w"): + pass + + # Make sure that the default config file is not included + with compile_cli.make_context("pip-compile", [req_file.as_posix()]) as ctx: + assert get_compile_command(ctx) == f"pip-compile {req_file.as_posix()}" + + def test_get_compile_command_escaped_filenames(tmpdir_cwd): """ Test that get_compile_command output (re-)escapes ' -- '-escaped filenames. From 655b9edff7983c80b7a13d546db387497fadcca9 Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Sun, 9 Jul 2023 22:02:17 +0200 Subject: [PATCH 02/11] Apply suggestions from code review Co-authored-by: Sviatoslav Sydorenko --- tests/test_utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 51fe987d4..a94485c2a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -424,9 +424,7 @@ def test_get_compile_command_does_not_include_default_config_if_reqs_file_in_sub (tmpdir_cwd / "subdir").mkdir() req_file = Path("subdir/requirements.in") req_file.touch() - - with open(req_file, "w"): - pass + req_file.write_bytes(b"") # Make sure that the default config file is not included with compile_cli.make_context("pip-compile", [req_file.as_posix()]) as ctx: From 64445d00524f2b76af6825c2f79424b179d914bc Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Sun, 9 Jul 2023 22:08:19 +0200 Subject: [PATCH 03/11] Add more tests --- tests/test_utils.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index a94485c2a..003894111 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -410,16 +410,25 @@ def test_get_compile_command_with_config(tmpdir_cwd, config_file, expected_comma assert get_compile_command(ctx) == expected_command +@pytest.mark.parametrize("config_file", ("pyproject.toml", ".pip-tools.toml")) +@pytest.mark.parametrize( + "config_file_content", + ( + pytest.param("", id="empty config file"), + pytest.param("[tool.pip-tools]", id="empty config section"), + pytest.param("[tool.pip-tools]\ndry-run = True", id="non-empty config section"), + ), +) def test_get_compile_command_does_not_include_default_config_if_reqs_file_in_subdir( - tmpdir_cwd, + tmpdir_cwd, config_file, config_file_content ): """ Test that ``get_compile_command`` does not include default config file if requirements file is in a subdirectory. Regression test for issue GH-1903. """ - default_config_file = Path("pyproject.toml") - default_config_file.touch() + default_config_file = Path(config_file) + default_config_file.write_text(config_file_content) (tmpdir_cwd / "subdir").mkdir() req_file = Path("subdir/requirements.in") From 845029a551601086bc5316126198f8fc5e246a54 Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Tue, 11 Jul 2023 01:39:57 +0200 Subject: [PATCH 04/11] Add tests for select_config_file --- tests/conftest.py | 9 ++++---- tests/test_utils.py | 51 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 024efa480..4cfafd74e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -454,11 +454,12 @@ def make_config_file(tmpdir_cwd): def _maker( pyproject_param: str, new_default: Any, config_file_name: str = CONFIG_FILE_NAME ) -> Path: - # Make a config file with this one config default override - config_path = Path(tmpdir_cwd) / pyproject_param - config_file = config_path / config_file_name - config_path.mkdir(exist_ok=True) + # Create a nested directory structure if config_file_name includes directories + config_dir = Path(tmpdir_cwd / config_file_name).parent + config_dir.mkdir(exist_ok=True, parents=True) + # Make a config file with this one config default override + config_file = Path(tmpdir_cwd / config_file_name) config_to_dump = {"tool": {"pip-tools": {pyproject_param: new_default}}} config_file.write_text(tomli_w.dumps(config_to_dump)) return config_file.relative_to(tmpdir_cwd) diff --git a/tests/test_utils.py b/tests/test_utils.py index 003894111..0d7b250f0 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -6,6 +6,7 @@ import shlex import sys from pathlib import Path +from textwrap import dedent import pip import pytest @@ -31,6 +32,7 @@ lookup_table, lookup_table_from_tuples, override_defaults_from_config_file, + select_config_file, ) @@ -708,3 +710,52 @@ def test_callback_config_file_defaults_unreadable_toml(make_config_file): "config", "/dev/null/path/does/not/exist/my-config.toml", ) + + +def test_select_config_file_no_files(tmpdir_cwd): + assert select_config_file(()) is None + + +@pytest.mark.parametrize("filename", ("pyproject.toml", ".pip-tools.toml")) +def test_select_config_file_returns_config_in_cwd(make_config_file, filename): + config_file = make_config_file("dry-run", True, filename) + assert select_config_file(()) == config_file + + +def test_select_config_file_returns_empty_config_file_in_cwd(tmpdir_cwd): + config_file = Path(".pip-tools.toml") + config_file.touch() + + assert select_config_file(()) == config_file + + +def test_select_config_file_cannot_find_config_in_cwd(tmpdir_cwd, make_config_file): + make_config_file("dry-run", True, "subdir/pyproject.toml") + assert select_config_file(()) is None + + +def test_select_config_file_with_config_file_in_subdir(tmpdir_cwd, make_config_file): + config_file = make_config_file("dry-run", True, "subdir/.pip-tools.toml") + + requirement_file = Path("subdir/requirements.in") + requirement_file.touch() + + assert select_config_file((requirement_file.as_posix(),)) == config_file + + +def test_select_config_file_prefers_pip_tools_toml_over_pyproject_toml(tmpdir_cwd): + pip_tools_file = Path(".pip-tools.toml") + pip_tools_file.touch() + + pyproject_file = Path("pyproject.toml") + pyproject_file.write_text( + dedent( + """\ + [build-system] + requires = ["setuptools>=63", "setuptools_scm[toml]>=7"] + build-backend = "setuptools.build_meta" + """ + ) + ) + + assert select_config_file(()) == pip_tools_file From 5cb88167a90f0477bb3a0118c2817068bbdea383 Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Thu, 13 Jul 2023 10:15:54 +0200 Subject: [PATCH 05/11] Test default config option --- tests/test_cli_compile.py | 12 ++++++++++++ tests/test_cli_sync.py | 13 +++++++++++++ 2 files changed, 25 insertions(+) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index edb4d2087..04158947d 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -2975,6 +2975,18 @@ def test_config_option(pip_conf, runner, tmp_path, make_config_file): assert "Dry-run, so nothing updated" in out.stderr +def test_default_config_option(pip_conf, runner, make_config_file, tmpdir_cwd): + make_config_file("dry-run", True) + + req_in = tmpdir_cwd / "requirements.in" + req_in.ensure() + + out = runner.invoke(cli) + + assert out.exit_code == 0 + assert "Dry-run, so nothing updated" in out.stderr + + def test_no_config_option_overrides_config_with_defaults( pip_conf, runner, tmp_path, make_config_file ): diff --git a/tests/test_cli_sync.py b/tests/test_cli_sync.py index 0ebcddf8d..6151c3625 100644 --- a/tests/test_cli_sync.py +++ b/tests/test_cli_sync.py @@ -374,6 +374,19 @@ def test_default_python_executable_option(run, runner): ] +@mock.patch("piptools.sync.run") +def test_default_config_option(run, runner, make_config_file, tmpdir_cwd): + make_config_file("dry-run", True) + + with open(sync.DEFAULT_REQUIREMENTS_FILE, "w") as reqs_txt: + reqs_txt.write("six==1.10.0") + + out = runner.invoke(cli) + + assert out.exit_code == 1 + assert "Would install:" in out.stdout + + @mock.patch("piptools.sync.run") def test_config_option(run, runner, make_config_file): config_file = make_config_file("dry-run", True) From 6114233993ba79f37e058bc0a22713ba6854de7c Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Thu, 27 Jul 2023 05:19:02 +0200 Subject: [PATCH 06/11] fix the bug --- piptools/locations.py | 4 ++-- piptools/scripts/compile.py | 13 +++++++------ piptools/scripts/sync.py | 13 +++++++------ piptools/utils.py | 13 +++++++++---- tests/conftest.py | 6 ++++-- tests/test_utils.py | 2 +- 6 files changed, 30 insertions(+), 21 deletions(-) diff --git a/piptools/locations.py b/piptools/locations.py index f31891cae..fa2413fc7 100644 --- a/piptools/locations.py +++ b/piptools/locations.py @@ -5,5 +5,5 @@ # The user_cache_dir helper comes straight from pip itself CACHE_DIR = user_cache_dir("pip-tools") -# The project defaults specific to pip-tools should be written to this filename -CONFIG_FILE_NAME = ".pip-tools.toml" +# The project defaults specific to pip-tools should be written to this filenames +DEFAULT_CONFIG_FILE_NAMES = (".pip-tools.toml", "pyproject.toml") diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index 8d57e3c1b..2c7f02f50 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -20,7 +20,7 @@ from .._compat import parse_requirements from ..cache import DependencyCache from ..exceptions import NoCandidateFound, PipToolsError -from ..locations import CACHE_DIR, CONFIG_FILE_NAME +from ..locations import CACHE_DIR, DEFAULT_CONFIG_FILE_NAMES from ..logging import log from ..repositories import LocalRequirementsRepository, PyPIRepository from ..repositories.base import BaseRepository @@ -251,9 +251,7 @@ def _determine_linesep( default=10, help="Maximum number of rounds before resolving the requirements aborts.", ) -@click.argument( - "src_files", nargs=-1, type=click.Path(exists=True, allow_dash=True), is_eager=True -) +@click.argument("src_files", nargs=-1, type=click.Path(exists=True, allow_dash=True)) @click.option( "--build-isolation/--no-build-isolation", is_flag=True, @@ -316,9 +314,12 @@ def _determine_linesep( allow_dash=False, path_type=str, ), - help=f"Read configuration from TOML file. By default, looks for a {CONFIG_FILE_NAME} or " - "pyproject.toml.", + help=( + f"Read configuration from TOML file. By default, looks for the following " + f"files in the given order: {', '.join(DEFAULT_CONFIG_FILE_NAMES)}. " + ), callback=override_defaults_from_config_file, + is_eager=True, ) @click.option( "--no-config", diff --git a/piptools/scripts/sync.py b/piptools/scripts/sync.py index 5352fb14d..54274a9be 100755 --- a/piptools/scripts/sync.py +++ b/piptools/scripts/sync.py @@ -17,7 +17,7 @@ from .. import sync from .._compat import Distribution, parse_requirements from ..exceptions import PipToolsError -from ..locations import CONFIG_FILE_NAME +from ..locations import DEFAULT_CONFIG_FILE_NAMES from ..logging import log from ..repositories import PyPIRepository from ..utils import ( @@ -86,9 +86,7 @@ help="Path to SSL client certificate, a single file containing " "the private key and the certificate in PEM format.", ) -@click.argument( - "src_files", required=False, type=click.Path(exists=True), nargs=-1, is_eager=True -) +@click.argument("src_files", required=False, type=click.Path(exists=True), nargs=-1) @click.option("--pip-args", help="Arguments to pass directly to pip install.") @click.option( "--config", @@ -100,9 +98,12 @@ allow_dash=False, path_type=str, ), - help=f"Read configuration from TOML file. By default, looks for a {CONFIG_FILE_NAME} or " - "pyproject.toml.", + help=( + f"Read configuration from TOML file. By default, looks for the following " + f"files in the given order: {', '.join(DEFAULT_CONFIG_FILE_NAMES)}. " + ), callback=override_defaults_from_config_file, + is_eager=True, ) @click.option( "--no-config", diff --git a/piptools/utils.py b/piptools/utils.py index 5e3e17260..ea01d4ce2 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -12,6 +12,8 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Callable, Iterable, Iterator, TypeVar, cast +from click.core import ParameterSource + if sys.version_info >= (3, 11): import tomllib else: @@ -31,7 +33,7 @@ from pip._vendor.pkg_resources import get_distribution from piptools._compat import PIP_VERSION -from piptools.locations import CONFIG_FILE_NAME +from piptools.locations import DEFAULT_CONFIG_FILE_NAMES from piptools.subprocess_utils import run_python_snippet if TYPE_CHECKING: @@ -367,8 +369,11 @@ def get_compile_command(click_ctx: click.Context) -> str: # Exclude config option if it's the default one if option_long_name == "--config": - default_config = select_config_file(click_ctx.params.get("src_files", ())) - if value == default_config: + parameter_source = click_ctx.get_parameter_source(option_name) + if ( + str(value) in DEFAULT_CONFIG_FILE_NAMES + or parameter_source == ParameterSource.DEFAULT + ): continue # Skip options without a value @@ -654,7 +659,7 @@ def select_config_file(src_files: tuple[str, ...]) -> Path | None: ( candidate_dir / config_file for candidate_dir in candidate_dirs - for config_file in (CONFIG_FILE_NAME, "pyproject.toml") + for config_file in DEFAULT_CONFIG_FILE_NAMES if (candidate_dir / config_file).is_file() ), None, diff --git a/tests/conftest.py b/tests/conftest.py index 842120b99..730e34aa7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -31,7 +31,7 @@ from piptools._compat import PIP_VERSION, Distribution from piptools.cache import DependencyCache from piptools.exceptions import NoCandidateFound -from piptools.locations import CONFIG_FILE_NAME +from piptools.locations import DEFAULT_CONFIG_FILE_NAMES from piptools.logging import log from piptools.repositories import PyPIRepository from piptools.repositories.base import BaseRepository @@ -452,7 +452,9 @@ def make_config_file(tmpdir_cwd): """ def _maker( - pyproject_param: str, new_default: Any, config_file_name: str = CONFIG_FILE_NAME + pyproject_param: str, + new_default: Any, + config_file_name: str = DEFAULT_CONFIG_FILE_NAMES[0], ) -> Path: # Create a nested directory structure if config_file_name includes directories config_dir = Path(tmpdir_cwd / config_file_name).parent diff --git a/tests/test_utils.py b/tests/test_utils.py index 672456b31..6d5198b6f 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -418,7 +418,7 @@ def test_get_compile_command_with_config(tmpdir_cwd, config_file, expected_comma ( pytest.param("", id="empty config file"), pytest.param("[tool.pip-tools]", id="empty config section"), - pytest.param("[tool.pip-tools]\ndry-run = True", id="non-empty config section"), + pytest.param("[tool.pip-tools]\ndry-run = true", id="non-empty config section"), ), ) def test_get_compile_command_does_not_include_default_config_if_reqs_file_in_subdir( From fd1c6baad445ecc41bdd1b0af407ccd0f26abe35 Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Thu, 27 Jul 2023 05:22:23 +0200 Subject: [PATCH 07/11] fix indentation --- tests/test_utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 6d5198b6f..42cfc6d22 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -750,10 +750,10 @@ def test_select_config_file_prefers_pip_tools_toml_over_pyproject_toml(tmpdir_cw pyproject_file.write_text( dedent( """\ - [build-system] - requires = ["setuptools>=63", "setuptools_scm[toml]>=7"] - build-backend = "setuptools.build_meta" - """ + [build-system] + requires = ["setuptools>=63", "setuptools_scm[toml]>=7"] + build-backend = "setuptools.build_meta" + """ ) ) From 7f9411147d73136e00794b5eeee8a50a7247834a Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Thu, 27 Jul 2023 17:56:00 +0200 Subject: [PATCH 08/11] Remove redundant cast to Path --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index f4952b9dc..d46169756 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -461,7 +461,7 @@ def _maker( config_dir.mkdir(exist_ok=True, parents=True) # Make a config file with this one config default override - config_file = Path(tmpdir_cwd / config_file_name) + config_file = tmpdir_cwd / config_file_name config_to_dump = {"tool": {"pip-tools": {pyproject_param: new_default}}} config_file.write_text(tomli_w.dumps(config_to_dump)) return cast(Path, config_file.relative_to(tmpdir_cwd)) From 90c2a2eb4569ac20f301c259d4321e9660727f6b Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Thu, 27 Jul 2023 18:08:00 +0200 Subject: [PATCH 09/11] Fix failing test --- tests/test_cli_compile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index e7a7ce0e2..5fc0e2a40 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -2979,7 +2979,7 @@ def test_default_config_option(pip_conf, runner, make_config_file, tmpdir_cwd): make_config_file("dry-run", True) req_in = tmpdir_cwd / "requirements.in" - req_in.ensure() + req_in.touch() out = runner.invoke(cli) From 37b302ee4caf70d6ab373db64ae5a15da9e70acd Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Thu, 27 Jul 2023 23:06:32 +0200 Subject: [PATCH 10/11] remove redundat space --- piptools/scripts/compile.py | 4 ++-- piptools/scripts/sync.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index e2eaaf0cb..acb0a937d 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -316,10 +316,10 @@ def _determine_linesep( ), help=( f"Read configuration from TOML file. By default, looks for the following " - f"files in the given order: {', '.join(DEFAULT_CONFIG_FILE_NAMES)}. " + f"files in the given order: {', '.join(DEFAULT_CONFIG_FILE_NAMES)}." ), - callback=override_defaults_from_config_file, is_eager=True, + callback=override_defaults_from_config_file, ) @click.option( "--no-config", diff --git a/piptools/scripts/sync.py b/piptools/scripts/sync.py index 54274a9be..307ce1b21 100755 --- a/piptools/scripts/sync.py +++ b/piptools/scripts/sync.py @@ -100,10 +100,10 @@ ), help=( f"Read configuration from TOML file. By default, looks for the following " - f"files in the given order: {', '.join(DEFAULT_CONFIG_FILE_NAMES)}. " + f"files in the given order: {', '.join(DEFAULT_CONFIG_FILE_NAMES)}." ), - callback=override_defaults_from_config_file, is_eager=True, + callback=override_defaults_from_config_file, ) @click.option( "--no-config", From d5ebe851d40bafd29d7b99fe1a81d9a7471255b2 Mon Sep 17 00:00:00 2001 From: Albert Tugushev Date: Mon, 7 Aug 2023 17:02:58 +0200 Subject: [PATCH 11/11] Update tests/conftest.py Co-authored-by: Ryan Walker --- tests/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index d46169756..4834f4bc1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -457,7 +457,7 @@ def _maker( config_file_name: str = DEFAULT_CONFIG_FILE_NAMES[0], ) -> Path: # Create a nested directory structure if config_file_name includes directories - config_dir = Path(tmpdir_cwd / config_file_name).parent + config_dir = (tmpdir_cwd / config_file_name).parent config_dir.mkdir(exist_ok=True, parents=True) # Make a config file with this one config default override