Version v1.0.2-alpha.0 should be greater than v1.0.1 but version.NewConstraint(...).Check returns false

[sebastian-sommerfeld-io]

Hey guys ... I stumbled upon this issue while implementing a tool to validate versions when I release one of my github projects. I want to make sure, that the version for my next release is greater than the latest version.

When checking for pre-release versions (alpha / beta) I stumbled upon a bug (from my point of view):

• Given my latest version is v1.0.1 which is a stable release
• When I check if v1.0.2 is greater than v1.0.1 then everything works fine
• When I check if v1.0.2-alpha.0 is greater than v1.0.1 then the result is false ... my expectation is that this check would pass, because I increased the minor version and marked it as unstable.
• When I check if v1.0.2-alpha.1 is greater than v1.0.2-alpha.0 then everything works fine again.

I wrote a short testcase in Go to validate

issue.go:

package services

import (
	"fmt"

	"github.com/hashicorp/go-version"
)

// The next version should be greater that the current version
func IsGreater(current string, next string) bool {
	parsedCurrent, err := version.NewVersion(current)
	if err != nil {
		fmt.Print(err)
		return false
	}

	parsedNext, err := version.NewVersion(next)
	if err != nil {
		fmt.Print(err)
		return false
	}

	// should be greater that current version
	constraint, err := version.NewConstraint("> " + parsedCurrent.Original())
	if err != nil {
		fmt.Print(err)
		return false
	}

	if !constraint.Check(parsedNext) {
		fmt.Print("next version " + parsedNext.Original() + " is not greater that current version " + parsedCurrent.Original())
	}
	return constraint.Check(parsedNext)
}

issue_test.go:

package services

import (
	"testing"

	"github.com/stretchr/testify/assert"
)

func Test_ShouldBeGreater(t *testing.T) {
	testCases := []struct {
		currentVersion string
		nextVersion    string
		shouldBeValid  bool
	}{
		{"v1.0.1", "v1.0.2", true},
		{"v1.0.1", "v1.1.0", true},
		{"v1.0.1", "v2.0.0", true},
		{"v1.0.1", "v1.0.2-alpha.0", true}, // fails but should be true
		{"v1.0.1", "v1.0.2-beta.0", true},  // fails but should be true
		{"v1.0.2-alpha.0", "v1.0.2-alpha.1", true},
		{"v1.0.2-alpha.0", "v1.0.2-beta.0", true},
		{"v1.0.2-beta.1", "v1.0.2-beta.2", true},
		{"v1.0.2-alpha.0", "v1.0.2", true},
		{"v1.0.2-beta.0", "v1.0.2", true},
		{"v1.0.2", "v1.0.2", false},
		{"v1.0.2", "v1.0.1", false},
		{"v1.0.2-beta.0", "v1.0.2-alpha.0", false},
		{"v1.0.2-beta.2", "v1.0.2-beta.1", false},
		{"v2.0.0", "v2.0.0", false},
	}
	for _, tc := range testCases {
		assert := assert.New(t)
		got := IsGreater(tc.currentVersion, tc.nextVersion)

		assert.NotNil(got)
		assert.Equal(tc.shouldBeValid, got, "Result did not meet expectation - next version = "+tc.nextVersion+", current version = "+tc.currentVersion)
	}
}

Could you take a look into this please? Feel free to reach out to me for information if you feel the need. Best regards.

[kke]

I don't know if it is mentioned in the docs, but the implementation and code comments are here: https://github.com/hashicorp/go-version/blob/main/constraint.go#L197-L215

	case cPre && vPre:
		// A constraint with a pre-release can only match a pre-release version
		// with the same base segments.
		return reflect.DeepEqual(c.Segments64(), v.Segments64())

	case !cPre && vPre:
		// A constraint without a pre-release can only match a version without a
		// pre-release.
		return false

So, it's intentional.

[chrismarget-j]

I've realized that my use case is not well aligned with the purpose of the package (or npm's purposes).

This library uses constraints to solve a package dependency problem: Selecting a compatible version from a pool of available releases.

I'm trying to use it in a client application, to make inferences about the server's capabilities: "If the server version is >=1.2, it can do feature X"

I'd been using a constraint ">=1.2" for that purpose, and not getting the Check() result I expected with obviously newer (but pre-release) server versions.

When the server version is, say "2.1-alpha", it does not satisfy the ">=1.2" constraint.

The constraint behavior is reasonable when selecting from available packages. It's just less suited to my scenario.

My original comment is below.

@kke said:

So, it's intentional.

I'm not sure that it is.

While the cited comments correctly describe the behavior of the code, this snippet considers pre-release tags only in isolation. There's no mention or consideration of the major/minor/patch values, so it's not a good match for the situation raised by @sebastian-sommerfeld-io.

The PR discussion for this code is here. That discussion gets closer to the mark, but doesn't quite address this case.

That discussion does, however, make clear that the behavior was intended to match npm's pre-release label handling, and includes a (now-dead) link relevant documentation. The npm link now lives here.

The npm document includes this nugget (emphasis mine):

For example, the range >1.2.3-alpha.3 would be allowed to match the version 1.2.3-alpha.7, but it would not be satisfied by 3.4.5-alpha.9, even though 3.4.5-alpha.9 is technically "greater than" 1.2.3-alpha.3 according to the SemVer sort rules. The version range only accepts prerelease tags on the 1.2.3 version. The version 3.4.5 would satisfy the range, because it does not have a prerelease flag, and 3.4.5 is greater than 1.2.3-alpha.7.

Clearly the npm parser would not fail the constraint check based only on the presence of a pre-release label, but would use that presence as a tiebreaker when considering versions with the same major/minor/patch value.

I think it's a bug.

I'm hoping @jbardin will weigh in on the original intentions.

[jbardin]

Yes, this is intentional as pointed out earlier in the code. One of the goals of the comparisons made in this package is to ensure that the typical constraints used in a production environment can never automatically update to a prerelease version. One must always explicitly opt-in to try prerelease versions using this scheme, otherwise prereleases are not considered.

@chrismarget-j, in a client-server situation, you can use the same constraints, but you must still take care have the client opt-in to test the alpha versions of a server. Depending on the use case, it could be critical that a client configured for >=1.2 never accept a connection to an alpha server release for security or data integrity purposes, but that's up to you to decide. Another approach is that if all clients are expected to be able to automatically upgrade to prerelease versions, perhaps a prerelease version is not suitable since there is no longer a difference between that an a normal release from the client's perspective. Either way, there is no single method for release versioning, and different situations require different solutions.

I'll close this out since the code is working as designed.

[chrismarget-j]

Thank you for your reply, @jbardin.

in a client-server situation, you can use the same constraints, but you must still take care have the client opt-in to test the alpha versions of a server.

Would you mind elaborating on this?

I've currently got something like this in the client code:

featureXok = version.MustConstraints(version.NewConstraint(">= 1.2.0"))

I use it to determine whether it's safe to use server "feature X":

  if featureXok.Check(client.serverVersion()) {
    // do something which relies on feature X
  }

This check fails when the *version.Version returned by the client.serverVersion() is, say, 3.1.0-alpha.

The failure (3.1.0-alpha is not greater or equal than 1.2.0) took me by surprise.

What would it mean to "have the client opt-in to test alpha versions of a server" in this case?

I'm thinking of adding Core() whenever making these kind of "server feature available" determinations, but perhaps you were alluding to a different approach?

[jbardin]

By "opt-in", I mean that the client would specifically need to use a constraint which allows alpha releases, and >= 1.2.0 purposely does not match prereleases. That could be done via configuration, runtime flags, special prerelease builds of the client, etc.

Whether a server supports a feature or not is not directly related to whether a particular version is acceptable to use. Version 4.0.0-alpha1234+sometimes_loses_data may support the feature you want, but may not be acceptable as a target for every client.

[chrismarget-j]

Thank you @jbardin.

the client would specifically need to use a constraint which allows alpha releases

I'm not sure it's possible for me to write a constraint which broadly considers pre-releases.

As it stands now, my client knows nothing about any alpha or sometimes_loses_data server builds which may exist now or in the future, and I have no way of anticipating them. It only knows that "feature X" appeared in 1.2.0.

I think we're zeroing in on the realization I had when I edited my first comment in this issue.

This package exists mostly for the reason you just raised: To determine "whether a particular version is acceptable to use".

It seems I'm a little off in the weeds trying to use it to create "feature available" milestones.