All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Adam Dinwoodie <adam@dinwoodie.org>,
	Ramsay Jones <ramsay@ramsayjones.plus.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] simple-ipc: work around issues with Cygwin's Unix socket emulation
Date: Wed, 10 Nov 2021 09:11:49 -0800	[thread overview]
Message-ID: <xmqqtugkx7e2.fsf@gitster.g> (raw)
In-Reply-To: <pull.1074.git.1636542550889.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Wed, 10 Nov 2021 11:09:10 +0000")

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Cygwin emulates Unix sockets by writing files with custom contents and
> then marking them as system files.
>
> The tricky problem is that while the file is written and its `system`
> bit is set, it is still identified as a file. This caused test failures
> when Git is too fast looking for the Unix sockets and then complains
> that there is a plain file in the way.
>
> Let's work around this by adding a delayed retry loop, specifically for
> Cygwin.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

OK, I was about to ask for an Ack from Cygwin folks but I see the
original that got Ack is more-or-less the same except for placement
of the comments (and this version of course is more polished than
the "this should work---please try it" version), so let me pretend
that this got tested-by from those who were happy with the original
in the old thread.

Thanks.


>     simple-ipc: work around issues with Cygwin's Unix socket emulation
>     
>     Adam Dinwoodie reported problems running the simple-ipc tests on Cygwin
>     [https://lore.kernel.org/git/20211104194619.GA12886@dinwoodie.org]. This
>     patch works around the underlying problem, which is rooted in Cygwin's
>     implementation details.
>     
>     With this patch, I could not reproduce the problem, even with sh
>     t0052-simple-ipc.sh --stress-limit=30.
>     
>     As per Junio's encouragement
>     [https://lore.kernel.org/git/xmqqee7ozyx4.fsf@gitster.g], I am
>     submitting this still in the -rc phase, hoping that it will make it into
>     v2.34.0 final.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1074%2Fdscho%2Fcygwin-vs-simple-ipc-workaround-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1074/dscho/cygwin-vs-simple-ipc-workaround-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1074
>
>  compat/simple-ipc/ipc-unix-socket.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c
> index 4e28857a0a1..28a79289d4f 100644
> --- a/compat/simple-ipc/ipc-unix-socket.c
> +++ b/compat/simple-ipc/ipc-unix-socket.c
> @@ -35,6 +35,28 @@ enum ipc_active_state ipc_get_active_state(const char *path)
>  		}
>  	}
>  
> +#ifdef __CYGWIN__
> +	/*
> +	 * Cygwin emulates Unix sockets by writing special-crafted files whose
> +	 * `system` bit is set.
> +	 *
> +	 * If we are too fast, Cygwin might still be in the process of marking
> +	 * the underlying file as a system file. Until then, we will not see a
> +	 * Unix socket here, but a plain file instead. Just in case that this
> +	 * is happening, wait a little and try again.
> +	 */
> +	{
> +		static const int delay[] = { 1, 10, 20, 40, -1 };
> +		int i;
> +
> +		for (i = 0; S_ISREG(st.st_mode) && delay[i] > 0; i++) {
> +			sleep_millisec(delay[i]);
> +			if (lstat(path, &st) == -1)
> +				return IPC_STATE__INVALID_PATH;
> +		}
> +	}
> +#endif
> +
>  	/* also complain if a plain file is in the way */
>  	if ((st.st_mode & S_IFMT) != S_IFSOCK)
>  		return IPC_STATE__INVALID_PATH;
>
> base-commit: 6c220937e2b26d85920bf2d38ff2464a0d57fd6b

  reply	other threads:[~2021-11-10 17:11 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-10 11:09 [PATCH] simple-ipc: work around issues with Cygwin's Unix socket emulation Johannes Schindelin via GitGitGadget
2021-11-10 17:11 ` Junio C Hamano [this message]
2021-11-12  8:50   ` Adam Dinwoodie

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=xmqqtugkx7e2.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=adam@dinwoodie.org \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=ramsay@ramsayjones.plus.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.