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

Update genproto and fake-gcs-server #202

Closed
kernle32dll opened this issue Apr 6, 2019 · 11 comments
Closed

Update genproto and fake-gcs-server #202

kernle32dll opened this issue Apr 6, 2019 · 11 comments

Comments

@kernle32dll
Copy link

Describe the Bug

Migrate contains old versions of both genproto and fake-gcs-server. These old versions transitively depend on a wrong import path of go lint, which breaks running go get -u in any project using migrate. This should be easy to fix by updating (at least) these two dependencies. However, as I have no deep knowledge how they are used in migrate, I can't estimate if something might break by updating, and needs fixing.

Steps to Reproduce

  1. New go project with go mod init ...
  2. go get github.com/golang-migrate/migrate/[email protected]
  3. go get -u

Migrate Version
v4.2.5 with go modules - probably older versions, too

Additional context

I found the affected dependencies via go mod graph.

Import path trough fake-gcs-server:

github.com/golang-migrate/migrate/[email protected] github.com/fsouza/[email protected]
github.com/fsouza/[email protected] google.golang.org/[email protected]
google.golang.org/[email protected] golang.org/x/[email protected]

Import path trough genproto:

github.com/golang-migrate/migrate/[email protected] google.golang.org/[email protected]
google.golang.org/[email protected] google.golang.org/[email protected]
google.golang.org/[email protected] golang.org/x/[email protected]

Fixed version for fake-gcs-server: 1.3.1 (Dec
Fixed version for genproto: 4f5b463f9597cbe0dd13a6a2cd4f85e788d27508 (Feb 27)

It basically boils down to getting all dependencies to use at least gRPC 1.17.0.

@kernle32dll
Copy link
Author

Well, looking at the current Master, things are a lot more complicated, as both dependencies mentioned have the required versions or later. However, new dependencies have been introduced, which again transitively depend on the wrong import path of lint. Urgh.

@kernle32dll
Copy link
Author

I did a quick dig trough the current master state. So, these need to be updated:

  • cloud.google.com/go
  • github.com/fsouza/fake-gcs-server
  • google.golang.org/api

However, even then things don't work, as google.golang.org/api has a cyclic dependency with go.opencensus.io which ultimately results in an too-old gepnproto version. I will create an issue there.

@jeanbza
Copy link

jeanbza commented Apr 8, 2019

This has been fixed. For whichever you depend on, please use:

  • cloud.google.com/go v0.37.4
  • google.golang.org/api v0.3.4
  • go.opencensus.io v0.20.1
  • Latest genproto semver

dhui added a commit that referenced this issue Apr 9, 2019
@dhui
Copy link
Member

dhui commented Apr 10, 2019

The specific issue where the suite of google dependencies depended on the wrong version of golint is fixed here: ddc7246
However, the underlying issue still hasn't been fixed yet, so go get -u still fails:

$ go get -u
...
go: sourcegraph.com/sourcegraph/[email protected]: parsing go.mod: unexpected module path "github.com/sourcegraph/go-diff"
...
go get: error loading module requirements
$

@jeanbza
Copy link

jeanbza commented Apr 10, 2019

Yeah, that's the third time I've heard of sourcegraph failing. I imagine there is the same problem somewhere else in your build graph with a different set of modules including sourcegraph. I'll be releasing an article later about the problem and how to go about fixing it. Unfortunately it likely requires a fix on the sourcegraph side.

@vorlif
Copy link

vorlif commented Apr 23, 2019

I think that the problems with the current version (v4.3.0) now only depends on fake-gcs-server. If I call go get -u I always get the error message github.com/golang/[email protected]: parsing go.mod: unexpected module path "golang.org/x/lint". But if I force a current version of fake-gcs-server with replace, I have no more error messages.

Command to force the current version of fake-gcs-server:

go mod edit -replace github.com/fsouza/[email protected]=github.com/fsouza/[email protected]

@jeanbza
Copy link

jeanbza commented Apr 23, 2019

For whom it may concern, I've written about this problem, why it occurs, and how to resolve it: https://github.com/golang/go/wiki/Resolving-Problems-From-Modified-Module-Path

@dhui dhui closed this as completed in 10e2545 Apr 27, 2019
@dhui
Copy link
Member

dhui commented Apr 27, 2019

Should be fixed in v4.3.1.
Namely, go get -u works for me at that commit.

Note, you probably shouldn't run go get -u since migrate has only be tested with the dependencies pinned at certain versions.

@jeanbza
Copy link

jeanbza commented Apr 29, 2019

Note, you probably shouldn't run go get -u since migrate has only be tested with the dependencies pinned at certain versions.

Quick note: due to how MVS works, pinning is not something you or your users can do in Go (without vendor and replace rule shenanigans).

The consequence for library authors is to strongly favour v1+ dependencies. v0 dependencies are allowed to break without major version bumps. This means that a stable library (such as this one) depending on an unstable library (such as, for example, github.com/kr/[email protected]) has the potential to break unexpectedly.

@dhui
Copy link
Member

dhui commented Apr 29, 2019

Thanks for the clarification @jadekler !

Also note, if you're using migrate as a library (e.g. depending on it), your mileage may vary when blindly running go get -u. Not only do you need to worry about unstable library dependencies, but we also make some minor backwards incompatible fixes (generally behavioral, not interface changes) that don't warrant a major version number change. We're pretty good about properly noting changes in our release notes, so you should check those before updating migrate.

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

No branches or pull requests

4 participants