Skip to content

Commit

Permalink
installer: respect source if the same version of a package has been l…
Browse files Browse the repository at this point in the history
…ocked from different sources (#8304)

Co-authored-by: David Hotham <[email protected]>
  • Loading branch information
radoering and dimbleby authored Aug 17, 2023
1 parent 36332d2 commit eb74d62
Show file tree
Hide file tree
Showing 7 changed files with 226 additions and 36 deletions.
3 changes: 1 addition & 2 deletions src/poetry/console/commands/show.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,7 @@ def _display_packages_information(
from poetry.utils.helpers import get_package_version_display_string

locked_packages = locked_repository.packages
pool = RepositoryPool(ignore_repository_names=True, config=self.poetry.config)
pool.add_repository(locked_repository)
pool = RepositoryPool.from_packages(locked_packages, self.poetry.config)
solver = Solver(
root,
pool=pool,
Expand Down
12 changes: 2 additions & 10 deletions src/poetry/installation/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,16 +286,8 @@ def _do_install(self) -> int:
)

# We resolve again by only using the lock file
pool = RepositoryPool(ignore_repository_names=True, config=self._config)

# Making a new repo containing the packages
# newly resolved and the ones from the current lock file
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)

pool.add_repository(repo)
packages = lockfile_repo.packages + locked_repository.packages
pool = RepositoryPool.from_packages(packages, self._config)

solver = Solver(
root,
Expand Down
38 changes: 33 additions & 5 deletions src/poetry/repositories/repository_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from poetry.config.config import Config
from poetry.repositories.abstract_repository import AbstractRepository
from poetry.repositories.exceptions import PackageNotFound
from poetry.repositories.repository import Repository
from poetry.utils.cache import ArtifactCache


Expand All @@ -19,7 +20,7 @@
from poetry.core.packages.dependency import Dependency
from poetry.core.packages.package import Package

from poetry.repositories.repository import Repository
_SENTINEL = object()


class Priority(IntEnum):
Expand All @@ -42,13 +43,12 @@ class RepositoryPool(AbstractRepository):
def __init__(
self,
repositories: list[Repository] | None = None,
ignore_repository_names: bool = False,
ignore_repository_names: object = _SENTINEL,
*,
config: Config | None = None,
) -> None:
super().__init__("poetry-repository-pool")
self._repositories: OrderedDict[str, PrioritizedRepository] = OrderedDict()
self._ignore_repository_names = ignore_repository_names

if repositories is None:
repositories = []
Expand All @@ -59,6 +59,34 @@ def __init__(
cache_dir=(config or Config.create()).artifacts_cache_directory
)

if ignore_repository_names is not _SENTINEL:
warnings.warn(
"The 'ignore_repository_names' argument to 'RepositoryPool.__init__' is"
" deprecated. It has no effect anymore and will be removed in a future"
" version.",
DeprecationWarning,
stacklevel=2,
)

@staticmethod
def from_packages(packages: list[Package], config: Config | None) -> RepositoryPool:
pool = RepositoryPool(config=config)
for package in packages:
if package.is_direct_origin():
continue

repo_name = package.source_reference or "PyPI"
try:
repo = pool.repository(repo_name)
except IndexError:
repo = Repository(repo_name)
pool.add_repository(repo)

if not repo.has_package(package):
repo.add_package(package)

return pool

@property
def repositories(self) -> list[Repository]:
"""
Expand Down Expand Up @@ -166,7 +194,7 @@ def package(
extras: list[str] | None = None,
repository_name: str | None = None,
) -> Package:
if repository_name and not self._ignore_repository_names:
if repository_name:
return self.repository(repository_name).package(
name, version, extras=extras
)
Expand All @@ -180,7 +208,7 @@ def package(

def find_packages(self, dependency: Dependency) -> list[Package]:
repository_name = dependency.source_name
if repository_name and not self._ignore_repository_names:
if repository_name:
return self.repository(repository_name).find_packages(dependency)

packages: list[Package] = []
Expand Down
8 changes: 8 additions & 0 deletions src/poetry/utils/env/mock_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ class MockEnv(NullEnv):
def __init__(
self,
version_info: tuple[int, int, int] = (3, 7, 0),
*,
python_implementation: str = "CPython",
platform: str = "darwin",
platform_machine: str = "amd64",
os_name: str = "posix",
is_venv: bool = False,
pip_version: str = "19.1",
Expand All @@ -31,6 +33,7 @@ def __init__(
self._version_info = version_info
self._python_implementation = python_implementation
self._platform = platform
self._platform_machine = platform_machine
self._os_name = os_name
self._is_venv = is_venv
self._pip_version: Version = Version.parse(pip_version)
Expand All @@ -42,6 +45,10 @@ def __init__(
def platform(self) -> str:
return self._platform

@property
def platform_machine(self) -> str:
return self._platform_machine

@property
def os(self) -> str:
return self._os_name
Expand All @@ -67,6 +74,7 @@ def get_marker_env(self) -> dict[str, Any]:
marker_env["python_version"] = ".".join(str(v) for v in self._version_info[:2])
marker_env["python_full_version"] = ".".join(str(v) for v in self._version_info)
marker_env["sys_platform"] = self._platform
marker_env["platform_machine"] = self._platform_machine
marker_env["interpreter_name"] = self._python_implementation.lower()
marker_env["interpreter_version"] = "cp" + "".join(
str(v) for v in self._version_info[:2]
Expand Down
23 changes: 9 additions & 14 deletions tests/console/commands/test_add.py
Original file line number Diff line number Diff line change
Expand Up @@ -902,21 +902,16 @@ def test_add_constraint_with_source(
mocker: MockerFixture,
) -> None:
repo = LegacyRepository(name="my-index", url="https://my-index.fake")
repo.add_package(get_package("cachy", "0.2.0"))
mocker.patch.object(
repo,
"_find_packages",
wraps=lambda _, name: [
Package(
"cachy",
Version.parse("0.2.0"),
source_type="legacy",
source_reference=repo.name,
source_url=repo._url,
yanked=False,
)
],
package = Package(
"cachy",
Version.parse("0.2.0"),
source_type="legacy",
source_reference=repo.name,
source_url=repo._url,
yanked=False,
)
mocker.patch.object(repo, "package", return_value=package)
mocker.patch.object(repo, "_find_packages", wraps=lambda _, name: [package])

poetry.pool.add_repository(repo)

Expand Down
167 changes: 162 additions & 5 deletions tests/installation/test_installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2558,9 +2558,8 @@ def test_installer_should_use_the_locked_version_of_git_dependencies_without_ref
)


# https://github.com/python-poetry/poetry/issues/6710
@pytest.mark.parametrize("env_platform", ["darwin", "linux"])
def test_installer_distinguishes_locked_packages_by_source(
def test_installer_distinguishes_locked_packages_with_local_version_by_source(
pool: RepositoryPool,
locker: Locker,
installed: CustomInstalledRepository,
Expand All @@ -2569,6 +2568,7 @@ def test_installer_distinguishes_locked_packages_by_source(
package: ProjectPackage,
env_platform: str,
) -> None:
"""https://github.com/python-poetry/poetry/issues/6710"""
# Require 1.11.0+cpu from pytorch for most platforms, but specify 1.11.0 and pypi on
# darwin.
package.add_dependency(
Expand Down Expand Up @@ -2661,6 +2661,110 @@ def test_installer_distinguishes_locked_packages_by_source(
)


@pytest.mark.parametrize("env_platform_machine", ["aarch64", "amd64"])
def test_installer_distinguishes_locked_packages_with_same_version_by_source(
pool: RepositoryPool,
locker: Locker,
installed: CustomInstalledRepository,
config: Config,
repo: Repository,
package: ProjectPackage,
env_platform_machine: str,
) -> None:
"""https://github.com/python-poetry/poetry/issues/8303"""
package.add_dependency(
Factory.create_dependency(
"kivy",
{
"version": "2.2.1",
"markers": "platform_machine == 'aarch64'",
"source": "pywheels",
},
)
)
package.add_dependency(
Factory.create_dependency(
"kivy",
{
"version": "2.2.1",
"markers": "platform_machine != 'aarch64'",
"source": "PyPI",
},
)
)

# Locking finds both the pypi and the pyhweels packages.
locker.locked(True)
locker.mock_lock_data(
{
"package": [
{
"name": "kivy",
"version": "2.2.1",
"optional": False,
"files": [],
"python-versions": "*",
},
{
"name": "kivy",
"version": "2.2.1",
"optional": False,
"files": [],
"python-versions": "*",
"source": {
"type": "legacy",
"url": "https://www.piwheels.org/simple",
"reference": "pywheels",
},
},
],
"metadata": {
"python-versions": "*",
"platform": "*",
"content-hash": "123456789",
},
}
)
installer = Installer(
NullIO(),
MockEnv(platform_machine=env_platform_machine),
package,
locker,
pool,
config,
installed=installed,
executor=Executor(
MockEnv(platform_machine=env_platform_machine),
pool,
config,
NullIO(),
),
)
result = installer.run()
assert result == 0

# Results of installation are consistent with the platform requirements.
version = "2.2.1"
if env_platform_machine == "aarch64":
source_type = "legacy"
source_url = "https://www.piwheels.org/simple"
source_reference = "pywheels"
else:
source_type = None
source_url = None
source_reference = None

assert isinstance(installer.executor, Executor)
assert len(installer.executor.installations) == 1
assert installer.executor.installations[0] == Package(
"kivy",
version,
source_type=source_type,
source_url=source_url,
source_reference=source_reference,
)


@pytest.mark.parametrize("env_platform", ["darwin", "linux"])
def test_explicit_source_dependency_with_direct_origin_dependency(
pool: RepositoryPool,
Expand All @@ -2675,12 +2779,13 @@ def test_explicit_source_dependency_with_direct_origin_dependency(
A dependency with explicit source should not be satisfied by
a direct origin dependency even if there is a version match.
"""
demo_url = "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl"
package.add_dependency(
Factory.create_dependency(
"demo",
{
"markers": "sys_platform != 'darwin'",
"url": "https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl",
"url": demo_url,
},
)
)
Expand All @@ -2698,6 +2803,50 @@ def test_explicit_source_dependency_with_direct_origin_dependency(
repo.add_package(get_package("pendulum", "1.4.4"))
repo.add_package(get_package("demo", "0.1.0"))

# Locking finds both the direct origin and the explicit source packages.
locker.locked(True)
locker.mock_lock_data(
{
"package": [
{
"name": "demo",
"version": "0.1.0",
"optional": False,
"files": [],
"python-versions": "*",
"dependencies": {"pendulum": ">=1.4.4"},
"source": {
"type": "url",
"url": demo_url,
},
},
{
"name": "demo",
"version": "0.1.0",
"optional": False,
"files": [],
"python-versions": "*",
"source": {
"type": "legacy",
"url": "https://www.demo.org/simple",
"reference": "repo",
},
},
{
"name": "pendulum",
"version": "1.4.4",
"optional": False,
"files": [],
"python-versions": "*",
},
],
"metadata": {
"python-versions": "*",
"platform": "*",
"content-hash": "123456789",
},
}
)
installer = Installer(
NullIO(),
MockEnv(platform=env_platform),
Expand Down Expand Up @@ -2725,8 +2874,16 @@ def test_explicit_source_dependency_with_direct_origin_dependency(
"demo",
"0.1.0",
source_type="url",
source_url="https://python-poetry.org/distributions/demo-0.1.0-py2.py3-none-any.whl",
source_url=demo_url,
),
]
else:
assert installer.executor.installations == [Package("demo", "0.1.0")]
assert installer.executor.installations == [
Package(
"demo",
"0.1.0",
source_type="legacy",
source_url="https://www.demo.org/simple",
source_reference="repo",
)
]
Loading

0 comments on commit eb74d62

Please sign in to comment.