cmd/go: 'vet' and 'test' duplicate flags from GOFLAGS if they are also specified explicitly #32471

elagergren-spideroak · 2019-06-06T22:38:59Z

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

$ go version
1.12.5

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

go env Output

$ go env
darwin/amd64

What did you do?

Set GOFLAGS=-vet=off then ran go test -vet=all.

What did you expect to see?

go test ... to not error out.

What did you see instead?

$ GOFLAGS=-test.vet=off go test -test.vet all
# _/tmp/bar
./bar_test.go:6:2: Logf format %f has arg X of wrong type string
FAIL	_/tmp/bar [build failed]

$ GOFLAGS=-vet=off go test -vet all
go test: vet flag may be set only once
run "go help test" or "go help testflag" for more information

$ GOFLAGS=-test.vet=off go test -vet all
# _/tmp/bar
./bar_test.go:6:2: Logf format %f has arg X of wrong type string
FAIL	_/tmp/bar [build failed]

$ GOFLAGS=-vet=off go test -test.vet all
go test: vet flag may be set only once
run "go help test" or "go help testflag" for more information

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

elagergren-spideroak · 2019-06-06T23:00:44Z

Similarly, GOFLAGS="-vet=all -vet=off" go test -v does not work. Related to #29053

dmitshur · 2019-06-10T19:09:39Z

/cc @jayconrod @bcmills

bcmills · 2019-06-10T20:43:29Z

What happens for the other flags with the test. prefix, such as -test.v and -test.run?

We should probably at least make -vet consistent with those.

elagergren-spideroak · 2019-06-11T01:53:32Z

$ GOFLAGS=-test.v go test -test.v=false
# _/tmp/bar
./bar_test.go:6:2: Logf format %f has arg X of wrong type string
FAIL	_/tmp/bar [build failed]

$ GOFLAGS=-test.v go test -v=false
# _/tmp/bar
./bar_test.go:6:2: Logf format %f has arg X of wrong type string
FAIL	_/tmp/bar [build failed]

$ GOFLAGS=-v go test -v=false
go test: v flag may be set only once
run "go help test" or "go help testflag" for more information

$ GOFLAGS=-v go test -test.v=false
go test: v flag may be set only once
run "go help test" or "go help testflag" for more information

elagergren-spideroak · 2019-06-11T01:56:02Z

-test.run exhibits the same behavior as -test.v and -test.vet.

agnivade · 2019-11-11T06:08:41Z

The errors seem to be consistent for all flags.

@elagergren-spideroak - It's not clear from the issue description, but for GOFLAGS=-vet=off go test -vet all, did you want the vet=all value to override the vet=off value set in GOFLAGS ?

amenzhinsky · 2019-11-11T08:57:02Z

I'm having the same problem with persistent environment variables, even if I set the corresponding env var to an empty string:

go env -w GOFLAGS="-trimpath"
GOFLAGS= go test -trimpath # or simply "go test -trimpath"

go test: trimpath flag may be set only once
run "go help test" or "go help testflag" for more information

amenzhinsky · 2019-11-11T20:40:05Z

It seems only go test and go vet have the issue, everything is alright with the others.

elagergren-spideroak · 2019-11-11T21:08:50Z

@agnivade Yeah, sorry. From the docs:

GOFLAGS
	...
        Flags listed on the command line
	are applied after this list and therefore override it.

agnivade · 2019-11-12T08:36:15Z

@amenzhinsky - Rightly noted. This was introduced in CL 126656.

Only go/internal/test/testflag.go has f, value, extraWord := cmdflag.Parse(cmd, usage, testFlagDefn, args, i)

and go/internal/vet/vetflag.go has f, value, extraWord := cmdflag.Parse("vet", usage, vetFlagDefn, args, i) due to which this happens.

The args come from modification with cmdflag.FindGOFLAGS(vetFlagDefn)

The other go subcommands have a different code path for flag processing.

Perhaps @rsc @bcmills will know why is this.

amenzhinsky · 2019-11-12T11:04:26Z

I think the easiest fix would be removing this:

go/src/cmd/go/internal/cmdflag/flag.go

Lines 123 to 125 in 99957b6

    
           if f.Present { 
        
           	SyntaxError(cmd, f.Name+" flag may be set only once") 
        
           }

