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

[QUESTION] Incorrect loose parsing of version? #164

Open
janslow opened this issue Aug 3, 2016 · 11 comments
Open

[QUESTION] Incorrect loose parsing of version? #164

janslow opened this issue Aug 3, 2016 · 11 comments
Assignees
Labels
Enhancement new feature or improvement

Comments

@janslow
Copy link

janslow commented Aug 3, 2016

I'm using the semver.valid() function (with loose = true) to check if strings are possibly semantic versions and I've got an unexpected result with one of the inputs:

> semver.valid("9.4.1208.jre7", false); // Strict, fails as expected
null
> semver.valid("9.4.1208.jre7", true); // Loose, incorrectly parses `patch` number.
'9.4.120-8.jre7'

When in loose mode I'd expect this to either fail to parse it and return null or parse it as 9.4.1208-jre7.

Can anyone explain this behaviour or is it a bug?

@janslow
Copy link
Author

janslow commented Aug 3, 2016

Also using semver@5.3.0

@isaacs
Copy link
Contributor

isaacs commented Aug 4, 2016

That is indeed quite curious! I agree with your expectation here. This is a bug.

To determine whether this is going to be a breaking change, though, we should check the registry to make sure there aren't any examples of this kind of oddness in the wild.

@blanchma
Copy link

I have the expectation that this
semver.valid('2.0',false)
should return at leas 2.0.0 but is returning null.

@janslow
Copy link
Author

janslow commented Sep 14, 2016

@blanchma, 2.0 isn't a valid semantic version, so must return null in strict mode (i.e., semver.valid(v, false)).

As it still isn't supported in loose mode, IMO, you should create a separate issue about expanding the scope of loose parsing to include MAJOR.MINOR versions, as this issue is about a string which is being parsed into the wrong version.

@isaacs
Copy link
Contributor

isaacs commented Sep 14, 2016

@blanchma Yeah, 2.0 is a version range, which matches any 2.0.x semver. A semver is always 3 parts. Also, that issue is off-topic, so please create a new issue if you'd like to discuss this.

Addressed on #167

@megawac
Copy link
Contributor

megawac commented Oct 5, 2016

I have mixed feelings about this, and loose mode in general. Personally I think it makes more sense in extensions/more general version parsers (e.g. https://github.com/megawac/semvish)

@Mark-Hatch-Bose
Copy link

Mark-Hatch-Bose commented Apr 21, 2019

>>> a = SemVer("2.10.1.1+gdd213db", loose=True)
>>> b = SemVer("2.10.100.1+gdd213db", loose=True)

>>> vars(a)
{'micro_versions': [1], 'minor': 10, 'raw': '2.10.1.1+gdd213db', 'major': 2, 'patch': 1, 'version': '2.10.1.1', 'build': ['gdd213db'], 'loose': True, 'prerelease': []}

>>> vars(b)
{'micro_versions': [], 'minor': 10, 'raw': '2.10.100.1+gdd213db', 'major': 2, 'patch': 10, 'version': '2.10.10-0.1', 'build': ['gdd213db'], 'loose': True, 'prerelease': [0, 1]}

I'm seeing this as well.
For some reason, if the patch number is multiple digits, it treats the final number as a prerelease number rather than a micro version.

This is because I'm trying to use commits since tag from git describe --long as a micro_version.

>>> versions = ['2.14.9.0+g98b2e1f','2.14.10.1+geabdbc8','2.14.11.0+gabcdefg']
>>> range_ = '2.X'
>>> print(max_satisfying(versions, range_,loose=True,include_prerelease=True))
2.14.9.0+g98b2e1f
>>> print(max_satisfying(versions, range_,loose=True))
2.14.9.0+g98b2e1f

Even adding include_prerelease=True seems to give unexpected results to max_satisfying.

@isaacs
Copy link
Contributor

isaacs commented Apr 22, 2019

@Mark-Hatch-Bose Yes, that is the same issue as referenced earlier in this issue, which would be addressed by PR #167, but we never landed that, and it would be a breaking change. In general, loose versions are not very well specified, and have some weird edge cases as a result.

Would the solutions discussed in #167 solve your problem?

@Mark-Hatch-Bose
Copy link

From #167

That'd block 1.2.3.4 but 1.2.34.5 would still be misinterpreted as 1.2.3-4.5.

The PR looks hopeful to my understanding but the comment above makes this seem like it's still not quite there yet.

Still confused why a solely numeric version 1.2.34.5 gets converted to 1.2.3-4.5. Is that ever desired?
It is understandable with non-numeric values like 1.2.3foo.5 is interpreted as 1.2.3-foo.5 as mentioned in the PR.
But with solely numeric values seems to make more sense to require '-' for prerelease interpretation.

Maybe this is best solved with an optional parameter defining the behavior desired?

Or do we know of examples where the above use-case (numeric only loose version desired to be interpreted as prerelease for any single digit beyond patches) is actually being used? It seems farfetched.

@isaacs
Copy link
Contributor

isaacs commented Apr 23, 2019

Still confused why a solely numeric version 1.2.34.5 gets converted to 1.2.3-4.5.

Currently, because loose mode does not require a - to prefix a prerelease section, and 4.5 is a valid prerelease section (and .5 is not), and 1.2.3 is a valid major-minor-patch tuple, it interprets the 1.2.3 as the tuple, and 4.5 as the prerelease.

Is that ever desired?

Not that I can see, no. There aren't a lot of folks using loose mode with >3 numbers in the tuple, as far as I can tell from the inactivity on this issue, which is presumably why it was forgotten for 2 years.

@Mark-Hatch-Bose
Copy link

Mark-Hatch-Bose commented Apr 24, 2019

Ah I see, thanks for the explanation. I didn't understand the parsing order I think.

If the change is made for loose versioning to support 1.2.34.5 (major 1, minor 2, patch 34, micro 5), this would be very helpful for my particular use-case. As mentioned in conan-io/conan#4630, it allows me to convert a git describe --long commits since tag to a micro version, allowing me to uniquely and monotonically increase my versions with every commit. This is used for accurate rebuilding against latest code without requiring SemVer (major, minor, patch) versioning changes for every commit.

@settings settings bot removed the in-progress label Dec 14, 2019
greenfenderpunk1987 added a commit to greenfenderpunk1987/node-semver that referenced this issue Oct 11, 2020
Cff01 added a commit to Cff01/node-semver that referenced this issue Dec 10, 2021
Let '.' start non-numeric prerelease in loose mode
@darcyclarke darcyclarke changed the title Incorrect loose parsing of version [QUESTION] Incorrect loose parsing of version? Jul 28, 2022
@darcyclarke darcyclarke added the Question further information is requested label Jul 28, 2022
@lukekarrys lukekarrys added Enhancement new feature or improvement and removed Question further information is requested labels Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement new feature or improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants