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

Add support for controlling prerelease resolution. #350

Merged
merged 2 commits into from
Jan 23, 2017

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Jan 21, 2017

This change both allows control over whether prereleases are allowed
when resolving requirements and changes the default from allowing
prereleases to disallowing them - in line with pip behavior.

Support includes a pip-alike --pre cli option, support for --pre in
requirements.txt files, and API support for consumers of the pex
library, notably pants.

This closes #28.

This change both allows control over whether prereleases are allowed
when resolving requirements and changes the default from allowing
prereleases to disallowing them - in line with pip behavior.

Support includes a pip-alike `--pre` cli option, support for `--pre` in
requirements.txt files, and API support for consumers of the pex
library, notably pants.
@jsirois
Copy link
Member Author

jsirois commented Jan 21, 2017

I'm happy to update CHANGES.rst once we've setlled on a release version which would seem to depend on how the new allow_prereleases/--pre knob is rolled out. As-is the default is behavior changing, so I think we technically would need to release 2.0.0. If the default were changed to including pre-releases, we could release 1.2.0. Either way, I'm pretty sure we can't release as 1.1.21 per semver.

Copy link
Contributor

@kwlzn kwlzn left a comment

Choose a reason for hiding this comment

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

lgtm - thanks for the excellent test coverage!

# `specifiers` field which is a `SpecifierSet`, but we can't be sure we have that. We can be
# sure though we have the `Requirement.specs` list of `(op, version)` tuples in all versions of
# setuptools we depend upon (we use `Requirement.specs` elsewhere in the codebase as well).
# Kill this re-parsing once our lower-bound setuptools dependency give access to
Copy link
Contributor

Choose a reason for hiding this comment

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

if we do indeed bump to 2.x.x or 1.2.x, this might also be a good time to bump the lower setuptools bound too. afaik, it's completely arbitrary now that pants setuptools versions are modernized.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I'll take that up in a follow-up.

SETUPTOOLS_REQUIREMENT = 'setuptools>=5.7,<31.0'
PACKAGING_REQUIREMENT = 'packaging>=16.8'

# NB: We exlude 7.0b1 since it will use `packaging` if present and we install a modern version of
Copy link
Contributor

Choose a reason for hiding this comment

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

s/exlude/exclude/

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@kwlzn
Copy link
Contributor

kwlzn commented Jan 23, 2017

re: version bumps, I'm good with whatever you think is the most reasonable (2.x.x or 1.2.x). imo, the fact that we were resolving pre-release versions is sort of in the gray area of "bug" territory. ostensibly, we shouldn't have been doing that - so correcting that "bug" here and thus aligning with the spirit of the "correct" design for 1.x may be sufficient grounds for excepting this to a 1.2.x release (and explicitly noting the flag to restore the broken behavior) may be fine. I doubt anyone is relying on this behavior in practice - but may be wrong. ultimately, I had always imagined a 2.x would carry a much larger set of breaking changes, but either way not that big of a deal - will let you decide.

@jsirois
Copy link
Member Author

jsirois commented Jan 23, 2017

Yeah - I have the same sentiment re 2.x. Though the correct answer, it has perceived connotations that will be wrong. I'll follow up with the CHANGES.rst in a new PR.

@jsirois jsirois merged commit 7e8faa2 into pex-tool:master Jan 23, 2017
@jsirois jsirois deleted the jsirois/issues/28 branch January 23, 2017 20:07
@jsirois jsirois mentioned this pull request Jan 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter prerelease versions in package iterators by default
2 participants