Add PyPI registry correctly to pool depending on other sources (#3406)

In the event where we defined sources that were set as
secondary = True, we would end up with PyPI being after this source
when it should have acted as default in that case.

The main issue stems from the fact that it's not because you have
sources configured that PyPI should not be a default. Instead, PyPI
should be default if there are no sources with secondary = False and
not default if there are sources with secondary = True.

poetry/factory.py

@@ -153,14 +153,15 @@ class Factory(BaseFactory):
 
 
             poetry.pool.add_repository(repository, is_default, secondary=is_secondary)
             poetry.pool.add_repository(repository, is_default, secondary=is_secondary)
 
 
-        # Always put PyPI last to prefer private repositories
-        # but only if we have no other default source
-        if not poetry.pool.has_default():
-            has_sources = bool(sources)
-            poetry.pool.add_repository(PyPiRepository(), not has_sources, has_sources)
-        else:
+        # Put PyPI last to prefer private repositories
+        # unless we have no default source AND no primary sources
+        # (default = false, secondary = false)
+        if poetry.pool.has_default():
             if io.is_debug():
             if io.is_debug():
                 io.write_line("Deactivating the PyPI repository")
                 io.write_line("Deactivating the PyPI repository")
+        else:
+            default = not poetry.pool.has_primary_repositories()
+            poetry.pool.add_repository(PyPiRepository(), default, not default)
 
 
     @classmethod
     @classmethod
     def create_legacy_repository(
     def create_legacy_repository(

poetry/repositories/pool.py

@@ -25,6 +25,7 @@ class Pool(BaseRepository):
         self._lookup: Dict[str, int] = {}
         self._lookup: Dict[str, int] = {}
         self._repositories: List[Repository] = []
         self._repositories: List[Repository] = []
         self._default = False
         self._default = False
+        self._has_primary_repositories = False
         self._secondary_start_idx = None
         self._secondary_start_idx = None
 
 
         for repository in repositories:
         for repository in repositories:
@@ -41,6 +42,9 @@ class Pool(BaseRepository):
     def has_default(self) -> bool:
     def has_default(self) -> bool:
         return self._default
         return self._default
 
 
+    def has_primary_repositories(self) -> bool:
+        return self._has_primary_repositories
+
     def has_repository(self, name: str) -> bool:
     def has_repository(self, name: str) -> bool:
         name = name.lower() if name is not None else None
         name = name.lower() if name is not None else None
 
 
@@ -84,6 +88,7 @@ class Pool(BaseRepository):
             self._repositories.append(repository)
             self._repositories.append(repository)
             self._lookup[repository_name] = len(self._repositories) - 1
             self._lookup[repository_name] = len(self._repositories) - 1
         else:
         else:
+            self._has_primary_repositories = True
             if self._secondary_start_idx is None:
             if self._secondary_start_idx is None:
                 self._repositories.append(repository)
                 self._repositories.append(repository)
                 self._lookup[repository_name] = len(self._repositories) - 1
                 self._lookup[repository_name] = len(self._repositories) - 1

tests/fixtures/with_non_default_multiple_secondary_sources/pyproject.toml

+[tool.poetry]
+name = "my-package"
+version = "1.2.3"
+description = "Some description."
+authors = [
+    "Your Name <you@example.com>"
+]
+license = "MIT"
+
+# Requirements
+[tool.poetry.dependencies]
+python = "~2.7 || ^3.6"
+
+[tool.poetry.dev-dependencies]
+
+[[tool.poetry.source]]
+name = "foo"
+url = "https://foo.bar/simple/"
+secondary = true
+
+[[tool.poetry.source]]
+name = "bar"
+url = "https://bar.baz/simple/"
+secondary = true

tests/fixtures/with_non_default_multiple_sources/pyproject.toml

+[tool.poetry]
+name = "my-package"
+version = "1.2.3"
+description = "Some description."
+authors = [
+    "Your Name <you@example.com>"
+]
+license = "MIT"
+
+# Requirements
+[tool.poetry.dependencies]
+python = "~2.7 || ^3.6"
+
+[tool.poetry.dev-dependencies]
+
+[[tool.poetry.source]]
+name = "foo"
+url = "https://foo.bar/simple/"
+secondary = true
+
+[[tool.poetry.source]]
+name = "bar"
+url = "https://bar.baz/simple/"

tests/fixtures/with_non_default_secondary_source/pyproject.toml

+[tool.poetry]
+name = "my-package"
+version = "1.2.3"
+description = "Some description."
+authors = [
+    "Your Name <you@example.com>"
+]
+license = "MIT"
+
+# Requirements
+[tool.poetry.dependencies]
+python = "~2.7 || ^3.6"
+
+[tool.poetry.dev-dependencies]
+
+[[tool.poetry.source]]
+name = "foo"
+url = "https://foo.bar/simple/"
+secondary = true

tests/test_factory.py

@@ -176,6 +176,64 @@ def test_poetry_with_non_default_source():
     assert isinstance(poetry.pool.repositories[1], PyPiRepository)
     assert isinstance(poetry.pool.repositories[1], PyPiRepository)
 
 
 
 
+def test_poetry_with_non_default_secondary_source():
+    poetry = Factory().create_poetry(fixtures_dir / "with_non_default_secondary_source")
+
+    assert len(poetry.pool.repositories) == 2
+
+    assert poetry.pool.has_default()
+
+    repository = poetry.pool.repositories[0]
+    assert repository.name == "PyPI"
+    assert isinstance(repository, PyPiRepository)
+
+    repository = poetry.pool.repositories[1]
+    assert repository.name == "foo"
+    assert isinstance(repository, LegacyRepository)
+
+
+def test_poetry_with_non_default_multiple_secondary_sources():
+    poetry = Factory().create_poetry(
+        fixtures_dir / "with_non_default_multiple_secondary_sources"
+    )
+
+    assert len(poetry.pool.repositories) == 3
+
+    assert poetry.pool.has_default()
+
+    repository = poetry.pool.repositories[0]
+    assert repository.name == "PyPI"
+    assert isinstance(repository, PyPiRepository)
+
+    repository = poetry.pool.repositories[1]
+    assert repository.name == "foo"
+    assert isinstance(repository, LegacyRepository)
+
+    repository = poetry.pool.repositories[2]
+    assert repository.name == "bar"
+    assert isinstance(repository, LegacyRepository)
+
+
+def test_poetry_with_non_default_multiple_sources():
+    poetry = Factory().create_poetry(fixtures_dir / "with_non_default_multiple_sources")
+
+    assert len(poetry.pool.repositories) == 3
+
+    assert not poetry.pool.has_default()
+
+    repository = poetry.pool.repositories[0]
+    assert repository.name == "bar"
+    assert isinstance(repository, LegacyRepository)
+
+    repository = poetry.pool.repositories[1]
+    assert repository.name == "foo"
+    assert isinstance(repository, LegacyRepository)
+
+    repository = poetry.pool.repositories[2]
+    assert repository.name == "PyPI"
+    assert isinstance(repository, PyPiRepository)
+
+
 def test_poetry_with_no_default_source():
 def test_poetry_with_no_default_source():
     poetry = Factory().create_poetry(fixtures_dir / "sample_project")
     poetry = Factory().create_poetry(fixtures_dir / "sample_project")