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

os/exec: LookPath inconsistency on windows/arm64 #44379

Closed
rsc opened this issue Feb 18, 2021 · 8 comments
Closed

os/exec: LookPath inconsistency on windows/arm64 #44379

rsc opened this issue Feb 18, 2021 · 8 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Feb 18, 2021

After a recent Windows 10 update on my Surface Pro X, os/exec TestLookPath is failing.
It appears to be a bug introduced in the update in cmd.exe itself.

The transcript below seems to show that cmd.exe is not respecting the order of extensions in PATHEXT:
even though EXE comes before BAT, cmd is prefering a.bat over a.exe.

C:\Users\russc\tmp>dir
 Volume in drive C is Local Disk
 Volume Serial Number is 6254-048B

 Directory of C:\Users\russc\tmp

02/18/2021  08:13 AM    <DIR>          .
02/18/2021  08:10 AM    <DIR>          ..
               0 File(s)              0 bytes
               2 Dir(s)  420,601,499,648 bytes free

C:\Users\russc\tmp>mkdir p1

C:\Users\russc\tmp>copy con p1\a.bat
@echo a1.bat
^Z
        1 file(s) copied.

C:\Users\russc\tmp>copy ..\go\bin\go.exe p1\a.exe
        1 file(s) copied.

C:\Users\russc\tmp>p1\a.exe
Go is a tool for managing Go source code.
...

C:\Users\russc\tmp>p1\a.bat
a1.bat

C:\Users\russc\tmp>p1\a
a1.bat

C:\Users\russc\tmp>set PATHEXT
PATHEXT=.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC

C:\Users\russc\tmp>p1\a.BAT
a1.bat

C:\Users\russc\tmp>p1\a.EXE
Go is a tool for managing Go source code.
...

C:\Users\russc\tmp>set PATH=C:\users\russc\tmp\p1

C:\Users\russc\tmp>a
a1.bat

C:\Users\russc\tmp>

Edit: I am running Windows 10 Home Insider Preview, Build 21301.rs_prerelease.210123-1645, on a Surface Pro X.

@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 18, 2021
@rsc rsc added this to the Go1.17 milestone Feb 18, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/293709 mentions this issue: os/exec: disable failing LookPathTest on windows/arm64

@zx2c4
Copy link
Contributor

zx2c4 commented Feb 18, 2021

What makes you so certain this is arm64 only? Have you tried updating to the latest insider ring on amd64?

@rsc
Copy link
Contributor Author

rsc commented Feb 18, 2021

@jstarks says it repros on x64 as well. Will update the CL.

@jstarks
Copy link

jstarks commented Feb 18, 2021

I can confirm that this is broken on x64 as well. We (Microsoft) are looking into it.

@jstarks
Copy link

jstarks commented Feb 18, 2021

We've confirmed that this is an OS bug in prerelease versions of Windows. We'll fix this in an upcoming preview build. Thanks for the report, Russ.

gopherbot pushed a commit that referenced this issue Feb 19, 2021
For #44379.

Change-Id: I9a3cf4d511a8286117f877c2ff9dbde56fa55983
Reviewed-on: https://go-review.googlesource.com/c/go/+/293709
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: Russ Cox <rsc@golang.org>
@dmitshur
Copy link
Contributor

@jstarks Thank you for fixing the bug and the update here.

@rsc Is there anything that needs to be done for Go 1.17 on Go's side, or is the fix in the OS sufficient? This issue can be updated to re-enable the failing LookPathTest that was skipped in CL 293709 (and possibly move it out of Go 1.17 milestone if it isn't expected to happen before the release).

@zx2c4
Copy link
Contributor

zx2c4 commented May 21, 2021

I would think that trying to provide workarounds for prerelease OS builds is not going to be very productive. Microsoft is pretty good generally about trying not to break things in major releases, and users of prerelease accept that some things might not work very well. 21301 is quite old at this point too (though I'm not sure in which version it was fixed).

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/341455 mentions this issue: os/exec: re-enable LookPathTest/16 on windows/arm64

@dmitshur dmitshur self-assigned this Aug 12, 2021
@dmitshur dmitshur added the Testing An issue that has been verified to require only test changes, not just a test failure. label Aug 12, 2021
@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 NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

5 participants