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.

Sign up for GitHub

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

Jump to bottom

x/review/git-codereview: "sh": executable file not found in %PATH% error in TestSubmitInteractive #13406

Closed
alexbrainman opened this issue Nov 26, 2015 · 18 comments
Closed

x/review/git-codereview: "sh": executable file not found in %PATH% error in TestSubmitInteractive #13406

alexbrainman opened this issue Nov 26, 2015 · 18 comments
Labels
FrozenDueToAge OS-Windows
Milestone
Unreleased

Comments

@alexbrainman
Copy link
Member

Our builders fails http://build.golang.org/log/688066e2307059052f9a0bd805045467b1edf181 with:

--- FAIL: TestSubmitInteractive (1.10s)
    util_test.go:265: git-codereview submit
    util_test.go:265: git-codereview submit -i
    util_test.go:289: died
        stdout:
        stderr:
        git-codereview: editor exited with: exec: "sh": executable file not found in %PATH%
FAIL
FAIL    golang.org/x/review/git-codereview  94.112s

From what I can gather, CL 16677 assumes that sh (and vim) are found in the PATH. Not so on my computer (and builders too). I tried running "git rebase -i" command. And that, obviously, finds and executes git.exe, which then finds (I don't know how) and executes both sh.exe and vim.exe.

I don't see how we can implement this on windows. I don't use git-codereview on windows. Not for submitting anyway. So I am happy just to let this go. And skip failing test. Unless others have some good ideas.

/cc @aclements

Alex
@aclements
Copy link
Member

Are you sure git is invoking sh.exe? Because of the strcspn in https://github.com/git/git/blob/aa826b651ae3012d1039453b36ed6f1eab939ef9/run-command.c#L161, I think git may be starting vim.exe directly. Does it work in Go on Windows to do, e.g.,

gitEditor := <whatever git var GIT_EDITOR says>
cmd := exec.Command(gitEditor, "test")
cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr
cmd.Run()

? If so, I can just follow git's logic a little more closely.

@aclements aclements self-assigned this Dec 2, 2015
@alexbrainman
Copy link
Member Author

Are you sure git is invoking sh.exe?

I created little repo so I can immitate "git rebase -i" command, and then used "Process Explorer" (https://technet.microsoft.com/en-us/sysinternals/processexplorer.aspx) to view process tree:

git rebase -i master
"C:\Program Files\Git\bin\sh.exe" "C:\Program Files\Git/libexec/git-core\git-rebase" -i master
"C:\Program Files\Git\bin\sh.exe"
"C:\Program Files\Git\share\vim\vim74\vim.exe" c:/dev/go/src/a/.git/rebase-merge/git-rebase-todo

Please note that neither "C:\Program Files\Git\bin" nor "C:\Program Files\Git\share\vim\vim74" is in my PATH.

Mind you it appears I can run "sh" from my command line, and it starts running "C:\Program Files\Git\bin\sh.exe". I don't understand how that happens, I suspect "git" program installed some hook (dll) on my system during its installation to intercept calls like that. I have no other explanation. I don't know of any other program that does that.

Because of the strcspn in https://github.com/git/git/blob/aa826b651ae3012d1039453b36ed6f1eab939ef9/run-command.c#L161, I think git may be starting vim.exe directly.

I agree, git is doing funny buggers under the covers.

Does it work in Go on Windows to do, e.g. ...?

"git var GIT_EDITOR" says "vi". But I cannot run vi from command window:

C:\dev\go\src>vi
'vi' is not recognized as an internal or external command,
operable program or batch file.

Same will happen, if I try to run your program.

Alex

@aclements
Copy link
Member

I'd forgotten that git rebase is (frighteningly) entirely written in shell, so it's not going through prepare_shell_command at all. I would guess it's the git binary itself seeing that git-rebase is a shell script and invoking the baked-in sh.exe. Just to make sure I understand, if you start "C:\Program Files\Git\bin\sh.exe" by hand, what do you get for

echo $GIT_SEQUENCE_EDITOR
echo $GIT_EDITOR
git config sequence.editor
git var GIT_EDITOR

?

I'm not sure what the right answer here is. What would people expect in a Windows environment? Should we just disable this test on Windows?

@alexbrainman
Copy link
Member Author

if you start "C:\Program Files\Git\bin\sh.exe" by hand, what do you get for ... ?

I get:

C:\dev\go\src>"C:\Program Files\Git\bin\sh.exe"
sh.exe"-3.1$ echo $GIT_SEQUENCE_EDITOR

sh.exe"-3.1$ echo $GIT_EDITOR

sh.exe"-3.1$ git config sequence.editor
sh.exe"-3.1$ git var GIT_EDITOR
vi
sh.exe"-3.1$ exit
exit

C:\dev\go\src>

I'm not sure what the right answer here is. What would people expect in a Windows environment?

I am certainly not a git expert. I suppose if you need an editor available on any Windows computer, you cannot go past "notpead". It is very basic and noone uses it, but you can open a text file with it to allow editing. I don't expect it to return any useful exit code. And it is GUI program (not command line).

Maybe it is possible to invoke git shell via git command somehow?

Should we just disable this test on Windows?

I have no problem with that. But it also means that "git submit -i" is broken too. Isn't it? Mind you, I don't use that facility myself.

Alex

@rsc rsc added this to the Unreleased milestone Dec 28, 2015
@gopherbot
Copy link

CL https://golang.org/cl/18557 mentions this issue.

gopherbot pushed a commit to golang/review that referenced this issue Jan 13, 2016
@alexbrainman
git-codereview: skip TestSubmitInteractive on windows (fixes build)
a838955 
Updates golang/go#13406

Change-Id: Iadc8586253e794947f5048676d317f55bd1d6b62
Reviewed-on: https://go-review.googlesource.com/18557
Reviewed-by: Austin Clements <austin@google.com>
@rsc
Copy link
Contributor

rsc commented Jan 29, 2016

The test is fixed. That may be the best we can do.

@rsc rsc closed this as completed Jan 29, 2016
@alexbrainman
Copy link
Member Author

The test is fixed. ...

I don't think so. I have added to skip TestSubmitInteractive on winodws in CL 18557. But the test is still broken.

That may be the best we can do.

Yes, I don't see way to improve TestSubmitInteractive. But I am also concerned that, perhaps, "git submit -i" doesn't work on windows too. If someone tell me how I can test "git submit -i" manually, I will try it on my windows computer. Alternatively we can just document that "git submit -i" is not supported on windows. Or we can also do nothing too.

Alex

@aclements
Copy link
Member

If someone tell me how I can test "git submit -i" manually, I will try it on my windows computer.

I don't know how to test it all the way without a Gerrit instance (or a fake Gerrit instance like the tests make), but you can test the interactive part by just running "git submit -i" in the Go repository. You can even just be on master without any extra commits. It should just bring up an editor with an empty submit list and some comments explaining what you could do if there was something in the list.

@alexbrainman
Copy link
Member Author

It should just bring up an editor with an empty submit list and ...

It does that. I guess we should just leave everything as is.

Alex

@aclements
Copy link
Member

Interesting. Does running "git-codereview submit -i" directly work (without invoking it through the git alias)? I wonder if git sets up some environment before invoking the alias that makes submit -i work, but that isn't set up in the test.

@alexbrainman
Copy link
Member Author

Does running "git-codereview submit -i" directly work

C:\dev\go\src>git-codereview submit -i
git-codereview: editor exited with: exec: "sh": executable file not found in %PATH%

C:\dev\go\src>

Alex

@aclements
Copy link
Member

Ah hah, so git is adding stuff to PATH when it runs an alias. That's missing in the test, so it fails. The only thing I see if obviously adding is the compiled-in GIT_EXEC_PATH. Mind running a few more things?

git --exec-path
git -c alias.x='!echo $PATH' x

The second one works on Unix and I think git will run the alias with its built in POSIX shell, so it should be the same on Windows. If it doesn't work, you may need to tweak it by changing $PATH to %PATH% or something.

Also, is there a sh.exe in the exec path printed by the fist one?

@alexbrainman
Copy link
Member Author

Mind running a few more things?

C:\dev\go\src>git --exec-path
C:\Program Files (x86)\Git/libexec/git-core

C:\dev\go\src>git -c alias.x='!echo $PATH' x
git: '$PATH'' is not a git command. See 'git --help'.

C:\dev\go\src>git -c alias.x='!echo %PATH%' x
git: 'C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program' is not a git command. See 'git --help'.

C:\dev\go\src>

... is there a sh.exe in the exec path printed by the fist one?

There is no sh.exe there, but there are many others:

C:\dev\go\src>dir "C:\Program Files (x86)\Git\libexec\git-core\"
 Volume in drive C has no label.
 Volume Serial Number is 1234-ABCD

 Directory of C:\Program Files (x86)\Git\libexec\git-core

19/12/2014  04:42 PM    <DIR>          .
19/12/2014  04:42 PM    <DIR>          ..
17/12/2014  09:20 PM            36,821 git-add--interactive
19/12/2014  04:42 PM         1,503,744 git-add.exe
17/12/2014  09:20 PM            23,163 git-am
19/12/2014  04:42 PM         1,503,744 git-annotate.exe
19/12/2014  04:42 PM         1,503,744 git-apply.exe
19/12/2014  04:42 PM         1,503,744 git-archive.exe
17/12/2014  09:20 PM            11,997 git-bisect
19/12/2014  04:42 PM         1,503,744 git-bisect--helper.exe
19/12/2014  04:42 PM         1,503,744 git-blame.exe
19/12/2014  04:42 PM         1,503,744 git-branch.exe
19/12/2014  04:42 PM         1,503,744 git-bundle.exe
19/12/2014  04:42 PM         1,503,744 git-cat-file.exe
19/12/2014  04:42 PM         1,503,744 git-check-attr.exe
19/12/2014  04:42 PM         1,503,744 git-check-ignore.exe
19/12/2014  04:42 PM         1,503,744 git-check-mailmap.exe
19/12/2014  04:42 PM         1,503,744 git-check-ref-format.exe
19/12/2014  04:42 PM         1,503,744 git-checkout-index.exe
19/12/2014  04:42 PM         1,503,744 git-checkout.exe
19/12/2014  04:42 PM         1,503,744 git-cherry-pick.exe
19/12/2014  04:42 PM         1,503,744 git-cherry.exe
17/12/2014  09:20 PM               687 git-citool
19/12/2014  04:42 PM         1,503,744 git-clean.exe
19/12/2014  04:42 PM         1,503,744 git-clone.exe
19/12/2014  04:42 PM         1,503,744 git-column.exe
19/12/2014  04:42 PM         1,503,744 git-commit-tree.exe
19/12/2014  04:42 PM         1,503,744 git-commit.exe
19/12/2014  04:42 PM         1,503,744 git-config.exe
19/12/2014  04:42 PM         1,503,744 git-count-objects.exe
19/12/2014  04:42 PM           781,824 git-credential-store.exe
19/12/2014  04:42 PM            29,696 git-credential-wincred.exe
19/12/2014  04:42 PM         1,503,744 git-credential.exe
19/12/2014  04:42 PM           790,528 git-daemon.exe
19/12/2014  04:42 PM         1,503,744 git-describe.exe
19/12/2014  04:42 PM         1,503,744 git-diff-files.exe
19/12/2014  04:42 PM         1,503,744 git-diff-index.exe
19/12/2014  04:42 PM         1,503,744 git-diff-tree.exe
19/12/2014  04:42 PM         1,503,744 git-diff.exe
17/12/2014  09:20 PM            13,300 git-difftool
17/12/2014  09:20 PM             1,968 git-difftool--helper
19/12/2014  04:42 PM         1,503,744 git-fast-export.exe
19/12/2014  04:42 PM           809,472 git-fast-import.exe
19/12/2014  04:42 PM         1,503,744 git-fetch-pack.exe
19/12/2014  04:42 PM         1,503,744 git-fetch.exe
17/12/2014  09:20 PM            11,658 git-filter-branch
19/12/2014  04:42 PM         1,503,744 git-fmt-merge-msg.exe
19/12/2014  04:42 PM         1,503,744 git-for-each-ref.exe
19/12/2014  04:42 PM         1,503,744 git-format-patch.exe
02/07/2014  10:45 PM               581 git-format-patch.exe.manifest
19/12/2014  04:42 PM         1,503,744 git-fsck-objects.exe
19/12/2014  04:42 PM         1,503,744 git-fsck.exe
19/12/2014  04:42 PM         1,503,744 git-gc.exe
19/12/2014  04:42 PM         1,503,744 git-get-tar-commit-id.exe
19/12/2014  04:42 PM         1,503,744 git-grep.exe
17/12/2014  09:20 PM               687 git-gui
17/12/2014  09:20 PM             1,372 git-gui--askpass
17/12/2014  09:20 PM             1,112 git-gui--askyesno
17/12/2014  09:20 PM           100,439 git-gui.tcl
19/12/2014  04:42 PM         1,503,744 git-hash-object.exe
19/12/2014  04:42 PM         1,503,744 git-help.exe
19/12/2014  04:42 PM           781,824 git-http-backend.exe
19/12/2014  04:42 PM           804,864 git-http-fetch.exe
19/12/2014  04:42 PM           923,648 git-http-push.exe
19/12/2014  04:42 PM           788,992 git-imap-send.exe
19/12/2014  04:42 PM         1,503,744 git-index-pack.exe
19/12/2014  04:42 PM         1,503,744 git-init-db.exe
19/12/2014  04:42 PM         1,503,744 git-init.exe
19/12/2014  04:42 PM         1,503,744 git-log.exe
19/12/2014  04:42 PM         1,503,744 git-ls-files.exe
19/12/2014  04:42 PM         1,503,744 git-ls-remote.exe
19/12/2014  04:42 PM         1,503,744 git-ls-tree.exe
19/12/2014  04:42 PM         1,503,744 git-mailinfo.exe
19/12/2014  04:42 PM         1,503,744 git-mailsplit.exe
19/12/2014  04:42 PM         1,503,744 git-merge-base.exe
19/12/2014  04:42 PM         1,503,744 git-merge-file.exe
19/12/2014  04:42 PM         1,503,744 git-merge-index.exe
17/12/2014  09:20 PM             2,231 git-merge-octopus
17/12/2014  09:20 PM             3,482 git-merge-one-file
19/12/2014  04:42 PM         1,503,744 git-merge-ours.exe
19/12/2014  04:42 PM         1,503,744 git-merge-recursive.exe
17/12/2014  09:20 PM               944 git-merge-resolve
19/12/2014  04:42 PM         1,503,744 git-merge-subtree.exe
19/12/2014  04:42 PM         1,503,744 git-merge-tree.exe
19/12/2014  04:42 PM         1,503,744 git-merge.exe
17/12/2014  09:20 PM             8,377 git-mergetool
17/12/2014  09:20 PM             7,562 git-mergetool--lib
19/12/2014  04:42 PM         1,503,744 git-mktag.exe
19/12/2014  04:42 PM         1,503,744 git-mktree.exe
19/12/2014  04:42 PM         1,503,744 git-mv.exe
19/12/2014  04:42 PM         1,503,744 git-name-rev.exe
19/12/2014  04:42 PM         1,503,744 git-notes.exe
17/12/2014  09:20 PM               109 git-p4
19/12/2014  04:42 PM         1,503,744 git-pack-objects.exe
19/12/2014  04:42 PM         1,503,744 git-pack-redundant.exe
19/12/2014  04:42 PM         1,503,744 git-pack-refs.exe
17/12/2014  09:20 PM             2,313 git-parse-remote
19/12/2014  04:42 PM         1,503,744 git-patch-id.exe
02/07/2014  10:45 PM               577 git-patch-id.exe.manifest
19/12/2014  04:42 PM         1,503,744 git-prune-packed.exe
19/12/2014  04:42 PM         1,503,744 git-prune.exe
17/12/2014  09:20 PM             8,558 git-pull
19/12/2014  04:42 PM         1,503,744 git-push.exe
17/12/2014  09:20 PM             3,349 git-quiltimport
19/12/2014  04:42 PM         1,503,744 git-read-tree.exe
17/12/2014  09:20 PM            15,282 git-rebase
17/12/2014  09:20 PM             2,135 git-rebase--am
17/12/2014  09:20 PM            27,816 git-rebase--interactive
17/12/2014  09:20 PM             3,754 git-rebase--merge
19/12/2014  04:42 PM         1,503,744 git-receive-pack.exe
19/12/2014  04:42 PM         1,503,744 git-reflog.exe
17/12/2014  09:20 PM             4,180 git-relink
19/12/2014  04:42 PM         1,503,744 git-remote-ext.exe
19/12/2014  04:42 PM         1,503,744 git-remote-fd.exe
19/12/2014  04:42 PM           819,712 git-remote-ftp.exe
19/12/2014  04:42 PM           819,712 git-remote-ftps.exe
19/12/2014  04:42 PM           819,712 git-remote-http.exe
19/12/2014  04:42 PM           819,712 git-remote-https.exe
19/12/2014  04:42 PM           825,344 git-remote-testsvn.exe
19/12/2014  04:42 PM         1,503,744 git-remote.exe
19/12/2014  04:42 PM         1,503,744 git-repack.exe
19/12/2014  04:42 PM         1,503,744 git-replace.exe
17/12/2014  09:20 PM             3,813 git-request-pull
19/12/2014  04:42 PM         1,503,744 git-rerere.exe
19/12/2014  04:42 PM         1,503,744 git-reset.exe
19/12/2014  04:42 PM         1,503,744 git-rev-list.exe
19/12/2014  04:42 PM         1,503,744 git-rev-parse.exe
19/12/2014  04:42 PM         1,503,744 git-revert.exe
19/12/2014  04:42 PM         1,503,744 git-rm.exe
17/12/2014  09:20 PM            44,197 git-send-email
19/12/2014  04:42 PM         1,503,744 git-send-pack.exe
17/12/2014  09:20 PM             1,990 git-sh-i18n
19/12/2014  04:42 PM           776,192 git-sh-i18n--envsubst.exe
17/12/2014  09:20 PM             8,168 git-sh-setup
19/12/2014  04:42 PM         1,503,744 git-shortlog.exe
19/12/2014  04:42 PM         1,503,744 git-show-branch.exe
19/12/2014  04:42 PM           775,168 git-show-index.exe
19/12/2014  04:42 PM         1,503,744 git-show-ref.exe
19/12/2014  04:42 PM         1,503,744 git-show.exe
19/12/2014  04:42 PM         1,503,744 git-stage.exe
17/12/2014  09:20 PM            13,560 git-stash
19/12/2014  04:42 PM         1,503,744 git-status.exe
19/12/2014  04:42 PM         1,503,744 git-stripspace.exe
17/12/2014  09:20 PM            31,941 git-submodule
17/12/2014  09:21 PM            15,633 git-subtree
17/12/2014  09:20 PM            61,528 git-svn
19/12/2014  04:42 PM         1,503,744 git-symbolic-ref.exe
19/12/2014  04:42 PM         1,503,744 git-tag.exe
19/12/2014  04:42 PM         1,503,744 git-unpack-file.exe
19/12/2014  04:42 PM         1,503,744 git-unpack-objects.exe
19/12/2014  04:42 PM         1,503,744 git-update-index.exe
02/07/2014  10:45 PM               581 git-update-index.exe.manifest
19/12/2014  04:42 PM         1,503,744 git-update-ref.exe
02/07/2014  10:45 PM               579 git-update-ref.exe.manifest
19/12/2014  04:42 PM         1,503,744 git-update-server-info.exe
02/07/2014  10:45 PM               587 git-update-server-info.exe.manifest
19/12/2014  04:42 PM         1,503,744 git-upload-archive.exe
19/12/2014  04:42 PM           793,088 git-upload-pack.exe
19/12/2014  04:42 PM         1,503,744 git-var.exe
19/12/2014  04:42 PM         1,503,744 git-verify-pack.exe
19/12/2014  04:42 PM         1,503,744 git-verify-tag.exe
17/12/2014  09:20 PM             4,398 git-web--browse
19/12/2014  04:42 PM         1,503,744 git-whatchanged.exe
19/12/2014  04:42 PM         1,503,744 git-write-tree.exe
19/12/2014  04:42 PM         1,503,744 git.exe
19/12/2014  04:41 PM    <DIR>          mergetools
             163 File(s)    175,045,269 bytes
               3 Dir(s)   3,214,245,888 bytes free

C:\dev\go\src>

Alex

@aclements
Copy link
Member

git -c alias.x='!echo $PATH' x
git: '$PATH'' is not a git command. See 'git --help'.

Hmm. That clearly didn't work. It looks like it split the arguments on the space after echo. How do you quote things in command prompt so they get passed as one argument and not expanded? Does git -c alias.x="!echo $PATH" x work? I just want to see that PATH that git aliases are run with. Alternatively, you could add x = !echo $PATH to the [alias] section of .gitconfig and just run git x, which would avoid any shell quoting/splitting issues.

@alexbrainman
Copy link
Member Author

How do you quote things in command prompt so they get passed as one argument and not expanded?

I don't think you really want to know. But, if interested, search http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx for cmd.exe.

Does git -c alias.x="!echo $PATH" x work?

It does indeed:

C:\dev\go\src>git -c alias.x="!echo $PATH" x
/libexec/git-core:/bin:/bin:/mingw/bin:/c/Windows/system32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/:/cmd:/c/Program Files (x86)/Mercurial:/c/dev/mingw64_4.9.1/bin:/c/dev/my/bin/:/c/dev/go/bin

C:\dev\go\src>

I will add my PATH for completeness:

C:\dev\go\src>echo %PATH%
C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program Files (x86)\Git\cmd;C:\Program Files (x86)\Mercurial;c:\dev\mingw64_4.9.1\bin;c:\dev\my\bin\;C:\dev\go\bin

C:\dev\go\src>

Alex

@mattn
Copy link
Member

mattn commented Feb 2, 2016

'C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Program' is not a git command. See 'git --help'.

C:\Program is valid? Seems broken with the space in the C:\Program Files.

@aclements
Copy link
Member

I don't think you really want to know. But, if interested, search http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx for cmd.exe.

Yikes!

/libexec/git-core:/bin:/bin:/mingw/bin:/c/Windows/system32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/:/cmd:/c/Program Files (x86)/Mercurial:/c/dev/mingw64_4.9.1/bin:/c/dev/my/bin/:/c/dev/go/bin

Based on this, I'm guessing that MingW is actually what's putting some sh.exe in git's PATH. Unfortunately, I think that also means there's no good way to fake it in the git-codereview test short of some gnarly hack like the aliased PATH echo that I don't think we want to do.

@alexbrainman
Copy link
Member Author

SGTM

@golang golang locked and limited conversation to collaborators Feb 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge OS-Windows
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@mattn @rsc @aclements @gopherbot @alexbrainman