From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 17 Jul 2021 05:13:44 GMT
Subject: Re: [PATCH v3 11/34] fsmonitor-fs-listen-win32: stub in backend for Windows
Message-ID: <xmqqa6mlplo7.fsf@gitster.g>
In-Reply-To: <87y2a6w61l.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

>>> > +ifdef FSMONITOR_DAEMON_BACKEND
>>> > +	COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND
>>> > +	COMPAT_OBJS += compat/fsmonitor/fsmonitor-fs-listen-$(FSMONITOR_DAEMON_BACKEND).o
>>> > +endif
>>> > +
>>> >  ifeq ($(TCLTK_PATH),)
>>> >  NO_TCLTK = NoThanks
>>> >  endif
>>> ...
>>>
>>> Why put this in an ifdef?
>>
>> Why not? What benefit does this question bring to improving this patch
>> series?
>
> I think that when adding code to the Makefile it makes sense to follow
> the prevailing pattern, unless there's a good reason to do otherwise,
> e.g. on my build:
> 	
> 	$ grep "''" GIT-BUILD-OPTIONS 
> 	NO_CURL=''
> 	NO_EXPAT=''
> 	NO_PERL=''
> 	NO_PTHREADS=''
> 	NO_PYTHON=''
> 	NO_UNIX_SOCKETS=''
> 	X=''
>
> Why does the FSMONITOR_DAEMON_BACKEND option require a nonexistent line
> as opposed to an empty one?

I do not quite get the question.

#!/bin/sh
cat >make.file <<\EOF
all::
ifeq ($(FSMONITOR_DAEMON_BACKEND),)
	echo it is empty
endif
ifdef FSMONITOR_DAEMON_BACKEND
	echo it is undefined
endif
EOF

echo "unset???"
make -f make.file

echo "set to empty???"
make -f make.file FSMONITOR_DAEMON_BACKEND=

These two make invocations will give us the same result, showing
that "is it set to empty" and "is it unset" are the same.