fix: don't merge sources into config (#7560)

b6fb3a39 · Randy Döring · GitHub · a3f6437b · b6fb3a39 · b6fb3a39
Commit b6fb3a39 authored Feb 27, 2023 by Randy Döring Committed by GitHub Feb 27, 2023
8 changed files
--- a/src/poetry/console/commands/source/add.py
+++ b/src/poetry/console/commands/source/add.py
@@ -83,13 +83,9 @@ class SourceAddCommand(Command):
            self.line(f"Adding source with name <c1>{name}</c1>.")
            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(

--- a/src/poetry/factory.py
+++ b/src/poetry/factory.py
@@ -23,6 +23,7 @@ from poetry.poetry import Poetry
 if TYPE_CHECKING:
+    from collections.abc import Iterable
    from pathlib import Path
    from cleo.io.io import IO
@@ -89,16 +90,14 @@ class Factory(BaseFactory):
            disable_cache,
        )
-        # Configuring sources
+        poetry.set_pool(
-        config.merge(
+            self.create_pool(
-            {
+                config,
-                "sources": {
+                poetry.local_config.get("source", []),
-                    source["name"]: source
+                io,
-                    for source in poetry.local_config.get("source", [])
+                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 @@ class Factory(BaseFactory):
    @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 @@ class Factory(BaseFactory):
        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)

--- a/src/poetry/installation/chef.py
+++ b/src/poetry/installation/chef.py
@@ -30,6 +30,7 @@ if TYPE_CHECKING:
    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 @@ class IsolatedEnv(BaseIsolatedEnv):
        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 @@ class IsolatedEnv(BaseIsolatedEnv):
            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 @@ class IsolatedEnv(BaseIsolatedEnv):
 class Chef:
-    def __init__(self, config: Config, env: Env) -> None:
+    def __init__(self, config: Config, env: Env, pool: RepositoryPool) -> None:
-        self._config = config
        self._env = env
+        self._pool = pool
        self._cache_dir = (
            Path(config.get("cache-dir")).expanduser().joinpath("artifacts")
        )
@@ -113,7 +112,7 @@ class Chef:
        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,

--- a/src/poetry/installation/executor.py
+++ b/src/poetry/installation/executor.py
@@ -79,7 +79,7 @@ class Executor:
        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)

--- a/tests/console/commands/self/conftest.py
+++ b/tests/console/commands/self/conftest.py
 from __future__ import annotations
 from typing import TYPE_CHECKING
+from typing import Any
 from typing import Callable
 import pytest
@@ -14,6 +15,8 @@ from poetry.utils.env import EnvManager
 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)

--- a/tests/console/commands/test_config.py
+++ b/tests/console/commands/test_config.py
@@ -24,6 +24,7 @@ if TYPE_CHECKING:
    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 @@ virtualenvs.prompt = "{{project_name}}-py{{python_version}}"
    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")

--- 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()

--- 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")
    )