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' sometimes adds new, unnecessary require sections #67948

Closed
cespare opened this issue Jun 12, 2024 · 8 comments
Closed

cmd/go: 'mod tidy' sometimes adds new, unnecessary require sections #67948

cespare opened this issue Jun 12, 2024 · 8 comments
Assignees
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@cespare
Copy link
Contributor

cespare commented Jun 12, 2024

Go version

go version go1.22.3 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN='/home/caleb/bin'
GOCACHE='/home/caleb/.cache/go-build'
GOENV='/home/caleb/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/caleb/p/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/caleb/p/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/caleb/apps/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/caleb/apps/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.3'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/caleb/3p/pomerium/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4079397003=/tmp/go-build -gno-record-gcc-switches'

What did you do?

This is spun off of #56471. That proposal suggests having go mod tidy automatically join multiple require sections. However, there's an underlying bug where go mod tidy itself is often responsible for these multiple sections. This issue is about the bug.


On a large private Go module at work, we have three require sections. These were all created by go mod tidy. The first one has direct dependencies and the second and third require sections have indirect dependencies.

We can reproduce the error easily: if we delete the second require section (the first of the two indirect sections) and then run go mod tidy, it recreates the separate indirect require section rather than adding the missing entries to the existing require section.

I cannot share the work code, but by looking through issues linked to #56471, I located a public repo where the same issue occurred. Here's how to reproduce the issue using that repo:

$ git clone https://github.com/pomerium/pomerium
...
$ cd pomerium
$ git checkout -q f54b1a7d098fac76c7ddbccdb9351483466b7b6a
# Observe that the go.mod contains three sections.
# Delete the second section.
$ sed -i '78,123d' go.mod
# Now go.mod contains two sections, direct and indirect.
$ go mod tidy
...
# Observe that go.mod contains three sections again.
# 'go mod tidy' added a section.

What did you see happen?

go mod tidy added a new extra indirect require section.

What did you expect to see?

I would expect go mod tidy to add entries to the existing require section.

More generally: if I only ever edit go.mod automatically, never by hand, I would expect the Go tool to maintain at most two require sections (one direct and one indirect).

Note that it is somewhat possible to work around the issue. If you manually join the two require sections, go mod tidy will keep them together as one large section (it won't split them up). But this manual work shouldn't be required. Also, it's only temporary: future additions to the code may cause go mod tidy to add new sections to go.mod.

@cespare
Copy link
Contributor Author

cespare commented Jun 12, 2024

/cc @matloob @samthanawalla per owners

@matloob matloob added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Jun 12, 2024
@thediveo
Copy link

thediveo commented Jun 12, 2024

I can probably offer another example: https://github.com/siemens/turtlefinder/blob/f21b66938827bb8d9ec1a720b34c870465e5620e/go.mod

The third require section in this case has even a mix of direct and indirect if I understand it correctly.

@samthanawalla samthanawalla self-assigned this Jun 27, 2024
klauspost added a commit to klauspost/mc that referenced this issue Jul 3, 2024
Combine into direct and indirect sections. See golang/go#67948 for why this is a mess.

Enables running `go mod tidy` without blowing up things.
harshavardhana pushed a commit to minio/mc that referenced this issue Jul 3, 2024
Combine into direct and indirect sections. See golang/go#67948 for why this is a mess.

Enables running `go mod tidy` without blowing up things.
@matloob
Copy link
Contributor

matloob commented Sep 30, 2024

I just had a chance to look into this. On the pomerium@f54b1a7d098fac76c7ddbccdb9351483466b7b6a example the behavior is as expected (though perhaps we should change the expectation). The third require block has a mix of direct and indirect requirements. When the go mod tidy is looking for the direct and indirect blocks to add requirements to, it expects a block with all direct requirements and another with all indirect requirements. If no such block is found it creates a new one.

So when the second block is removed, there's a block with all direct requirements, and one that's a mix. go mod tidy sees that there's no block with all indirect requirements, so it creates a new block to hold the indirect requirements.

@cespare Is this also the case for your go.mod file at work? Are the direct and indirect blocks all direct and all indirect before go mod tidy is run?

@thediveo Yes, go mod tidy will preserve three blocks. It sees that the first has all direct requirements, the second has all indirect requirements, and the third has a mix. So it will move new direct requirements (and requirements changing from direct to indirect) to the first require block, and indirect requirements will go into the second require block.

@matloob matloob added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Sep 30, 2024
@cespare
Copy link
Contributor Author

cespare commented Sep 30, 2024

@matloob Ah! You're correct. In my private go.mod there are two direct requirements hidden amongst 53 indirect requirements and I missed them.

I'm not sure how they got there but it seems likely it was due to human editing of the file. (If I simply add a new direct import of a previously-indirect dependency, I verified that go mod tidy moves the requirement into the other block when it deletes the // indirect comment.)

Anyway, I think this bug is invalid but it'd still be great to see #56471 happen.

@cespare cespare closed this as not planned Won't fix, can't repro, duplicate, stale Sep 30, 2024
@matloob
Copy link
Contributor

matloob commented Sep 30, 2024

I think this issue gives some support that we should do something. I think we have evidence that it's easy to get into a confusing state (having a requirement marked direct buried among requirements marked indirect) and we should have a better way to get out of it.

@thediveo
Copy link

Is the three block design intended, als if so, why? Or should it be always only two blocks?

@matloob
Copy link
Contributor

matloob commented Sep 30, 2024

@thediveo The design we have was intended: we wanted to allow users to be able to define their own sections that we'd leave alone. So we only modify the all-direct and all-indirect sections. But I think it's probably the case that most people don't want the third section. I think we'll need to figure out if anyone's intentionally using the functionality that maintains the third section and if not, maybe change the default to always produce two sections. See #56471 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants