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
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@crvv
Copy link
Contributor

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 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

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 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

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 Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 12, 2019
@bcmills bcmills modified the milestones: Go1.13, Go1.14 Apr 12, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@crvv crvv closed this as completed Jun 23, 2022
@crvv
Copy link
Contributor Author

crvv commented Jun 23, 2022

This is not reproducible on 1.18.3.

@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules 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

6 participants