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

Errors will happen when upgrading the libraries #1647

Closed
KateGo520 opened this issue Jun 21, 2020 · 6 comments · Fixed by #1979
Closed

Errors will happen when upgrading the libraries #1647

KateGo520 opened this issue Jun 21, 2020 · 6 comments · Fixed by #1979

Comments

@KateGo520
Copy link

KateGo520 commented Jun 21, 2020

(The purpose of this report is to alert CrunchyData/postgres-operator to the possible problems when CrunchyData/postgres-operator try to upgrade the following dependencies)

An error will happen when upgrading library spf13/cobra :

github.com/spf13/cobra

-Version: V1.0.0 (Latest commit a684a6d on 11 Apr)
-Where did you use it:
https://github.com/CrunchyData/postgres-operator/search?q=spf13%2Fcobra%2Fdoc&unscoped_q=spf13%2Fcobra%2Fdoc
-Detail:

github.com/spf13/cobra/go.mod

module github.com/spf13/cobra
go 1.12
require (
	github.com/cpuguy83/go-md2man/v2 v2.0.0
	github.com/inconshreveable/mousetrap v1.0.0
	github.com/mitchellh/go-homedir v1.1.0
	github.com/spf13/pflag v1.0.3
	github.com/spf13/viper v1.4.0
	gopkg.in/yaml.v2 v2.2.2
)

github.com/spf13/cobra/doc/man_docs.go

package doc
import (
	"github.com/cpuguy83/go-md2man/v2/md2man"
	"github.com/spf13/cobra"
	"github.com/spf13/pflag"
)

This problem was introduced since spf13/cobra v0.0.6 . Now you used version v0.0.5 . If you try to upgrade spf13/cobra to version v0.0.6 and above, you will get an error--- no package exists at "github.com/cpuguy83/go-md2man/v2/md2man"

I investigated the libraries' (spf13/cobra >= v0.0.6) release information and found the root cause of this issue is that----

  1. These dependencies all added Go modules in the recent versions.

  2. They all comply with the specification of "Releasing Modules for v2 or higher" available in the Modules documentation. Quoting the specification:

A package that has migrated to Go Modules must include the major version in the import path to reference any v2+ modules. For example, Repo github.com/my/module migrated to Modules on version v3.x.y. Then this repo should declare its module path with MAJOR version suffix "/v3" (e.g., module github.com/my/module/v3), and its downstream project should use "github.com/my/module/v3/mypkg" to import this repo’s package.

  1. This "github.com/my/module/v3/mypkg" is not the physical path. So earlier versions of Go (including those that don't have minimal module awareness) plus all tooling (like dep, glide, govendor, etc) don't have minimal module awareness as of now and therefore don't handle import paths correctly See golang/dep#1962, golang/dep#2139.

Note: creating a new branch is not required. If instead you have been previously releasing on master and would prefer to tag v3.0.0 on master, that is a viable option. (However, be aware that introducing an incompatible API change in master can cause issues for non-modules users who issue a go get -u given the go tool is not aware of semver prior to Go 1.11 or when module mode is not enabled in Go 1.11+).
Pre-existing dependency management solutions such as dep currently can have problems consuming a v2+ module created in this way. See for example dep#1962.
https://github.com/golang/go/wiki/Modules#releasing-modules-v2-or-higher

Solution

1. Migrate to Go Modules.

Go Modules is the general trend of ecosystem, if you want a better upgrade package experience, migrating to Go Modules is a good choice.

Migrate to modules will be accompanied by the introduction of virtual paths(It was discussed above).

This "github.com/my/module/v3/mypkg" is not the physical path. So Go versions older than 1.9.7 and 1.10.3 plus all third-party dependency management tools (like dep, glide, govendor, etc) don't have minimal module awareness as of now and therefore don't handle import paths correctly.

Then the downstream projects might be negatively affected in their building if they are module-unaware (Go versions older than 1.9.7 and 1.10.3; Or use third-party dependency management tools, such as: Dep, glide, govendor…).

2. Maintaining v2+ libraries that use Go Modules in Vendor directories.

If CrunchyData/postgres-operator want to keep using the dependency manage tools (like dep, glide, govendor, etc), and still want to upgrade the dependencies, can choose this fix strategy.
Manually download the dependencies into the vendor directory and do compatibility dispose(materialize the virtual path or delete the virtual part of the path). Avoid fetching the dependencies by virtual import paths. This may add some maintenance overhead compared to using modules.

There are 3 module users downstream, such as jufi-git/postgres-operator, CrunchyData/pgo-osb, CrunchyData/postgres-operator…)
https://github.com/search?q=CrunchyData%2Fpostgres-operator+filename%3Ago.mod

