bitprophet / releases

A powerful Sphinx changelog-generating extension.
http://releases.readthedocs.io/
BSD 2-Clause "Simplified" License
176 stars 41 forks source link

Fix compatibility with >=python-semanticversion-2.7.0 #86

Open rbarrois opened 5 years ago

rbarrois commented 5 years ago

Context

The recent versions of python-semanticversion made changes to private APIs, removing the interaction between Version(x, partial=True) and Spec() (partial=True was designed for implementing the Spec class only).

This breaks releases, has mentioned in #84 and #85; it has also been reported as affecting twine's doc building (https://github.com/pypa/twine/issues/492).

The attached patch switches to the recommended usage of python-semanticversion (whose docs are still lacking in that regard, sorry :disappointed:), while keeping all tests green - both with updated and old python-semanticversion releases.

Implementation notes

The code used Version(..., partial=True) to exclude ranges of version whose major component didn't match a bugfix/issue range; the code went akin to:

Version('1', partial=True) in Spec('>=1.0')

This no longer works; this patch changes that behaviour to exclude families where no actual release matches the bugfix/issue range - this should be more accurate.

The patch also uses Version.coerce, the intended API to manage non semver-compliant version strings.

Compatibility

The patch has been tested with both python-semanticversion==2.6.0 and python-semanticversion==2.8.1; it can be included to an upgraded version of releases even if users haven't yet upgraded python-semanticversion.

rbarrois commented 5 years ago

The two failing tests seem to me to be a travis-side issue: I haven't been able to reproduce them locally with the same Sphinx and Python version.

Travis reports an issue with a sphinx build exiting with an unexpected error code, but the trace mentions that the build has succeeded and no error message...

jaraco commented 4 years ago

I've confirmed this change fixes releases where it currently fails. Please find a way to release this change.

rixx commented 4 years ago

Any news on this PR? It would be nice to be able to unpin python-semanticversion.

bitprophet commented 4 years ago

I'm actually leaning towards just pinning our requirements at semantic-version<2.7 (re #84), since they've now proven a tendency to break APIs on minor versions.

Do you have needs for features added in semantic-version in 2.7 or 2.8? To my knowledge Releases itself does not, so I'd rather punt until we do.

rbarrois commented 4 years ago

@bitprophet As the maintainer of python-semanticversion, I feel a bit hurt by the wording of your decision — derivating a "tendency" from a single change to a private, non-documented API seems a bit harsh.

Since releases is a release management tool, pinning the version on your side forces that decision onto users of releases too; they'll have to choose between:

How can we avoid this blocking situation? Would you be interested by a further pull request that would either vendor python-semanticversion, or simply include a minimal subset of its functionalities?

bitprophet commented 4 years ago

@rbarrois That's fair, "tendency" was poor word choice on my part, I should have put something less implying a string of events - apologies.

I'm confused by the assertion that the API call is private though - all our subclass did was call Version with partial=True and that seems to be part of the public API. Am I overlooking something? I definitely didn't have a mental note that we were using its undocumented innards (like I do have for all of our Sphinx interactions...).

EDIT after reading the PR more and discovering the commit message especially: got it. partial I guess was undocumented previously?


Re: the rest: I'm well aware of the tradeoffs involved in pinning, sadly. It's never ideal but especially with limited dev time it's often a very tempting band-aid.

That said, since I last visited this ticket, I've decided to pop out a 2.0 of Releases for Sphinx reasons (dropping older Sphinxes; no API change on our end); so I'm tempted to merge this after all, and if necessary raise our semantic_version requirement (but hopefully not?).

bitprophet commented 4 years ago

Unfortunately as-is the PR breaks the test suite. If I can't figure out exactly why, I may try creating a variant that is less of a delta from master while still being compliant with modern versions of semantic_version.

bitprophet commented 4 years ago

Still poking but it always seems to come back to the fact that a lot of the logic in Releases /really/ wants that partial-version feature.

Many such spots feel like they logically should kinda be able to be Spec objects instead (and in fact this is suggested in semantic_version's docs, eg SimpleSpec('1.x.x')), but Specs are not Versions and so cannot be treated quite like a partial Version could (eg you cannot ask if one of these Specs matches another, as they lack truncate).

Trying to see if I can work around this one way or another (e.g. trying to use 'floor' versions, 1.0.0 instead of '1.*' etc).

bitprophet commented 4 years ago

Ran out of working tendons but have an interim branch (autolinked above). Some changes the same as this PR, some different. Only one test fails vs the many failing in this PR, so I think I am on the right track and missing one spot somewhere.

The changes here imply I want to keep a Version subclass around that "acts partial" but in a way that does not rely on the existing/deprecated, internal-to-semantic_version partial kwarg. (It would instead just isolate the dumb "tack on zeroes" stuff seen in my branch.)

As seen in TODOs, also, a lot of the Releases-level code is probably kinda dumb and wants reorganizing anyways. Whether I'll do that as part of this or punt, not sure.

rbarrois commented 4 years ago

@bitprophet awesome, thanks for looking into this! I'll take a look at your branch later today, hopefully I'll be able to help you finding a good solution, or to see what kind of APIs would be needed over in python-semantic_version to support your use case ;)