From 43b43c350b16245aaf49e9866e22455a567b0f19 Mon Sep 17 00:00:00 2001 From: finswimmer Date: Fri, 1 Nov 2024 11:13:18 +0100 Subject: [PATCH 1/8] feat(config): add get_property() to ConfigSource --- src/poetry/config/config_source.py | 7 ++ src/poetry/config/dict_config_source.py | 14 ++++ src/poetry/config/file_config_source.py | 15 ++++ tests/config/test_dict_config_source.py | 68 ++++++++++++++++++ tests/config/test_file_config_source.py | 91 +++++++++++++++++++++++++ 5 files changed, 195 insertions(+) create mode 100644 tests/config/test_dict_config_source.py create mode 100644 tests/config/test_file_config_source.py diff --git a/src/poetry/config/config_source.py b/src/poetry/config/config_source.py index ae9da9da050..ed5e492e9af 100644 --- a/src/poetry/config/config_source.py +++ b/src/poetry/config/config_source.py @@ -5,7 +5,14 @@ from typing import Any +class PropertyNotFoundError(ValueError): + pass + + class ConfigSource(ABC): + @abstractmethod + def get_property(self, key: str) -> Any: ... + @abstractmethod def add_property(self, key: str, value: Any) -> None: ... diff --git a/src/poetry/config/dict_config_source.py b/src/poetry/config/dict_config_source.py index 942d76ea1b4..4b5a87a1699 100644 --- a/src/poetry/config/dict_config_source.py +++ b/src/poetry/config/dict_config_source.py @@ -3,6 +3,7 @@ from typing import Any from poetry.config.config_source import ConfigSource +from poetry.config.config_source import PropertyNotFoundError class DictConfigSource(ConfigSource): @@ -13,6 +14,19 @@ def __init__(self) -> None: def config(self) -> dict[str, Any]: return self._config + def get_property(self, key: str) -> Any: + keys = key.split(".") + config = self._config + + for i, key in enumerate(keys): + if key not in config: + raise PropertyNotFoundError(f"Key {'.'.join(keys)} not in config") + + if i == len(keys) - 1: + return config[key] + + config = config[key] + def add_property(self, key: str, value: Any) -> None: keys = key.split(".") config = self._config diff --git a/src/poetry/config/file_config_source.py b/src/poetry/config/file_config_source.py index eed4cd053ca..6d8677bb5b2 100644 --- a/src/poetry/config/file_config_source.py +++ b/src/poetry/config/file_config_source.py @@ -8,6 +8,7 @@ from tomlkit import table from poetry.config.config_source import ConfigSource +from poetry.config.config_source import PropertyNotFoundError if TYPE_CHECKING: @@ -30,6 +31,20 @@ def name(self) -> str: def file(self) -> TOMLFile: return self._file + def get_property(self, key: str) -> Any: + keys = key.split(".") + + config = self.file.read() if self.file.exists() else {} + + for i, key in enumerate(keys): + if key not in config: + raise PropertyNotFoundError(f"Key {'.'.join(keys)} not in config") + + if i == len(keys) - 1: + return config[key] + + config = config[key] + def add_property(self, key: str, value: Any) -> None: with self.secure() as toml: config: dict[str, Any] = toml diff --git a/tests/config/test_dict_config_source.py b/tests/config/test_dict_config_source.py new file mode 100644 index 00000000000..925833d5a85 --- /dev/null +++ b/tests/config/test_dict_config_source.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +import pytest + +from poetry.config.config_source import PropertyNotFoundError +from poetry.config.dict_config_source import DictConfigSource + + +def test_dict_config_source_add_property() -> None: + config_source = DictConfigSource() + assert config_source._config == {} + + config_source.add_property("system-git-client", True) + assert config_source._config == {"system-git-client": True} + + config_source.add_property("virtualenvs.use-poetry-python", False) + assert config_source._config == { + "virtualenvs": { + "use-poetry-python": False, + }, + "system-git-client": True, + } + + +def test_dict_config_source_remove_property() -> None: + config_data = { + "virtualenvs": { + "use-poetry-python": False, + }, + "system-git-client": True, + } + + config_source = DictConfigSource() + config_source._config = config_data + + config_source.remove_property("system-git-client") + assert config_source._config == { + "virtualenvs": { + "use-poetry-python": False, + } + } + + config_source.remove_property("virtualenvs.use-poetry-python") + assert config_source._config == {"virtualenvs": {}} + + +def test_dict_config_source_get_property() -> None: + config_data = { + "virtualenvs": { + "use-poetry-python": False, + }, + "system-git-client": True, + } + + config_source = DictConfigSource() + config_source._config = config_data + + assert config_source.get_property("virtualenvs.use-poetry-python") is False + assert config_source.get_property("system-git-client") is True + + +def test_dict_config_source_get_property_should_raise_if_not_found() -> None: + config_source = DictConfigSource() + + with pytest.raises( + PropertyNotFoundError, match="Key virtualenvs.use-poetry-python not in config" + ): + _ = config_source.get_property("virtualenvs.use-poetry-python") diff --git a/tests/config/test_file_config_source.py b/tests/config/test_file_config_source.py new file mode 100644 index 00000000000..c5fbac18ae4 --- /dev/null +++ b/tests/config/test_file_config_source.py @@ -0,0 +1,91 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + +import pytest +import tomlkit + +from poetry.config.config_source import PropertyNotFoundError +from poetry.config.file_config_source import FileConfigSource +from poetry.toml import TOMLFile + + +if TYPE_CHECKING: + from pathlib import Path + + +def test_file_config_source_add_property(tmp_path: Path) -> None: + config = tmp_path.joinpath("config.toml") + config.touch() + + config_source = FileConfigSource(TOMLFile(config)) + + assert config_source._file.read() == {} + + config_source.add_property("system-git-client", True) + assert config_source._file.read() == {"system-git-client": True} + + config_source.add_property("virtualenvs.use-poetry-python", False) + assert config_source._file.read() == { + "virtualenvs": { + "use-poetry-python": False, + }, + "system-git-client": True, + } + + +def test_file_config_source_remove_property(tmp_path: Path) -> None: + config_data = { + "virtualenvs": { + "use-poetry-python": False, + }, + "system-git-client": True, + } + + config = tmp_path.joinpath("config.toml") + with config.open(mode="w") as f: + f.write(tomlkit.dumps(config_data)) + + config_source = FileConfigSource(TOMLFile(config)) + + config_source.remove_property("system-git-client") + assert config_source._file.read() == { + "virtualenvs": { + "use-poetry-python": False, + } + } + + config_source.remove_property("virtualenvs.use-poetry-python") + assert config_source._file.read() == {"virtualenvs": {}} + + +def test_file_config_source_get_property(tmp_path: Path) -> None: + config_data = { + "virtualenvs": { + "use-poetry-python": False, + }, + "system-git-client": True, + } + + config = tmp_path.joinpath("config.toml") + with config.open(mode="w") as f: + f.write(tomlkit.dumps(config_data)) + + config_source = FileConfigSource(TOMLFile(config)) + + assert config_source.get_property("virtualenvs.use-poetry-python") is False + assert config_source.get_property("system-git-client") is True + + +def test_file_config_source_get_property_should_raise_if_not_found( + tmp_path: Path, +) -> None: + config = tmp_path.joinpath("config.toml") + config.touch() + + config_source = FileConfigSource(TOMLFile(config)) + + with pytest.raises( + PropertyNotFoundError, match="Key virtualenvs.use-poetry-python not in config" + ): + _ = config_source.get_property("virtualenvs.use-poetry-python") From 677ba8126cfe9a36d6226226f17dd491eff7e32b Mon Sep 17 00:00:00 2001 From: finswimmer Date: Tue, 5 Nov 2024 07:23:13 +0100 Subject: [PATCH 2/8] feat(config): provide method to remove empty config category For now only applied to FileConfigSource on remove_property --- src/poetry/config/config_source.py | 19 +++++++++ src/poetry/config/file_config_source.py | 5 +++ tests/config/test_config_source.py | 53 +++++++++++++++++++++++++ tests/config/test_file_config_source.py | 2 +- 4 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 tests/config/test_config_source.py diff --git a/src/poetry/config/config_source.py b/src/poetry/config/config_source.py index ed5e492e9af..112092a03c1 100644 --- a/src/poetry/config/config_source.py +++ b/src/poetry/config/config_source.py @@ -18,3 +18,22 @@ def add_property(self, key: str, value: Any) -> None: ... @abstractmethod def remove_property(self, key: str) -> None: ... + + +def drop_empty_config_category( + keys: list[str], config: dict[Any, Any] +) -> dict[Any, Any]: + config_ = {} + + for key, value in config.items(): + if not keys or key != keys[0]: + config_[key] = value + continue + if keys and key == keys[0]: + if isinstance(value, dict): + value = drop_empty_config_category(keys[1:], value) + + if value != {}: + config_[key] = value + + return config_ diff --git a/src/poetry/config/file_config_source.py b/src/poetry/config/file_config_source.py index 6d8677bb5b2..9170e0ef254 100644 --- a/src/poetry/config/file_config_source.py +++ b/src/poetry/config/file_config_source.py @@ -9,6 +9,7 @@ from poetry.config.config_source import ConfigSource from poetry.config.config_source import PropertyNotFoundError +from poetry.config.config_source import drop_empty_config_category if TYPE_CHECKING: @@ -77,6 +78,10 @@ def remove_property(self, key: str) -> None: current_config = current_config[key] + current_config = drop_empty_config_category(keys=keys[:-1], config=config) + config.clear() + config.update(current_config) + @contextmanager def secure(self) -> Iterator[TOMLDocument]: if self.file.exists(): diff --git a/tests/config/test_config_source.py b/tests/config/test_config_source.py new file mode 100644 index 00000000000..6943dbb64d7 --- /dev/null +++ b/tests/config/test_config_source.py @@ -0,0 +1,53 @@ +from __future__ import annotations + +from typing import Any + +import pytest + +from poetry.config.config_source import drop_empty_config_category + + +@pytest.mark.parametrize( + ["config_data", "expected"], + [ + ( + { + "category_a": { + "category_b": { + "category_c": {}, + }, + }, + "system-git-client": True, + }, + {"system-git-client": True}, + ), + ( + { + "category_a": { + "category_b": { + "category_c": {}, + "category_d": {"some_config": True}, + }, + }, + "system-git-client": True, + }, + { + "category_a": { + "category_b": { + "category_d": {"some_config": True}, + } + }, + "system-git-client": True, + }, + ), + ], +) +def test_drop_empty_config_category( + config_data: dict[Any, Any], expected: dict[Any, Any] +) -> None: + assert ( + drop_empty_config_category( + keys=["category_a", "category_b", "category_c"], config=config_data + ) + == expected + ) diff --git a/tests/config/test_file_config_source.py b/tests/config/test_file_config_source.py index c5fbac18ae4..2edef353fe3 100644 --- a/tests/config/test_file_config_source.py +++ b/tests/config/test_file_config_source.py @@ -56,7 +56,7 @@ def test_file_config_source_remove_property(tmp_path: Path) -> None: } config_source.remove_property("virtualenvs.use-poetry-python") - assert config_source._file.read() == {"virtualenvs": {}} + assert config_source._file.read() == {} def test_file_config_source_get_property(tmp_path: Path) -> None: From eaf33cf30add4a02e787dfc39f08bef6c2f3767d Mon Sep 17 00:00:00 2001 From: finswimmer Date: Mon, 4 Nov 2024 06:56:34 +0100 Subject: [PATCH 3/8] feat(config): add ConfigSourceMigration Rename or delete keys from ConfigSource. Migrate existing values including unset them. --- src/poetry/config/config_source.py | 27 ++++++++ tests/config/test_config_source.py | 105 +++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/src/poetry/config/config_source.py b/src/poetry/config/config_source.py index 112092a03c1..0172b0271d2 100644 --- a/src/poetry/config/config_source.py +++ b/src/poetry/config/config_source.py @@ -1,10 +1,15 @@ from __future__ import annotations +import dataclasses + from abc import ABC from abc import abstractmethod from typing import Any +UNSET = object() + + class PropertyNotFoundError(ValueError): pass @@ -20,6 +25,28 @@ def add_property(self, key: str, value: Any) -> None: ... def remove_property(self, key: str) -> None: ... +@dataclasses.dataclass +class ConfigSourceMigration: + old_key: str + new_key: str | None + value_migration: dict[Any, Any] = dataclasses.field(default_factory=dict) + + def apply(self, config_source: ConfigSource) -> None: + try: + old_value = config_source.get_property(self.old_key) + except PropertyNotFoundError: + return + + new_value = ( + self.value_migration[old_value] if self.value_migration else old_value + ) + + config_source.remove_property(self.old_key) + + if self.new_key is not None and new_value is not UNSET: + config_source.add_property(self.new_key, new_value) + + def drop_empty_config_category( keys: list[str], config: dict[Any, Any] ) -> dict[Any, Any]: diff --git a/tests/config/test_config_source.py b/tests/config/test_config_source.py index 6943dbb64d7..abd81399adf 100644 --- a/tests/config/test_config_source.py +++ b/tests/config/test_config_source.py @@ -4,7 +4,10 @@ import pytest +from poetry.config.config_source import UNSET +from poetry.config.config_source import ConfigSourceMigration from poetry.config.config_source import drop_empty_config_category +from poetry.config.dict_config_source import DictConfigSource @pytest.mark.parametrize( @@ -51,3 +54,105 @@ def test_drop_empty_config_category( ) == expected ) + + +def test_config_source_migration_rename_key() -> None: + config_data = { + "virtualenvs": { + "prefer-active-python": True, + }, + "system-git-client": True, + } + + config_source = DictConfigSource() + config_source._config = config_data + + migration = ConfigSourceMigration( + old_key="virtualenvs.prefer-active-python", + new_key="virtualenvs.use-poetry-python", + ) + + migration.apply(config_source) + + config_source._config = { + "virtualenvs": { + "use-poetry-python": True, + }, + "system-git-client": True, + } + + +def test_config_source_migration_remove_key() -> None: + config_data = { + "virtualenvs": { + "prefer-active-python": True, + }, + "system-git-client": True, + } + + config_source = DictConfigSource() + config_source._config = config_data + + migration = ConfigSourceMigration( + old_key="virtualenvs.prefer-active-python", + new_key=None, + ) + + migration.apply(config_source) + + config_source._config = { + "virtualenvs": {}, + "system-git-client": True, + } + + +def test_config_source_migration_unset_value() -> None: + config_data = { + "virtualenvs": { + "prefer-active-python": True, + }, + "system-git-client": True, + } + + config_source = DictConfigSource() + config_source._config = config_data + + migration = ConfigSourceMigration( + old_key="virtualenvs.prefer-active-python", + new_key="virtualenvs.use-poetry-python", + value_migration={True: UNSET, False: True}, + ) + + migration.apply(config_source) + + config_source._config = { + "virtualenvs": {}, + "system-git-client": True, + } + + +def test_config_source_migration_complex_migration() -> None: + config_data = { + "virtualenvs": { + "prefer-active-python": True, + }, + "system-git-client": True, + } + + config_source = DictConfigSource() + config_source._config = config_data + + migration = ConfigSourceMigration( + old_key="virtualenvs.prefer-active-python", + new_key="virtualenvs.use-poetry-python", + value_migration={True: None, False: True}, + ) + + migration.apply(config_source) + + config_source._config = { + "virtualenvs": { + "use-poetry-python": None, + }, + "system-git-client": True, + } From cd78de8abcd678b57921ca415f04aec96dc4ba70 Mon Sep 17 00:00:00 2001 From: finswimmer Date: Mon, 4 Nov 2024 11:08:23 +0100 Subject: [PATCH 4/8] feat(cli): add --migration option to config command --- src/poetry/console/commands/config.py | 28 ++++++++++++++++++++++ tests/console/commands/test_config.py | 34 +++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/poetry/console/commands/config.py b/src/poetry/console/commands/config.py index 706e21158dc..f79c5b75520 100644 --- a/src/poetry/console/commands/config.py +++ b/src/poetry/console/commands/config.py @@ -16,6 +16,8 @@ from poetry.config.config import boolean_normalizer from poetry.config.config import boolean_validator from poetry.config.config import int_normalizer +from poetry.config.config_source import UNSET +from poetry.config.config_source import ConfigSourceMigration from poetry.console.commands.command import Command @@ -25,6 +27,17 @@ from poetry.config.config_source import ConfigSource +CONFIG_MIGRATIONS = [ + ConfigSourceMigration( + old_key="experimental.system-git-client", new_key="system-git-client" + ), + ConfigSourceMigration( + old_key="virtualenvs.prefer-active-python", + new_key="virtualenvs.use-poetry-python", + value_migration={True: UNSET, False: True}, + ), +] + class ConfigCommand(Command): name = "config" @@ -39,6 +52,7 @@ class ConfigCommand(Command): option("list", None, "List configuration settings."), option("unset", None, "Unset configuration setting."), option("local", None, "Set/Get from the project's local configuration."), + option("migrate", None, "Migrate outdated configuration settings."), ] help = """\ @@ -98,6 +112,9 @@ def handle(self) -> int: from poetry.locations import CONFIG_DIR from poetry.toml.file import TOMLFile + if self.option("migrate"): + self._migrate() + config = Config.create() config_file = TOMLFile(CONFIG_DIR / "config.toml") @@ -325,3 +342,14 @@ def _list_configuration( message = f"{k + key} = {json.dumps(value)}" self.line(message) + + def _migrate(self) -> None: + from poetry.config.file_config_source import FileConfigSource + from poetry.locations import CONFIG_DIR + from poetry.toml.file import TOMLFile + + config_file = TOMLFile(CONFIG_DIR / "config.toml") + config_source = FileConfigSource(config_file) + + for migration in CONFIG_MIGRATIONS: + migration.apply(config_source) diff --git a/tests/console/commands/test_config.py b/tests/console/commands/test_config.py index 11ddfcbdbc3..117b8dc68ab 100644 --- a/tests/console/commands/test_config.py +++ b/tests/console/commands/test_config.py @@ -2,7 +2,9 @@ import json import os +import textwrap +from pathlib import Path from typing import TYPE_CHECKING import pytest @@ -18,8 +20,6 @@ if TYPE_CHECKING: - from pathlib import Path - from cleo.testers.command_tester import CommandTester from pytest_mock import MockerFixture @@ -566,3 +566,33 @@ def test_config_solver_lazy_wheel( repo = LegacyRepository("foo", "https://foo.com") assert not repo._lazy_wheel + + +def test_config_migrate( + tester: CommandTester, mocker: MockerFixture, tmp_path: Path +) -> None: + config_dir = tmp_path / "config" + mocker.patch("poetry.locations.CONFIG_DIR", config_dir) + + config_file = Path(config_dir / "config.toml") + config_data = textwrap.dedent("""\ + [experimental] + system-git-client = true + + [virtualenvs] + prefer-active-python = false + """) + with config_file.open("w") as fh: + fh.write(config_data) + + tester.execute("--migrate") + + expected_config = textwrap.dedent("""\ + system-git-client = true + + [virtualenvs] + use-poetry-python = true + """) + + with config_file.open("r") as fh: + assert fh.read() == expected_config From a7d3504a8e60ed70af1c3331685581b090421e70 Mon Sep 17 00:00:00 2001 From: finswimmer Date: Tue, 5 Nov 2024 19:28:12 +0100 Subject: [PATCH 5/8] feat(cli): add support for --local for --migrate --- src/poetry/console/commands/config.py | 6 +++++ tests/console/commands/test_config.py | 34 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/poetry/console/commands/config.py b/src/poetry/console/commands/config.py index f79c5b75520..17ba2dd814b 100644 --- a/src/poetry/console/commands/config.py +++ b/src/poetry/console/commands/config.py @@ -349,6 +349,12 @@ def _migrate(self) -> None: from poetry.toml.file import TOMLFile config_file = TOMLFile(CONFIG_DIR / "config.toml") + + if self.option("local"): + config_file = TOMLFile(self.poetry.file.path.parent / "poetry.toml") + if not config_file.exists(): + raise RuntimeError("No local config file found") + config_source = FileConfigSource(config_file) for migration in CONFIG_MIGRATIONS: diff --git a/tests/console/commands/test_config.py b/tests/console/commands/test_config.py index 117b8dc68ab..9c811532526 100644 --- a/tests/console/commands/test_config.py +++ b/tests/console/commands/test_config.py @@ -24,6 +24,7 @@ from pytest_mock import MockerFixture from poetry.config.dict_config_source import DictConfigSource + from poetry.poetry import Poetry from tests.types import CommandTesterFactory from tests.types import FixtureDirGetter from tests.types import ProjectFactory @@ -596,3 +597,36 @@ def test_config_migrate( with config_file.open("r") as fh: assert fh.read() == expected_config + + +def test_config_migrate_local_config(tester: CommandTester, poetry: Poetry) -> None: + local_config = poetry.file.path.parent / "poetry.toml" + config_data = textwrap.dedent("""\ + [experimental] + system-git-client = true + + [virtualenvs] + prefer-active-python = false + """) + + with local_config.open("w") as fh: + fh.write(config_data) + + tester.execute("--migrate --local") + + expected_config = textwrap.dedent("""\ + system-git-client = true + + [virtualenvs] + use-poetry-python = true + """) + + with local_config.open("r") as fh: + assert fh.read() == expected_config + + +def test_config_migrate_local_config_should_raise_if_not_found( + tester: CommandTester, +) -> None: + with pytest.raises(RuntimeError, match="No local config file found"): + tester.execute("--migrate --local") From 8c6bb9530a8cc316e326ee4c7e47877370d221a9 Mon Sep 17 00:00:00 2001 From: finswimmer Date: Tue, 5 Nov 2024 20:39:02 +0100 Subject: [PATCH 6/8] docs: add information about --migrate --- docs/cli.md | 1 + docs/configuration.md | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/docs/cli.md b/docs/cli.md index b815a51c7ac..5d5717962a4 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -630,6 +630,7 @@ Without `--` this command will fail if `${GITLAB_JOB_TOKEN}` starts with a hyphe * `--unset`: Remove the configuration element named by `setting-key`. * `--list`: Show the list of current config variables. * `--local`: Set/Get settings that are specific to a project (in the local configuration file `poetry.toml`). +* `--migrate`: Migrate outdated configuration settings. ## run diff --git a/docs/configuration.md b/docs/configuration.md index 8004061dc39..fcf08e7c866 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -114,6 +114,21 @@ This also works for secret settings, like credentials: export POETRY_HTTP_BASIC_MY_REPOSITORY_PASSWORD=secret ``` +## Migrate outdated configs + +If poetry renames or remove config options it might be necessary to migrate explicit set options. This is possible +by running: + +```bash +poetry config --migrate +``` + +If you need to migrate a local config run: + +```bash +poetry config --migrate --local +``` + ## Default Directories Poetry uses the following default directories: From 0b12771bdee84b96a8a97ed750cec187cc61eca5 Mon Sep 17 00:00:00 2001 From: finswimmer Date: Thu, 7 Nov 2024 07:00:45 +0100 Subject: [PATCH 7/8] feat(cli): add info messages about applied migration --- src/poetry/config/config_source.py | 21 ++++++++++++++++++++- src/poetry/console/commands/config.py | 6 +++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/poetry/config/config_source.py b/src/poetry/config/config_source.py index 0172b0271d2..61f7b37b648 100644 --- a/src/poetry/config/config_source.py +++ b/src/poetry/config/config_source.py @@ -1,11 +1,19 @@ from __future__ import annotations import dataclasses +import json from abc import ABC from abc import abstractmethod +from typing import TYPE_CHECKING from typing import Any +from cleo.io.null_io import NullIO + + +if TYPE_CHECKING: + from cleo.io.io import IO + UNSET = object() @@ -31,7 +39,9 @@ class ConfigSourceMigration: new_key: str | None value_migration: dict[Any, Any] = dataclasses.field(default_factory=dict) - def apply(self, config_source: ConfigSource) -> None: + def apply(self, config_source: ConfigSource, io: IO | None = None) -> None: + io = io or NullIO() + try: old_value = config_source.get_property(self.old_key) except PropertyNotFoundError: @@ -43,8 +53,17 @@ def apply(self, config_source: ConfigSource) -> None: config_source.remove_property(self.old_key) + msg = f"{self.old_key} = {json.dumps(old_value)}" + if self.new_key is not None and new_value is not UNSET: + msg += f" -> {self.new_key} = {json.dumps(new_value)}" config_source.add_property(self.new_key, new_value) + elif self.new_key is None: + msg += " -> Removed from config" + elif self.new_key and new_value is UNSET: + msg += f" -> {self.new_key} = Not explicit set" + + io.write_line(msg) def drop_empty_config_category( diff --git a/src/poetry/console/commands/config.py b/src/poetry/console/commands/config.py index 17ba2dd814b..cb0083ef63c 100644 --- a/src/poetry/console/commands/config.py +++ b/src/poetry/console/commands/config.py @@ -357,5 +357,9 @@ def _migrate(self) -> None: config_source = FileConfigSource(config_file) + self.io.write_line("Starting config migration ...") + for migration in CONFIG_MIGRATIONS: - migration.apply(config_source) + migration.apply(config_source, io=self.io) + + self.io.write_line("Config migration successfully done.") From 99ea24b161f656d0bce0cc60ff08dccc48be6c2a Mon Sep 17 00:00:00 2001 From: finswimmer Date: Sat, 16 Nov 2024 19:04:48 +0100 Subject: [PATCH 8/8] feat: add confirmation step --- src/poetry/config/config_source.py | 24 +++++++++--- src/poetry/console/commands/config.py | 21 ++++++++-- tests/console/commands/test_config.py | 56 +++++++++++++++++++-------- 3 files changed, 75 insertions(+), 26 deletions(-) diff --git a/src/poetry/config/config_source.py b/src/poetry/config/config_source.py index 61f7b37b648..e06c934e70a 100644 --- a/src/poetry/config/config_source.py +++ b/src/poetry/config/config_source.py @@ -39,25 +39,22 @@ class ConfigSourceMigration: new_key: str | None value_migration: dict[Any, Any] = dataclasses.field(default_factory=dict) - def apply(self, config_source: ConfigSource, io: IO | None = None) -> None: + def dry_run(self, config_source: ConfigSource, io: IO | None = None) -> bool: io = io or NullIO() try: old_value = config_source.get_property(self.old_key) except PropertyNotFoundError: - return + return False new_value = ( self.value_migration[old_value] if self.value_migration else old_value ) - config_source.remove_property(self.old_key) - msg = f"{self.old_key} = {json.dumps(old_value)}" if self.new_key is not None and new_value is not UNSET: msg += f" -> {self.new_key} = {json.dumps(new_value)}" - config_source.add_property(self.new_key, new_value) elif self.new_key is None: msg += " -> Removed from config" elif self.new_key and new_value is UNSET: @@ -65,6 +62,23 @@ def apply(self, config_source: ConfigSource, io: IO | None = None) -> None: io.write_line(msg) + return True + + def apply(self, config_source: ConfigSource) -> None: + try: + old_value = config_source.get_property(self.old_key) + except PropertyNotFoundError: + return + + new_value = ( + self.value_migration[old_value] if self.value_migration else old_value + ) + + config_source.remove_property(self.old_key) + + if self.new_key is not None and new_value is not UNSET: + config_source.add_property(self.new_key, new_value) + def drop_empty_config_category( keys: list[str], config: dict[Any, Any] diff --git a/src/poetry/console/commands/config.py b/src/poetry/console/commands/config.py index cb0083ef63c..8002e537811 100644 --- a/src/poetry/console/commands/config.py +++ b/src/poetry/console/commands/config.py @@ -357,9 +357,22 @@ def _migrate(self) -> None: config_source = FileConfigSource(config_file) - self.io.write_line("Starting config migration ...") + self.io.write_line("Checking for required migrations ...") - for migration in CONFIG_MIGRATIONS: - migration.apply(config_source, io=self.io) + required_migrations = [ + migration + for migration in CONFIG_MIGRATIONS + if migration.dry_run(config_source, io=self.io) + ] - self.io.write_line("Config migration successfully done.") + if not required_migrations: + self.io.write_line("Already up to date.") + return + + if not self.io.is_interactive() or self.confirm( + "Proceed with migration?: ", False + ): + for migration in required_migrations: + migration.apply(config_source) + + self.io.write_line("Config migration successfully done.") diff --git a/tests/console/commands/test_config.py b/tests/console/commands/test_config.py index 9c811532526..d4925b85129 100644 --- a/tests/console/commands/test_config.py +++ b/tests/console/commands/test_config.py @@ -569,32 +569,54 @@ def test_config_solver_lazy_wheel( assert not repo._lazy_wheel +current_config = """\ +[experimental] +system-git-client = true + +[virtualenvs] +prefer-active-python = false +""" + +config_migrated = """\ +system-git-client = true + +[virtualenvs] +use-poetry-python = true +""" + + +@pytest.mark.parametrize( + ["proceed", "expected_config"], + [ + ("yes", config_migrated), + ("no", current_config), + ], +) def test_config_migrate( - tester: CommandTester, mocker: MockerFixture, tmp_path: Path + proceed: str, + expected_config: str, + tester: CommandTester, + mocker: MockerFixture, + tmp_path: Path, ) -> None: config_dir = tmp_path / "config" mocker.patch("poetry.locations.CONFIG_DIR", config_dir) config_file = Path(config_dir / "config.toml") - config_data = textwrap.dedent("""\ - [experimental] - system-git-client = true - - [virtualenvs] - prefer-active-python = false - """) with config_file.open("w") as fh: - fh.write(config_data) - - tester.execute("--migrate") + fh.write(current_config) - expected_config = textwrap.dedent("""\ - system-git-client = true + tester.execute("--migrate", inputs=proceed) - [virtualenvs] - use-poetry-python = true + expected_output = textwrap.dedent("""\ + Checking for required migrations ... + experimental.system-git-client = true -> system-git-client = true + virtualenvs.prefer-active-python = false -> virtualenvs.use-poetry-python = true """) + output = tester.io.fetch_output() + assert output.startswith(expected_output) + with config_file.open("r") as fh: assert fh.read() == expected_config @@ -612,7 +634,7 @@ def test_config_migrate_local_config(tester: CommandTester, poetry: Poetry) -> N with local_config.open("w") as fh: fh.write(config_data) - tester.execute("--migrate --local") + tester.execute("--migrate --local", inputs="yes") expected_config = textwrap.dedent("""\ system-git-client = true @@ -629,4 +651,4 @@ def test_config_migrate_local_config_should_raise_if_not_found( tester: CommandTester, ) -> None: with pytest.raises(RuntimeError, match="No local config file found"): - tester.execute("--migrate --local") + tester.execute("--migrate --local", inputs="yes")