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 fmt shouldn't send network request in module mode #30975

Closed
crvv opened this issue Mar 21, 2019 · 5 comments
Closed

cmd/go: go fmt shouldn't send network request in module mode #30975

crvv opened this issue Mar 21, 2019 · 5 comments
Labels
GoCommand modules NeedsInvestigation
Milestone

Comments

@crvv
Copy link
Contributor

@crvv crvv commented Mar 21, 2019

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

master branch, 0fe1986

Does this issue reproduce with the latest release?

Partially.

What did you do?

mkdir ~/test
cd ~/test
go mod init github.com/a/b
echo 'package main\nimport "github.com/a/b/c"\n' > main.go

And then there are three situations.

  1. cd ~/test; go fmt
  2. cd ~/test; go fmt ./main.go
  3. cd ~; go fmt ~/test/main.go

What did you expect to see?

go fmt reformats the file main.go in the three situations.
If I didn't run go mod init github.com/a/b, everything works fine.

What did you see instead?

On master branch, with commit eca5c83, go fmt runs git ls-remote -q https://github.com/a/b and fails in the three situations.

build github.com/a/b: cannot load github.com/a/b/c: cannot find module providing package github.com/a/b/c

Go1.12 fails in situation 1 and 2, and it succeeds in situation 3.

I think there are two things to be fixed.

  1. go fmt doesn't need to find package github.com/a/b/c and it shouldn't run git. It can just reformat the file.
  2. If the module github.com/a/b is in the directory ~/test and I am doing something with ~/test(such as cd ~/test; go build), go should look for github.com/a/b and github.com/a/b/c in ~/test. It shouldn't use git or request github.com to find the current module.
@mvdan
Copy link
Member

@mvdan mvdan commented Mar 21, 2019

Duplicate of #30577? Also, consider using gofmt instead, if you want a lower-level tool which will never resolve or fetch any packages.

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Mar 21, 2019

Hmm, we could probably improve behavior here by ignoring imports when loading packages and rejecting package paths outside of the main module. We may be able to avoid loading the build list and going out to network that way.

#30577 is a dup that points to #27841: adding a -mod flag for go fmt. I don't think it makes sense to format sources in other modules though, so we don't need to load them.

@crvv
Copy link
Contributor Author

@crvv crvv commented Mar 21, 2019

I don't understand why go fmt needs to load packages.
If it just does the following things, what will be wrong?

go fmt path/to/file.go => gofmt -l -w path/to/file.go
go fmt . => for f in ./*.go; do gofmt -l -w $f; done
go fmt ./... => for f in **/*.go; do gofmt -l -w $f; done
go fmt github.com/a/b => for f in $GOPATH/src/github.com/a/b/*.go; do gofmt -l -w $f; done

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Mar 21, 2019

go fmt arguments should be interpreted the same way go build interprets arguments for consistency. For example, go fmt ./... should not recurse into testdata directories or process hidden files.

@bcmills bcmills added the NeedsInvestigation label Apr 12, 2019
@bcmills bcmills added this to the Go1.13 milestone Apr 12, 2019
@bcmills bcmills removed this from the Go1.13 milestone Apr 12, 2019
@bcmills bcmills added this to the Go1.14 milestone Apr 12, 2019
@rsc rsc removed this from the Go1.14 milestone Oct 9, 2019
@rsc rsc added this to the Backlog milestone Oct 9, 2019
@crvv crvv closed this as completed Jun 23, 2022
@crvv
Copy link
Contributor Author

@crvv crvv commented Jun 23, 2022

This is not reproducible on 1.18.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand modules NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

5 participants