From 3ea0a49b7715d78886f28c24398e23582e8f2dee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 25 Feb 2023 17:16:11 +0100 Subject: [PATCH] fix: don't merge sources into config --- src/poetry/console/commands/source/add.py | 6 +--- src/poetry/factory.py | 24 +++++++------- src/poetry/installation/chef.py | 15 ++++----- src/poetry/installation/executor.py | 2 +- tests/console/commands/self/conftest.py | 10 ++++-- tests/console/commands/test_config.py | 39 +++++++++++++++++++++++ tests/installation/test_chef.py | 11 ++++--- tests/installation/test_executor.py | 12 +++---- 8 files changed, 80 insertions(+), 39 deletions(-) diff --git a/src/poetry/console/commands/source/add.py b/src/poetry/console/commands/source/add.py index f258e404af0..81a0ca66e91 100644 --- a/src/poetry/console/commands/source/add.py +++ b/src/poetry/console/commands/source/add.py @@ -83,13 +83,9 @@ def handle(self) -> int: self.line(f"Adding source with name {name}.") sources.append(source_to_table(new_source)) - self.poetry.config.merge( - {"sources": {source["name"]: source for source in sources}} - ) - # ensure new source is valid. eg: invalid name etc. try: - pool = Factory.create_pool(self.poetry.config, NullIO()) + pool = Factory.create_pool(self.poetry.config, sources, NullIO()) pool.repository(name) except ValueError as e: self.line_error( diff --git a/src/poetry/factory.py b/src/poetry/factory.py index ff03718b48d..43960b43285 100644 --- a/src/poetry/factory.py +++ b/src/poetry/factory.py @@ -23,6 +23,7 @@ if TYPE_CHECKING: + from collections.abc import Iterable from pathlib import Path from cleo.io.io import IO @@ -89,16 +90,14 @@ def create_poetry( disable_cache, ) - # Configuring sources - config.merge( - { - "sources": { - source["name"]: source - for source in poetry.local_config.get("source", []) - } - } + poetry.set_pool( + self.create_pool( + config, + poetry.local_config.get("source", []), + io, + disable_cache=disable_cache, + ) ) - poetry.set_pool(self.create_pool(config, io, disable_cache=disable_cache)) plugin_manager = PluginManager(Plugin.group, disable_plugins=disable_plugins) plugin_manager.load_plugins() @@ -114,7 +113,8 @@ def get_package(cls, name: str, version: str) -> ProjectPackage: @classmethod def create_pool( cls, - config: Config, + auth_config: Config, + sources: Iterable[dict[str, Any]] = (), io: IO | None = None, disable_cache: bool = False, ) -> RepositoryPool: @@ -128,9 +128,9 @@ def create_pool( pool = RepositoryPool() - for source in config.get("sources", {}).values(): + for source in sources: repository = cls.create_package_source( - source, config, disable_cache=disable_cache + source, auth_config, disable_cache=disable_cache ) is_default = source.get("default", False) is_secondary = source.get("secondary", False) diff --git a/src/poetry/installation/chef.py b/src/poetry/installation/chef.py index 2a4c02c7451..d3f7b09d38e 100644 --- a/src/poetry/installation/chef.py +++ b/src/poetry/installation/chef.py @@ -30,6 +30,7 @@ from poetry.core.packages.utils.link import Link from poetry.config.config import Config + from poetry.repositories import RepositoryPool from poetry.utils.env import Env @@ -42,9 +43,9 @@ class ChefBuildError(ChefError): class IsolatedEnv(BaseIsolatedEnv): - def __init__(self, env: Env, config: Config) -> None: + def __init__(self, env: Env, pool: RepositoryPool) -> None: self._env = env - self._config = config + self._pool = pool @property def executable(self) -> str: @@ -60,7 +61,6 @@ def install(self, requirements: Collection[str]) -> None: from poetry.core.packages.project_package import ProjectPackage from poetry.config.config import Config - from poetry.factory import Factory from poetry.installation.installer import Installer from poetry.packages.locker import Locker from poetry.repositories.installed_repository import InstalledRepository @@ -72,13 +72,12 @@ def install(self, requirements: Collection[str]) -> None: dependency = Dependency.create_from_pep_508(requirement) package.add_dependency(dependency) - pool = Factory.create_pool(self._config) installer = Installer( NullIO(), self._env, package, Locker(self._env.path.joinpath("poetry.lock"), {}), - pool, + self._pool, Config.create(), InstalledRepository.load(self._env), ) @@ -87,9 +86,9 @@ def install(self, requirements: Collection[str]) -> None: class Chef: - def __init__(self, config: Config, env: Env) -> None: - self._config = config + def __init__(self, config: Config, env: Env, pool: RepositoryPool) -> None: self._env = env + self._pool = pool self._cache_dir = ( Path(config.get("cache-dir")).expanduser().joinpath("artifacts") ) @@ -113,7 +112,7 @@ def _prepare( from subprocess import CalledProcessError with ephemeral_environment(self._env.python) as venv: - env = IsolatedEnv(venv, self._config) + env = IsolatedEnv(venv, self._pool) builder = ProjectBuilder( directory, python_executable=env.executable, diff --git a/src/poetry/installation/executor.py b/src/poetry/installation/executor.py index a4ca1238cee..eb7302aa63d 100644 --- a/src/poetry/installation/executor.py +++ b/src/poetry/installation/executor.py @@ -79,7 +79,7 @@ def __init__( self._authenticator = Authenticator( config, self._io, disable_cache=disable_cache, pool_size=self._max_workers ) - self._chef = Chef(config, self._env) + self._chef = Chef(config, self._env, pool) self._chooser = Chooser(pool, self._env, config) self._executor = ThreadPoolExecutor(max_workers=self._max_workers) diff --git a/tests/console/commands/self/conftest.py b/tests/console/commands/self/conftest.py index 68aedf1ac21..381e22e2f90 100644 --- a/tests/console/commands/self/conftest.py +++ b/tests/console/commands/self/conftest.py @@ -1,6 +1,7 @@ from __future__ import annotations from typing import TYPE_CHECKING +from typing import Any from typing import Callable import pytest @@ -14,6 +15,8 @@ if TYPE_CHECKING: + from collections.abc import Iterable + import httpretty from cleo.io.io import IO @@ -44,9 +47,12 @@ def pool(repo: TestRepository) -> RepositoryPool: def create_pool_factory( repo: Repository, -) -> Callable[[Config, IO, bool], RepositoryPool]: +) -> Callable[[Config, Iterable[dict[str, Any]], IO, bool], RepositoryPool]: def _create_pool( - config: Config, io: IO, disable_cache: bool = False + config: Config, + sources: Iterable[dict[str, Any]] = (), + io: IO | None = None, + disable_cache: bool = False, ) -> RepositoryPool: pool = RepositoryPool() pool.add_repository(repo) diff --git a/tests/console/commands/test_config.py b/tests/console/commands/test_config.py index 22fca7dcafe..283146d2c31 100644 --- a/tests/console/commands/test_config.py +++ b/tests/console/commands/test_config.py @@ -24,6 +24,7 @@ from poetry.config.dict_config_source import DictConfigSource from tests.types import CommandTesterFactory from tests.types import FixtureDirGetter + from tests.types import ProjectFactory @pytest.fixture() @@ -156,6 +157,44 @@ def test_list_displays_set_get_local_setting( assert tester.io.fetch_output() == expected +def test_list_must_not_display_sources_from_pyproject_toml( + project_factory: ProjectFactory, + fixture_dir: FixtureDirGetter, + command_tester_factory: CommandTesterFactory, + config: Config, + config_cache_dir: Path, +): + source = fixture_dir("with_non_default_source") + pyproject_content = (source / "pyproject.toml").read_text(encoding="utf-8") + poetry = project_factory("foo", pyproject_content=pyproject_content) + tester = command_tester_factory("config", poetry=poetry) + + tester.execute("--list") + + cache_dir = json.dumps(str(config_cache_dir)) + venv_path = json.dumps(os.path.join("{cache-dir}", "virtualenvs")) + expected = f"""cache-dir = {cache_dir} +experimental.new-installer = true +experimental.system-git-client = false +installer.max-workers = null +installer.modern-installation = true +installer.no-binary = null +installer.parallel = true +repositories.foo.url = "https://foo.bar/simple/" +virtualenvs.create = true +virtualenvs.in-project = null +virtualenvs.options.always-copy = false +virtualenvs.options.no-pip = false +virtualenvs.options.no-setuptools = false +virtualenvs.options.system-site-packages = false +virtualenvs.path = {venv_path} # {config_cache_dir / 'virtualenvs'} +virtualenvs.prefer-active-python = false +virtualenvs.prompt = "{{project_name}}-py{{python_version}}" +""" + + assert tester.io.fetch_output() == expected + + def test_set_pypi_token(tester: CommandTester, auth_config_source: DictConfigSource): tester.execute("pypi-token.pypi mytoken") tester.execute("--list") diff --git a/tests/installation/test_chef.py b/tests/installation/test_chef.py index ac084455850..e9046c5433f 100644 --- a/tests/installation/test_chef.py +++ b/tests/installation/test_chef.py @@ -63,6 +63,7 @@ def test_get_cached_archive_for_link( Tag("py3", "none", "any"), ], ), + Factory.create_pool(config), ) mocker.patch.object( @@ -87,6 +88,7 @@ def test_get_cached_archives_for_link(config: Config, mocker: MockerFixture): MockEnv( marker_env={"interpreter_name": "cpython", "interpreter_version": "3.8.3"} ), + Factory.create_pool(config), ) distributions = Path(__file__).parent.parent.joinpath("fixtures/distributions") @@ -110,6 +112,7 @@ def test_get_cache_directory_for_link(config: Config, config_cache_dir: Path): MockEnv( marker_env={"interpreter_name": "cpython", "interpreter_version": "3.8.3"} ), + Factory.create_pool(config), ) directory = chef.get_cache_directory_for_link( @@ -125,7 +128,7 @@ def test_get_cache_directory_for_link(config: Config, config_cache_dir: Path): def test_prepare_sdist(config: Config, config_cache_dir: Path) -> None: - chef = Chef(config, EnvManager.get_system_env()) + chef = Chef(config, EnvManager.get_system_env(), Factory.create_pool(config)) archive = ( Path(__file__) @@ -142,7 +145,7 @@ def test_prepare_sdist(config: Config, config_cache_dir: Path) -> None: def test_prepare_directory(config: Config, config_cache_dir: Path): - chef = Chef(config, EnvManager.get_system_env()) + chef = Chef(config, EnvManager.get_system_env(), Factory.create_pool(config)) archive = Path(__file__).parent.parent.joinpath("fixtures/simple_project").resolve() @@ -155,7 +158,7 @@ def test_prepare_directory_with_extensions( config: Config, config_cache_dir: Path ) -> None: env = EnvManager.get_system_env() - chef = Chef(config, env) + chef = Chef(config, env, Factory.create_pool(config)) archive = ( Path(__file__) @@ -169,7 +172,7 @@ def test_prepare_directory_with_extensions( def test_prepare_directory_editable(config: Config, config_cache_dir: Path): - chef = Chef(config, EnvManager.get_system_env()) + chef = Chef(config, EnvManager.get_system_env(), Factory.create_pool(config)) archive = Path(__file__).parent.parent.joinpath("fixtures/simple_project").resolve() diff --git a/tests/installation/test_executor.py b/tests/installation/test_executor.py index b0e02443042..1f2a6c37dd1 100644 --- a/tests/installation/test_executor.py +++ b/tests/installation/test_executor.py @@ -202,7 +202,7 @@ def test_execute_executes_a_batch_of_operations( config.merge({"cache-dir": tmp_dir}) prepare_spy = mocker.spy(Chef, "_prepare") - chef = Chef(config, env) + chef = Chef(config, env, Factory.create_pool(config)) chef.set_directory_wheel([copy_wheel(), copy_wheel()]) chef.set_sdist_wheel(copy_wheel()) @@ -661,7 +661,7 @@ def test_executor_should_write_pep610_url_references_for_directories( "demo", "0.1.2", source_type="directory", source_url=url.as_posix() ) - chef = Chef(config, tmp_venv) + chef = Chef(config, tmp_venv, Factory.create_pool(config)) chef.set_directory_wheel(wheel) executor = Executor(tmp_venv, pool, config, io) @@ -692,7 +692,7 @@ def test_executor_should_write_pep610_url_references_for_editable_directories( develop=True, ) - chef = Chef(config, tmp_venv) + chef = Chef(config, tmp_venv, Factory.create_pool(config)) chef.set_directory_wheel(wheel) executor = Executor(tmp_venv, pool, config, io) @@ -756,7 +756,7 @@ def test_executor_should_write_pep610_url_references_for_git( source_url="https://github.com/demo/demo.git", ) - chef = Chef(config, tmp_venv) + chef = Chef(config, tmp_venv, Factory.create_pool(config)) chef.set_directory_wheel(wheel) executor = Executor(tmp_venv, pool, config, io) @@ -794,7 +794,7 @@ def test_executor_should_write_pep610_url_references_for_git_with_subdirectories source_subdirectory="two", ) - chef = Chef(config, tmp_venv) + chef = Chef(config, tmp_venv, Factory.create_pool(config)) chef.set_directory_wheel(wheel) executor = Executor(tmp_venv, pool, config, io) @@ -950,8 +950,6 @@ def test_build_backend_errors_are_reported_correctly_if_caused_by_subprocess( mock_file_downloads: None, env: MockEnv, ): - mocker.patch.object(Factory, "create_pool", return_value=pool) - error = BuildBackendException( CalledProcessError(1, ["pip"], output=b"Error on stdout") )