Re: [PATCH 05/10] send-pack.c: remove #ifdef NO_PTHREADS

[Jeff King <peff@peff.net>]

On Sat, Oct 27, 2018 at 09:09:58AM +0200, Nguyễn Thái Ngọc Duy wrote:

> While at there correct "#include cache.h" position. It must be one of
> the first includes.
> [...]
> @@ -1,4 +1,5 @@
>  #include "builtin.h"
> +#include "cache.h"

I think it's actually fine as it is. The rule is not about cache.h in
particular, but that the first include must be one of the special ones.
And builtin.h is such a special one. After that, there are no ordering
rules.

> @@ -203,9 +204,8 @@ static int receive_status(int in, struct ref *refs)
>  static int sideband_demux(int in, int out, void *data)
>  {
>  	int *fd = data, ret;
> -#ifdef NO_PTHREADS
> -	close(fd[1]);
> -#endif
> +	if (!HAVE_THREADS)
> +		close(fd[1]);
>  	ret = recv_sideband("send-pack", fd[0], out);
>  	close(out);
>  	return ret;

This one is a very interesting case. Your conversion here isn't wrong,
but what we actually want to know is: is "struct async" implemented as a
separate process or not.

And "struct async" will continue to switch on NO_PTHREADS, as it can
never use these dummy bits. I can't think of a way that HAVE_THREADS
would ever diverge from how "struct async" works, but my gut feeling is
that we probably ought to have a separate variable for the async code to
tell how it works. Even if it ends up being the same as HAVE_THREADS,
it helps untangle the two cases.

-Peff