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: 'go mod tidy' breaks our build #45223

Closed
bradfitz opened this issue Mar 24, 2021 · 5 comments
Closed

cmd/go: 'go mod tidy' breaks our build #45223

bradfitz opened this issue Mar 24, 2021 · 5 comments

Comments

@bradfitz
Copy link
Contributor

go mod tidy breaks our build.

Does tidy not work with replace directives? Our closed-source mono/megarepo is a Go module with git submodule directories of a few other [all open source] Go repos. (Yes, painful, and we all wish there was something less terrible.)

In any case, I hear that go mod tidy shouldn't be able to break our builds, and yet:

bradfitz@tsdev:~/src/tailscale.io$ go version
go version go1.16.2 linux/amd64

bradfitz@tsdev:~/src/tailscale.io$ cat go.mod 
module tailscale.io

go 1.13

require (
        github.com/NYTimes/gziphandler v1.1.1
        github.com/apenwarr/fixconsole v0.0.0-20191012055117-5a9f6489cc29
        github.com/apenwarr/git-subtrac v0.0.0-20200907023842-6f55d3a89654
        github.com/aws/aws-sdk-go v1.35.13
        github.com/blakesmith/ar v0.0.0-20190502131153-809d4375e1fb
        github.com/bradfitz/ip2asn v0.0.0-20200601024024-5f8625d7bed5
        github.com/coreos/go-oidc v2.2.1+incompatible
        github.com/coreos/go-semver v0.3.0 // indirect
        github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f // indirect
        github.com/crewjam/saml v0.4.2
        github.com/davecgh/go-spew v1.1.1
        github.com/fsnotify/fsnotify v1.4.10-0.20200417215612-7f4cf4dd2b52
        github.com/gdamore/tcell/v2 v2.0.1-0.20201017141208-acf90d56d591
        github.com/google/go-cmp v0.5.4
        github.com/goreleaser/nfpm v1.10.3 // indirect
        github.com/gorilla/csrf v1.7.0
        github.com/gorilla/handlers v1.4.2
        github.com/jackc/pgconn v1.6.1
        github.com/jackc/pgx/v4 v4.7.1
        github.com/klauspost/compress v1.11.12
        github.com/kr/pty v1.1.8 // indirect
        github.com/lxn/walk v0.0.0-20201110160827-18ea5e372cdb
        github.com/lxn/win v0.0.0-20201111105847-2a20daff6a55
        github.com/mastahyeti/cms v0.0.7
        github.com/mitchellh/go-ps v1.0.0
        github.com/pborman/getopt v0.0.0-20190409184431-ee0cd42419d3
        github.com/peterbourgon/ff/v3 v3.0.0
        github.com/pkg/diff v0.0.0-20200914180035-5b29258ca4f7
        github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 // indirect
        github.com/rivo/tview v0.0.0-20201118063654-f007e9ad3893
        github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027 // indirect
        github.com/tailscale/hujson v0.0.0-20200924210142-dde312d0d6a2
        github.com/tailscale/wireguard-go v0.0.0-20210210202228-3cc76ed5f222
        github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802 // indirect
        go.etcd.io/bbolt v1.3.4
        go.etcd.io/etcd v0.0.0-00010101000000-000000000000
        go4.org/mem v0.0.0-20201119185036-c04c5a6ff174
        golang.org/x/crypto v0.0.0-20210317152858-513c2a44f670
        golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d
        golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
        golang.org/x/sys v0.0.0-20210317225723-c4fcb01b228e
        golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba
        google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 // indirect
        gopkg.in/Knetic/govaluate.v3 v3.0.0 // indirect
        gopkg.in/square/go-jose.v2 v2.4.1 // indirect
        honnef.co/go/tools v0.1.0
        inet.af/netaddr v0.0.0-20210222205655-a1ec2b7b8c44
        inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22 // indirect
        inet.af/peercred v0.0.0-20210318190834-4259e17bb763 // indirect
        tailscale.com v0.0.0-00010101000000-000000000000
)