As the import paths have different meanings between the projects adopting module repos and the non-module repos, materialize the virtual path is a better way to solve the issue, while ensuring compatibility with downstream module users. A textbook example provided by repo github.com/moby/moby is here:
https://github.com/moby/moby/blob/master/VENDORING.md
https://github.com/moby/moby/blob/master/vendor.conf
In the vendor directory, github.com/moby/moby adds the /vN subdirectory in the corresponding dependencies.
This will help more downstream module users to work well with your package.

3. Request upstream to do compatibility processing.

The spf13/cobra have 1212 module-unaware users in github, such as: maliceio/cli, Azure/acr-docker-credential-helper, alyyousuf7/spotifyshare…
https://github.com/search?q=spf13%2Fcobra+filename%3Avendor.conf+filename%3Avendor.json+filename%3Aglide.toml+filename%3AGodep.toml+filename%3AGodep.json&type=Code

Summary

You can make a choice when you meet this DM issues by balancing your own development schedules/mode against the affects on the downstream projects.

For this issue, Solution 1 can maximize your benefits and with minimal impacts to your downstream projects the ecosystem.

References

Do you plan to upgrade the libraries in near future?
Hope this issue report can help you ^_^
Thank you very much for your attention.

Best regards,
Kate

@jkatz
Copy link
Contributor

jkatz commented Jun 21, 2020

Thanks for the analysis @KateGo520!

We had looked at go modules a few months back and is something we could consider again. However, based on our current version number combined with go modules opinions on later versions, there are some challenges shifting to that.

For the forseeable future we will be on dep, though subject to change should anyone wish to take on the conversion.

We do still use dep, and in our requirements file, we do have this dependency pinned to 0.0.5 (which has been pinned there for awhile). We do try to generally keep our dependencies up-to-date, though we aim to do it safely, especially for packages that are major dependencies.

We can look into updating the Cobra dependency and seeing what happens and what code changes are required.

@KateGo520
Copy link
Author

@jkatz Thanks for your reply. This report is a warm prompt for you to prevent or combat this issue. Hope this can help you when upgrading Cobra dependency.

Thanks again.
Kate

@jkatz
Copy link
Contributor

jkatz commented Jun 21, 2020

@KateGo520 I'm going to take a quick look now to see what it takes and if it's relatively quick/safe. It also reminded me to do a check on the updates available for other dependencies.

jkatz pushed a commit to jkatz/postgres-operator that referenced this issue Jun 21, 2020
An unintended side effect of CrunchyData#1576 and others is that the
directory structure is now conducive to using go modules. After
my latest wrangling with dep to try to do some routine upgrades,
I thought it prudent to attempt to make the switch. It turns
out that it was not too bad this go around, so now seems as likely
of a time to try to make the switch.

Issue: CrunchyData#1647
@jkatz
Copy link
Contributor

jkatz commented Jun 21, 2020

@KateGo520 After fighting with dep for about an hour, for fun I tried again moving to go modules. Turned out it was much easier this time, likely due to some project restructuring found as part of #1576. Pending review of others who know more about go than me, we can look to get that in, and then subsequently evaluate updating cobra.

Thanks!

@KateGo520
Copy link
Author

KateGo520 commented Jun 21, 2020

@jkatz See your PR #1648 . Thank you very much for your efforts!

@KateGo520
Copy link
Author

KateGo520 commented Jun 22, 2020

@jkatz A minor issue in your PR. SEE Problems encountered when migrating to modules

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 a pull request may close this issue.

2 participants