| Message ID | 20210913085600.35506-2-carenas@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | windows: allow building without NO_UNIX_SOCKETS | expand |
Hi Carlo, On Mon, 13 Sep 2021, Carlo Marcelo Arenas Belón wrote: > In preparation for a future patch that will allow building with > Unix Sockets in Windows, workaround a couple of issues from the > Mingw-W64 compatibility layer. > > test -S is not able to detect that a file is a socket, so use Is that really true, even with recent MSYS2 versions? I thought I saw some patch flying around on the Cygwin mailing list that added support for Unix sockets... > test -e instead (through a library function). > > `mkdir -m` will always fail with permission problems, so instead > call mkdir followed by chmod. Maybe explain that Windows' permission model is a lot more sophisticated (using Access Control Lists) and is therefore unable to interpret permissions like `0700`. And we probably need to mention then, too, that it is funny that `mkdir -m` complains while `chmod` does _not_... Ah, historical reasons... Thanks, Dscho > > The last invocation of mkdir would likely need the same treatment > but SYMLINK is unlikely to be enabled on Windows so it has been > punted for now. > > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > --- > v2: > * avoid the confusing -f test as suggested by Bagas > * try to help casual readers as suggested by Junio > > t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++-------- > 1 file changed, 24 insertions(+), 8 deletions(-) > > diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh > index ebd5fa5249..1f7b1e29e6 100755 > --- a/t/t0301-credential-cache.sh > +++ b/t/t0301-credential-cache.sh > @@ -9,6 +9,21 @@ test -z "$NO_UNIX_SOCKETS" || { > test_done > } > > +test_path_is_socket () { > + test -S "$1" > +} > + > +# in Windows, Unix Sockets look just like regular files > +uname_s=$(uname -s) > +case $uname_s in > +*MINGW*) > + test_socket_exist=test_path_exists > + ;; > +*) > + test_socket_exist=test_path_is_socket > + ;; > +esac A more canonical way would probably be to imitate what we do with `pwd` in `t/test-lib.sh`: test_path_is_socket () { test -S "$1" } case $uname_s in *MINGW*) test_path_is_socket () { # `test -S` cannot detect Win10's Unix sockets test -e "$1" } ;; esac > + > # don't leave a stale daemon running > test_atexit 'git credential-cache exit' > > @@ -21,7 +36,7 @@ test_expect_success 'socket defaults to ~/.cache/git/credential/socket' ' > rmdir -p .cache/git/credential/ > " && > test_path_is_missing "$HOME/.git-credential-cache" && > - test -S "$HOME/.cache/git/credential/socket" > + $test_socket_exist "$HOME/.cache/git/credential/socket" > ' > > XDG_CACHE_HOME="$HOME/xdg" > @@ -31,7 +46,7 @@ helper_test cache > > test_expect_success "use custom XDG_CACHE_HOME if set and default sockets are not created" ' > test_when_finished "git credential-cache exit" && > - test -S "$XDG_CACHE_HOME/git/credential/socket" && > + $test_socket_exist "$XDG_CACHE_HOME/git/credential/socket" && > test_path_is_missing "$HOME/.git-credential-cache/socket" && > test_path_is_missing "$HOME/.cache/git/credential/socket" > ' > @@ -48,7 +63,7 @@ test_expect_success 'credential-cache --socket option overrides default location > username=store-user > password=store-pass > EOF > - test -S "$HOME/dir/socket" > + $test_socket_exist "$HOME/dir/socket" > ' > > test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' > @@ -62,7 +77,7 @@ test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' > username=store-user > password=store-pass > EOF > - test -S "$HOME/.cache/git/credential/socket" && > + $test_socket_exist "$HOME/.cache/git/credential/socket" && > XDG_CACHE_HOME="$HOME/xdg" && > export XDG_CACHE_HOME && > check approve cache <<-\EOF && > @@ -71,7 +86,7 @@ test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' > username=store-user > password=store-pass > EOF > - test -S "$XDG_CACHE_HOME/git/credential/socket" > + $test_socket_exist "$XDG_CACHE_HOME/git/credential/socket" > ' > > test_expect_success 'use user socket if user directory exists' ' > @@ -79,14 +94,15 @@ test_expect_success 'use user socket if user directory exists' ' > git credential-cache exit && > rmdir \"\$HOME/.git-credential-cache/\" > " && > - mkdir -p -m 700 "$HOME/.git-credential-cache/" && > + mkdir -p "$HOME/.git-credential-cache/" && > + chmod 700 "$HOME/.git-credential-cache/" && > check approve cache <<-\EOF && > protocol=https > host=example.com > username=store-user > password=store-pass > EOF > - test -S "$HOME/.git-credential-cache/socket" > + $test_socket_exist "$HOME/.git-credential-cache/socket" > ' > > test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' ' > @@ -103,7 +119,7 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to > username=store-user > password=store-pass > EOF > - test -S "$HOME/.git-credential-cache/socket" > + $test_socket_exist "$HOME/.git-credential-cache/socket" > ' > > helper_test_timeout cache --timeout=1 > -- > 2.33.0.481.g26d3bed244 > >
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> +test_path_is_socket () { >> + test -S "$1" >> +} >> + >> +# in Windows, Unix Sockets look just like regular files >> +uname_s=$(uname -s) >> +case $uname_s in >> +*MINGW*) >> + test_socket_exist=test_path_exists >> + ;; >> +*) >> + test_socket_exist=test_path_is_socket >> + ;; >> +esac > > A more canonical way would probably be to imitate what we do with `pwd` in > `t/test-lib.sh`: Thanks for bringing up a better practice. Referring to a variable when calling a function gives a "we are doing something unusual" signal and it loses half its abstraction value at the callsites. E.g. >> test_when_finished "git credential-cache exit" && >> - test -S "$XDG_CACHE_HOME/git/credential/socket" && >> + $test_socket_exist "$XDG_CACHE_HOME/git/credential/socket" && >> test_path_is_missing "$HOME/.git-credential-cache/socket" && >> test_path_is_missing "$HOME/.cache/git/credential/socket" I actually do not think it is so bad to just use test_path_exists without per-platform conditional in this case, but if we want to be more conservative, I agree with you that case ... in *MINGW*) test_path_is_socket () { test_path_exists "$@" } ;; *) test_path_is_socket () { test -S "$1" } ;; esac is the way to go.
diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh index ebd5fa5249..1f7b1e29e6 100755 --- a/t/t0301-credential-cache.sh +++ b/t/t0301-credential-cache.sh @@ -9,6 +9,21 @@ test -z "$NO_UNIX_SOCKETS" || { test_done } +test_path_is_socket () { + test -S "$1" +} + +# in Windows, Unix Sockets look just like regular files +uname_s=$(uname -s) +case $uname_s in +*MINGW*) + test_socket_exist=test_path_exists + ;; +*) + test_socket_exist=test_path_is_socket + ;; +esac + # don't leave a stale daemon running test_atexit 'git credential-cache exit' @@ -21,7 +36,7 @@ test_expect_success 'socket defaults to ~/.cache/git/credential/socket' ' rmdir -p .cache/git/credential/ " && test_path_is_missing "$HOME/.git-credential-cache" && - test -S "$HOME/.cache/git/credential/socket" + $test_socket_exist "$HOME/.cache/git/credential/socket" ' XDG_CACHE_HOME="$HOME/xdg" @@ -31,7 +46,7 @@ helper_test cache test_expect_success "use custom XDG_CACHE_HOME if set and default sockets are not created" ' test_when_finished "git credential-cache exit" && - test -S "$XDG_CACHE_HOME/git/credential/socket" && + $test_socket_exist "$XDG_CACHE_HOME/git/credential/socket" && test_path_is_missing "$HOME/.git-credential-cache/socket" && test_path_is_missing "$HOME/.cache/git/credential/socket" ' @@ -48,7 +63,7 @@ test_expect_success 'credential-cache --socket option overrides default location username=store-user password=store-pass EOF - test -S "$HOME/dir/socket" + $test_socket_exist "$HOME/dir/socket" ' test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' @@ -62,7 +77,7 @@ test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' username=store-user password=store-pass EOF - test -S "$HOME/.cache/git/credential/socket" && + $test_socket_exist "$HOME/.cache/git/credential/socket" && XDG_CACHE_HOME="$HOME/xdg" && export XDG_CACHE_HOME && check approve cache <<-\EOF && @@ -71,7 +86,7 @@ test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' username=store-user password=store-pass EOF - test -S "$XDG_CACHE_HOME/git/credential/socket" + $test_socket_exist "$XDG_CACHE_HOME/git/credential/socket" ' test_expect_success 'use user socket if user directory exists' ' @@ -79,14 +94,15 @@ test_expect_success 'use user socket if user directory exists' ' git credential-cache exit && rmdir \"\$HOME/.git-credential-cache/\" " && - mkdir -p -m 700 "$HOME/.git-credential-cache/" && + mkdir -p "$HOME/.git-credential-cache/" && + chmod 700 "$HOME/.git-credential-cache/" && check approve cache <<-\EOF && protocol=https host=example.com username=store-user password=store-pass EOF - test -S "$HOME/.git-credential-cache/socket" + $test_socket_exist "$HOME/.git-credential-cache/socket" ' test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' ' @@ -103,7 +119,7 @@ test_expect_success SYMLINKS 'use user socket if user directory is a symlink to username=store-user password=store-pass EOF - test -S "$HOME/.git-credential-cache/socket" + $test_socket_exist "$HOME/.git-credential-cache/socket" ' helper_test_timeout cache --timeout=1
In preparation for a future patch that will allow building with Unix Sockets in Windows, workaround a couple of issues from the Mingw-W64 compatibility layer. test -S is not able to detect that a file is a socket, so use test -e instead (through a library function). `mkdir -m` will always fail with permission problems, so instead call mkdir followed by chmod. The last invocation of mkdir would likely need the same treatment but SYMLINK is unlikely to be enabled on Windows so it has been punted for now. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- v2: * avoid the confusing -f test as suggested by Bagas * try to help casual readers as suggested by Junio t/t0301-credential-cache.sh | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-)