Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pex --only-binary X --lock .... #2433

Merged
merged 1 commit into from
Jun 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Release Notes

## 2.4.1

This release fixes `pex --only-binary X --lock ...` to work with lock
files also created with `--only-binary X`. The known case here is a
`--style universal` lock created with `--only-binary X` to achieve a
partially wheel-only universal lock.

* Fix `pex --only-binary X --lock ...`. (#2433)

## 2.4.0

This release brings new support for preserving arguments passed to the
Expand Down
6 changes: 5 additions & 1 deletion pex/resolve/lock_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,13 @@ def __init__(
self._network_configuration = network_configuration
self._password_entries = password_entries
self._cache = cache

# Since a VCSArtifactDownloadManager is only used for VCS requirements, a build is both
# required and preferred by the user.
self._build_configuration = attr.evolve(
build_configuration, allow_wheels=False, allow_builds=True, prefer_older_binary=False
build_configuration, allow_builds=True, prefer_older_binary=False
Copy link
Collaborator

@huonw huonw Jun 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was initially confused why changes to VCSArtifactDownloadManager would improve the behaviour for what was tested. That test seems to only use PyPI requirements, not VCS ones.

My understanding is that this error happens eagerly, on construction of this BuildConfiguration object, and so when a VCSArtifactDownloadManager is constructed, it'll eagerly do the checking and throw the error here. And, at least one VCSArtifactDownloadManager is constructed for every resolve-from-lock (one for each subset), thus, failures even when no VCS requirements are involved.

Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

)

self._pip_version = pip_version
self._resolver = resolver
self._use_pip_config = use_pip_config
Expand Down
2 changes: 1 addition & 1 deletion pex/version.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2015 Pex project contributors.
# Licensed under the Apache License, Version 2.0 (see LICENSE).

__version__ = "2.4.0"
__version__ = "2.4.1"
85 changes: 85 additions & 0 deletions tests/integration/test_issue_2432.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# Copyright 2024 Pex project contributors.
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import os.path
from textwrap import dedent

from pex.compatibility import commonpath
from pex.targets import LocalInterpreter
from pex.typing import TYPE_CHECKING
from testing import run_pex_command
from testing.cli import run_pex3

if TYPE_CHECKING:
from typing import Any, Iterable, Optional


def test_lock_use_no_build_wheel(tmpdir):
# type: (Any)-> None

lock = os.path.join(str(tmpdir), "lock")
run_pex3(
"lock",
"create",
"ansicolors==1.1.8",
"--only-binary",
"ansicolors",
"-o",
lock,
"--style",
"universal",
"--indent",
"2",
).assert_success()

def assert_pex_from_lock(
extra_args=(), # type: Iterable[str]
expected_error=None, # type: Optional[str]
):
# type: (...) -> None

pex_root = os.path.realpath(os.path.join(str(tmpdir), "pex_root"))
args = [
"--pex-root",
pex_root,
"--runtime-pex-root",
pex_root,
"--lock",
lock,
"--",
"-c",
"import colors, os; print(os.path.realpath(colors.__file__))",
]
result = run_pex_command(args=list(extra_args) + args)
if expected_error:
result.assert_failure()
assert expected_error in result.error
else:
result.assert_success()
assert pex_root == commonpath((pex_root, result.output.strip()))

assert_pex_from_lock()

# A redundant --only-binary should not matter.
assert_pex_from_lock(extra_args=["--only-binary", "ansicolors"])

# An extraneous --only-build should not matter.
assert_pex_from_lock(extra_args=["--only-build", "cowsay"])

# However; a conflicting --only-build should matter.
assert_pex_from_lock(
extra_args=["--only-build", "ansicolors"],
expected_error=dedent(
"""\
Failed to resolve all requirements for {target_description} from {lock}:

Configured with:
build: True
only_build: ansicolors
use_wheel: True

Dependency on ansicolors not satisfied, 1 incompatible candidate found:
1.) ansicolors 1.1.8 (via: ansicolors==1.1.8) does not have any compatible artifacts:
"""
).format(lock=lock, target_description=LocalInterpreter.create().render_description()),
)