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

cmd/go: replace directive on +incompatible version fails on verifying with sum.golang.org consistently #33795

Open
leighmcculloch opened this issue Aug 23, 2019 · 8 comments

Comments

@leighmcculloch
Copy link
Contributor

commented Aug 23, 2019

What version of Go are you using (go version)?

$ go version
go version go1.13rc1 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/leighmcculloch/local/bin"
GOCACHE="/home/leighmcculloch/.cache/go-build"
GOENV="/home/leighmcculloch/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/leighmcculloch/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/leighmcculloch/local/bin/go/1.13rc1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/leighmcculloch/local/bin/go/1.13rc1/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/leighmcculloch/devel/test/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build816830480=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I run go mod tidy on the following code.

go.mod:

module test

go 1.13

require (
        github.com/throttled/throttled v2.2.2+incompatible
)

replace github.com/throttled/throttled v2.2.2+incompatible => github.com/bartekn/throttled v2.2.3-0.20181102161246-c99eef3ad70a+incompatible

main.go:

package main

import (
        "fmt"

        "github.com/throttled/throttled"
)

func main() {
        fmt.Println(throttled.HTTPRateLimiter{})
}

What did you expect to see?

I expected go mod tidy to exit status code of success (0).

$ go mod tidy
$ echo $?
0

What did you see instead?

The go mod tidy command had a failure exit status code (1) and a 410 Gone error message.

$ go mod tidy
go: downloading github.com/throttled/throttled v2.2.2
verifying github.com/throttled/throttled@v2.2.2: github.com/throttled/throttled@v2.2.2: reading https://sum.golang.org/lookup/github.com/throttled/throttled@v2.2.2: 410 Gone
$ echo $?
1

If I visit the URL https://sum.golang.org/lookup/github.com/throttled/throttled@v2.2.2 it returns the text:

not found: invalid version

If I remove the replace directive, go mod tidy returns success and there is no verifying error message. The output in the section above where the exit status is 0 is what I see when the replace statement is removed. The problem I'm experiencing is isolated to when the replace directive is in use.

The version of the throttled package is v2.2.2+incompatible and so if the tool wants to query the sumdb about it, it should be attempting to query the URL https://sum.golang.org/lookup/github.com/throttled/throttled@v2.2.2+incompatible, but for some reason it is attempting to go to the URL without the +incompatible suffix.

It also seems strange that it is attempting to download the source package when I have a replace directive present pointing to the fork. Possibly it shouldn't be doing that either?

Context

