Final and prerelease control for package selection #13647
Conversation
notatallshaw
force-pushed
the
final-and-prerelease-control
branch
from
b0a12c8 to
5b4b0c1
Compare
notatallshaw
force-pushed
the
final-and-prerelease-control
branch
from
5b4b0c1 to
dbd03ab
Compare
|
Very nice - and very neeeded :) |
ichard26
left a comment
ichard26
left a comment
There was a problem hiding this comment.
First pass review. I'll need to take another look at this. Overall, logic looks good, but I'd like to take another look at the code.
| if finder.release_control is not None: | ||
| # Use ordered args to preserve the user's original command-line order | ||
| # This is important because later flags can override earlier ones | ||
| for attr_name, value in finder.release_control.get_ordered_args(): | ||
| args.extend(("--" + attr_name.replace("_", "-"), value)) | ||
|
|
There was a problem hiding this comment.
I can't wait until we eliminate the subprocess build env installer so we can just let this configuration be inherited through the finder for free.
| self.cmd_opts.add_option(cmdoptions.all_releases()) | ||
| self.cmd_opts.add_option(cmdoptions.only_final()) |
There was a problem hiding this comment.
As I noted in the --uploaded-prior-to PR, I believe it would be better if we grouped our command options more aggressively. Any command that accepts --no-binary (and friends) should also support --only-final and --all-releases for free.
This can be addressed as a follow up since the scope would be larger than any of these two PRs.
| """ | ||
| return self._order[:] | ||
|
|
||
| def allows_prereleases(self, canonical_name: str) -> bool | None: |
There was a problem hiding this comment.
| def allows_prereleases(self, canonical_name: str) -> bool | None: | |
| def allows_prereleases(self, canonical_name: NormalizedName) -> bool | None: |
If this API is going to break on unnormalized names, let's enforce that in the function signature.
| def handle_mutual_excludes( | ||
| self, value: str, target: set[str], other: set[str], attr_name: str | ||
| ) -> None: |
There was a problem hiding this comment.
Please add a docstring describing what this does. It's not too bad to grok manually, but some help can't hurt :)
| from pip._internal.exceptions import CommandError | ||
|
|
||
|
|
||
| class ReleaseControl: |
There was a problem hiding this comment.
Hmm, could we replace this with a dataclass? I don't think this object is performance sensitive enough slots is essential:
from dataclasses import dataclass, field
# TODO: add slots=True when Python 3.9 is dropped
@dataclass
class ReleaseControl:
"""Helper for managing which release types can be installed."""
all_releases: set[str] = field(default_factory=set)
only_final: set[str] = field(default_factory=set)
# Track the order of arguments as (attribute_name, value) tuples
# This is used to reconstruct arguments in the correct order for subprocesses
_order: list[tuple[str, str]] = field(
init=False, default_factory=list, compare=False, repr=False
)3 participants
Fixes #13221
This PR adds two new flags
--all-releasesand--only-final, they control package selection with the exact same semantics as--no-binaryand--only-binary, but for pre vs final releases instead of binary vs. source distributions.They are mutually exclusive with
--pre, as there are edge cases to do with the semantics of--preand these flags I didn't want to have to define. However, when only--preis used it is converted to the equivalent--all-releases :all:in the code, to keep the logic simple.The main feature I think users will be interested is only allowing final releases, to prevent pre-releases from being selected in any situation, e.g.