replace (
        github.com/AllenDang/w32 => github.com/apenwarr/w32 v0.0.0-20190407064951-aa00fec
        github.com/tailscale/winipcfg-go => ./winipcfg-go
        github.com/tailscale/wireguard-go => ./wireguard-go
        go.etcd.io/bbolt => ./bbolt
        go.etcd.io/etcd => ./etcdserver
        tailscale.com => ./oss
)

A build of a tailscale.com target builds here, from the oss/ replace directory:

bradfitz@tsdev:~/src/tailscale.io$ go install tailscale.com/cmd/tailscaled
bradfitz@tsdev:~/src/tailscale.io$ echo $?
0

And then go mod tidy works:

bradfitz@tsdev:~/src/tailscale.io$ go mod tidy
warning: ignoring symlink /home/bradfitz/src/tailscale.io/out/native
bradfitz@tsdev:~/src/tailscale.io$ echo $?
0

Which results in changes:

bradfitz@tsdev:~/src/tailscale.io$ git diff go.mod
diff --git a/go.mod b/go.mod
index 8650959e3..5c41b032d 100644
--- a/go.mod
+++ b/go.mod
@@ -3,6 +3,8 @@ module tailscale.io
 go 1.13
 
 require (
+       cloud.google.com/go v0.46.3 // indirect
+       github.com/Microsoft/go-winio v0.4.16 // indirect
        github.com/NYTimes/gziphandler v1.1.1
        github.com/apenwarr/fixconsole v0.0.0-20191012055117-5a9f6489cc29
        github.com/apenwarr/git-subtrac v0.0.0-20200907023842-6f55d3a89654
@@ -16,14 +18,19 @@ require (
        github.com/davecgh/go-spew v1.1.1
        github.com/fsnotify/fsnotify v1.4.10-0.20200417215612-7f4cf4dd2b52
        github.com/gdamore/tcell/v2 v2.0.1-0.20201017141208-acf90d56d591
+       github.com/go-git/go-git/v5 v5.2.0 // indirect
+       github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef // indirect
+       github.com/golang/protobuf v1.4.2 // indirect
        github.com/google/go-cmp v0.5.4
-       github.com/goreleaser/nfpm v1.10.3 // indirect
+       github.com/google/uuid v1.1.2 // indirect
        github.com/gorilla/csrf v1.7.0
        github.com/gorilla/handlers v1.4.2
+       github.com/gorilla/websocket v1.4.2 // indirect
+       github.com/imdario/mergo v0.3.11 // indirect
        github.com/jackc/pgconn v1.6.1
        github.com/jackc/pgx/v4 v4.7.1
+       github.com/kevinburke/ssh_config v0.0.0-20201106050909-4977a11b4351 // indirect
        github.com/klauspost/compress v1.11.12
-       github.com/kr/pty v1.1.8 // indirect
        github.com/lxn/walk v0.0.0-20201110160827-18ea5e372cdb
        github.com/lxn/win v0.0.0-20201111105847-2a20daff6a55
        github.com/mastahyeti/cms v0.0.7
@@ -33,10 +40,12 @@ require (
        github.com/pkg/diff v0.0.0-20200914180035-5b29258ca4f7
        github.com/pquerna/cachecontrol v0.0.0-20180517163645-1555304b9b35 // indirect
        github.com/rivo/tview v0.0.0-20201118063654-f007e9ad3893
-       github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027 // indirect
+       github.com/sirupsen/logrus v1.7.0 // indirect
+       github.com/spf13/pflag v1.0.5 // indirect
        github.com/tailscale/hujson v0.0.0-20200924210142-dde312d0d6a2
        github.com/tailscale/wireguard-go v0.0.0-20210210202228-3cc76ed5f222
        github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802 // indirect
+       github.com/xanzy/ssh-agent v0.3.0 // indirect
        go.etcd.io/bbolt v1.3.4
        go.etcd.io/etcd v0.0.0-00010101000000-000000000000
        go4.org/mem v0.0.0-20201119185036-c04c5a6ff174
@@ -48,10 +57,10 @@ require (
        google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 // indirect
        gopkg.in/Knetic/govaluate.v3 v3.0.0 // indirect
        gopkg.in/square/go-jose.v2 v2.4.1 // indirect
+       gopkg.in/yaml.v2 v2.4.0 // indirect
+       gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 // indirect
        honnef.co/go/tools v0.1.0
        inet.af/netaddr v0.0.0-20210222205655-a1ec2b7b8c44
-       inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22 // indirect
-       inet.af/peercred v0.0.0-20210318190834-4259e17bb763 // indirect
        tailscale.com v0.0.0-00010101000000-000000000000
 )

And those changes break our build:

bradfitz@tsdev:~/src/tailscale.io$ go install tailscale.com/cmd/tailscaled
oss/wgengine/netstack/netstack.go:21:2: missing go.sum entry for module providing package inet.af/netstack/tcpip (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:22:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/adapters/gonet (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:23:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/buffer (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:24:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/header (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:25:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/link/channel (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:26:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/network/ipv4 (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:27:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/network/ipv6 (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:28:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/stack (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:29:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/transport/icmp (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:30:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/transport/tcp (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:31:2: missing go.sum entry for module providing package inet.af/netstack/tcpip/transport/udp (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/wgengine/netstack/netstack.go:32:2: missing go.sum entry for module providing package inet.af/netstack/waiter (imported by tailscale.com/wgengine/netstack); to add:
        go get tailscale.com/wgengine/netstack@v0.0.0-00010101000000-000000000000
oss/ipn/ipnserver/server.go:31:2: missing go.sum entry for module providing package inet.af/peercred (imported by tailscale.com/ipn/ipnserver); to add:
        go get tailscale.com/ipn/ipnserver@v0.0.0-00010101000000-000000000000

This should be reproducible without our closed-source repo since all the git submodules are public:

bradfitz@tsdev:~/src/tailscale.io$ cat .gitmodules 
[submodule "winipcfg-go"]
        path = winipcfg-go
        url = git@github.com:tailscale/winipcfg-go
[submodule "wireguard-go"]
        path = wireguard-go
        url = git@github.com:tailscale/wireguard-go
[submodule "oss"]
        path = oss
        url = git@github.com:tailscale/tailscale
[submodule "bbolt"]
        path = bbolt
        url = git@github.com:tailscale/bbolt
[submodule "etcdserver"]
        path = etcdserver
        url = git@github.com:tailscale/etcd.git

The revs of each submodule is currently:

160000 commit 5f0daad167277bcb436c01a7533fad01e3994751  bbolt
160000 commit ac33385a8e3ce10e045d9e73a95595ff58d68c9b  etcdserver
160000 commit 77ec80538acd1c01ba4058852be1d9e701a5f1e7  oss
160000 commit 84d0a1a3b210638dfe24d6169581a7af315c13f0  winipcfg-go
160000 commit 3cc76ed5f222ec82748ef3bd8c41d4b059e28cdb  wireguard-go
@bradfitz
Copy link
Contributor Author

I suppose I could prepare a demo repo for this.

bradfitz added a commit to tailscale/go-mod-tidy-broken that referenced this issue Mar 24, 2021
bradfitz added a commit to tailscale/go-mod-tidy-broken that referenced this issue Mar 24, 2021
bradfitz added a commit to tailscale/go-mod-tidy-broken that referenced this issue Mar 24, 2021
bradfitz added a commit to tailscale/go-mod-tidy-broken that referenced this issue Mar 24, 2021
bradfitz added a commit to tailscale/go-mod-tidy-broken that referenced this issue Mar 24, 2021
@bradfitz
Copy link
Contributor Author

It's very likely we're using this wrong. I tried to prepare a repro at https://github.com/tailscale/go-mod-tidy-broken but now I'm hitting something else I don't understand:

bradfitz@tsdev:~/src/github.com/tailscale/go-mod-tidy-broken$ git st
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean

bradfitz@tsdev:~/src/github.com/tailscale/go-mod-tidy-broken$ git rev-parse HEAD
13935ef26c4c951e0c1b8027aa2469ba4de3886b

bradfitz@tsdev:~/src/github.com/tailscale/go-mod-tidy-broken$ go install tailscale.com/cmd/tailscaled

bradfitz@tsdev:~/src/github.com/tailscale/go-mod-tidy-broken$ go mod tidy
go: finding module for package inet.af/netstack
github.com/tailscale/go-mod-tidy-broken imports
        inet.af/netstack: module inet.af/netstack@latest found (v0.0.0-20210317161235-a1bf4e56ef22), but does not contain package inet.af/netstack

bradfitz@tsdev:~/src/github.com/tailscale/go-mod-tidy-broken$ git grep v0.0.0-20210317161235-a1bf4e56ef22
go.mod: inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22 // indirect
go.sum:inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22 h1:DNtszwGa6w76qlIr+PbPEnlBJdiRV8SaxeigOy0q1gg=
go.sum:inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22/go.mod h1:GVx+5OZtbG4TVOW5ilmyRZAZXr1cNwfqUEkTOtWK0PM=

But that is literally the only version:

https://pkg.go.dev/inet.af/netstack@v0.0.0-20210317161235-a1bf4e56ef22?tab=versions

Screen Shot 2021-03-24 at 4 07 52 PM

@seankhliao
Copy link
Member

There are no go files in the root of the inet.af/netstack module, so no package, so you can't import that, in foo.go:

	_ "inet.af/netstack"

@jayconrod
Copy link
Contributor

jayconrod commented Mar 25, 2021

From the first set of error messages, it looks like go mod tidy removed the requirement on inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22. go install still needs that requirement though; it complains about a missing sum on version v0.0.0-00010101000000-000000000000, which I'm guessing is because another module replaced that version with a local directory, and that replacement isn't being applied.

So the question is why go mod tidy removes that requirement? tidy traverses the import graph, starting with the packages in the main module. It makes a list of the modules needed to build the packages it visits. When it's finished, it removes requirements on modules that didn't provide any packages.

tailscale.com/cmd/tailscaled won't automatically be one of the packages that tidy visits, so it's entirely possible that tidy removes some of the required modules needed to build that command if those modules aren't also needed by packages in the main module. There's an idiomatic workaround for this: you can create a file (usually named tools.go) like this in the main module:

// +build tools

package tools

import _ "tailscale.com/cmd/tailscaled"

This package won't actually get built because of the build tag. tidy ignores build constraints except ignore, so it will consider that import.

If that doesn't fix the issue, there are a couple other edge cases: when tidy lists packages in the main module, it doesn't follow symbolic links; it looks like there was a symlink warning for the out/native directory. That could be related? tidy also won't enter testdata directories or directories starting with . or _, but that's less common.

@bradfitz
Copy link
Contributor Author

@seankhliao, doh! Thanks. My attempt at a minimal repro wasn't quite accurate. That error message could've been a bit more helpful, but oh well.

@jayconrod, thanks for the link! I'd been splitting all our package main binaries in two halves (but apparently missed some) so we could empty import the package half. I didn't think using a build tag to hide it from the compiler but not from go mod tidy. Maybe something could give an error if it sees you empty importing a package main, pointing out this trick. But fiddly heuristics like that deep in the compiler also seems gross, so I'd understand the hesitance.

Anyway, sorry for the distraction! I think this'll be enough to get us working again. Still not exactly sure how we got into this state. I suspect Go 1.15 modifying the go.mod files for us papered over some of this previously.

@golang golang locked and limited conversation to collaborators Mar 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants