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: mod tidy -go=1.17 should move indirect dependencies to the second require part #47733

Open
FireFart opened this issue Aug 17, 2021 · 10 comments

Comments

@FireFart
Copy link

@FireFart FireFart commented Aug 17, 2021

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

$ go version
go version go1.17 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=""
GOCACHE="/home/firefart/.cache/go-build"
GOENV="/home/firefart/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/firefart/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/firefart/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/firefart/code/project/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-build661731482=/tmp/go-build -gno-record-gcc-switches"

What did you do?

ran go mod tidy -go=1.17 as described on https://golang.org/doc/go1.17

What did you expect to see?

I expected to see that the // indirect dependencies in my go.mod file get moved to the second indirect require part of the file.

What did you see instead?

Some new indirect dependencies were added but the old // indirect are left untouched. If I remove the indirect entries from the first require block and run the command again, they are only added to the bottom. This is no deal breaker but I think this could be improved to make the go.mod file a bit clearer by also moving all previous indirects to the second block.

Because the number of explicit requirements may be substantially larger in an expanded Go 1.17 go.mod file, the newly-added requirements on indirect dependencies in a go 1.17 module are maintained in a separate require block from the block containing direct dependencies.
@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Aug 17, 2021

Can you share your go.mod?

@FireFart
Copy link
Author

@FireFart FireFart commented Aug 17, 2021

Sure here is my go.mod from go 1.16

module xxxx/yyy

go 1.16

require (
	github.com/DATA-DOG/go-sqlmock v1.5.0
	github.com/aws/aws-sdk-go-v2 v1.8.0
	github.com/aws/aws-sdk-go-v2/credentials v1.3.2
	github.com/aws/aws-sdk-go-v2/service/s3 v1.12.0
	github.com/aws/aws-sdk-go-v2/service/sesv2 v1.4.2
	github.com/cloudflare/cloudflare-go v0.20.0
	github.com/felixge/httpsnoop v1.0.2 // indirect
	github.com/go-redis/redis/v8 v8.11.3
	github.com/go-sql-driver/mysql v1.6.0
	github.com/gorilla/handlers v1.5.1
	github.com/gorilla/mux v1.8.0
	github.com/prometheus/client_golang v1.11.0
	github.com/prometheus/common v0.30.0 // indirect
	github.com/prometheus/procfs v0.7.2 // indirect
	github.com/sirupsen/logrus v1.8.1
	github.com/slack-go/slack v0.9.4
	github.com/streadway/amqp v1.0.0
	github.com/stripe/stripe-go/v72 v72.60.0
	golang.org/x/net v0.0.0-20210813160813-60bc85c4be6d // indirect
	golang.org/x/sys v0.0.0-20210816074244-15123e1e1f71 // indirect
	golang.org/x/text v0.3.7 // indirect
	golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect
	google.golang.org/genproto v0.0.0-20210813162853-db860fec028c // indirect
	google.golang.org/grpc v1.40.0
	google.golang.org/protobuf v1.27.1
)

@andig
Copy link
Contributor

@andig andig commented Aug 17, 2021

I can confirm this finding.

I've also noticed another "weird" behaviour: while the second part is generated during go mod tidy -go=1.17, if I merge the two require sections and run the command again, it will remain a single section. This seems a bit inconsistent as running tidy multiple times will lead to different results.

@seankhliao seankhliao changed the title go mod tidy -go=1.17 should move indirect dependencies to the second require part cmd/go: mod tidy -go=1.17 should move indirect dependencies to the second require part Aug 17, 2021
@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Aug 17, 2021

@jayconrod jayconrod added this to the Go1.18 milestone Aug 17, 2021
@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Aug 17, 2021

@gopherbot Please backport to 1.17. This is annoying behavior in a new feature, and the fix shouldn't be very disruptive.

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 17, 2021

Backport issue(s) opened: #47756 (for 1.17).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 18, 2021

Change https://golang.org/cl/343431 mentions this issue: modfile: in SetRequireSeparateIndirect, arrange requirements consistently

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 18, 2021

Change https://golang.org/cl/343432 mentions this issue: cmd/go: write go.mod requirements more consistently for go 1.17+

@thomasf
Copy link

@thomasf thomasf commented Aug 29, 2021

I don't know if it's related but in some projects I now have three require blocks which I don't think is explicitly mentioned in this issue before.

The first one has mixed direct and indirect dependencies, the second one only has direct dependencies and the third one only has indirect dependencies.

The bottom sections are added by go get of previously uninstalled dependencies after the go 1.17 switch.

The release notes only speaks about a second section for indirect dependencies so I don't know if having three sections is considered a bug or not.

module redacted

go 1.17

require (
	github.com/aws/aws-sdk-go v1.40.32
	github.com/beorn7/perks v1.0.1 // indirect
	github.com/buger/jsonparser v1.1.1
	github.com/cespare/xxhash/v2 v2.1.2 // indirect
	github.com/getsentry/sentry-go v0.11.0
	github.com/golang/protobuf v1.5.2 // indirect
	github.com/gorilla/mux v1.8.0
	github.com/jmespath/go-jmespath v0.4.0 // indirect
	github.com/justinas/alice v1.2.0
	github.com/kr/text v0.2.0 // indirect
	github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
	github.com/peterbourgon/ff/v3 v3.1.0
	github.com/pmezard/go-difflib v1.0.0 // indirect
	github.com/prometheus/client_golang v1.11.0
	github.com/prometheus/client_model v0.2.0 // indirect
	github.com/prometheus/common v0.30.0 // indirect
	github.com/prometheus/procfs v0.7.3 // indirect
	github.com/rs/xid v1.3.0
	github.com/rs/zerolog v1.24.0
	github.com/stretchr/testify v1.7.0
	golang.org/x/crypto v0.0.0-20210817164053-32db794688a5
	golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c // indirect
	google.golang.org/protobuf v1.27.1 // indirect
	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
	gopkg.in/natefinch/lumberjack.v2 v2.0.0
	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
)

require (
	github.com/benbjohnson/hashfs v0.1.0
	github.com/davecgh/go-spew v1.1.1
)

require (
	github.com/MicahParks/keyfunc v0.7.0 // indirect
	github.com/dgrijalva/jwt-go v3.2.0+incompatible // indirect
	github.com/form3tech-oss/jwt-go v3.2.5+incompatible // indirect
	github.com/golang-jwt/jwt/v4 v4.0.0 // indirect
	github.com/gorilla/securecookie v1.1.1 // indirect
	github.com/pkg/errors v0.9.1 // indirect
)

If it's supposed to be three sections that I am sorry for making a noise about it if it isn't maybe go mod tidy should be able to fix this situation as well.

EDIT: It is possible that gopls 0.7.1 added a few of those new requirements, I don't know how gopls adds stuff but the majority of the requirements were definitely added by using plain go get ... command line though.

gopherbot pushed a commit to golang/mod that referenced this issue Sep 2, 2021
…ntly

SetRequireSeparateIndirect now makes a stronger attempt to keep
automatically added requirements in two blocks: one containing only
direct requirements and one containing only indirect
requirements. SetRequireSeparateIndirect will find or add these two
blocks (commented blocks are left alone). New requirements are added
to one of these two blocks. Existing requirements may be moved between
these two blocks if their markings change.

SetRequireSeparateIndirect attempts to preserve existing structure in
the file by not adding requirements to or moving requirements from
blocks with block-level comments and blocks other than the last
uncommented direct-only and indirect-only block.

As an exception to aid with migration, if the file contains a single
uncommented block of requirements (as would the be the case if the
user started with a 1.16 go.mod file, changed the go directive to
1.17, then ran 'go mod tidy'), SetRequireSeparateIndirect will split
the block into direct-only and indirect-only blocks.

This is a change in behavior, but it has no semantic effect, and it
should result in cleaner, more stable go.mod files.

For golang/go#47563
For golang/go#47733

Change-Id: Ifa20bb084c6bdaf1e00140600380857de8afa320
Reviewed-on: https://go-review.googlesource.com/c/mod/+/343431
Trust: Jay Conrod <jayconrod@google.com>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 8, 2021

Change https://golang.org/cl/348576 mentions this issue: modfile: in SetRequireSeparateIndirect, convert lines to blocks

gopherbot pushed a commit to golang/mod that referenced this issue Sep 13, 2021
When reading go.mod, SetRequireSeparateIndirect will insert new
requirements into the last uncommented direct-only or indirect-only
block OR line. If the last such statement is a line,
SetRequireSeparateIndirect converts it to a block before inserting new
requirements. Cleanup will convert it back to a line later if no
requirements are inserted.

For golang/go#47733

Change-Id: Id8ee3b0ce2d005488ddb3d9a5349115bd93938e7
Reviewed-on: https://go-review.googlesource.com/c/mod/+/348576
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants