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 run -mod=mod [files...] does not update go.mod and go.sum #52331

Closed
itchyny opened this issue Apr 13, 2022 · 12 comments
Closed

cmd/go: go run -mod=mod [files...] does not update go.mod and go.sum #52331

itchyny opened this issue Apr 13, 2022 · 12 comments
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@itchyny
Copy link
Contributor

itchyny commented Apr 13, 2022

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

$ go version
go version go1.18.1 darwin/arm64

Does this issue reproduce with the latest release?

Yes, reproducible with 1.18.1.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/itchyny/Library/Caches/go-build"
GOENV="/Users/itchyny/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/itchyny/.share/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/itchyny/.share/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.18.1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.18.1/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
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 arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/62/9dw3btnn6_s9skw9ps7bhz2m0000gn/T/go-build3182765003=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

main.go

package main

import "golang.org/x/sys/unix"

func main() {
	println(unix.Getpid())
}

go.mod

module tmp

go 1.17

No go.sum file.

Execute go run -mod=mod main.go.

What did you expect to see?

go run -mod=mod main.go runs the code and update the dependency to go.mod and go.sum, just like go run -mod=mod ..
This is the behavior of Go 1.17.9.

What did you see instead?

go run -mod=mod main.go resolves the package golang.org/x/sys/unix, and runs the code, but does not add the dependency to go.mod. The file go.sum is not created.
Alternatively if this is an intended behavior, document in Go Modules Reference that even with -mod=mod, go run [files...] does not update go.mod and go.sum, while go run . does.

@seankhliao
Copy link
Member

This is intentional from CL 339170

@itchyny
Copy link
Contributor Author

itchyny commented Apr 14, 2022

Thank you for notifying the change is intentional, and the pointer for CL. Let me clarify the behaviors of go run (and other build commands) and module mode. Specifying build target files reads the module files, uses the version or downloads modules, but never writes to the module files.

Command Module Missing dependency Update go.mod and go.sum
go run -mod=mod main.go mod Lookup and download module No (this issue)
go run -mod=readonly main.go readonly Error No
go run -mod=mod . mod Lookup and download module Yes
go run -mod=readonly . readonly Error No

@dmitshur
Copy link
Contributor

CC @matloob.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 14, 2022
@dmitshur dmitshur added this to the Backlog milestone Apr 14, 2022
@dmitshur dmitshur added the GoCommand cmd/go label Apr 14, 2022
@bcmills bcmills self-assigned this Apr 20, 2022
@bcmills bcmills modified the milestones: Backlog, Go1.19 Apr 20, 2022
@bcmills
Copy link
Member

bcmills commented Apr 20, 2022

This is intentional from CL 339170

I don't think it is — at least, that was not my intent during code review. 😅

If go run -mod=mod main.go ends up resolving module dependencies, the go.mod and go.sum files should be updated so that go run main.go would run the same binary using the same dependencies. The fact that the new dependency is (in some sense) “not relevant” to the main module should not change that behavior.

@seankhliao
Copy link
Member

wouldn't that leave it in an inconsistent or non reproducible state compared to go mod tidy, given that go run main.go ignores build constraints?

@bcmills
Copy link
Member

bcmills commented Apr 20, 2022

Yes, it would leave the module untidy — but lots of things (like go get) can leave the module untidy if you run them on packages outside of the main module.

@bcmills
Copy link
Member

bcmills commented Apr 21, 2022

I can reproduce this is a standalone cmd/go test, and am running a git bisect to try to identify the change that introduced it.

@bcmills
Copy link
Member

bcmills commented Apr 21, 2022

git bisect complete, attributed to CL 349600 (which looks very plausible).

b00222fcdd9f2aeb426887b005865eca1aec3631 is the first bad commit
commit b00222fcdd9f2aeb426887b005865eca1aec3631
Author: Jay Conrod <jayconrod@google.com>
Date:   Mon Sep 13 16:48:53 2021 -0700

    cmd/go: refactor {Allow,Disallow}WriteGoMod to ExplicitWriteGoMod

    Subcommands may now set the global flag modload.ExplicitWriteGoMod
    instead of calling {Allow,Disallow}WriteGoMod.

    When ExplicitWriteGoMod is false (default), modload.LoadPackages and
    ListModules will either update go.mod and go.sum or report an error if
    they need to be updated, depending on cfg.BuildMod.

    When ExplicitWriteGoMod is true, commands must explicitly call
    modload.WriteGoMod to update go.mod and go.sum or report an
    error. Commands that perform some operation after loading the build
    list (like downloading zips or building packages) and commands that
    load packages multiple times should set this. For now, only 'go get'
    and 'go mod download' set this.

    This CL is a pure refactor: no change in behavior is expected.
    There are some other minor changes in here, too: commitRequirements no
    longer sets the global requirements: that should be done separately first.

    Change-Id: I69942a808bb177faf7904a53aaf2d4ac68500e82
    Reviewed-on: https://go-review.googlesource.com/c/go/+/349600
    Trust: Jay Conrod <jayconrod@google.com>
    Reviewed-by: Bryan C. Mills <bcmills@google.com>

 src/cmd/go/internal/modcmd/download.go   |  22 ++++--
 src/cmd/go/internal/modcmd/why.go        |   1 +
 src/cmd/go/internal/modget/get.go        |   8 +-
 src/cmd/go/internal/modload/buildlist.go |   5 +-
 src/cmd/go/internal/modload/init.go      | 130 +++++++++++++------------------
 src/cmd/go/internal/modload/list.go      |   5 +-
 src/cmd/go/internal/modload/load.go      |  18 ++++-
 7 files changed, 96 insertions(+), 93 deletions(-)

@bcmills
Copy link
Member

bcmills commented Apr 21, 2022

@gopherbot, please backport to Go 1.18. This is a regression in the behavior of go run, and results in non-reproducible commands.

@gopherbot
Copy link

Backport issue(s) opened: #52468 (for 1.18).

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

@gopherbot
Copy link

Change https://go.dev/cl/401536 mentions this issue: cmd/go: write changes to go.mod and go.sum after loading the command-line-arguments package

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. and removed Documentation labels Apr 21, 2022
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 21, 2022
@gopherbot
Copy link

Change https://go.dev/cl/404094 mentions this issue: [release-branch.go1.18] cmd/go: write changes to go.mod and go.sum after loading the command-line-arguments package

gopherbot pushed a commit that referenced this issue May 9, 2022
…ter loading the command-line-arguments package

This entrypoint was missed in CL 349600, and the behavior happened not
to be covered by existing tests.

Updates #52331.
Fixes #52468.

Change-Id: Iccf12e8e633215abe4bfa1c3ca2fe3a8391b5ba5
Reviewed-on: https://go-review.googlesource.com/c/go/+/401536
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
(cherry picked from commit cf69725)
Reviewed-on: https://go-review.googlesource.com/c/go/+/404094
Reviewed-by: David Chase <drchase@google.com>
@rsc rsc unassigned bcmills Jun 22, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 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. release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants