every Repository is named (#5910)

* every Repository is named * assert repository not added to pool twice

every Repository is named (#5910)
* every Repository is named * assert repository not added to pool twice
c2c69a3c · David Hotham · GitHub · f97b3722 · c2c69a3c · c2c69a3c
Commit c2c69a3c authored Jul 13, 2022 by David Hotham Committed by GitHub Jul 13, 2022
17 changed files
--- a/src/poetry/console/commands/debug/resolve.py
+++ b/src/poetry/console/commands/debug/resolve.py
@@ -85,7 +85,7 @@ class DebugResolveCommand(InitCommand):
        pool = self.poetry.pool
-        solver = Solver(package, pool, Repository(), Repository(), self.io)
+        solver = Solver(package, pool, [], [], self.io)
        ops = solver.solve().calculate_operations()
@@ -98,13 +98,12 @@ class DebugResolveCommand(InitCommand):
            show_command.init_styles(self.io)
            packages = [op.package for op in ops]
-            repo = Repository(packages=packages)
            requires = package.all_requires
-            for pkg in repo.packages:
+            for pkg in packages:
                for require in requires:
                    if pkg.name == require.name:
-                        show_command.display_package_tree(self.io, pkg, repo)
+                        show_command.display_package_tree(self.io, pkg, packages)
                        break
            return 0
@@ -116,13 +115,13 @@ class DebugResolveCommand(InitCommand):
        if self.option("install"):
            env = EnvManager(self.poetry).get()
            pool = Pool()
-            locked_repository = Repository()
+            locked_repository = Repository("poetry-locked")
            for op in ops:
                locked_repository.add_package(op.package)
            pool.add_repository(locked_repository)
-            solver = Solver(package, pool, Repository(), Repository(), NullIO())
+            solver = Solver(package, pool, [], [], NullIO())
            with solver.use_environment(env):
                ops = solver.solve().calculate_operations()

--- a/src/poetry/console/commands/show.py
+++ b/src/poetry/console/commands/show.py
@@ -15,7 +15,6 @@ if TYPE_CHECKING:
    from poetry.core.packages.package import Package
    from poetry.core.packages.project_package import ProjectPackage
-    from poetry.repositories.installed_repository import InstalledRepository
    from poetry.repositories.repository import Repository
@@ -74,7 +73,6 @@ lists all packages available."""
        from poetry.puzzle.solver import Solver
        from poetry.repositories.installed_repository import InstalledRepository
        from poetry.repositories.pool import Pool
-        from poetry.repositories.repository import Repository
        from poetry.utils.helpers import get_package_version_display_string
        package = self.argument("package")
@@ -119,7 +117,7 @@ lists all packages available."""
            for p in packages:
                for require in requires:
                    if p.name == require.name:
-                        self.display_package_tree(self.io, p, locked_repo)
+                        self.display_package_tree(self.io, p, packages)
                        break
            return 0
@@ -131,8 +129,8 @@ lists all packages available."""
        solver = Solver(
            root,
            pool=pool,
-            installed=Repository(),
+            installed=[],
-            locked=locked_repo,
+            locked=locked_packages,
            io=NullIO(),
        )
        solver.provider.load_deferred(False)
@@ -174,11 +172,11 @@ lists all packages available."""
                    for p in packages:
                        self.display_package_tree(
-                            self.io, p, locked_repo, why_package=pkg
+                            self.io, p, locked_packages, why_package=pkg
                        )
                else:
-                    self.display_package_tree(self.io, pkg, locked_repo)
+                    self.display_package_tree(self.io, pkg, locked_packages)
                return 0
@@ -224,7 +222,9 @@ lists all packages available."""
            current_length = len(locked.pretty_name)
            if not self.io.output.is_decorated():
-                installed_status = self.get_installed_status(locked, installed_repo)
+                installed_status = self.get_installed_status(
+                    locked, installed_repo.packages
+                )
                if installed_status == "not-installed":
                    current_length += 4
@@ -300,7 +300,9 @@ lists all packages available."""
                color = "black;options=bold"
            else:
-                installed_status = self.get_installed_status(locked, installed_repo)
+                installed_status = self.get_installed_status(
+                    locked, installed_repo.packages
+                )
                if installed_status == "not-installed":
                    color = "red"
@@ -371,7 +373,7 @@ lists all packages available."""
        self,
        io: IO,
        package: Package,
-        installed_repo: Repository,
+        installed_packages: list[Package],
        why_package: Package | None = None,
    ) -> None:
        io.write(f"<c1>{package.pretty_name}</c1>")
@@ -408,14 +410,19 @@ lists all packages available."""
            packages_in_tree = [package.name, dependency.name]
            self._display_tree(
-                io, dependency, installed_repo, packages_in_tree, tree_bar, level + 1
+                io,
+                dependency,
+                installed_packages,
+                packages_in_tree,
+                tree_bar,
+                level + 1,
            )
    def _display_tree(
        self,
        io: IO,
        dependency: Dependency,
-        installed_repo: Repository,
+        installed_packages: list[Package],
        packages_in_tree: list[str],
        previous_tree_bar: str = "├",
        level: int = 1,
@@ -423,7 +430,7 @@ lists all packages available."""
        previous_tree_bar = previous_tree_bar.replace("├", "│")
        dependencies = []
-        for package in installed_repo.packages:
+        for package in installed_packages:
            if package.name == dependency.name:
                dependencies = package.requires
@@ -459,7 +466,12 @@ lists all packages available."""
                current_tree.append(dependency.name)
                self._display_tree(
-                    io, dependency, installed_repo, current_tree, tree_bar, level + 1
+                    io,
+                    dependency,
+                    installed_packages,
+                    current_tree,
+                    tree_bar,
+                    level + 1,
                )
    def _write_tree_line(self, io: IO, line: str) -> None:
@@ -517,9 +529,9 @@ lists all packages available."""
        return "update-possible"
    def get_installed_status(
-        self, locked: Package, installed_repo: InstalledRepository
+        self, locked: Package, installed_packages: list[Package]
    ) -> str:
-        for package in installed_repo.packages:
+        for package in installed_packages:
            if locked.name == package.name:
                return "installed"

--- a/src/poetry/installation/installer.py
+++ b/src/poetry/installation/installer.py
@@ -193,8 +193,8 @@ class Installer:
        solver = Solver(
            self._package,
            self._pool,
-            locked_repository,
+            locked_repository.packages,
-            locked_repository,
+            locked_repository.packages,
            self._io,
        )
@@ -213,7 +213,7 @@ class Installer:
    def _do_install(self) -> int:
        from poetry.puzzle.solver import Solver
-        locked_repository = Repository()
+        locked_repository = Repository("poetry-locked")
        if self._update:
            if self._locker.is_locked() and not self._lock:
                locked_repository = self._locker.locked_repository()
@@ -233,8 +233,8 @@ class Installer:
            solver = Solver(
                self._package,
                self._pool,
-                self._installed_repository,
+                self._installed_repository.packages,
-                locked_repository,
+                locked_repository.packages,
                self._io,
            )
@@ -291,7 +291,7 @@ class Installer:
        # Making a new repo containing the packages
        # newly resolved and the ones from the current lock file
-        repo = Repository()
+        repo = Repository("poetry-repo")
        for package in lockfile_repo.packages + locked_repository.packages:
            if not package.is_direct_origin() and not repo.has_package(package):
                repo.add_package(package)
@@ -299,7 +299,11 @@ class Installer:
        pool.add_repository(repo)
        solver = Solver(
-            root, pool, self._installed_repository, locked_repository, NullIO()
+            root,
+            pool,
+            self._installed_repository.packages,
+            locked_repository.packages,
+            NullIO(),
        )
        # Everything is resolved at this point, so we no longer need
        # to load deferred dependencies (i.e. VCS, URL and path dependencies)

--- a/src/poetry/packages/locker.py
+++ b/src/poetry/packages/locker.py
@@ -102,10 +102,10 @@ class Locker:
        from poetry.repositories import Repository
        if not self.is_locked():
-            return Repository()
+            return Repository("poetry-locked")
        lock_data = self.lock_data
-        packages = Repository()
+        packages = Repository("poetry-locked")
        locked_packages = cast("list[dict[str, Any]]", lock_data["package"])
        if not locked_packages:

--- a/src/poetry/puzzle/provider.py
+++ b/src/poetry/puzzle/provider.py
@@ -53,7 +53,6 @@ if TYPE_CHECKING:
    from poetry.core.version.markers import BaseMarker
    from poetry.repositories import Pool
-    from poetry.repositories import Repository
    from poetry.utils.env import Env
@@ -127,7 +126,7 @@ class Provider:
        pool: Pool,
        io: IO,
        env: Env | None = None,
-        installed: Repository | None = None,
+        installed: list[Package] | None = None,
    ) -> None:
        self._package = package
        self._pool = pool
@@ -140,7 +139,7 @@ class Provider:
        self._deferred_cache: dict[Dependency, Package] = {}
        self._load_deferred = True
        self._source_root: Path | None = None
-        self._installed = installed
+        self._installed_packages = installed if installed is not None else []
    @property
    def pool(self) -> Pool:
@@ -201,7 +200,7 @@ class Provider:
        This is useful when dealing with packages that are under development, not
        published on package sources and/or only available via system installations.
        """
-        if not self._installed:
+        if not self._installed_packages:
            return []
        logger.debug(
@@ -210,7 +209,7 @@ class Provider:
        )
        packages = [
            package
-            for package in self._installed.packages
+            for package in self._installed_packages
            if package.provides(specification)
        ]
        logger.debug(

--- a/src/poetry/puzzle/solver.py
+++ b/src/poetry/puzzle/solver.py
@@ -29,7 +29,6 @@ if TYPE_CHECKING:
    from poetry.puzzle.transaction import Transaction
    from poetry.repositories import Pool
-    from poetry.repositories import Repository
    from poetry.utils.env import Env
@@ -38,15 +37,15 @@ class Solver:
        self,
        package: ProjectPackage,
        pool: Pool,
-        installed: Repository,
+        installed: list[Package],
-        locked: Repository,
+        locked: list[Package],
        io: IO,
        provider: Provider | None = None,
    ) -> None:
        self._package = package
        self._pool = pool
-        self._installed = installed
+        self._installed_packages = installed
-        self._locked = locked
+        self._locked_packages = locked
        self._io = io
        if provider is None:
@@ -85,9 +84,9 @@ class Solver:
                )
        return Transaction(
-            self._locked.packages,
+            self._locked_packages,
            list(zip(packages, depths)),
-            installed_packages=self._installed.packages,
+            installed_packages=self._installed_packages,
            root_package=self._package,
        )
@@ -128,7 +127,7 @@ class Solver:
            self._overrides.append(self._provider._overrides)
        locked: dict[str, list[DependencyPackage]] = defaultdict(list)
-        for package in self._locked.packages:
+        for package in self._locked_packages:
            locked[package.name].append(
                DependencyPackage(package.to_dependency(), package)
            )

--- a/src/poetry/repositories/installed_repository.py
+++ b/src/poetry/repositories/installed_repository.py
@@ -33,6 +33,9 @@ logger = logging.getLogger(__name__)
 class InstalledRepository(Repository):
+    def __init__(self) -> None:
+        super().__init__("poetry-installed")
    @classmethod
    def get_package_paths(cls, env: Env, name: str) -> set[Path]:
        """

--- a/src/poetry/repositories/lockfile_repository.py
+++ b/src/poetry/repositories/lockfile_repository.py
@@ -15,6 +15,9 @@ class LockfileRepository(Repository):
    but also by source type, url, etc.
    """
+    def __init__(self) -> None:
+        super().__init__("poetry-lockfile")
    def has_package(self, package: Package) -> bool:
        return any(p == package for p in self.packages)

--- a/src/poetry/repositories/pool.py
+++ b/src/poetry/repositories/pool.py
@@ -17,12 +17,12 @@ class Pool(Repository):
        repositories: list[Repository] | None = None,
        ignore_repository_names: bool = False,
    ) -> None:
-        super().__init__()
+        super().__init__("poetry-pool")
        if repositories is None:
            repositories = []
-        self._lookup: dict[str | None, int] = {}
+        self._lookup: dict[str, int] = {}
        self._repositories: list[Repository] = []
        self._default = False
        self._has_primary_repositories = False
@@ -33,8 +33,6 @@ class Pool(Repository):
        self._ignore_repository_names = ignore_repository_names
-        super().__init__()
    @property
    def repositories(self) -> list[Repository]:
        return self._repositories
@@ -49,11 +47,11 @@ class Pool(Repository):
        return name.lower() in self._lookup
    def repository(self, name: str) -> Repository:
-        if name is not None:
        name = name.lower()
-        if name in self._lookup:
+        lookup = self._lookup.get(name)
-            return self._repositories[self._lookup[name]]
+        if lookup is not None:
+            return self._repositories[lookup]
        raise ValueError(f'Repository "{name}" does not exist.')
@@ -63,11 +61,10 @@ class Pool(Repository):
        """
        Adds a repository to the pool.
        """
-        # FIXME: surely it's a problem that the repository name can be None here?
+        repository_name = repository.name.lower()
-        # All nameless repositories will collide in self._lookup.
+        if repository_name in self._lookup:
-        repository_name = (
+            raise ValueError(f"{repository_name} already added")
-            repository.name.lower() if repository.name is not None else None
-        )
        if default:
            if self.has_default():
                raise ValueError("Only one repository can be the default")

--- a/src/poetry/repositories/repository.py
+++ b/src/poetry/repositories/repository.py
@@ -18,9 +18,7 @@ if TYPE_CHECKING:
 class Repository:
-    def __init__(
+    def __init__(self, name: str, packages: list[Package] | None = None) -> None:
-        self, name: str | None = None, packages: list[Package] | None = None
-    ) -> None:
        self._name = name
        self._packages: list[Package] = []
@@ -28,7 +26,7 @@ class Repository:
            self.add_package(package)
    @property
-    def name(self) -> str | None:
+    def name(self) -> str:
        return self._name
    @property

--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -333,7 +333,7 @@ def tmp_venv(tmp_dir: str) -> Iterator[VirtualEnv]:
 @pytest.fixture
 def installed() -> Repository:
-    return Repository()
+    return Repository("installed")
 @pytest.fixture(scope="session")

--- a/tests/installation/test_installer.py
+++ b/tests/installation/test_installer.py
@@ -152,7 +152,7 @@ def package() -> ProjectPackage:
 @pytest.fixture()
 def repo() -> Repository:
-    return Repository()
+    return Repository("repo")
 @pytest.fixture()

--- a/tests/installation/test_installer_old.py
+++ b/tests/installation/test_installer_old.py
@@ -105,7 +105,7 @@ def package() -> ProjectPackage:
 @pytest.fixture()
 def repo() -> Repository:
-    return Repository()
+    return Repository("repo")
 @pytest.fixture()

--- a/tests/mixology/version_solver/conftest.py
+++ b/tests/mixology/version_solver/conftest.py
@@ -24,7 +24,7 @@ class Provider(BaseProvider):
 @pytest.fixture
 def repo() -> Repository:
-    return Repository()
+    return Repository("repo")
 @pytest.fixture

--- a/tests/puzzle/test_provider.py
+++ b/tests/puzzle/test_provider.py
@@ -39,7 +39,7 @@ def root() -> ProjectPackage:
 @pytest.fixture
 def repository() -> Repository:
-    return Repository()
+    return Repository("repo")
 @pytest.fixture

--- a/tests/puzzle/test_solver.py
+++ b/tests/puzzle/test_solver.py
@@ -65,12 +65,12 @@ def installed() -> InstalledRepository:
 @pytest.fixture()
 def locked() -> Repository:
-    return Repository()
+    return Repository("locked")
 @pytest.fixture()
 def repo() -> Repository:
-    return Repository()
+    return Repository("repo")
 @pytest.fixture()
@@ -89,10 +89,10 @@ def solver(
    return Solver(
        package,
        pool,
-        installed,
+        installed.packages,
-        locked,
+        locked.packages,
        io,
-        provider=Provider(package, pool, io, installed=installed),
+        provider=Provider(package, pool, io, installed=installed.packages),
    )
@@ -2019,7 +2019,7 @@ def test_solver_does_not_raise_conflict_for_locked_conditional_dependencies(
    repo.add_package(package_a)
    repo.add_package(package_b)
-    solver._locked = Repository([package_a])
+    solver._locked = Repository("locked", [package_a])
    transaction = solver.solve(use_latest=[package_b.name])
    check_solver_result(
@@ -2298,7 +2298,12 @@ def test_solver_can_resolve_directory_dependencies_nested_editable(
    package = poetry.package
    solver = Solver(
-        package, pool, installed, locked, io, provider=Provider(package, pool, io)
+        package,
+        pool,
+        installed.packages,
+        locked.packages,
+        io,
+        provider=Provider(package, pool, io),
    )
    transaction = solver.solve()
@@ -2539,7 +2544,7 @@ def test_solver_can_solve_with_legacy_repository_using_proper_dists(
    repo = MockLegacyRepository()
    pool = Pool([repo])
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    package.add_dependency(Factory.create_dependency("isort", "4.3.4"))
@@ -2586,7 +2591,7 @@ def test_solver_can_solve_with_legacy_repository_using_proper_python_compatible_
    repo = MockLegacyRepository()
    pool = Pool([repo])
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    package.add_dependency(Factory.create_dependency("isort", "4.3.4"))
@@ -2620,7 +2625,7 @@ def test_solver_skips_invalid_versions(
    repo = MockPyPIRepository()
    pool = Pool([repo])
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    package.add_dependency(Factory.create_dependency("trackpy", "^0.4"))
@@ -2667,7 +2672,7 @@ def test_solver_chooses_most_recent_version_amongst_repositories(
    repo = MockLegacyRepository()
    pool = Pool([repo, MockPyPIRepository()])
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    transaction = solver.solve()
@@ -2693,7 +2698,7 @@ def test_solver_chooses_from_correct_repository_if_forced(
    repo = MockLegacyRepository()
    pool = Pool([repo, MockPyPIRepository()])
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    transaction = solver.solve()
@@ -2728,13 +2733,13 @@ def test_solver_chooses_from_correct_repository_if_forced_and_transitive_depende
        Factory.create_dependency("tomlkit", {"version": "^0.5", "source": "legacy"})
    )
-    repo = Repository()
+    repo = Repository("repo")
    foo = get_package("foo", "1.0.0")
    foo.add_dependency(Factory.create_dependency("tomlkit", "^0.5.0"))
    repo.add_package(foo)
    pool = Pool([MockLegacyRepository(), repo, MockPyPIRepository()])
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    transaction = solver.solve()
@@ -2774,7 +2779,7 @@ def test_solver_does_not_choose_from_secondary_repository_by_default(
    pool.add_repository(MockPyPIRepository(), secondary=True)
    pool.add_repository(MockLegacyRepository())
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    transaction = solver.solve()
@@ -2826,7 +2831,7 @@ def test_solver_chooses_from_secondary_if_explicit(
    pool.add_repository(MockPyPIRepository(), secondary=True)
    pool.add_repository(MockLegacyRepository())
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    transaction = solver.solve()
@@ -2883,7 +2888,7 @@ def test_solver_discards_packages_with_empty_markers(
    repo.add_package(package_b)
    repo.add_package(package_c)
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    transaction = solver.solve()
@@ -2989,7 +2994,7 @@ def test_solver_does_not_fail_with_locked_git_and_non_git_dependencies(
    repo.add_package(get_package("a", "1.2.3"))
    repo.add_package(Package("pendulum", "2.1.2"))
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    transaction = solver.solve()
@@ -3067,7 +3072,7 @@ def test_solver_synchronize_single(
    locked: Repository,
    io: NullIO,
 ):
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    package_a = get_package("a", "1.0")
    installed.add_package(package_a)
@@ -3086,7 +3091,7 @@ def test_solver_with_synchronization_keeps_critical_package(
    locked: Repository,
    io: NullIO,
 ):
-    solver = Solver(package, pool, installed, locked, io)
+    solver = Solver(package, pool, installed.packages, locked.packages, io)
    package_pip = get_package("setuptools", "1.0")
    installed.add_package(package_pip)

--- a/tests/repositories/test_pool.py
+++ b/tests/repositories/test_pool.py
@@ -10,7 +10,7 @@ from poetry.repositories.legacy_repository import LegacyRepository
 def test_pool_raises_package_not_found_when_no_package_is_found():
    pool = Pool()
-    pool.add_repository(Repository())
+    pool.add_repository(Repository("repo"))
    with pytest.raises(PackageNotFound):
        pool.package("foo", "1.0.0")
@@ -24,7 +24,7 @@ def test_pool():
 def test_pool_with_initial_repositories():
-    repo = Repository()
+    repo = Repository("repo")
    pool = Pool([repo])
    assert len(pool.repositories) == 1