[go] os/exec: fix GOOS=windows,Cmd.Run always calls LookPath before returning
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded this change for review.
os/exec: fix GOOS=windows,Cmd.Run always calls LookPath before returning
Follow up on CL 511458, see https://go-review.googlesource.com/c/go/+/511458/2..4/src/cmd/go/main.go#b270 .
Fixes #36768
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
GitHub-Last-Rev: 6f84ef7207fc3592bb6ddce1a17a183c0168d88f
GitHub-Pull-Request: golang/go#61517
---
M src/os/exec/exec.go
M src/os/exec/lp_windows.go
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go
index 138be29..d6b5a749 100644
--- a/src/os/exec/exec.go
+++ b/src/os/exec/exec.go
@@ -597,6 +597,15 @@
if filepath.Base(path) == path {
path = "." + string(filepath.Separator) + path
}
+ exts := pathExt()
+ if ext := filepath.Ext(path); ext != "" {
+ for _, e := range exts {
+ if strings.EqualFold(ext, e) {
+ // Assume that path has already been resolved.
+ return path, nil
+ }
+ }
+ }
if dir == "" {
return LookPath(path)
}
diff --git a/src/os/exec/lp_windows.go b/src/os/exec/lp_windows.go
index 066d38d..373a8f8 100644
--- a/src/os/exec/lp_windows.go
+++ b/src/os/exec/lp_windows.go
@@ -63,6 +63,11 @@
// As of Go 1.19, LookPath will instead return that path along with an error satisfying
// errors.Is(err, ErrDot). See the package documentation for more details.
func LookPath(file string) (string, error) {
+ return lookPath(file, pathExt())
+
+}
+
+func pathExt() []string {
var exts []string
x := os.Getenv(`PATHEXT`)
if x != "" {
@@ -78,7 +83,11 @@
} else {
exts = []string{".com", ".exe", ".bat", ".cmd"}
}
+ return exts
+}
+// lookPath implements LookPath for the given PATHEXT list.
+func lookPath(file string, exts []string) (string, error) {
if strings.ContainsAny(file, `:\/`) {
f, err := findExecutable(file, exts)
if err == nil {
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor.
Gerrit Bot uploaded patch set #2 to this change.
os/exec: fix GOOS=windows,Cmd.Run always calls LookPath before returning
Follow up on CL 511458, see https://go-review.googlesource.com/c/go/+/511458/2..4/src/cmd/go/main.go#b270 .
Fixes #36768
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
GitHub-Last-Rev: 6f84ef7207fc3592bb6ddce1a17a183c0168d88f
GitHub-Pull-Request: golang/go#61517
---
M src/os/exec/exec.go
M src/os/exec/lp_windows.go
2 files changed, 18 insertions(+), 0 deletions(-)
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
Ian Lance Taylor (Gerrit)
Gerrit Bot (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor.
Gerrit Bot uploaded patch set #3 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Ian Lance Taylor, TryBot-Result-1 by Gopher Robot
os/exec: fix GOOS=windows,Cmd.Run always calls LookPath before returning
Follow up on CL 511458, see https://go-review.googlesource.com/c/go/+/511458/2..4/src/cmd/go/main.go#b270 .
Fixes #36768
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
GitHub-Last-Rev: ad2b4838e2964dfe7fea1f3c223a871dca1b53d1
GitHub-Pull-Request: golang/go#61517
---
M src/os/exec/exec.go
M src/os/exec/lp_plan9.go
M src/os/exec/lp_unix.go
M src/os/exec/lp_wasm.go
M src/os/exec/lp_windows.go
5 files changed, 30 insertions(+), 0 deletions(-)
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
qiulaidongfeng (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor.
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor.
4 comments:
Commit Message:
Patch Set #3, Line 7:
os/exec: fix GOOS=windows,Cmd.Run always calls LookPath before returning```suggestion
os/exec: avoid calling LookPath in cmd.Start for resolved paths
```Patch Set #3, Line 11:
Fixes #36768```suggestion
For #36768.
```(This change by itself doesn't eliminate the `os.Stat` calls — it just avoids additional `Stat` calls if we cache the `LookPath` calls on the caller side.)
File src/os/exec/exec.go:
Patch Set #3, Line 600:
exts := pathExt()It would be nice to avoid a second call to (and allocation in) `pathExt` in the `LookPath` calls in this function.
I think probably the simplest way to do that is to move the definition of `lookExtensions` into `lp_windows.go`, and provide a `lookExtensions` implementation for the other platforms that just passes the path through:
```
// lookExtensions is a no-op on non-Windows platforms, since
// they do not restrict executables to specific extensions.
func lookExtensions(path, dir string) (string, error) {
return path, nil
}
```Then the call site within `Cmd.Start` simplifies to:
```
c.Path, c.Err = lookExtensions(c.Path, c.Dir)
if c.Err != nil {
return c.Err
}
```and we don't need per-OS implementations of `pathExt`, which is really only ever called on Windows.
The Windows version of `lookExtensions` could then call the unexported `lookPath` directly:
```go
func lookExtensions(path, dir string) (string, error) {
if filepath.Base(path) == path {
path = "." + string(filepath.Separator) + path
}
exts := pathExt()
…
if dir == "" {
return lookPath(path, exts)
}
if filepath.VolumeName(path) != "" {
return lookPath(path, exts)
}
… // and so on.
}
```
File src/os/exec/lp_windows.go:
(nit) remove trailing newline before the closing `}`
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor.
Gerrit Bot uploaded patch set #4 to this change.
os/exec: avoid calling LookPath in cmd.Start for resolved paths
Follow up on CL 511458, see https://go-review.googlesource.com/c/go/+/511458/2..4/src/cmd/go/main.go#b270 .
For #36768.
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
GitHub-Last-Rev: ad2b4838e2964dfe7fea1f3c223a871dca1b53d1
GitHub-Pull-Request: golang/go#61517
---
M src/os/exec/exec.go
M src/os/exec/lp_plan9.go
M src/os/exec/lp_unix.go
M src/os/exec/lp_wasm.go
M src/os/exec/lp_windows.go
5 files changed, 30 insertions(+), 0 deletions(-)
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor.
Gerrit Bot uploaded patch set #5 to this change.
os/exec: avoid calling LookPath in cmd.Start for resolved paths
Follow up on CL 511458, see https://go-review.googlesource.com/c/go/+/511458/2..4/src/cmd/go/main.go#b270 .
For #36768.
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
GitHub-Last-Rev: bac7e66496806d505270c5b90d53672d80a1ca29
GitHub-Pull-Request: golang/go#61517
---
M src/os/exec/exec.go
M src/os/exec/lp_plan9.go
M src/os/exec/lp_unix.go
M src/os/exec/lp_wasm.go
M src/os/exec/lp_windows.go
5 files changed, 65 insertions(+), 32 deletions(-)
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
qiulaidongfeng (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Ian Lance Taylor.
4 comments:
Commit Message:
Patch Set #3, Line 7:
os/exec: fix GOOS=windows,Cmd.Run always calls LookPath before returning
```suggestion […]
Done.
Patch Set #3, Line 11:
Fixes #36768
```suggestion […]
Done.
File src/os/exec/exec.go:
Patch Set #3, Line 600:
exts := pathExt()
It would be nice to avoid a second call to (and allocation in) `pathExt` in the `LookPath` calls in […]
Done.
File src/os/exec/lp_windows.go:
(nit) remove trailing newline before the closing `}`
Done.
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
Bryan Mills (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor, qiulaidongfeng.
Patch set 5:Run-TryBot +1Auto-Submit +1Code-Review +2
6 comments:
Commit Message:
Patch Set #3, Line 7:
os/exec: fix GOOS=windows,Cmd.Run always calls LookPath before returning
Done.
Please remember to mark comment threads in Gerrit as resolved when they are done.
Patch Set #3, Line 11:
Fixes #36768
Done.
Acknowledged
Patchset:
Fixed.
Acknowledged
Patchset:
TRY=windows-amd64-longtest
File src/os/exec/exec.go:
Patch Set #3, Line 600:
exts := pathExt()
Done.
Acknowledged
File src/os/exec/lp_windows.go:
Done.
Acknowledged
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
Matthew Dempsky (Gerrit)
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor, qiulaidongfeng.
Patch set 5:Code-Review +1
Gopher Robot (Gerrit)
Gopher Robot submitted this change.
os/exec: avoid calling LookPath in cmd.Start for resolved paths
Follow up on CL 511458, see https://go-review.googlesource.com/c/go/+/511458/2..4/src/cmd/go/main.go#b270 .
For #36768.
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
Change-Id: Icc2a4dbb1219b1d69dd10a900478957b0e975847
GitHub-Last-Rev: bac7e66496806d505270c5b90d53672d80a1ca29
GitHub-Pull-Request: golang/go#61517
Reviewed-on: https://go-review.googlesource.com/c/go/+/512155
Auto-Submit: Bryan Mills <bcm...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Matthew Dempsky <mdem...@google.com>
Reviewed-by: Bryan Mills <bcm...@google.com>
Run-TryBot: Bryan Mills <bcm...@google.com>
---
M src/os/exec/exec.go
M src/os/exec/lp_plan9.go
M src/os/exec/lp_unix.go
M src/os/exec/lp_wasm.go
M src/os/exec/lp_windows.go
5 files changed, 65 insertions(+), 32 deletions(-)
diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go
index 138be29..a23d1c4 100644
--- a/src/os/exec/exec.go
+++ b/src/os/exec/exec.go
@@ -590,32 +590,6 @@
return c.Wait()
}
-// lookExtensions finds windows executable by its dir and path.
-// It uses LookPath to try appropriate extensions.
-// lookExtensions does not search PATH, instead it converts `prog` into `.\prog`.
-func lookExtensions(path, dir string) (string, error) {
- if filepath.Base(path) == path {
- path = "." + string(filepath.Separator) + path
- }
- if dir == "" {
- return LookPath(path)
- }
- if filepath.VolumeName(path) != "" {
- return LookPath(path)
- }
- if len(path) > 1 && os.IsPathSeparator(path[0]) {
- return LookPath(path)
- }
- dirandpath := filepath.Join(dir, path)
- // We assume that LookPath will only add file extension.
- lp, err := LookPath(dirandpath)
- if err != nil {
- return "", err
- }
- ext := strings.TrimPrefix(lp, dirandpath)
- return path + ext, nil
-}
-
// Start starts the specified command but does not wait for it to complete.
//
// If Start returns successfully, the c.Process field will be set.
@@ -649,13 +623,11 @@
}
return c.Err
}
- if runtime.GOOS == "windows" {
- lp, err := lookExtensions(c.Path, c.Dir)
- if err != nil {
- return err
- }
- c.Path = lp
+ lp, err := lookExtensions(c.Path, c.Dir)
+ if err != nil {
+ return err
}
+ c.Path = lp
if c.Cancel != nil && c.ctx == nil {
return errors.New("exec: command with a non-nil Cancel was not created with CommandContext")
}
diff --git a/src/os/exec/lp_plan9.go b/src/os/exec/lp_plan9.go
index 9344b14..dffdbac 100644
--- a/src/os/exec/lp_plan9.go
+++ b/src/os/exec/lp_plan9.go
@@ -64,3 +64,9 @@
}
return "", &Error{file, ErrNotFound}
}
+
+// lookExtensions is a no-op on non-Windows platforms, since
+// they do not restrict executables to specific extensions.
+func lookExtensions(path, dir string) (string, error) {
+ return path, nil
+}
diff --git a/src/os/exec/lp_unix.go b/src/os/exec/lp_unix.go
index fd2c6ef..3787132 100644
--- a/src/os/exec/lp_unix.go
+++ b/src/os/exec/lp_unix.go
@@ -80,3 +80,9 @@
}
return "", &Error{file, ErrNotFound}
}
+
+// lookExtensions is a no-op on non-Windows platforms, since
+// they do not restrict executables to specific extensions.
+func lookExtensions(path, dir string) (string, error) {
+ return path, nil
+}
diff --git a/src/os/exec/lp_wasm.go b/src/os/exec/lp_wasm.go
index f2c8e9c..3c81904 100644
--- a/src/os/exec/lp_wasm.go
+++ b/src/os/exec/lp_wasm.go
@@ -21,3 +21,9 @@
// Wasm can not execute processes, so act as if there are no executables at all.
return "", &Error{file, ErrNotFound}
}
+
+// lookExtensions is a no-op on non-Windows platforms, since
+// they do not restrict executables to specific extensions.
+func lookExtensions(path, dir string) (string, error) {
+ return path, nil
+}
diff --git a/src/os/exec/lp_windows.go b/src/os/exec/lp_windows.go
index 066d38d..7f13347 100644
--- a/src/os/exec/lp_windows.go
+++ b/src/os/exec/lp_windows.go
@@ -63,6 +63,45 @@
// As of Go 1.19, LookPath will instead return that path along with an error satisfying
// errors.Is(err, ErrDot). See the package documentation for more details.
func LookPath(file string) (string, error) {
+ return lookPath(file, pathExt())
+}
+
+// lookExtensions finds windows executable by its dir and path.
+// It uses LookPath to try appropriate extensions.
+// lookExtensions does not search PATH, instead it converts `prog` into `.\prog`.
+func lookExtensions(path, dir string) (string, error) {
+ if filepath.Base(path) == path {
+ path = "." + string(filepath.Separator) + path
+ }
+ exts := pathExt()
+ if ext := filepath.Ext(path); ext != "" {
+ for _, e := range exts {
+ if strings.EqualFold(ext, e) {
+ // Assume that path has already been resolved.
+ return path, nil
+ }
+ }
+ }
+ if dir == "" {
+ return lookPath(path, exts)
+ }
+ if filepath.VolumeName(path) != "" {
+ return lookPath(path, exts)
+ }
+ if len(path) > 1 && os.IsPathSeparator(path[0]) {
+ return lookPath(path, exts)
+ }
+ dirandpath := filepath.Join(dir, path)
+ // We assume that LookPath will only add file extension.
+ lp, err := lookPath(dirandpath, exts)
+ if err != nil {
+ return "", err
+ }
+ ext := strings.TrimPrefix(lp, dirandpath)
+ return path + ext, nil
+}
+
+func pathExt() []string {
var exts []string
x := os.Getenv(`PATHEXT`)
if x != "" {
@@ -78,7 +117,11 @@
} else {
exts = []string{".com", ".exe", ".bat", ".cmd"}
}
+ return exts
+}
+// lookPath implements LookPath for the given PATHEXT list.
+func lookPath(file string, exts []string) (string, error) {
if strings.ContainsAny(file, `:\/`) {
f, err := findExecutable(file, exts)
if err == nil {
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
Alex Brainman (Gerrit)
Ian Lance Taylor (Gerrit)
Ian Lance Taylor has created a revert of this change.
To view, visit change 512155. To unsubscribe, or for help writing mail filters, visit settings.
Will Drakus (Gerrit)
Will Drakus added 1 comment![]( "Open in Gerrit")
Related details
Code-Review
No-Holds
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |