git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Carlo Arenas <carenas@gmail.com>
Cc: Bagas Sanjaya <bagasdotme@gmail.com>,
	git@vger.kernel.org, lehmacdj@gmail.com
Subject: Re: [PATCH 1/3] t0301: fixes for windows compatibility
Date: Mon, 13 Sep 2021 11:01:54 -0700	[thread overview]
Message-ID: <xmqqtuiofj71.fsf@gitster.g> (raw)
In-Reply-To: <CAPUEspi1NzM34whXOT8T7NgJ1SCBGTPETcF0MY-UHz+kRqCTZQ@mail.gmail.com> (Carlo Arenas's message of "Mon, 13 Sep 2021 00:13:50 -0700")

Carlo Arenas <carenas@gmail.com> writes:

> On Sun, Sep 12, 2021 at 10:34 PM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>> On 13/09/21 03.28, Carlo Marcelo Arenas Belón wrote:
>> > test -S is not able to detect that a file is a socket, so use
>> > test -f instead.
>>
>> Isn't test -f just check for socket as regular file?
>
> and that is exactly how they look; ironically a -f check in Linux
> fails for sockets so maybe better to do -e?
>
> an empty file with nothing that indicates in Windows Explorer or a
> stat call (from WSL or git bash), that they are anything else.

It actually is a quite attractive idea to use "-e", or even more
preferrably, test_path_exists.  For example:

@@ -31,7 +42,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 $FLAG "$XDG_CACHE_HOME/git/credential/socket" &&
 	test_path_is_missing "$HOME/.git-credential-cache/socket" &&
 	test_path_is_missing "$HOME/.cache/git/credential/socket"
 '

test_path_exists contrasts better with the two test_path_is_missing
and explains what is being tested better.  Before this part, we have
run some "git credential" test, and there are three possible places
that the socket may appear (XDG, HOME/.git-credential-cache/ and
HOME/.cache/git/credential/), and we want to make sure only one of
them gets it.

One possible downside is that it makes us rely more on our knowledge
that we communicate via unix-domain socket (i.e. what the "socket"
the test is checking is).  By assuming that a mere presence of some
filesystem entity at the inspected path is OK, we may not notice a
breakage that creates a regular file or a directory there by mistake,
yet successfully carry out the credential tests.  It may even be a
good thing, if future ourselves have somehow found out how to use a
regular file or a directory for IPC instead of using a socket ;-).

Thanks.




  reply	other threads:[~2021-09-13 18:01 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-12 20:28 [PATCH 0/3] windows: allow building without NO_UNIX_SOCKETS Carlo Marcelo Arenas Belón
2021-09-12 20:28 ` [PATCH 1/3] t0301: fixes for windows compatibility Carlo Marcelo Arenas Belón
2021-09-13  1:04   ` Junio C Hamano
2021-09-13  5:34   ` Bagas Sanjaya
2021-09-13  7:13     ` Carlo Arenas
2021-09-13 18:01       ` Junio C Hamano [this message]
2021-09-12 20:28 ` [PATCH 2/3] credential-cache: check for windows specific errors Carlo Marcelo Arenas Belón
2021-09-13  1:10   ` Junio C Hamano
2021-09-12 20:28 ` [PATCH 3/3] git-compat-util: include declaration for unix sockets Carlo Marcelo Arenas Belón
2021-09-13  8:55 ` [PATCH v2 0/3] windows: allow building without NO_UNIX_SOCKETS Carlo Marcelo Arenas Belón
2021-09-13  8:55   ` [PATCH v2 1/3] t0301: fixes for windows compatibility Carlo Marcelo Arenas Belón
2021-09-13 11:50     ` Johannes Schindelin
2021-09-13 18:09       ` Junio C Hamano
2021-09-13  8:55   ` [PATCH v2 2/3] credential-cache: check for windows specific errors Carlo Marcelo Arenas Belón
2021-09-13 11:58     ` Johannes Schindelin
2021-09-13  8:56   ` [PATCH v2 3/3] git-compat-util: include declaration for unix sockets Carlo Marcelo Arenas Belón
2021-09-13 11:59     ` Johannes Schindelin
2021-09-13 11:42   ` [PATCH v2 0/3] windows: allow building without NO_UNIX_SOCKETS Johannes Schindelin
2021-09-14  7:25   ` [PATCH v3 " Carlo Marcelo Arenas Belón
2021-09-14  7:25     ` [PATCH v3 1/3] t0301: fixes for windows compatibility Carlo Marcelo Arenas Belón
2021-11-02  0:37       ` Ævar Arnfjörð Bjarmason
2021-09-14  7:25     ` [PATCH v3 2/3] credential-cache: check for windows specific errors Carlo Marcelo Arenas Belón
2021-09-14 16:43       ` Junio C Hamano
2021-09-14 19:09       ` What should happen in credential-cache on recoverable error without SPAWN option? Junio C Hamano
2021-09-14 19:33         ` Jeff King
2021-09-14  7:26     ` [PATCH v3 3/3] git-compat-util: include declaration for unix sockets in windows Carlo Marcelo Arenas Belón
2021-09-14 16:37     ` [PATCH v3 0/3] windows: allow building without NO_UNIX_SOCKETS Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtuiofj71.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=bagasdotme@gmail.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=lehmacdj@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).