[go] cmd/go: skip "exclude all Go files" error in fmt
Daniel Martí (Gerrit)
Daniel Martí has uploaded this change for review.
cmd/go: skip "exclude all Go files" error in fmt
Otherwise, one can't run "go fmt" on a directory containing Go files if
none of them are buildable (e.g. because of build tags). This is
counter-intuitive, as fmt will format all Go files anyway.
If we encounter such a load error, ignore it and carry on. All other
load errors, such as when a package can't be found, should still be
shown to the user.
Add a test for the two kinds of load errors. Use fmt -n so that any
changes to the formatting of the files in testdata don't actually get
applied. The load errors still occur with -n, so the test does its job.
Fixes #22183.
Change-Id: I99d0c0cdd29015b6a3f5286a9bbff50757c78e0d
---
M src/cmd/go/go_test.go
M src/cmd/go/internal/fmtcmd/fmt.go
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index a4f6452..b593bcc 100644
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -4686,3 +4686,11 @@
t.Fatalf("bad output from compressed go binary:\ngot %q; want %q", out, "hello upx")
}
}
+
+func TestFmtLoadErrors(t *testing.T) {
+ tg := testgo(t)
+ defer tg.cleanup()
+ tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata"))
+ tg.runFail("fmt", "does-not-exist")
+ tg.run("fmt", "-n", "exclude")
+}
diff --git a/src/cmd/go/internal/fmtcmd/fmt.go b/src/cmd/go/internal/fmtcmd/fmt.go
index 2ff6dd5..eb96823 100644
--- a/src/cmd/go/internal/fmtcmd/fmt.go
+++ b/src/cmd/go/internal/fmtcmd/fmt.go
@@ -9,6 +9,7 @@
"os"
"path/filepath"
"runtime"
+ "strings"
"sync"
"cmd/go/internal/base"
@@ -55,7 +56,16 @@
}
}()
}
- for _, pkg := range load.Packages(args) {
+ for _, pkg := range load.PackagesAndErrors(args) {
+ if pkg.Error != nil {
+ if strings.HasPrefix(pkg.Error.Err, "build constraints exclude all Go files") {
+ // Skip this error, as we will format
+ // all files regardless.
+ } else {
+ base.Errorf("can't load package: %s", pkg.Error)
+ continue
+ }
+ }
// Use pkg.gofiles instead of pkg.Dir so that
// the command only applies to this package,
// not to packages in subdirectories.
To view, visit change 75930. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Martí (Gerrit)
Whoops, I intended to update the existing CL but forgot to carry over the Change-Id. Will abandon the old one.
Gobot Gobot (Gerrit)
Gobot Gobot (Gerrit)
Build is still in progress...
This change failed on darwin-amd64-10_11:
See https://storage.googleapis.com/go-build-log/ed023776/darwin-amd64-10_11_09a94e21.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
Daniel Martí (Gerrit)
This change failed on darwin-amd64-10_11:
See https://storage.googleapis.com/go-build-log/ed023776/darwin-amd64-10_11_09a94e21.log
I see that adding deps to cmd/go isn't trivial. Should I update via mkdeps, or should I find a way to not add strings as a dependency?
Gobot Gobot (Gerrit)
6 of 21 TryBots failed:
Failed on darwin-amd64-10_11: https://storage.googleapis.com/go-build-log/ed023776/darwin-amd64-10_11_09a94e21.log
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/ed023776/linux-amd64_5166f8bf.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/ed023776/linux-386_271f425a.log
Failed on freebsd-amd64-110: https://storage.googleapis.com/go-build-log/ed023776/freebsd-amd64-110_dc5e6ca4.log
Failed on openbsd-amd64-60: https://storage.googleapis.com/go-build-log/ed023776/openbsd-amd64-60_d6bc89b0.log
Failed on linux-arm: https://storage.googleapis.com/go-build-log/ed023776/linux-arm_5065dca5.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 1:TryBot-Result -1
Ian Lance Taylor (Gerrit)
This change failed on darwin-amd64-10_11:
See https://storage.googleapis.com/go-build-log/ed023776/darwin-amd64-10_11_09a94e21.logI see that adding deps to cmd/go isn't trivial. Should I update via mkdeps, or should I find a way to not add strings as a dependency?
Just run mkdeps. There are other dependencies on strings underneath cmd/go, so this is not a problem.
Daniel Martí (Gerrit)
Daniel Martí uploaded patch set #2 to this change.
cmd/go: skip "exclude all Go files" error in fmt
Otherwise, one can't run "go fmt" on a directory containing Go files if
none of them are buildable (e.g. because of build tags). This is
counter-intuitive, as fmt will format all Go files anyway.
If we encounter such a load error, ignore it and carry on. All other
load errors, such as when a package can't be found, should still be
shown to the user.
Add a test for the two kinds of load errors. Use fmt -n so that any
changes to the formatting of the files in testdata don't actually get
applied. The load errors still occur with -n, so the test does its job.
Fixes #22183.
Change-Id: I99d0c0cdd29015b6a3f5286a9bbff50757c78e0d
---
M src/cmd/dist/deps.go
M src/cmd/go/go_test.go
M src/cmd/go/internal/fmtcmd/fmt.go
3 files changed, 20 insertions(+), 1 deletion(-)
To view, visit change 75930. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Daniel Martí (Gerrit)
Just run mkdeps. There are other dependencies on strings underneath cmd/go, so this is not a problem.
Thanks - done and rebased.
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 2:TryBot-Result +1
Russ Cox (Gerrit)
Patch set 2:Code-Review +2
Russ Cox (Gerrit)
Russ Cox merged this change.
cmd/go: skip "exclude all Go files" error in fmt
Otherwise, one can't run "go fmt" on a directory containing Go files if
none of them are buildable (e.g. because of build tags). This is
counter-intuitive, as fmt will format all Go files anyway.
If we encounter such a load error, ignore it and carry on. All other
load errors, such as when a package can't be found, should still be
shown to the user.
Add a test for the two kinds of load errors. Use fmt -n so that any
changes to the formatting of the files in testdata don't actually get
applied. The load errors still occur with -n, so the test does its job.
Fixes #22183.
Change-Id: I99d0c0cdd29015b6a3f5286a9bbff50757c78e0d
Reviewed-on: https://go-review.googlesource.com/75930
Run-TryBot: Daniel Martí <mv...@mvdan.cc>
TryBot-Result: Gobot Gobot <go...@golang.org>
Reviewed-by: Russ Cox <r...@golang.org>
---
M src/cmd/dist/deps.go
M src/cmd/go/go_test.go
M src/cmd/go/internal/fmtcmd/fmt.go
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/cmd/dist/deps.go b/src/cmd/dist/deps.go
index ffc3b47..294ca22 100644
--- a/src/cmd/dist/deps.go
+++ b/src/cmd/dist/deps.go
@@ -167,6 +167,7 @@
"os", // cmd/go/internal/fmtcmd
"path/filepath", // cmd/go/internal/fmtcmd
"runtime", // cmd/go/internal/fmtcmd
+ "strings", // cmd/go/internal/fmtcmd
"sync", // cmd/go/internal/fmtcmd
},
diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index 25cc18f..854de79 100644
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -4906,3 +4906,11 @@
tg.run("install", "-i", "p2")
tg.mustExist(p1)