add a Win32 askpass #371

rimrul · 2021-08-13T20:43:37Z

It's currently as bare-bones as it gets, but seems to be functional.

The prompt does get wrapped into multiple lines if it gets too long. The code is obviously heavily based on the sample code for line edits and slightly on git-askyesno.

I intend to add the Git for Windows icon on the right (similar to the GCM askpass) at some point in the near future, but that'll probably happen after v2.33.0 is out.

The name isn't set in stone yet, but I think shipping two significantly different git-askpass.exe that are more or less intended for the same purpose might lead to confusion.

Changes since v1:

rename to git-askpass
includ the changes to the Makefile that I forgot to add
switch from DialogBoxW() to DialogBoxParamW(). This allows passing the prompt directly and gives us easier access to the result of the Dialog
return non-zero on cancel (alligns behaviour closer with OpenSSH ssh-askpass)
drop the changing of the default button from the sample code (alligns behaviour closer with OpenSSH ssh-askpass)
pass prompt directly
prefer %ls over %S
also show single arg prompts
fixed checksums that where off due to wrong EOLs
prepared env.sh to also handle MSYSTEM=ARM64
added the new askpass to env.sh

dscho

Very nice!

dscho · 2021-08-13T21:50:29Z

git-extra/PKGBUILD

@@ -52,7 +52,10 @@ source=('inputrc'
        'update-via-pacman.bat'
        'git-credential-helper-selector.c'
        'git-credential-helper-selector.manifest'
-        'git-credential-helper-selector.rc')
+        'git-credential-helper-selector.rc'
+        'git-win32-askpass.c'


I intend to stop shipping GCM soon (and only retain GCM Core, its official successor). At that point at the latest, using the name git-askpass would actually bring a benefit.

Would that cause a new name conflict with git-ecosystem/git-credential-manager#385 or obsolete that issue?

I think it will obsolete git-ecosystem/git-credential-manager#385

git-extra/git-win32-askpass.c

rimrul · 2021-08-14T08:49:27Z

I think I've addressed all concerns. I've also added the new askpass to env.sh and prepared env.sh to handle ARM64.

This is a drop-in replacement for `git gui--askpass`. Since we added an option to use an external `ssh` found on the `PATH` (git-for-windows#367) we'll need an `askpass` implementation that can be called by any windows application even without understanding shebang lines. `git gui--askpass` probably also has the same problems with screenreaders that `git gui--askyesno` had(git-for-windows#234), so we'll likely get improved accessibility as a positive side-effect. Signed-off-by: Matthias Aßhauer <mha1993@live.de>

Signed-off-by: Matthias Aßhauer <mha1993@live.de>

The new `git-askpass.exe` doesn't use shebang lines and thus works with non-msys2 OpenSSH. It's also likely to interact more nicely with screen readers. Signed-off-by: Matthias Aßhauer <mha1993@live.de>

dscho · 2021-08-14T09:42:21Z

I modified this a little, locally. This is the range-diff:

1:  7e0a028098 ! 1:  da9ba5d86b git-extra: include an simple Win32 `git askpass`
    @@ Commit message
         Signed-off-by: Matthias Aßhauer <mha1993@live.de>
     
      ## git-extra/Makefile ##
    +@@ git-extra/Makefile: CXXFLAGS ?= -Wall
    + all: $(BUILDDIR)/create-shortcut.exe $(BUILDDIR)/WhoUses.exe \
    + 	$(BUILDDIR)/blocked-file-util.exe $(BUILDDIR)/proxy-lookup.exe \
    + 	$(BUILDDIR)/git-askyesno.exe \
    ++	$(BUILDDIR)/git-askpass.exe \
    + 	$(BUILDDIR)/git-credential-helper-selector.exe
    + 
    + $(BUILDDIR)/create-shortcut.exe: $(BUILDDIR)/create-shortcut.o
     @@ git-extra/Makefile: $(BUILDDIR)/git-credential-helper-selector.exe: \
      	$(BUILDDIR)/git-credential-helper-selector.o \
      	$(BUILDDIR)/git-credential-helper-selector.res
    @@ git-extra/git-askpass.c (new)
     +		case WM_INITDIALOG:
     +			/* Display the prompt */
     +			SetDlgItemTextW(hDlg, IDC_PROMPT, (wchar_t *) lParam);
    -+			return TRUE; 
    ++			if (GetDlgCtrlID((HWND) wParam) != IDE_PASSWORDEDIT) {
    ++				SetFocus(GetDlgItem(hDlg, IDE_PASSWORDEDIT));
    ++				return FALSE;
    ++			}
    ++			return TRUE;
     +		case WM_COMMAND:
     +			switch(wParam)
     +			{
     +				case IDOK:
     +					/* Get number of characters. */
     +					cchPassword = (WORD) SendDlgItemMessage(hDlg,
    -+															IDE_PASSWORDEDIT,
    -+															EM_LINELENGTH,
    -+															(WPARAM) 0,
    -+															(LPARAM) 0);
    ++										IDE_PASSWORDEDIT,
    ++										EM_LINELENGTH,
    ++										(WPARAM) 0,
    ++										(LPARAM) 0);
     +
     +					lpszPassword = (wchar_t *) malloc(sizeof(wchar_t) * (cchPassword + 1));
     +					if (!lpszPassword) {
    @@ git-extra/git-askpass.c (new)
     +
     +					/* Get the characters. */
     +					SendDlgItemMessageW(hDlg,
    -+									   IDE_PASSWORDEDIT,
    -+									   EM_GETLINE,
    -+									   (WPARAM) 0,       /* line 0 */
    -+									   (LPARAM) lpszPassword);
    ++							    IDE_PASSWORDEDIT,
    ++							    EM_GETLINE,
    ++							    (WPARAM) 0,       /* line 0 */
    ++							    (LPARAM) lpszPassword);
     +
     +					/* Null-terminate the string. */
     +					lpszPassword[cchPassword] = 0;
    @@ git-extra/git-askpass.c (new)
     +{
     +	INT_PTR res;
     +	wchar_t *prompt = NULL;
    -+	
    ++
     +	if (argc < 2) {
     +		MessageBoxW(NULL, L"Usage: git askpass <prompt>", L"Error!", MB_OK);
     +		return 1;
    @@ git-extra/git-askpass.c (new)
     +	free(prompt);
     +	return !res;
     +}
    - \ No newline at end of file
     
      ## git-extra/git-askpass.h (new) ##
     @@
2:  1976019925 = 2:  dc55377e79 git-extra: set `DISPLAY` and `SSH_ASKPASS` for ARM64
3:  350810e4a9 = 3:  c292bffd28 env.sh: prefer git-askpass.exe over git-gui--askpass

Summary:

git-askpass.exe is now built upon make
The edit box gets the focus
Whitespace issues were fixed via git rebase --whitespace=fix
I fixed some indentation manually

dscho · 2021-08-14T09:43:37Z

git-extra is being built and uploaded here: https://dev.azure.com/Git-for-Windows/git/_build/results?buildId=82787&view=results

dscho · 2021-08-14T09:44:00Z

Thank you so much @rimrul!!!

dscho · 2021-08-14T09:49:17Z

Oh, and once this is built, we will have to fix mingit/release.sh, I think: it would probably see a conflict otherwise because libexec/git-core/* is moved to bin/ in MinGit.

rimrul · 2021-08-14T10:13:05Z

libexec/git-core/* is moved to bin/ in MinGit.

Is it? I'm looking at the contents of MinGit-2.32.0.2-64-bit.zip and git-askpass.exe (and about 46 other files) is in libexec/git-core not bin/

dscho · 2021-08-14T12:27:02Z

Hrm. I thought I had moved them all into bin/. It's actually quite important.

dscho · 2021-08-14T18:27:22Z

Hrm. I thought I had moved them all into bin/. It's actually quite important.

Turns out that I only moved the .exe files from libexec/git-core/ to bin/ that originate from the mingw-w64-git package:

build-extra/mingit/release.sh

Lines 126 to 140 in 36a1cbd

    
           pacman -Ql mingw-w64-$ARCH-git | 
        
           sed 's|^[^ ]* /||' | 
        
           grep "^mingw$BITNESS/libexec/git-core/.*\.exe$" | 
        
           sort >"$SCRIPT_PATH"/sorted-libexec-exes && 
        
           MOVED_FILE=etc/libexec-moved.txt && 
        
           comm -12 "$SCRIPT_PATH"/sorted-all "$SCRIPT_PATH"/sorted-libexec-exes \ 
        
           	>"$SCRIPT_PATH"/root/$MOVED_FILE && 
        
           if test ! -s "$SCRIPT_PATH"/root/$MOVED_FILE 
        
           then 
        
           	die "Could not find any .exe files in libexec/git-core/" 
        
           fi && 
        
           BIN_DIR=mingw$BITNESS/bin && 
        
           mkdir -p "$SCRIPT_PATH"/root/$BIN_DIR && 
        
           (cd / && 
        
            cp $(cat "$SCRIPT_PATH"/root/$MOVED_FILE) "$SCRIPT_PATH"/root/$BIN_DIR/) &&

So I think we're actually good here.

rimrul requested a review from dscho August 13, 2021 20:43

rimrul mentioned this pull request Aug 13, 2021

Add choice for external OpenSSH #367

Merged

dscho reviewed Aug 13, 2021

View reviewed changes

rimrul force-pushed the win32-askpass branch 4 times, most recently from b9184fd to 350810e Compare August 14, 2021 08:42

rimrul marked this pull request as ready for review August 14, 2021 08:46

rimrul changed the title ~~WIP: add a Win32 askpass~~ add a Win32 askpass Aug 14, 2021

rimrul added 3 commits August 14, 2021 11:38

git-extra: set DISPLAY and SSH_ASKPASS for ARM64

dc55377

Signed-off-by: Matthias Aßhauer <mha1993@live.de>

env.sh: prefer git-askpass.exe over git-gui--askpass

c292bff

The new `git-askpass.exe` doesn't use shebang lines and thus works with non-msys2 OpenSSH. It's also likely to interact more nicely with screen readers. Signed-off-by: Matthias Aßhauer <mha1993@live.de>

dscho force-pushed the win32-askpass branch from 350810e to c292bff Compare August 14, 2021 09:42

dscho merged commit b818f12 into git-for-windows:main Aug 14, 2021

rimrul deleted the win32-askpass branch August 14, 2021 09:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add a Win32 askpass #371

add a Win32 askpass #371

rimrul commented Aug 13, 2021 •

edited

Loading

dscho left a comment

dscho Aug 13, 2021

rimrul Aug 13, 2021

dscho Aug 14, 2021

rimrul commented Aug 14, 2021

dscho commented Aug 14, 2021

dscho commented Aug 14, 2021

dscho commented Aug 14, 2021

dscho commented Aug 14, 2021

rimrul commented Aug 14, 2021

dscho commented Aug 14, 2021

dscho commented Aug 14, 2021

add a Win32 askpass #371

add a Win32 askpass #371

Conversation

rimrul commented Aug 13, 2021 • edited Loading

dscho left a comment

Choose a reason for hiding this comment

dscho Aug 13, 2021

Choose a reason for hiding this comment

rimrul Aug 13, 2021

Choose a reason for hiding this comment

dscho Aug 14, 2021

Choose a reason for hiding this comment

rimrul commented Aug 14, 2021

dscho commented Aug 14, 2021

dscho commented Aug 14, 2021

dscho commented Aug 14, 2021

dscho commented Aug 14, 2021

rimrul commented Aug 14, 2021

dscho commented Aug 14, 2021

dscho commented Aug 14, 2021

rimrul commented Aug 13, 2021 •

edited

Loading