cmd/go: tip breaks program flags in "go run" #64738

mvdan · 2023-12-15T12:57:16Z

Go version

go version devel go1.22-d73b4322ed Thu Dec 14 22:24:40 2023 +0000 linux/amd64

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

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/mvdan/.cache/go-build'
GOENV='/home/mvdan/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/mvdan/go/pkg/mod'
GONOPROXY='github.com/cue-unity'
GONOSUMDB='github.com/cue-unity'
GOOS='linux'
GOPATH='/home/mvdan/go'
GOPRIVATE='github.com/cue-unity'
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/mvdan/tip'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/mvdan/tip/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.22-d73b4322ed Thu Dec 14 22:24:40 2023 +0000'
GCCGO='gccgo'
GOAMD64='v3'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
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-build2866565515=/tmp/go-build -gno-record-gcc-switches'

What did you do?

$ go version
go version devel go1.22-d73b4322ed Thu Dec 14 22:24:40 2023 +0000 linux/amd64
$ go run cuelang.org/go/cmd/cue@v0.5.0-beta.5 import -p json
go: -p must be a positive integer: 0
$ /usr/bin/go version
go version go1.21.5 linux/amd64
$ /usr/bin/go run cuelang.org/go/cmd/cue@v0.5.0-beta.5 import -p json
$

What did you expect to see?

Same behavior between Go tip and 1.21; the flag should be passed to the cue program without any error from cmd/go.

What did you see instead?

It seems like cmd/go tries to parse the -p json flag, perhaps not realizing that go run takes an arbitrary number of flags and arguments to pass on to the main package being run:

$ go run -h
usage: go run [build flags] [-exec xprog] package [arguments...]

In my case, import -p json go after the single package argument, so cmd/go should not attempt to parse them.

cc @bcmills I suspect this might be a very recent regression due to https://go-review.googlesource.com/c/go/+/546635. I haven't bisected, but I've also never run into this error before.

The text was updated successfully, but these errors were encountered:

mauri870 · 2023-12-15T13:52:05Z

Confirmed that this is a regression from CL 546635. I see that we are also backporting it to go 1.21 in CL550055.

mauri870 · 2023-12-15T14:40:15Z

@bcmills This appears to be enough to fix the issue, but I'm not sure if it is the correct fix:

diff --git a/src/cmd/go/internal/toolchain/select.go b/src/cmd/go/internal/toolchain/select.go
index 84fa7f685c..835d8117cc 100644
--- a/src/cmd/go/internal/toolchain/select.go
+++ b/src/cmd/go/internal/toolchain/select.go
@@ -15,6 +15,7 @@ import (
        "os"
        "path/filepath"
        "runtime"
+       "slices"
        "strconv"
        "strings"

@@ -556,7 +557,15 @@ func goInstallVersion() bool {

        // Make a best effort to parse flags so that module flags like -modcacherw
        // will take effect (see https://go.dev/issue/64282).
-       args := os.Args[2:]
+
+       // All flags past pkg@version should apply to pkg and not to the go
+       // command itself.
+       pkgIdx := slices.Index(os.Args, arg)
+       if pkgIdx == -1 {
+               return false
+       }
+
+       args := os.Args[2:pkgIdx]
        for len(args) > 0 {
                var err error
                _, args, err = cmdflag.ParseOne(cmdFlags, args)

bcmills · 2023-12-15T14:46:05Z

Thanks for the report! This is a surprising gap in our test coverage. 😅
I'll send a fix today.

gopherbot · 2023-12-15T21:24:30Z

Change https://go.dev/cl/550237 mentions this issue: cmd/go/internal/toolchain: simplify flag parsing and stop at the first non-flag

gopherbot · 2023-12-19T14:25:03Z

Change https://go.dev/cl/551215 mentions this issue: cmd/go/internal/toolchain: revert "make a best effort to parse 'go run' and 'go install' flags"

gopherbot · 2023-12-19T14:44:42Z

Change https://go.dev/cl/551255 mentions this issue: cmd/go: add test case for issue 64738

The straight revert in CL 551215 fixed this issue. Add a test case to make sure we don't reintroduce it. Test case copied from CL 550237 (by bcmills). Fixes #64738. Change-Id: I9654a1fd46fe1a1cc63ee6645a552ec21d720ad0 Reviewed-on: https://go-review.googlesource.com/c/go/+/551255 Reviewed-by: Michael Matloob <matloob@golang.org> Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Bryan Mills <bcmills@google.com>

…n' and 'go install' flags" This caused other problems, and for the purposes of the Go 1.22 release, we can just roll back to the Go 1.21 behavior and then decide in January what the correct path forward is. Revert of CL 546635. Unfixes golang#64282. Fixes golang#64738. For golang#57001. This reverts commit e44b8b1. Change-Id: I78753c76dcd0bc6dbc90caa17f73248c42e5f64a Reviewed-on: https://go-review.googlesource.com/c/go/+/551215 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Than McIntosh <thanm@google.com>

The straight revert in CL 551215 fixed this issue. Add a test case to make sure we don't reintroduce it. Test case copied from CL 550237 (by bcmills). Fixes golang#64738. Change-Id: I9654a1fd46fe1a1cc63ee6645a552ec21d720ad0 Reviewed-on: https://go-review.googlesource.com/c/go/+/551255 Reviewed-by: Michael Matloob <matloob@golang.org> Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Bryan Mills <bcmills@google.com>

mauri870 added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Dec 15, 2023

prattmic added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Dec 15, 2023

gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 15, 2023

bcmills self-assigned this Dec 15, 2023

bcmills added this to the Go1.22 milestone Dec 15, 2023

bcmills added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Dec 15, 2023

gopherbot closed this as completed in 52dbffe Dec 19, 2023

rsc mentioned this issue Dec 19, 2023

cmd/go: flag modcacherw does not take effect in the target package #64282

Closed

dmitshur assigned rsc Dec 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: tip breaks program flags in "go run" #64738

cmd/go: tip breaks program flags in "go run" #64738

mvdan commented Dec 15, 2023

mauri870 commented Dec 15, 2023

mauri870 commented Dec 15, 2023

bcmills commented Dec 15, 2023

gopherbot commented Dec 15, 2023

gopherbot commented Dec 19, 2023

gopherbot commented Dec 19, 2023

cmd/go: tip breaks program flags in "go run" #64738

cmd/go: tip breaks program flags in "go run" #64738

Comments

mvdan commented Dec 15, 2023

Go version

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

What did you do?

What did you expect to see?

What did you see instead?

mauri870 commented Dec 15, 2023

mauri870 commented Dec 15, 2023

bcmills commented Dec 15, 2023

gopherbot commented Dec 15, 2023

gopherbot commented Dec 19, 2023

gopherbot commented Dec 19, 2023

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