git-for-windows / git

A fork of Git containing Windows-specific patches.
http://gitforwindows.org/
Other
8.37k stars 2.54k forks source link

`start-ssh-agent.cmd` doesn't execute `@CALL cmd %*` at the end of the script #3975

Open AnastaZIuk opened 2 years ago

AnastaZIuk commented 2 years ago

Setup

I'm using Git for windows, 64-bit

$ git --version --build-options

git version 2.37.1.windows.1
cpu: x86_64
built from commit: 323a69709944b193bb5cee81ff09fe9a4a686df5
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
$ cmd.exe /c ver

Microsoft Windows [Version 10.0.22000.795]
# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
> type "$env:USERPROFILE\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt

Editor Option: CustomEditor
Custom Editor Path: C:\Program Files\Notepad++\notepad++.exe
Default Branch Option:  
Path Option: Cmd
SSH Option: OpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFAlways
Bash Terminal Option: MinTTY
Git Pull Behavior Option: Merge
Use Credential Manager: Enabled
Performance Tweaks FSCache: Enabled
Enable Symlinks: Disabled
Enable Pseudo Console Support: Disabled
Enable FSMonitor: Disabled

Details

CMD

   cd "C:\Program Files\Git\cmd"
   ~~cmd~~ start-ssh-agent.exe /k echo "test"

it should perform ssh initialization and at the end if there is no fail it should execute @CALL cmd %* so in my case cmd /k echo "test"

nothing executed because there is a typo,

instead of doing

@IF NOT ERRORLEVEL 1 @(
    @CALL cmd %*
)

it should be

@IF NOT !ERRORLEVEL! == 1 @(
    @CALL cmd %*
)

I don't know the repository I should look into

dscho commented 2 years ago
cd "C:\Program Files\Git\cmd"
cmd start-ssh-agent.exe /k echo "test"

This is the output when I try it here:

C:\Users\me>cd "C:\Program Files\Git\cmd"

C:\Program Files\Git\cmd> cmd start-ssh-agent.exe /k echo "test"
"test"

C:\Program Files\Git\cmd>

In other words: it works as expected here, without the indicated patch. Maybe this has something to do with delayed expansion? But I see that the script does enable that.

Color me puzzled.

AnastaZIuk commented 2 years ago

indeed it works for windows powershell, but it doesn't work for windows cmd image

AnastaZIuk commented 2 years ago
cd "C:\Program Files\Git\cmd"
cmd start-ssh-agent.exe /k echo "test"

This is the output when I try it here:

C:\Users\me>cd "C:\Program Files\Git\cmd"

C:\Program Files\Git\cmd> cmd start-ssh-agent.exe /k echo "test"
"test"

C:\Program Files\Git\cmd>

In other words: it works as expected here, without the indicated patch. Maybe this has something to do with delayed expansion? But I see that the script does enable that.

Color me puzzled.

and after the patch it works for both

dscho commented 2 years ago

but it doesn't work for windows cmd

I tried this in CMD and in PowerShell, and it works for me in both. I am not opposed to accepting a PR with the patch (you will want to modify this file), but I want to understand first why it is needed.

dscho commented 2 years ago

It's hard to quote parts of a screenshot, so I won't. Pro-tip: paste text as text, not as a picture, in the future.

The thing about the "Found" line is that I do not get it, and when I run dir %TEMP%\ssh-??????* I also don't get any hit.

Even funnier: I do not get the SSH_AGENT_PID variable, either, so maybe something is going fundamentally wrong here.

AnastaZIuk commented 2 years ago

It's hard to quote parts of a screenshot, so I won't. Pro-tip: paste text as text, not as a picture, in the future.

oh right, sorry

well I can see that this script uses almost everywhere delayed expansion or variable expansion, and in this line https://github.com/git-for-windows/MINGW-packages/blob/05e0ebb4a53a6d58aa49c57a4a7040f149533473/mingw-w64-git/start-ssh-agent.cmd#L83 I created issue for it seems the syntax is incorrect? https://ss64.com/nt/delayedexpansion.html I'm not an expert in batch/cmd scripting, but this line simply doesn't work in my cmd

dscho commented 2 years ago

this line simply doesn't work in my cmd

The concern I have is that it might actually work as designed and that something in the command preceding it goes wrong, and we would only paper over that.

mfriedrich74 commented 2 years ago

Also please note that the proposed patch changes the check from not >=1 to <>1

On Mon, Aug 8, 2022, 6:59 AM Johannes Schindelin @.***> wrote:

this line simply doesn't work in my cmd

The concern I have is that it might actually work as designed and that something in the command preceding it goes wrong, and we would only paper over that.

— Reply to this email directly, view it on GitHub https://github.com/git-for-windows/git/issues/3975#issuecomment-1207973943, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZH5SEAMNFOEZ557YMXCQ3VYDSAPANCNFSM552IKL4A . You are receiving this because you are subscribed to this thread.Message ID: @.***>