semver.bump_prerelease on *-rc9 results in lower version

Which version of Python is the problem with?

3.8

What semver version are you using?

No response

What OS are you using? (Add more in the Environment section)

macOS

Situation

When calling semver.bump_prerelease on a version with a prerelease of -rc9, it gets bumped to -rc10. However. according to the semver spec (https://semver.org/#spec-item-11 section 11.4 items 1+2), 'rc9' is considered a string and will be compared to 'rc10' using ASCII sort order, which considers 'rc10' to be lower than 'rc9'.

How to reproduce

>>> import semver
>>> version1 = '1.0.0-rc9'
>>> version2 = semver.bump_prerelease(version1)
>>> version2
'1.0.0-rc10'
>>> semver.compare(version1, version2)
1

Expected behavior

Not entirely sure how the bump_prerelease version should behalve when it gets rc9. ASCII-wise, after 9 you could consider a, but you get rca, rcb which feels a bit odd. Another option could be that the function just returns some kind of error, indicating it cannot determine how to increase rc9 to a higher value.

Environment

No response

@Learloj

Thanks @Learloj for your issue.

I think we need to distinguish between bumping and comparing. 🙂

Not entirely sure how the bump_prerelease version should behalve when it gets rc9.

I've looked into node's semver library and this is a short shell session what they do:

$ npm install semver
added 1 package in 562ms
$ node
Welcome to Node.js v20.18.1.
Type ".help" for more information.
> const semver = require('semver');
undefined
> v = semver.parse("2.3.4-rc9");
SemVer {
  options: {},
  loose: false,
  includePrerelease: false,
  raw: '2.3.4-rc9',
  major: 2,
  minor: 3,
  patch: 4,
  prerelease: [ 'rc9' ],
  build: [],
  version: '2.3.4-rc9'
}
> semver.inc(v, "prerelease")
'2.3.4-rc9.0'

Look at the last two lines. I was also surprised how they bump a prerelease. They just append .0 and bump that. Would that be an option?

@tomschr

Hi @tomschr ! Kudos for your fast reply! That does not happen often 😁

I think it is a very elegant way to resolve this indeed:

Split the pre release tag by dots and consider the last value
If the value is numeric, increase it
If not, add .0

That way it will always be considered a higher version according to the semver spec.

If it is ok, I don't mind working on a PR to implement this.

@Learloj

Great, thank you @Learloj! 👍

If it is ok, I don't mind working on a PR to implement this.

Absolutely! I'd love to get some support. 😍

To make it hopefully easier for you, I've described all the details at the Contributing to semver section. I think you only need to consider the sub sections 6.2, 6.3, and 6.4.

Thank you! ❤

@tomschr

@tomschr while working on improving the tests to cover this issue, I ran into another possible bug. When bumping the pre release version on a version that has no prerelease part yet, semver only adds the prerelease. As a version without a prerelease is considered newer than the same version with a prerelease, we basically get a version that is lower than the version which we bumped, which feels wrong to me.

To demonstrate:

>>> version = semver.Version.parse('1.2.3')
>>> newversion = version.bump_prerelease()
>>> newversion
Version(major=1, minor=2, patch=3, prerelease='rc.1', build=None)
>>> version.compare(newversion)
1

Checking npm, I see that when you bump a prerelease on a version without a prerelease part, they also bump the minor version to ensure the resulting version is considered higher:

> v = semver.parse("2.3.4");
SemVer {
  options: {},
  loose: false,
  includePrerelease: false,
  raw: '2.3.4',
  major: 2,
  minor: 3,
  patch: 4,
  prerelease: [],
  build: [],
  version: '2.3.4'
}
> semver.inc(v, "prerelease")
'2.3.5-0'

How shall I proceed with this? Keep the current implementation for this (and not add a test to validate if the outcome of the bump is higher than the input), or shall I change the behavior and also increase the minor in such cases?

@Learloj

Hi @Learloj,
thanks for all your efforts. 👍 Yes, unfortunately the .bump_prerelease() method has this problem. I didn't change that due to backward compatibility reasons. To mitigate this problem, use the .next_version() method.

Correcting the implementation of .bump_prerelease() would introduce a backward incompatible change which I'd like to avoid. If it's necessary we could do that, but we would have to increase the major version to 4. 🤔

Just a crazy idea and I'd like your opinion on that. What about if we introduce an .inc() method similar to the NPM version? This could work like this:

>>> version = semver.Version.parse('1.2.3')
>>> version.inc(part="prerelease")
Version(major=1, minor=2, patch=3, prerelease='0', build=None)

The implementation works in the same way as semver's NPM version. The next step would be to deprecate all the bump_*() versions. There should be only one way. If wanted, we could add shortcuts like .inc_major(), inc_minor()` etc.

So to summarize it, I see these options:

Make a semver 4 release and fix the .bump_prerelease() method (and maybe others).
Deprecate all .bump_*() methods and propagate .next_version() as the preferred method.
Introduce an .inc() method similar to NPM and deprecate all .bump_*() methods.

Perhaps some could be even combined. What do you think would be the best option here?

My only concerns are that this change is:

Backward compatible, or
If we introduce a backward incompatible change, we raise the major version and document the migration path.

@tomschr

@tomschr fair point. I think it's a bit confusing that we have multiple ways to 'increase' the version, which behave differently. I also think that adding a new method, even if we'd deprecate the others, might create more confusion, plus it'd affect a lot more people who'd need to adjust their code to step away from the bump_* methods.

Regarding the inc method, isn't that exactly what next_version() currently does? I checked the implementations, and both increase the patch version if there was no prerelease set (though npm adds 0 and semver adds rc.1 as prerelease).

I agree with your statement regarding backwards compatibility, but this change also feels so small for me that it's a bit drastic to increase the major version of semver. What we could do, is introduce an argument to bump_prerelease() named something like ensure_newer_version or bump_patch_when_empty or some other logical name, which would default to false. That way, we keep backwards compatibility, and when someone sets it to true, you'd get the 'expected' behaviour which bumps the patch version if there was no prerelease version set, resulting in a newer version. Then later, when there are other bigger changes and semver gets bumped to major version 4, we could consider adjusting the default value of this argument to true.

If this is not desired, then I think deprecating the bump_*() methods in favour of next_version() would not be a bad idea, as they both (should) do the same thing. I don't think it's necessary to add a inc() method, unless I am missing a big difference between what next_version() does, and what inc() in NPM does.

Let me know what you prefer, I am fine with creating a PR for either solution.

@Learloj

@Learloj

I think it's a bit confusing that we have multiple ways to 'increase' the version, which behave differently

Yes, was probably a bad idea. If I recall correctly, it was for historical reasons and to maintaining backward compatibility. But maybe it would be better/easier/cleaner if we have only one method that works for all cases.

Regarding the inc method, isn't that exactly what next_version() currently does?

Yes, that's pretty similar. It was just an idea. Or we could sell that as an alias to next_version() for people knowing or coming from the NPM semver library. Anyway, that's probably a different topic and not relevant for now.

What we could do, is introduce an argument to bump_prerelease() named something like ensure_newer_version or bump_patch_when_empty or some other logical name, which would default to false

Ahh, what a nice idea! I like it! Soo... I'd say, go for that. 👍

I'm not so sure about the naming of the optional argument. I have a slight preference of bump_when_empty. But if you have an idea to make it shorter, let us know.

If this is not desired, then I think deprecating the bump_*() methods in favour of next_version() would not be a bad idea, [...]

Maybe that should be the preferred way anyway, independent from this PR.

Thank you for all your ideas! ❤ That was very valuable. 👍

Fix python-semver#460 Raising a prerelease version always results in …

@tomschr

@tomschr please have a look at my PR and provide any and all feedback (don't hold back :-) ).

Fix python-semver#460 Raising a prerelease version always results in …

Fix python-semver#460 Improve bump_prerelease to alway get a newer ve…

Fix bump_build() silently returning an unchanged version for digitles…

Navigation Menu

Repository navigation

semver.bump_prerelease on *-rc9 results in lower version #460

Which version of Python is the problem with?

What semver version are you using?

What OS are you using? (Add more in the Environment section)

Situation

How to reproduce

Expected behavior

Environment

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

semver.bump_prerelease on *-rc9 results in lower version #460

Description

Which version of Python is the problem with?

What semver version are you using?

What OS are you using? (Add more in the Environment section)

Situation

How to reproduce

Expected behavior

Environment

Activity

tomschr commented on Jan 22, 2025

Learloj commented on Jan 22, 2025

tomschr commented on Jan 22, 2025

Learloj commented on Jan 25, 2025

tomschr commented on Jan 26, 2025

Learloj commented on Jan 26, 2025

tomschr commented on Jan 26, 2025

Learloj commented on Jan 26, 2025

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions