os/exec: windows LookPath is broken

[gopherbot]

by atkaaz:

Before filing a bug, please check whether it has been fixed since the
latest release. Search the issue tracker and check that you're running the
latest version of Go:

Run "go version" and compare against
http://golang.org/doc/devel/release.html  If a newer version of Go exists,
install it and retry what you did to reproduce the problem.

Thanks.

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
on Windows btw.
1. set PATHEXT=.BAT
2. go run hello.go
3.

What is the expected output?
no errors and hello world program runs

What do you see instead?
go build command-line-arguments: exec:
"c:\\Go\\pkg\\tool\\windows_386\\8g.exe": file does not exist

Which compiler are you using (5g, 6g, 8g, gccgo)?
8g

Which operating system are you using?
Windows 7 64bit

Which version are you using?  (run 'go version')
go version go1.1.2 windows/386


Please provide any additional information below.
LookPath is called with "c:\Go\pkg\tool\windows_386\8g.exe" and the fact that
PATHEXT is being set in an evil way let's say, make LookPath ....

This apparently works on linux because absolute path is starting with / or other
variants. But on windows it can be drive letter...

quick jump to LookPath function(in master):
http://code.google.com/p/go/source/browse/src/pkg/os/exec/lp_windows.go?name=default#51

This issue was encountered by one person (not me), here:
https://groups.google.com/d/msg/golang-nuts/mDzGuofHvxk/i1n0STnVl0cJ


unsetting PATHEXT will cause it to work, or setting it to the default which is:
PATHEXT=.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC

[gopherbot]

Comment 1 by atkaaz:

oops, wait, it looks fixed =) I'll try latest

[gopherbot]

Comment 2 by atkaaz:

nope, it's not fixed:
c:\>go.exe version
go version devel +037a28ab0725 Thu Aug 22 12:13:54 2013 +0900 windows/amd64
c:\>go.exe run -x hello.go
WORK=C:\Temp\go-build062400227
mkdir -p $WORK\command-line-arguments\_obj\
mkdir -p $WORK\command-line-arguments\_obj\exe\
cd c:\
"s:\\workspace.go.2013\\go64\\pkg\\tool\\windows_amd64\\6g.exe" -o
"C:\\Temp\\go-build062400227\\co
mmand-line-arguments\\_obj\\_go_.6" -p command-line-arguments -complete -D _/c_ -I
"C:\\Temp\\go-bu
ild062400227" "c:\\hello.go"
go build command-line-arguments: exec:
"s:\\workspace.go.2013\\go64\\pkg\\tool\\windows_amd64\\6g.e
xe": file does not exist

[alexbrainman]

Comment 3:

The problem is PATHEXT, not absolute filename. This is how Windows works.
C:\>find
FIND: Parameter format not correct
C:\>set PATHEXT=.BAT
C:\>find
'find' is not recognized as an internal or external command, operable program or batch
file.
C:\>
Go is doing the same.
Please, feel free to argue this point.
Alex

Status changed to WorkingAsIntended.

[gopherbot]

Comment 4 by atkaaz:

(excuse my $d $t $p$g  prompt)
Ok, but if I'm using your example it should be like this:
Fri 08/23/2013 10:25:16.88 C:\Users\user>find.exe
FIND: Parameter format not correct
Fri 08/23/2013 10:25:18.98 C:\Users\user>set PATHEXT=.BAT
Fri 08/23/2013 10:25:24.37 C:\Users\user>find.exe
FIND: Parameter format not correct
Fri 08/23/2013 10:26:56.61 C:\Users\user>c:\windows\system32\find
'c:\windows\system32\find' is not recognized as an internal or external command,
operable program or batch file.
Fri 08/23/2013 10:27:03.66 C:\Users\user>c:\windows\system32\find.exe
FIND: Parameter format not correct
Because Go is using this filename:
"s:\\workspace.go.2013\\go64\\pkg\\tool\\windows_amd64\\6g.exe"
which is equivalent to c:\windows\system32\find.exe above, if it didn't have .exe
already, then it would make sense to not find it(just as above)
Fri 08/23/2013 10:34:04.41 C:\Users\user>set PATHEXT=.EXE
Fri 08/23/2013 10:34:27.72 C:\Users\user>c:\windows\system32\find
FIND: Parameter format not correct
Fri 08/23/2013 10:34:30.75 C:\Users\user>set PATHEXT=.BAT
Fri 08/23/2013 10:34:56.68 C:\Users\user>c:\windows\system32\find
'c:\windows\system32\find' is not recognized as an internal or external command,
operable program or batch file.
(this makes sense, to me)
The thing is, currently, even if the file name is absolute (path+name+extension)
LookPath is still using PATHEXT to validate the extension, so if it's not ending in .bat
(if PATHEXT=.BAT) then it says it doesn't find it, but I say that if the file exists
(possibly regardless of the extension?) it should be found...
Maybe the extension should be limited(to ie. exe, com, dll, whichever), but in windows
command prompt if the file is absolute it will be executed as if double clicked, so
something like file.jpg would open default program for .jpg files:
Fri 08/23/2013 10:27:08.94 C:\Users\user>"Documents\!screenshots\applications ffox.png"
Fri 08/23/2013 10:31:14.73 C:\Users\user>"Documents\!screenshots\applications ffox"
'"Documents\!screenshots\applications ffox"' is not recognized as an internal or
external command,
operable program or batch file.
(the first one just worked, opened file in default viewer)
Maybe Go doesn't want to have this behaviour though, if it tries to execute the file in
some other way (as if it were an .exe) ...
but still the point is that if the file is absolute/exists it gets "executed"(think: as
if double clicked) on Windows

[gopherbot]

Comment 5 by atkaaz:

also, in case anyone was wondering at this point, this works too (it opens the .png
file, even though its extension wasn't specified, just like it does for .exe files for
example)
Fri 08/23/2013 10:36:51.06 C:\Users\user>set PATHEXT=.PNG
Fri 08/23/2013 10:38:47.79 C:\Users\user>"Documents\!screenshots\applications ffox"
Fri 08/23/2013 10:38:52.32 C:\Users\user>
but PATHEXT shouldn't interfere with absolute filenames, or well, rather when the
filename already has its extension specified (just like above when I used just find.exe
[extension was specified])
So in other words, as I am just realizing this as I write, if the filename already has
the extension specified, it should be searched in path, or if it already contains a path
ie. c:\windows\system32\find.exe  then it shouldn't be searched in path but since it
does contain .exe it should be stat-ed to see if it exists, regardless of PATHEXT's value

[alexbrainman]

Comment 6:

Lets start again. You are saying the problem is with os/exec.LookPath function. Please,
provide a small Go program to demonstrate that problem then. Thank you.
Alex

Status changed to WaitingForReply.

[gopherbot]

Comment 7 by atkaaz:

That's a great idea, I kinda wanted to do that, thank you for this opportunity.
http://play.golang.org/p/g4MD29_6JT
or pasting it here just in case:
package main
import "os"
import "os/exec"
func main() {
    testEXE()
    testBAT()
}
func setup(extensions string) {
    setEnv("PATHEXT", extensions)
    setEnv("PATH", "C:\\windows\\system32")
}
func findWhere() {
    lookPath("where")
    lookPath("where.exe")
    lookPath("c:\\windows\\system32\\where.exe")
}
func testEXE() { //all work
    setup(".EXE")
    findWhere()
}
func testBAT() { //all fail
    setup(".BAT")
    findWhere()
}
func lookPath(file string) {
    if ffile, err := exec.LookPath(file); err != nil {
        println("Not found:", err.Error())
    } else {
        println("    Found:", ffile)
    }
}
func setEnv(key, value string) {
    os.Setenv(key, value)
    if now_val := os.Getenv(key); value != now_val {
        println("Couldn't set `", key, "` env var, current value `", now_val, "`, wanted value `", value, "`")
    }
}

[gopherbot]

Comment 8 by atkaaz:

output:
    Found: C:\windows\system32\where.exe
    Found: C:\windows\system32\where.exe
    Found: c:\windows\system32\where.exe
Not found: exec: "where": executable file not found in %PATH%
Not found: exec: "where.exe": executable file not found in %PATH%
Not found: exec: "c:\\windows\\system32\\where.exe": file does not exist

[gopherbot]

Comment 9 by atkaaz:

expected output:
output:
    Found: C:\windows\system32\where.exe
    Found: C:\windows\system32\where.exe
    Found: c:\windows\system32\where.exe
Not found: exec: "where": executable file not found in %PATH%
    Found: C:\windows\system32\where.exe
    Found: C:\windows\system32\where.exe

[remyoudompheng]

Comment 10:

Labels changed: added os-windows.

[alexbrainman]

Comment 11:

atkaaz,
You convinced me there is a problem here. I think the problem is the way we interpret
one of the rules in http://technet.microsoft.com/en-us/library/cc723564.aspx, in
particular, it says:
"... If the command name includes a file extension, the shell searches each directory
for the exact file name specified by the command name. ...".
We only consider etxtensions listed in PATHEXT
https://code.google.com/p/go/source/browse/src/pkg/os/exec/lp_windows.go#32, but, I
think, we need to allow for ANY extension.
Also, it continues to say:
"... If the command name does not include a file extension, the shell adds the
extensions listed in the PATHEXT environment variable, one by one, and searches the
directory for that file name.... "
but we go and try extensions in PATHEXT
https://code.google.com/p/go/source/browse/src/pkg/os/exec/lp_windows.go#37 regardles of
whether we have extension in the name already.
I think the fix should be easy enough to make, but, given that we made mistake already,
I think we should write new test that will verify some of these rules. Perhaps something
similar to TestWinSplitListTestsAreValid from path/filepath package.
Would you like to have a go and make this change? If not, I will do it myself. Just le
me know.
Thank you.
Alex

Labels changed: added go1.2, packagebug, removed priority-triage.

Owner changed to @alexbrainman.

Status changed to Accepted.

[gopherbot]

Comment 12 by atkaaz:

You put it so eloquently.
I'd love to take a stab at it, but considering that I am new to Go and I've no
idea(currently) how to use the CL stuff, it might be more work for people reviewing the
code(that I'll write)  than writing it yourself(?) but if time is not an issue (ie. 1-2
days max) then I'll proceed reading http://golang.org/doc/contribute.html (I'll still
read this anyway)  and submit this change[exactly as you described it], I just hope it
won't be more work for the people reviewing the code having to suggest changes to it 
than actually having someone write it  who already knows how to write it.

[alexbrainman]

Comment 13:

Everyone have to start somewhere. Do it if you like.
I hope you can use linux or darwin, because I don't now if codereview works on Windows
(it probably works, but I don't know how to do). Just ask for help if you get stuck.
Alex

[gopherbot]

Comment 14 by atkaaz:

I'll do it. 
 Codereview seems to work after doing this https://code.google.com/p/go-wiki/wiki/CodeReview , unless you're referring to something else, I'll see.
 I can also try it on linux (virtualbox) if I really have to.
Thanks.

[gopherbot]

Comment 15 by atkaaz:

since 48 hours passed and I'm still yet not done, I felt like I should report that I am
still currently working on it, not stuck it's just that I'm trying to reorganize the
code while at the same time attempting to cover all/most test cases for this lookpath
and haven't yet implemented it but i think i can do it, just need few more hours