Moreover all subcommands and tools accept flags overriding (correct me if I'm wrong, checked on that vert quickly).

ianlancetaylor · 2019-12-05T21:56:42Z

@bcmills Is this going to happen for 1.14?

bcmills · 2019-12-05T22:16:23Z

Yes.

gopherbot · 2019-12-11T17:16:14Z

Change https://golang.org/cl/210937 mentions this issue: cmd/go: restore default vet analyzers for targets in GOROOT

gopherbot · 2019-12-11T20:15:01Z

Change https://golang.org/cl/210940 mentions this issue: cmd/go: allow arguments to 'go test' and 'go vet' to duplicate or override flags from GOFLAGS

…rride flags from GOFLAGS This is a minimal fix for Go 1.14, but this parsing logic is much too complex and seems like it will cause more trouble going forward. I intend to mail a followup change to refactor this logic for 1.15. Updates #32471 Change-Id: I00ed07dcf3a23c9cd4ffa8cf764921fb5c18bcd6 Reviewed-on: https://go-review.googlesource.com/c/go/+/210940 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Jay Conrod <jayconrod@google.com>

bcmills · 2019-12-11T22:01:46Z

A minimal fix for 1.14 is now merged. I plan to follow that up with a cleaner refactoring for 1.15.

This fixes a regression introduced in CL 209498, found while investigating #32471. Also fix $WORK replacement in cmd/go/internal/work.(*Builder).Showcmd when b.WorkDir includes a backslash and appears in a quoted string. That fix is needed in order to write a precise test that passes under Windows, since Windows directories nearly always include backslashes. Updates #35837 Change-Id: I5fddc5435d5d283a3e598989209d873b59b0a39c Reviewed-on: https://go-review.googlesource.com/c/go/+/210937 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com>

-mod=vendor is not needed if it's already set in GOFLAGS. So lets not set it unless it's needed. This is an issue for commands such as `go test` and `go vet` ( see: golang/go#32471 ). This avois such an issue.

gopherbot · 2019-12-13T22:11:45Z

Change https://golang.org/cl/211358 mentions this issue: cmd/go/internal/{test,vet}: use a standard flag.FlagSet to parse flags

@estroz

* Only add explicit -mod=vendor if needed -mod=vendor is not needed if it's already set in GOFLAGS. So lets not set it unless it's needed. This is an issue for commands such as `go test` and `go vet` ( see: golang/go#32471 ). This avois such an issue. * Add comments for -mod=vendor flag addition This takes into us the suggestions from @estroz and adds the relevant comments for the addition of the -mod=vendor GOFLAG Co-authored-by: Eric Stroczynski <estroczy@redhat.com>

dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 10, 2019

dmitshur added this to the Go1.14 milestone Jun 10, 2019

bcmills added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 10, 2019

rsc modified the milestones: Go1.14, Backlog Oct 9, 2019

agnivade removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 11, 2019

agnivade mentioned this issue Nov 26, 2019

cmd/go: 'go test -mod=mod' fails if GOFLAGS contains a default -mod setting #35827

Closed

bcmills changed the title ~~cmd/go: vet flag may be set only once~~ cmd/go: 'vet' and 'test' duplicate flags from GOFLAGS if they are also specified explicitly Nov 26, 2019

bcmills self-assigned this Nov 26, 2019

bcmills modified the milestones: Backlog, Go1.14 Nov 26, 2019

mvdan mentioned this issue Dec 4, 2019

go vet -mod=vendor break if GOFLAGS env sets mod too #35955

Closed

bcmills removed this from the Go1.14 milestone Dec 11, 2019

bcmills added this to the Go1.15 milestone Dec 11, 2019

bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 11, 2019

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

sthagen mentioned this issue Dec 12, 2019

cmd/go: restore default vet analyzers for targets in GOROOT sthagen/golang-go#78

Merged

jhrozek mentioned this issue Dec 12, 2019

Compliance remediations openshift/compliance-operator#44

Merged

7 tasks

JAORMX mentioned this issue Dec 12, 2019

Only add explicit -mod=vendor if needed operator-framework/operator-sdk#2332

Merged

bcmills mentioned this issue Dec 13, 2019

cmd/go: invalidate cache entries for test runs using different timeouts #36134

Closed

joelanford mentioned this issue Feb 12, 2020

go test: mod flag may be set only once operator-framework/operator-sdk#2556

Closed

gopherbot closed this as completed in 583419e Feb 25, 2020

golang locked and limited conversation to collaborators Feb 24, 2021

gopherbot added the FrozenDueToAge label Feb 24, 2021

rsc unassigned bcmills Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: 'vet' and 'test' duplicate flags from GOFLAGS if they are also specified explicitly #32471

cmd/go: 'vet' and 'test' duplicate flags from GOFLAGS if they are also specified explicitly #32471

elagergren-spideroak commented Jun 6, 2019 •

edited

elagergren-spideroak commented Jun 6, 2019

dmitshur commented Jun 10, 2019

bcmills commented Jun 10, 2019

elagergren-spideroak commented Jun 11, 2019 •

edited

elagergren-spideroak commented Jun 11, 2019 •

edited

agnivade commented Nov 11, 2019 •

edited

amenzhinsky commented Nov 11, 2019

amenzhinsky commented Nov 11, 2019

elagergren-spideroak commented Nov 11, 2019 •

edited

agnivade commented Nov 12, 2019

amenzhinsky commented Nov 12, 2019 •

edited

ianlancetaylor commented Dec 5, 2019

bcmills commented Dec 5, 2019

gopherbot commented Dec 11, 2019

gopherbot commented Dec 11, 2019

bcmills commented Dec 11, 2019

gopherbot commented Dec 13, 2019

cmd/go: 'vet' and 'test' duplicate flags from GOFLAGS if they are also specified explicitly #32471

cmd/go: 'vet' and 'test' duplicate flags from GOFLAGS if they are also specified explicitly #32471

Comments

elagergren-spideroak commented Jun 6, 2019 • edited

What version of Go are you using (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?

elagergren-spideroak commented Jun 6, 2019

dmitshur commented Jun 10, 2019

bcmills commented Jun 10, 2019

elagergren-spideroak commented Jun 11, 2019 • edited

elagergren-spideroak commented Jun 11, 2019 • edited

agnivade commented Nov 11, 2019 • edited

amenzhinsky commented Nov 11, 2019

amenzhinsky commented Nov 11, 2019

elagergren-spideroak commented Nov 11, 2019 • edited

agnivade commented Nov 12, 2019

amenzhinsky commented Nov 12, 2019 • edited

ianlancetaylor commented Dec 5, 2019

bcmills commented Dec 5, 2019

gopherbot commented Dec 11, 2019

gopherbot commented Dec 11, 2019

bcmills commented Dec 11, 2019

gopherbot commented Dec 13, 2019

elagergren-spideroak commented Jun 6, 2019 •

edited

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

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

elagergren-spideroak commented Jun 11, 2019 •

edited

elagergren-spideroak commented Jun 11, 2019 •

edited

agnivade commented Nov 11, 2019 •

edited

elagergren-spideroak commented Nov 11, 2019 •

edited

amenzhinsky commented Nov 12, 2019 •

edited