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

Replace use of github.com/hashicorp/go-version #1753

Closed
MrAlias opened this issue Feb 5, 2025 · 0 comments · Fixed by #1766
Closed

Replace use of github.com/hashicorp/go-version #1753

MrAlias opened this issue Feb 5, 2025 · 0 comments · Fixed by #1766
Assignees
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Feb 5, 2025

We currently use github.com/hashicorp/go-version to represent versions throughout the project. There are many limitations and unexpected behavior that that this project contains that warrant a move away from its use.

Issues

No way to construct a new value without also returning an error.

If you want to create a new version.Version you can only do so by passing a string that is parsed with version.NewVersion. This means an error type is always returned with the parsed version. If you know the version is valid (i.e. a static value in a test), you should be able to create a version directly

Comparisons are not evaluated according to semantic conventions

We use version.Constraints to match package versioning. However, github.com/hashicorp/go-version does not evaluate these as specified by semantic versioning.

The package takes its own approach to handling prereleases.

This means if we add a package constraint like > 1.34.0 and use a package with a version like "1.34.1-0.20250205150531-85fab8be9320" it will evaluate to false even though according to semantic versioning this should be true

Version is not comparable

The Version type is defined as such:

// Version represents a single version.
type Version struct {
	metadata string
	pre      string
	segments []int64
	si       int
	original string
}

Based on the way the segments field is defined, this type is not comparable.

An Equal method is provided, but it would be more idiomatic if we could compare equality with the = Go operator.

Also, this mean the Version cannot be used directly as a map key. This results in us having to manually make the parts comparable:

type verKey struct {
major, minor, patch uint64
prerelease string
metadata string
}

The version package name sits on a useful variable name

Commonly using version for a variable name will be descriptive of what we are trying to hold. However, this will conflict with the package name.

Code health

Over the last 11 years there have been 73 commits: https://github.com/hashicorp/go-version/graphs/contributors

There are 23 open Pull Requests (some > 10 years old): https://github.com/hashicorp/go-version/pulls

There are 18 open issues and 30 closed issues: https://github.com/hashicorp/go-version/issues

There are 8 releases: https://github.com/hashicorp/go-version/releases

Licensing

github.com/hashicorp/go-version is licensed under the Mozilla Public License, version 2.0. This is a weak copyleft license that prohibits trademark use, and requires source disclosure, license and copyright notice, and same license inheritance.

This is not a blocking issue as we are complying with all the terms currently, however there are more permissively licensed projects we could consider.

Proposal

Switch to using github.com/Masterminds/semver/v3

Auditing the package, there appears to be a 100% feature overlap with how we currently use github.com/hashicorp/go-version

  • There is a Version type that represents Go versions
  • The Version type supports comparison
  • There is a Constraint type that supports matching versions
  • Supports sorting versions.
  • Supports JSON and other text marshaling
  • Supports Go >= 1.21

Additionally ...

MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 6, 2025
Resolve open-telemetry#1753

Replace with github.com/Masterminds/semver/v3
@MrAlias MrAlias self-assigned this Feb 6, 2025
@MrAlias MrAlias added this to the v0.21.0 milestone Feb 6, 2025
MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 6, 2025
Replace with github.com/Masterminds/semver/v3

Resolve open-telemetry#1753
MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 6, 2025
Replace with github.com/Masterminds/semver/v3

Resolve open-telemetry#1753
MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 6, 2025
Replace with github.com/Masterminds/semver/v3

Resolve open-telemetry#1753
MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 6, 2025
Replace with github.com/Masterminds/semver/v3

Resolve open-telemetry#1753
MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 7, 2025
Replace with github.com/Masterminds/semver/v3

Resolve open-telemetry#1753
MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 7, 2025
Replace with github.com/Masterminds/semver/v3

Resolve open-telemetry#1753
MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 10, 2025
Replace with github.com/Masterminds/semver/v3

Resolve open-telemetry#1753
MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 12, 2025
Replace with github.com/Masterminds/semver/v3

Resolve open-telemetry#1753
MrAlias added a commit to MrAlias/opentelemetry-go-instrumentation that referenced this issue Feb 13, 2025
Replace with github.com/Masterminds/semver/v3

Resolve open-telemetry#1753
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant