Subject: Re: [PATCH v3 11/34] fsmonitor-fs-listen-win32: stub in backend for Windows

From: Junio C Hamano <[email-masked]>
Date: Mon, 19 Jul 2021 12:58:51 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

>>>>> Why put this in an ifdef?
>>>> ...
>>> 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
>> ifndef 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.
>
> Indeed, which is why I'm pointing out that wrapping it in an ifdef is
> pointless, which is why we don't do it for the other ones.
>
> We do have a bunch of ifdef'd things there for perf etc., I'm not sure
> if it matters or not for those.

Sorry, but I still do not get the question.  There are bunch of
ifndef in Makefile in addition to ifeq/ifneq and your question

    FSMONITOR_DAEMON_BACKEND option require a nonexistent line as
    opposed to an empty one?

is asking "why is it X" when X is not quite true.  I presume that
your "wrapping it in an ifdef" refers to a construct like this:

>>> > +ifdef FSMONITOR_DAEMON_BACKEND
>>> > +	COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND
>>> > +	COMPAT_OBJS += compat/fsmonitor/fsmonitor-fs-listen-$(FSMONITOR_DAEMON_BACKEND).o
>>> > +endif

but is your suggestion that it should be written like this instead?

>>> > +ifneq ($(FSMONITOR_DAEMON_BACKEND),)
>>> > +	COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND
>>> > +	COMPAT_OBJS += compat/fsmonitor/fsmonitor-fs-listen-$(FSMONITOR_DAEMON_BACKEND).o
>>> > +endif

I do not think the latter is any easier to follow (and we have many
ifdef and ifndef in our Makefile already).  Perhaps I will see what
you mean when I see your "better alternative", but so far, I am not
successfully guessing what it is.