The above example is a reduced example of an error I'm seeing while I'm trying to migrate a larger Go repo from dep to Modules (stellar/go#1634).

@FiloSottile FiloSottile changed the title go/cmd: replace directive on +incompatible version fails on verifying with sum.golang.org consistently cmd/go: replace directive on +incompatible version fails on verifying with sum.golang.org consistently Aug 23, 2019

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Aug 23, 2019

/cc @bcmills

@FiloSottile FiloSottile added this to the Go1.14 milestone Aug 23, 2019

@leighmcculloch

This comment has been minimized.

Copy link
Contributor Author

commented Aug 23, 2019

I tried this on Go 1.12.9, and the go mod tidy command works as expected there, so this only seems to be broken on Go 1.13rc1. I assume that's because in 1.12 there is no call out to the sumdb.

leighmcculloch added a commit to stellar/go that referenced this issue Aug 26, 2019
services/horizon: make our fork of throttled the primary dependency (#…
…1642)

Make @bartekn's fork of the `throttled` dependency the primary dependency for that dependency instead of being just a custom source, which is now hosted at `github.com/stellar/throttled`.

This is part of the initiative to move to Modules (#1634) and is a small change to simplify our Gopkg.toml/lock files to make that transition possible and simpler.

Using the fork seems to be confusing the go tooling in go1.12 and go1.13rc1, with both behaving a little differently. There is an issue open golang/go#33795 about it. The issue seems to be limited to replacing modules that don't have a `go.mod`, which is the case with the `throttled` module. Since this fork is the only fork of a dependency we use and it is simple package, it is straight forward for us to switch to using the fork fully in name rather than to try and make it work consistently with both versions, and we did that in bartekn/throttled#2. See that issue for more details about why we felt like this is the right move.

For context, in modules a `replace` directive is how we tell the go toolchain that we want to use a fork of a module. However, a replace statement only applies when go commands are executed inside this repo as the main module and so importers of our repo won't be using the fork. This doesn't really matter for `throttled` because we only use it in `internal` and `main` modules of `horizon`, but because of this the go toolchain does care about the original source since importers of our repo will get the original source, not the replacement.

The revision of `stellar/throttled` that we are importing did change. You can see the diff here:
stellar/throttled@c99eef3...89d7581
@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 28, 2019

When you say that go mod tidy “works as expected” in Go 1.12.9, did it actually modify the go.mod file, or did it fail silently (#27063)?

@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 28, 2019

For reference, here is what @heschik and I observe with GOSUMDB=off:

example.com$ GOSUMDB=off go mod tidy
go: downloading github.com/throttled/throttled v2.2.2
go: extracting github.com/throttled/throttled v2.2.2
go: test imports
        github.com/throttled/throttled tested by
        github.com/throttled/throttled.test imports
        github.com/throttled/throttled/store: looping trying to add package

It's trying to resolve github.com/throttled/throttled v2.2.2 because it sees the v2.2.2 tag upstream and for some reason doesn't realize that it should be +incompatible.

I see the same problem (albeit with a worse diagnostic) using go1.12.9:

example.com$ go1.12.9 mod tidy
go: import "test" ->
        import "github.com/throttled/throttled" ->
        test ->
        import "github.com/throttled/throttled/store": looping trying to add package
@leighmcculloch

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

Hmm, trying with a clean modcache in a new container I'm now seeing the exact same error as you when running with go1.12.9. Possibly something in my environment state was causing this to not to happen before? I no longer have the container I was running those tests in unfortunately to inspect it.

It does seem like somewhere the +incompatible is being lost though, even in earlier versions of Go.

@heschik

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

From what I can see, if you start with the go.mod as specified in the original bug, you get one successful tidy, and then after that the error.

FTR, the proximate cause of the problem seems to be that tidy is looking at upstream throttled, which has tests that reference packages (store and children) that are deleted in the replace.

@leighmcculloch

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

The full output of what I see when I run go1.12.9 mod tidy in a new container with the files in the pr description is:

$ go1.12.9 mod tidy                                                                                                                                                                                                                                                                       
go: finding github.com/bartekn/throttled v2.2.3-0.20181102161246-c99eef3ad70a+incompatible
go: downloading github.com/bartekn/throttled v2.2.3-0.20181102161246-c99eef3ad70a+incompatible
go: extracting github.com/bartekn/throttled v2.2.3-0.20181102161246-c99eef3ad70a+incompatible
go: downloading github.com/throttled/throttled v2.2.2
go: finding github.com/hashicorp/golang-lru v0.5.3
go: downloading github.com/hashicorp/golang-lru v0.5.3
go: extracting github.com/throttled/throttled v2.2.2
go: extracting github.com/hashicorp/golang-lru v0.5.3
go: finding github.com/throttled/throttled v2.2.2
go: import "test" ->
        import "github.com/throttled/throttled" ->
        test ->
        import "github.com/throttled/throttled/store": looping trying to add package

Doing same on go1.13rc1, first time I see:

$ go1.13rc1 mod tidy 
go: downloading github.com/bartekn/throttled v2.2.3-0.20181102161246-c99eef3ad70a+incompatible
go: extracting github.com/bartekn/throttled v2.2.3-0.20181102161246-c99eef3ad70a+incompatible
go: downloading github.com/throttled/throttled v2.2.2
go: finding github.com/hashicorp/golang-lru v0.5.3
go: downloading github.com/hashicorp/golang-lru v0.5.3
go: extracting github.com/hashicorp/golang-lru v0.5.3
verifying github.com/throttled/throttled@v2.2.2: github.com/throttled/throttled@v2.2.2: reading https://sum.golang.org/lookup/github.com/throttled/throttled@v2.2.2: 410 Gone

Noteworthy is the line in both outputs that references github.com/throttled/throttled with version v2.2.2 that doesn't have +incompatible.

@heschik I don't see success on the first attempt like you, with either go1.12.9 or go1.13rc1.

@heschik

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

Sorry -- with GOSUMDB=off the first succeeds, then I get the loopy error on the second.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.