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

Problems encountered when migrating to modules #2

Closed
wants to merge 2 commits into from

Conversation

KateGo520
Copy link

Background

If you opted into Go Modules, you must comply with the specification of "Releasing Modules for v2 or higher" available in the Modules documentation and enforced since the most recent versions of Go. Quoting the specification:

A package that has opted in to modules must include the major version in the import path to import any v2+ modules
To preserve import compatibility, the go command requires that modules with major version v2 or later use a module path with that major version as the final element. For example, version v2.0.0 of example.com/m must instead use module path example.com/m/v2.
https://github.com/golang/go/wiki/Modules#releasing-modules-v2-or-higher

Issue 1: The module path declaration is wrong

The latest version of CrunchyData/postgres-operator is v4.3.2, it’s a v2+ module. So, you need to declare the module path like this:

module github.com/crunchydata/postgres-operator/v4
go 1.13
require (
…

(This is for the commit with the version tag v4.x.y)
Or, the downstream modules users cannot require your package normally, they will get an error like:

invalid version: module contains a go.mod file, so major version must be compatible: should be v0 or v1, not v4

** Solution **:

  1. Update the go.mod file to include a /v4 at the end of the module path in the module directive (module github.com/crunchydata/postgres-operator/v4).
  2. Update import statements within the module to also use /v4 (e.g., import "github.com/crunchydata/postgres-operator/v4/internal/config").
    You can see the import statements within the module here:
    https://github.com/CrunchyData/postgres-operator/search?q=github.com%2Fcrunchydata%2Fpostgres-operator&unscoped_q=github.com%2Fcrunchydata%2Fpostgres-operator
  3. Tag the release with v4.x.y.

Issue 2: github.com/evanphx/json-patch stuck in the older version

Before you opted into Go Modules, the version of evanphx/json-patch is v4.6.0.
https://github.com/CrunchyData/postgres-operator/blob/master/Gopkg.toml#L16

After you opted into Go Modules, the version of evanphx/json-patch turn to v4.2.0.

github.com/evanphx/json-patch v4.2.0+incompatible

If you didn't do it on purpose, then I think the root cause of this issue is:

  1. Since the version v4.6.0, the evanphx/json-patch opted into Go Modules.
  2. evanphx/json-patch didn’t comply with the specification of "Releasing Modules for v2 or higher".
    github.com/evanphx/json-patch/go.mod
module github.com/evanphx/json-patch
go 1.12
require github.com/pkg/errors v0.8.1

(It should be “module github.com/evanphx/json-patch/v4”)

** Solution **:
Since it's against one of the design choices of Go, it'll be a bit of a hack. Instead of go get github.com/evanphx/json-patch@version-tag, the install procedure would be something like:
(1)Search for the tag you want (in browser)
(2)Get the commit hash for the tag you want
(3)Run go get github.com/evanphx/json-patch@commit-hash
(4)Edit the go.mod file to put a comment about which version you actually used
For example:
If you want to get the github.com/evanphx/json-patch v4.6.0(Latest commit bf22ed9 on 21 Dec 2019), you need to edit the go.mod file like this:

module github.com/crunchydata/postgres-operator
go 1.13
require (
	github.com/evanphx/json-patch bf22ed9
	…
)

This way seems good, but if the users try to use go get -u, they will get the old version and break again. So be cautious here.

References

@jkatz
Copy link
Owner

jkatz commented Oct 19, 2020

Superseded by CrunchyData#1979

@jkatz jkatz closed this Oct 19, 2020
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