[go] cmd/go: improve handling of os.DevNull on Windows

11 views
Skip to first unread message

Damien Neil (Gerrit)

unread,
Nov 9, 2022, 5:05:55 PM11/9/22
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gopher Robot, Bryan Mills, golang-co...@googlegroups.com

Damien Neil submitted this change.

View Change


Approvals: Bryan Mills: Looks good to me, approved Gopher Robot: TryBots succeeded Damien Neil: Run TryBots
cmd/go: improve handling of os.DevNull on Windows

The "go test" and "go build" commands have special-case behavior when
passed "-o /dev/null". These checks are case-sensitive and assume that
os.DevNull is an absolute path. Windows filesystems are case-insensitive
and os.DevNull is NUL, which is not an absolute path.

CL 145220 changed filepath.IsAbs to report "NUL" as absolute to work
around this issue; that change is being rolled back and a better fix here
is to compare the value of -o against os.DevNull before attempting to
merge it with a base path. Make that fix.

On Windows, accept any capitilization of "NUL" as the null device.

This change doesn't cover every possible name for the null device, such
as "-o //./NUL", but this test is for efficiency rather than correctness.
Accepting just the most common name is fine.

For #56217.

Change-Id: I60b59b671789fc456074d3c8bc755a74ea8d5765
Reviewed-on: https://go-review.googlesource.com/c/go/+/449117
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
Reviewed-by: Bryan Mills <bcm...@google.com>
---
M src/cmd/go/internal/base/path.go
M src/cmd/go/internal/test/test.go
M src/cmd/go/internal/work/build.go
3 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/src/cmd/go/internal/base/path.go b/src/cmd/go/internal/base/path.go
index 4d8715e..ebe4f15 100644
--- a/src/cmd/go/internal/base/path.go
+++ b/src/cmd/go/internal/base/path.go
@@ -7,6 +7,7 @@
import (
"os"
"path/filepath"
+ "runtime"
"strings"
"sync"
)
@@ -54,3 +55,17 @@
// We don't cover tests, only the code they test.
return strings.HasSuffix(file, "_test.go")
}
+
+// IsNull reports whether the path is a common name for the null device.
+// It returns true for /dev/null on Unix, or NUL (case-insensitive) on Windows.
+func IsNull(path string) bool {
+ if path == os.DevNull {
+ return true
+ }
+ if runtime.GOOS == "windows" {
+ if strings.EqualFold(path, "NUL") {
+ return true
+ }
+ }
+ return false
+}
diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 6ec32df..5a56009 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1010,13 +1010,16 @@
if testC || testNeedBinary() {
// -c or profiling flag: create action to copy binary to ./test.out.
target := filepath.Join(base.Cwd(), testBinary+cfg.ExeSuffix)
+ isNull := false
if testO != "" {
target = testO
- if !filepath.IsAbs(target) {
+ if base.IsNull(target) {
+ isNull = true
+ } else if !filepath.IsAbs(target) {
target = filepath.Join(base.Cwd(), target)
}
}
- if target == os.DevNull {
+ if isNull {
runAction = buildAction
} else {
pmain.Target = target
diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go
index 98babc0..848f070 100644
--- a/src/cmd/go/internal/work/build.go
+++ b/src/cmd/go/internal/work/build.go
@@ -482,7 +482,7 @@
pkgs = omitTestOnly(pkgsFilter(pkgs))

// Special case -o /dev/null by not writing at all.
- if cfg.BuildO == os.DevNull {
+ if base.IsNull(cfg.BuildO) {
cfg.BuildO = ""
}


To view, visit change 449117. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I60b59b671789fc456074d3c8bc755a74ea8d5765
Gerrit-Change-Number: 449117
Gerrit-PatchSet: 3
Gerrit-Owner: Damien Neil <dn...@google.com>
Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
Gerrit-Reviewer: Damien Neil <dn...@google.com>
Gerrit-Reviewer: Gopher Robot <go...@golang.org>
Gerrit-MessageType: merged
Reply all
Reply to author
Forward
0 new messages