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: fmt does not respect or have defined -mod #27841

Open
emikalendra opened this Issue Sep 25, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@emikalendra

emikalendra commented Sep 25, 2018

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

go version go1.11 darwin/amd64

Does this issue reproduce with the latest release?

Yes. 1.11 is the current latest release

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

darwin/amd64

What did you do?

$ mkdir -p /tmp/scratch/fmtex
$ cd /tmp/scratch/fmtex
$ go mod init fmtex
$ cat <<EOF > hello.go
package main

import (
  "go.uber.org/zap"
)

func main() {
    logger, _ := zap.NewProduction()
    defer logger.Sync()
    logger.Info("Hello")
}
EOF

$ go run hello.go
$ go mod vendor
$ chmod -R 776 $GOPATH/pkg/mod/*
$ rm -rf $GOPATH/pkg/mod
$ export GOPROXY="off"
$ export GOFLAGS="-mod=vendor"
$ go run hello.go
{"level":"info","ts":1537854151.3013902,"caller":"fmtex/hello.go:10","msg":"Hello"}

This demonstrates that we have an example program using the vendor directory and GOPROXY is set to off.

Now we want to run go fmt

$ go fmt .
go: go.uber.org/atomic@v1.3.2: module lookup disabled by GOPROXY=off
go: go.uber.org/multierr@v1.1.0: module lookup disabled by GOPROXY=off
go: go.uber.org/zap@v1.9.1: module lookup disabled by GOPROXY=off
go: error loading module requirements

try using -mod=vendor

$ go fmt -mod=vendor .
flag provided but not defined: -mod
usage: go fmt [-n] [-x] [packages]
Run 'go help fmt' for details.

try using gofmt

$ gofmt -l .
hello.go

What did you expect to see?

go fmt should format source files using gofmt -l -w . . The documentation indicates:

Fmt runs the command 'gofmt -l -w' on the packages named
by the import paths. It prints the names of the files that are modified.

What did you see instead?

Instead of go fmt running gofmt -l -w . it first tries to load modules ignoring -mod=vendor. It is also unclear why go fmt requires loading modules if it just runs gofmt which works without loading modules.

@emikalendra emikalendra changed the title from go fmt does respect or have defined -mod to go fmt does not respect or have defined -mod Sep 25, 2018

@agnivade agnivade changed the title from go fmt does not respect or have defined -mod to cmd/go: fmt does not respect or have defined -mod Sep 25, 2018

@agnivade agnivade added this to the Go1.12 milestone Sep 25, 2018

@agnivade

This comment has been minimized.

Member

agnivade commented Sep 25, 2018

@agnivade agnivade added the modules label Sep 25, 2018

@myitcv

This comment has been minimized.

Member

myitcv commented Sep 25, 2018

I think you have some sort of environment issue here. The following works for me:

export GOPATH=$(mktemp -d)
unset GOPROXY GOFLAGS
cd $(mktemp -d)
go mod init example.com/fmtex
cat <<EOF > hello.go
package main

import (
  "go.uber.org/zap"
)

func main() {
    logger, _ := zap.NewProduction()
    defer logger.Sync()
    logger.Info("Hello")
}
EOF

go run hello.go
go mod vendor
GOPROXY="off" GOFLAGS="-mod=vendor" go run hello.go
GOPROXY="off" GOFLAGS="-mod=vendor" go fmt .
GOPROXY="off" GOFLAGS="-mod=vendor" gofmt .

gives the output:

$ egrunner -out std script.sh
$ export GOPATH=/tmp/tmp.6JmYsgOqIM
$ unset GOPROXY GOFLAGS
$ cd /tmp/tmp.oZGjJxN1NV
$ go mod init example.com/fmtex
go: creating new go.mod: module example.com/fmtex
$ cat <<EOF >hello.go
package main

import (
  "go.uber.org/zap"
)

func main() {
    logger, _ := zap.NewProduction()
    defer logger.Sync()
    logger.Info("Hello")
}
EOF
$ go run hello.go
go: finding go.uber.org/zap v1.9.1
go: downloading go.uber.org/zap v1.9.1
go: finding go.uber.org/multierr v1.1.0
go: finding go.uber.org/atomic v1.3.2
go: downloading go.uber.org/multierr v1.1.0
go: downloading go.uber.org/atomic v1.3.2
{"level":"info","ts":1537870453.3103888,"caller":"tmp.vm8Xl8pJwF/hello.go:10","msg":"Hello"}
$ go mod vendor
$ GOPROXY="off" GOFLAGS="-mod=vendor" go run hello.go
{"level":"info","ts":1537870454.1195014,"caller":"tmp.vm8Xl8pJwF/hello.go:10","msg":"Hello"}
$ GOPROXY="off" GOFLAGS="-mod=vendor" go fmt .
hello.go
$ GOPROXY="off" GOFLAGS="-mod=vendor" gofmt .
package main

import (
        "go.uber.org/zap"
)
...
@emikalendra

This comment has been minimized.

emikalendra commented Sep 25, 2018

@myitcv I think you missed the step to remove the cached modules. Without the step, the cache is being used, ignoring -mod=vendor.

Modifying your example to include changing to a new GOPATH. Using rm -rf in the original example was probably not a good idea.

export GOPATH=$(mktemp -d)
unset GOPROXY GOFLAGS
cd $(mktemp -d)
go mod init example.com/fmtex
cat <<EOF > hello.go
package main

import (
  "go.uber.org/zap"
)

func main() {
    logger, _ := zap.NewProduction()
    defer logger.Sync()
    logger.Info("Hello")
}
EOF

go run hello.go
go mod vendor
GOPROXY="off" GOFLAGS="-mod=vendor" go run hello.go
GOPROXY="off" GOFLAGS="-mod=vendor" go fmt .
GOPROXY="off" GOFLAGS="-mod=vendor" gofmt -l .

export GOPATH=$(mktemp -d)
GOPROXY="off" GOFLAGS="-mod=vendor" go run hello.go
GOPROXY="off" GOFLAGS="-mod=vendor" go fmt .
GOPROXY="off" GOFLAGS="-mod=vendor" gofmt -l .

Gives output:

$ export GOPATH=$(mktemp -d)
$ unset GOPROXY GOFLAGS
$ cd $(mktemp -d)
$ go mod init example.com/fmtex
go: creating new go.mod: module example.com/fmtex
$ cat <<EOF > hello.go
> package main
> 
> import (
>   "go.uber.org/zap"
> )
> 
> func main() {
>     logger, _ := zap.NewProduction()
>     defer logger.Sync()
>     logger.Info("Hello")
> }
> EOF
$ 
$ go run hello.go
go mod vendor
GOPROXY="off" GOFLAGS="-mod=vendor" go run hello.go
GOPROXY="off" GOFLAGS="-mod=vendor" go fmt .
GOPROXY="off" GOFLAGS="-mod=vendor" gofmt -l .

export GOPATH=$(mktemp -d)
GOPROXY="off" GOFLAGS="-mod=vendor" go run hello.go
GOPROXY="off" GOFLAGS="-mod=vendor" go fmt .
GOPROXY="off" GOFLAGS="-mod=vendor" gofmt -l .
go: finding go.uber.org/zap v1.9.1
go: downloading go.uber.org/zap v1.9.1
go: finding go.uber.org/multierr v1.1.0
go: finding go.uber.org/atomic v1.3.2
go: downloading go.uber.org/multierr v1.1.0
go: downloading go.uber.org/atomic v1.3.2
{"level":"info","ts":1537886400.8363662,"caller":"/hello.go:10","msg":"Hello"}
$ go mod vendor
$ GOPROXY="off" GOFLAGS="-mod=vendor" go run hello.go
{"level":"info","ts":1537886401.973213,"caller":"/hello.go:10","msg":"Hello"}
$ GOPROXY="off" GOFLAGS="-mod=vendor" go fmt .
hello.go
$ GOPROXY="off" GOFLAGS="-mod=vendor" gofmt -l .
 
$ export GOPATH=$(mktemp -d)

$ GOPROXY="off" GOFLAGS="-mod=vendor" go run hello.go
{"level":"info","ts":1537886402.843414,"caller":"/hello.go:10","msg":"Hello"}
$ GOPROXY="off" GOFLAGS="-mod=vendor" go fmt .
go: go.uber.org/zap@v1.9.1: module lookup disabled by GOPROXY=off
go: go.uber.org/atomic@v1.3.2: module lookup disabled by GOPROXY=off
go: go.uber.org/multierr@v1.1.0: module lookup disabled by GOPROXY=off
go: error loading module requirements
$ GOPROXY="off" GOFLAGS="-mod=vendor" gofmt -l .

@myitcv

This comment has been minimized.

Member

myitcv commented Sep 25, 2018

I think you missed the step to remove the cached modules.

I did indeed forget to ask why you had included that :)

Makes sense now, thanks.

cc @rsc @bcmills to confirm expected behaviour here. I suspect this is also an issue for the other tools (vet, fix etc?)

Using rm -rf in the original example was probably not a good idea.

You probably wanted go clean -modcache?

@emikalendra

This comment has been minimized.

emikalendra commented Sep 25, 2018

You probably wanted go clean -modcache?

Yes. Thank you. That would have made the example clearer.

@rsc

This comment has been minimized.

Contributor

rsc commented Oct 25, 2018

Most commands take all the build flags (documented in go help build) but a few, including fmt, know that most of the build flags are irrelevant if you're only loading packages and not building them, and so they only accept -n and -x. But -mod is now also relevant to package loading and should be included in those commands. Probably anything using base.AddBuildFlagsNX should be checked to see if should be something like AddLoadFlags instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment