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 download" doesn't record checksums for replacements in workspace mode #51946

Closed
h31 opened this issue Mar 25, 2022 · 10 comments
Closed
Assignees
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@h31
Copy link

h31 commented Mar 25, 2022

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

$ go version
go version go1.18 darwin/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="/Users/artyom/Library/Caches/go-build"
GOENV="/Users/artyom/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/artyom/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/artyom/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.18/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.18/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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 x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/pr/5k6c2yx97cjb9t5fh0httxv80000gn/T/go-build740782791=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

  • git clone https://github.com/sergunya/my_workspace; cd my_workspace/
  • rm go.work.sum
  • go clean -modcache
  • cd cmd/my_app/
  • go mod download
  • go list -m -json all

What did you expect to see?

github.com/samber/lo@v1.3.2-0.20220305174447-4edabde35217 necessary to build my_app module is downloaded and has a "Dir" property in go list output.

What did you see instead?

There's no "Dir" property.

I tried to debug Go source code and found that checksumOk("") in src/cmd/go/internal/modload/build.go:275 returns false for github.com/samber/lo. Building my_app using go build adds the necessary checksum to go.work.sum and the issue disappears. It doesn't help to run go mod tidy or go work sync before go mod download.

@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
@seankhliao
Copy link
Member

cc @bcmills @matloob

@matloob matloob self-assigned this May 11, 2022
@bcmills
Copy link
Contributor

bcmills commented May 11, 2022

This may be a bad interaction with #44435.

@matloob matloob added this to the Go1.19 milestone May 11, 2022
@ianlancetaylor
Copy link
Contributor

@bcmills @matloob This issue is marked for 1.19. Should it move to Backlog? Thanks.

@bcmills
Copy link
Contributor

bcmills commented Jul 14, 2022

I attempted to diagnose this issue, and the diagnostic steps revealed a livelock in the module graph computation (filed as #53874). Once that is resolved we should revisit this issue promptly.

@bcmills bcmills assigned bcmills and unassigned matloob Jul 14, 2022
@bcmills
Copy link
Contributor

bcmills commented Jul 14, 2022

With CL 417594 (the fix for #53874) patched in, I see the following:

.gopath/src/my_workspace/cmd/my_app$ go clean -modcache

.gopath/src/my_workspace/cmd/my_app$ GOPROXY=off go build -o /dev/null .
main.go:4:2: github.com/rs/zerolog@v1.26.1: module lookup disabled by GOPROXY=off
main.go:5:2: github.com/rs/zerolog@v1.26.1: module lookup disabled by GOPROXY=off
main.go:6:2: github.com/rs/zerolog@v1.26.1: module lookup disabled by GOPROXY=off

.gopath/src/my_workspace/cmd/my_app$ go mod download

.gopath/src/my_workspace/cmd/my_app$ GOPROXY=off go build -o /dev/null .

And go mod download -json reports:

{
        "Path": "github.com/samber/lo",
        "Version": "v1.3.2-0.20220305174447-4edabde35217",
        "Info": "/tmp/tmp.gAss6FA3Dd/.gopath/pkg/mod/cache/download/github.com/samber/lo/@v/v1.3.2-0.20220305174447-4edabde35217.info",
        "GoMod": "/tmp/tmp.gAss6FA3Dd/.gopath/pkg/mod/cache/download/github.com/samber/lo/@v/v1.3.2-0.20220305174447-4edabde35217.mod",
        "Zip": "/tmp/tmp.gAss6FA3Dd/.gopath/pkg/mod/cache/download/github.com/samber/lo/@v/v1.3.2-0.20220305174447-4edabde35217.zip",
        "Dir": "/tmp/tmp.gAss6FA3Dd/.gopath/pkg/mod/github.com/samber/lo@v1.3.2-0.20220305174447-4edabde35217",
        "Sum": "h1:6ALV695vge95bcdzsU1NfpRBWzo1/PGW7k5COD63xag=",
        "GoModSum": "h1:2I7tgIv8Q1SG2xEIkRq0F2i2zgxVpnyPOP0d3Gj2r+A="
}

@h31, could you confirm whether that patch is a complete fix for your issue?

@bcmills bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 14, 2022
@bcmills
Copy link
Contributor

bcmills commented Jul 14, 2022

Hmm. With the patch applied, the go mod download step now does correctly download the replacement module, but it doesn't record the checksum in go.work.sum, and that causes go list -m not to report the directory. The subsequent go build command populates the entry in go.work.sum, which then allows go list to report the directory.

That seems off: it seems to me that the go mod download step should record the entry in go.work.sum.

.gopath/src/my_workspace/cmd/my_app$ rm ../../go.work.sum

.gopath/src/my_workspace/cmd/my_app$ go clean -modcache

.gopath/src/my_workspace/cmd/my_app$ go mod download

.gopath/src/my_workspace/cmd/my_app$ GOPROXY=off go list -m -json=Dir github.com/samber/lo
{}

.gopath/src/my_workspace/cmd/my_app$ GOPROXY=off go build -o /dev/null .

.gopath/src/my_workspace/cmd/my_app$ GOPROXY=off go list -m -json=Dir github.com/samber/lo
{
        "Dir": "/tmp/tmp.gAss6FA3Dd/.gopath/pkg/mod/github.com/samber/lo@v1.3.2-0.20220305174447-4edabde35217"
}

@bcmills bcmills changed the title cmd/go: "go mod download" doesn't fetch necessary modules in workspace mode cmd/go: "go mod download" doesn't record checksums for replacements in workspace mode Jul 14, 2022
@bcmills bcmills removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 14, 2022
@bcmills
Copy link
Contributor

bcmills commented Jul 14, 2022

The missing checksums are closely related to this TODO:
https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/modcmd/download.go;l=129-132;drc=5f305ae8e5796ea3821088863a6842117c58da72

If go mod tidy has been run within an individual module, then (at go 1.17 and higher) the main module's go.mod file lists all of the dependencies needed to build and test every package within the main module.

However, that does not hold in workspace mode: one module in the workspace can upgrade another's dependencies, and if the dependencies include nested modules, the act of upgrading can move packages from one module into or out of another module whose path is a shared prefix. Moreover, if import statements are added or removed in an upgraded package, the set of transitive dependencies needed to build that package may also change.

So in workspace mode, go mod download downloads a much more conservative set of module dependencies. Perhaps in that case we should update go.work.sum with checksums for that full set as well.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/417654 mentions this issue: cmd/go: save zip sums for downloaded modules in 'go mod download' in a workspace

@bcmills
Copy link
Contributor

bcmills commented Jul 14, 2022

Found the fix and it's tiny. This might make 1.19 after all.

@bcmills bcmills modified the milestones: Go1.20, Go1.19 Jul 14, 2022
@gopherbot gopherbot moved this to Done in Release Blockers Jul 14, 2022
@bcmills
Copy link
Contributor

bcmills commented Jul 14, 2022

@h31, this should be fixed at HEAD and in the final go 1.19 (and any remaining release candidates).

In the meantime, you should be able to work around this issue by using go list all to populate go.work.sum before calling go list -m -json:

.gopath/src/my_workspace$ rm go.work.sum; (cd cmd/my_app && go1.18.4 clean -modcache && go1.18.4 mod download && go1.18.4 list -f '{{""}}' all && go1.18.4 list -m -json github.com/samber/lo)
{
        "Path": "github.com/samber/lo",
        "Version": "v1.3.1",
        "Replace": {
                "Path": "github.com/samber/lo",
                "Version": "v1.3.2-0.20220305174447-4edabde35217",
                "Time": "2022-03-05T17:44:47Z",
                "Dir": "/tmp/tmp.gAss6FA3Dd/.gopath/pkg/mod/github.com/samber/lo@v1.3.2-0.20220305174447-4edabde35217",
                "GoMod": "/tmp/tmp.gAss6FA3Dd/.gopath/pkg/mod/cache/download/github.com/samber/lo/@v/v1.3.2-0.20220305174447-4edabde35217.mod",
                "GoVersion": "1.18"
        },
        "Indirect": true,
        "Dir": "/tmp/tmp.gAss6FA3Dd/.gopath/pkg/mod/github.com/samber/lo@v1.3.2-0.20220305174447-4edabde35217",
        "GoMod": "/tmp/tmp.gAss6FA3Dd/.gopath/pkg/mod/cache/download/github.com/samber/lo/@v/v1.3.2-0.20220305174447-4edabde35217.mod",
        "GoVersion": "1.18"
}

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 1, 2022
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
…a workspace

Within a single module we expect all needed checksums to have already
been recorded by a previous call to 'go get' or 'go mod tidy' in that
module. However, when we combine multiple modules in a workspace, they
may upgrade each other's dependencies, so a given module might be
upgraded above the highest version recorded in the individual go.sum
files for the workspace modules.

Since the checksums might not be present in individual go.sum files,
record them in go.work.sum.

Fixes golang#51946.

Change-Id: Icb4ea874b9e5c5b1950d42650974a24b5d6543d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/417654
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
@golang golang locked and limited conversation to collaborators Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
Status: Done
Development

No branches or pull requests

7 participants