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 edit doesn't respect direct/indirect blocks in go.mod #51983

Closed
andig opened this issue Mar 28, 2022 · 11 comments
Closed

cmd/go: mod edit doesn't respect direct/indirect blocks in go.mod #51983

andig opened this issue Mar 28, 2022 · 11 comments
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Unfortunate
Milestone

Comments

@andig
Copy link
Contributor

andig commented Mar 28, 2022

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

$ go version
go version go1.18 darwin/arm64

Does this issue reproduce with the latest release?

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

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/andig/Library/Caches/go-build"
GOENV="/Users/andig/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/andig/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/andig/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.18/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.18/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.18"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/andig/htdocs/evcc/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/sv/rs_453y57xj86xsbz3kw1mbc0000gn/T/go-build1274200129=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.18 darwin/arm64
GOROOT/bin/go tool compile -V: compile version go1.18
uname -v: Darwin Kernel Version 21.4.0: Mon Feb 21 20:36:53 PST 2022; root:xnu-8020.101.4~2/RELEASE_ARM64_T8101
ProductName:	macOS
ProductVersion:	12.3
BuildVersion:	21E230
lldb --version: lldb-1316.0.9.41
Apple Swift version 5.6 (swiftlang-5.6.0.323.62 clang-1316.0.20.8)

What did you do?

Repo at evcc-io/evcc@6f2abf4:

go get github.com/panta/machineid

go.mod requires 1.17 at this point.

What did you expect to see?

Single require section.

What did you see instead?

2nd require section is added in go.mod:

require github.com/panta/machineid v1.0.1 // indirect

Replacing

github.com/denisbrodbeck/machineid

with

github.com/panta/machineid

in the code and running

go mod tidy

does not remove the 2nd require section.

UPDATE I think the root cause of

I think it's due to the require sections being impure

is in #51983 (comment).

@seankhliao
Copy link
Member

I think it's due to the require sections being impure: github.com/mergermarket/go-pkcs7 is a direct dependency but it's placed in the indirect section. Moving sections for it (or just dropping the dependency) results in it getting placed in the expected section.

cc @bcmills @matloob

@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Mar 28, 2022
@andig
Copy link
Contributor Author

andig commented Mar 28, 2022

Interesting. When I

go mod tidy

before the go get, go-pkcs7 is not being moved from the indirect to the direct section. Wondering why it's in the indirect section?

Update

When I

git reset d8a6efc35a6e70be4bb3b086b678a10a5046fe30
go mod tidy

to before go-pkcs7 was added and reset the go.mod before the tidy, go-pkcs7 appears in the direct section.

So question is why go mod tidy doesn't move it from indirect to direct section.

@seankhliao seankhliao changed the title cmd/go: go get creates duplicate require section cmd/go: mod tidy doesn't move direct dependency from indirect to direct block Mar 28, 2022
@andig andig closed this as completed Apr 6, 2022
@andig
Copy link
Contributor Author

andig commented Apr 13, 2022

Notice that #52296 has a flow to produce a go.mod with two separate require blocks where the latter contains not only indirect files. Looks like an issue in go mod.

@andig andig reopened this Apr 13, 2022
@andig andig changed the title cmd/go: mod tidy doesn't move direct dependency from indirect to direct block cmd/go: mod mixes direct and indirect dependencies in second block when upgrading packages Apr 13, 2022
@thaJeztah
Copy link
Contributor

Notice that #52296 has a flow to produce a go.mod with two separate require blocks where the latter contains not only indirect files. Looks like an issue in go mod.

Ah, yes, I noticed that as well; looks like a bug

@seankhliao seankhliao changed the title cmd/go: mod mixes direct and indirect dependencies in second block when upgrading packages cmd/go: mod edit doesn't respect direct/indirect blocks in go.mod Apr 13, 2022
@seankhliao
Copy link
Member

That seems like a different issue than the one originally reported here

@andig
Copy link
Contributor Author

andig commented Apr 14, 2022

@seankhliao I think that‘s actually the root cause of this issue as it creates the impure section. #52296 is about module graph evaluation but it does accidentally show how to create an impure section which I wasn‘t able to reproduce otherwise (and hence closed the issue). With the repro it should be possible to fix the root cause.

@bcmills
Copy link
Member

bcmills commented Apr 19, 2022

@andig, could you summarize the repro steps here? (But, to set expectations: if they involve go mod edit, that's probably not going to be worth fixing. go get and go mod tidy are the supported ways to edit dependencies with correct direct/indirect markings. 😅)

@andig
Copy link
Contributor Author

andig commented Apr 20, 2022

It is indeed caused by go mod edit:

mkdir foobar && cd foobar

cat > main.go <<EOF
package main

import (
    "github.com/sirupsen/logrus"
)

func main() {
    logrus.Info("Hello foobar")
}
EOF

go mod init foobar

go mod edit -require github.com/sirupsen/logrus@v1.7.0
go mod tidy

cat go.mod

cat > main.go <<EOF
package main

import (
    "github.com/containerd/containerd/pkg/apparmor"
    "github.com/sirupsen/logrus"
)

func main() {
    if apparmor.HostSupports() {
        logrus.Infof("Running Foobar Deluxe, with AppArmor")
    } else {
        logrus.Infof("Running Foobar Basic")
    }
}
EOF

# >>> the next step causes the problem
go mod edit -require github.com/containerd/containerd@v1.6.2
go mod tidy

cat go.mod
module foobar

go 1.18

require github.com/sirupsen/logrus v1.8.1

require (
	github.com/containerd/containerd v1.6.2
	golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e // indirect
)

@andig
Copy link
Contributor Author

andig commented Apr 20, 2022

It is indeed caused by go mod edit:

@bcmills That being said, it would be nice if tidy caught the problem and corrected the indirect block? It wouldn't be tidy otherwise ;)

@bcmills
Copy link
Member

bcmills commented Apr 21, 2022

@andig, we've rewritten the direct-organizing logic at least twice now to try to fix the heuristic. (It intentionally doesn't mess with existing block structure, in case projects have their own block structure they're trying to maintain.)

I think the only viable improvement would be to give up on maintaining existing structure entirely. That would need a full proposal (since it would be a visible change in behavior), but even then there are all kinds of thorny issues around comment handling that I really don't have the bandwidth to address. 😩

@bcmills bcmills added this to the Unplanned milestone Apr 21, 2022
@andig
Copy link
Contributor Author

andig commented Apr 21, 2022

No problem. Then let‘s just close this issue? +1 for the „unfortunate“ label :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Unfortunate
Projects
None yet
Development

No branches or pull requests

5 participants