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: generate should set GOFLAGS env variable #27898

Open
myitcv opened this issue Sep 27, 2018 · 4 comments
Open

cmd/go: generate should set GOFLAGS env variable #27898

myitcv opened this issue Sep 27, 2018 · 4 comments
Labels
FeatureRequest GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@myitcv
Copy link
Member

myitcv commented Sep 27, 2018

Please answer these questions before submitting your issue. Thanks!

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

go version go1.11 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/go-modules-by-example/.gopath"
GOPROXY=""
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/go-modules-by-example/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build952142611=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Given the following setup:

$ cd $(mktemp -d)
$ go mod init example.com/hello
go: creating new go.mod: module example.com/hello
$ cat <<EOD >hello.go
package hello
EOD
$ cat <<"EOD" >hello_js_wasm.go
// +build constraint

package hello

//go:generate echo "GOARCH: $GOARCH; GOOS: $GOOS; GOFILE: $GOFILE; GOLINE: $GOLINE; GOPACKAGE: $GOPACKAGE"
EOD

Then the following two command produce no output, i.e. go generate doesn't find any directives:

$ go generate
$ GOOS=js GOARCH=wasm go generate

Only when we combine the setting of GOOS, GOARCH and build tags is the directive run:

$ GOOS=js GOARCH=wasm go generate -tags constraint
GOARCH: wasm; GOOS: js; GOFILE: hello_js_wasm.go; GOLINE: 5; GOPACKAGE: hello

This makes sense; per go help generate, GOOS and GOARCH are passed through by go generate to each directive:

$ go help generate
usage: go build [-o output] [-i] [build flags] [packages]
...

Go generate sets several variables when it runs the generator:

        $GOARCH
                The execution architecture (arm, amd64, etc.)
        $GOOS
                The execution operating system (linux, windows, etc.)
        $GOFILE
                The base name of the file.
        $GOLINE
                The line number of the directive in the source file.
        $GOPACKAGE
                The name of the package of the file containing the directive.
        $DOLLAR
                A dollar sign.
...

However build tags are not, despite build tags clearly influencing the files that are scanned for directives.

So assuming the observed behaviour is correct/consistent, I'd like to propose that we add a GOBUILDTAGS env variable to complement GOOS and GOARCH. This would pass through the -tags build flag:

        -tags 'tag list'
                a space-separated list of build tags to consider satisfied during the
                build. For more information about build tags, see the description of
                build constraints in the documentation for the go/build package.

The use case here is needing to pass GOOS, GOARCH and build tags as supplied to go generate to a generator that uses go/packages for type checking.

cc @robpike @rsc @bcmills

@bcmills bcmills added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. FeatureRequest GoCommand cmd/go labels Sep 27, 2018
@bcmills bcmills added this to the Go1.12 milestone Sep 27, 2018
@mvdan
Copy link
Member

mvdan commented Sep 27, 2018

I think the use case here goes beyond go/packages - it extends to any use case where one may want to import/build the package in question as part of the generation process. Be it go/packages, go list, go build, or any other means.

Also, if there's a way to accomplish this already, please bring it up. Paul and myself weren't able to come up with a way, other than adding something like GOBUILDTAGS.

@rsc
Copy link
Contributor

rsc commented Oct 25, 2018

I am skeptical of introducing a new GOBUILDTAGS environment variable that generate writes but that nothing else reads.

(The others that are unique to generate are GOFILE, GOLINE, GOPACKAGE, and DOLLAR, which are all easier to understand as not-read-by-the-go-command.)

@rsc
Copy link
Contributor

rsc commented Oct 25, 2018

In the case in the original report, it's clear that the relevant file is specific to wasm+js+constraint. Why not just say that on the command line of the generator: -tags 'wasm js constraint'?

@bcmills bcmills added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 25, 2018
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 25, 2018
@myitcv
Copy link
Member Author

myitcv commented Oct 28, 2018

Stepping back a bit just in case I'm jumping rather too quickly to a solution before properly articulating the problem.

go/packages defines a Config type. Taking the Env field first:

...
    // Env is the environment to use when invoking the build system's query tool.
    // If Env is nil, the current environment is used.
    // As in os/exec's Cmd, only the last value in the slice for
    // each environment key is used. To specify the setting of only
    // a few variables, append to the current environment, as in:
    //
    //	opt.Env = append(os.Environ(), "GOOS=plan9", "GOARCH=386")
    //
    Env []string
...

Hence we can use the current environment's GOOS and GOARCH, or override. In many situations, the GOOS and GOARCH of the current environment will suffice. Hence the GOOS and GOARCH used to invoke go generate will probably suffice. So nothing to override in most cases.

Config also defines .BuildFlags:

    // BuildFlags is a list of command-line flags to be passed through to
    // the build system's query tool.
    BuildFlags []string

This is where there is currently a gap, because these would need to be explicitly set: if I pass -tags constraint to go generate, I don't have a way to automatically pass these through the generator to go/packages.

If, however, the -tags flag is set via GOFLAGS then that value is automatically available to go/packages via the generator's environment:

$ cd $(mktemp -d)
$ go mod init example.com/hello
go: creating new go.mod: module example.com/hello
$ cat <<EOD >hello.go
package hello
EOD
$ cat <<"EOD" >hello_js_wasm.go
// +build constraint

package hello

//go:generate echo "GOARCH: $GOARCH; GOOS: $GOOS; GOFILE: $GOFILE; GOLINE: $GOLINE; GOPACKAGE: $GOPACKAGE; GOFLAGS: $GOFLAGS"
EOD
$ GOFLAGS=-mod=vendor GOOS=js GOARCH=wasm go generate -tags constraint
GOARCH: wasm; GOOS: js; GOFILE: hello_js_wasm.go; GOLINE: 5; GOPACKAGE: hello; GOFLAGS: -mod=vendor

So if instead of setting GOBUILDTAGS we have go generate set the effective GOFLAGS (i.e. the combined result of the GOFLAGS it was passed plus any command line flags it was passed) in the environment for the generator, then I think we have all bases covered.

Why not just say that on the command line of the generator: -tags 'wasm js constraint'?

Absolutely, this would work. But then these constraints would need to be maintained at the site of every go:generate directive (probably by setting GOFLAGS so that each generator doesn't also need to grow another flag just for the purpose of accepting tags/other).

I am skeptical of introducing a new GOBUILDTAGS environment variable that generate writes but that nothing else reads.

Have re-read what's written above, I can quite agree now. I don't know how we didn't come up with the effective-GOFLAGS option earlier. Would that work to your mind?

@myitcv myitcv changed the title cmd/go: generate should set GOBUILDTAGS env variable cmd/go: generate should set GOFLAGS env variable Feb 4, 2019
@andybons andybons modified the milestones: Go1.12, Go1.13 Feb 12, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@agnivade agnivade removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants