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

Minimum version selection #3617

Merged
merged 1 commit into from
Nov 8, 2024
Merged

Minimum version selection #3617

merged 1 commit into from
Nov 8, 2024

Conversation

akutz
Copy link
Member

@akutz akutz commented Nov 8, 2024

Description

This patch updates this project's root go.mod file to specify the minimum required version of Go and minimum required versions for dependencies.

This change ensures consumers of the GoVmomi module are not forced to use a version of Golang or dependency just because this project's root go.mod file specifies them.

The release of Go 1.21 changed the meaning of the Go version specified in go.mod (https://go.dev/doc/modules/gomod-ref#go-notes):

The go directive sets the minimum version of Go required to use this module. Before Go 1.21, the directive was advisory only; now it is a mandatory requirement: Go toolchains refuse to use modules declaring newer Go versions.

This means if GoVmomi specified Go 1.23.2 in its go.mod file, any project that imported GoVmomi would also be required to use Go 1.23.2.

Of course the govc and vcsim binaries still specify the latest Go version and newest dependencies to mitigate CVEs. Except for these two packages, the rest of GoVmomi is never built as a binary artifact, ex. executable / plug-in. Thus it is fine for the rest of the project to use an older Go version and older dependencies.

The only downside to this change is that govc and vcsim may no longer be installed by running:

  • go install github.com/vmware/govmomi/govc@latest
  • go install github.com/vmware/govmomi/vcsim@latest

This is because both packages are now distinct sub-modules that still depend on the other packages in this project. Therefore these sub-mods use the following replace directive:

replace github.com/vmware/govmomi -> ../

This teaches govc and vcsim where to look for the imported GoVmomi packages. This ensures that building these binaries will pick up any local changes to the rest of the project.

However, the go install command does not support installing modules from a remote location if the module contains a replace directive. This means users will need to either clone all of github.com/vmware/govmomi before running go -C govc install or go -C vcsim install, or they can download the released binaries directly.

Closes: NA

How Has This Been Tested?

Ran the following commands locally:

  • make install
  • gorelease release --snapshot --clean
  • make test

Guidelines

Please read and follow the CONTRIBUTION guidelines of this project.

@akutz akutz requested a review from dougm November 8, 2024 16:30
@akutz akutz force-pushed the feature/mvs branch 2 times, most recently from 1f1bfdf to f34d089 Compare November 8, 2024 16:39
Copy link
Member

@dougm dougm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @akutz !

@@ -1,24 +1,21 @@
module github.com/vmware/govmomi

go 1.22
go 1.21 // required for slices/maps package and built-in clear function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had bumped to 1.22 here just because we support the latest 2 Go minor versions and only test w/ those versions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, how do you want to handle this? Should we consider changing this behavior to "leave version alone unless the library needs a newer version?"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that sounds good, let's leave it alone for now

This patch updates this project's root `go.mod` file to specify the
minimum required version of Go and minimum required versions for
dependencies.

This change ensures consumers of the GoVmomi module are not forced to
use a version of Golang or dependency just because this project's root
`go.mod` file specifies them.

The release of Go 1.21 changed the meaning of the Go version specified
in  `go.mod` (https://go.dev/doc/modules/gomod-ref#go-notes):

    The go directive sets the minimum version of Go required to use this
    module. Before Go 1.21, the directive was advisory only; now it is a
    mandatory requirement: Go toolchains refuse to use modules declaring
    newer Go versions.

This means if GoVmomi specified Go 1.23.2 in its `go.mod` file, any
project that imported GoVmomi would also be required to use Go 1.23.2.

Of course the `govc` and `vcsim` binaries still specify the latest Go
version and newest dependencies to mitigate CVEs. Except for these two
packages, the rest of GoVmomi is never built as a binary artifact, ex.
executable / plug-in. Thus it is fine for the rest of the project to use
an older Go version and older dependencies.

Signed-off-by: akutz <akutz@vmware.com>

BREAKING: The `go install` command may no longer be used to install
govc and vcsim directly from GitHub.

This is because both packages are now distinct sub-modules that still
depend on the other packages in this project. Therefore these sub-mods
use the following replace directive:

    replace github.com/vmware/govmomi -> ../

This teaches govc and vcsim where to look for the imported GoVmomi
packages. This ensures that building these binaries will pick up any
local changes to the rest of the project.

However, the `go install` command does not support installing modules
from a remote location if the module contains a `replace` directive.
This means users will need to either clone all of
github.com/vmware/govmomi before running `go -C govc install` or
`go -C vcsim install`, or they can download the released binaries
directly.
@akutz akutz merged commit 8f2493a into vmware:main Nov 8, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants