Re: [bug-gettext] [PATCH] Add a --disable-tools option

[Stefano Lattarini]

Hi Thomas.

On 05/28/2013 09:07 PM, Thomas Petazzoni wrote:
> Dear Stefano Lattarini,
> 
> On Tue, 28 May 2013 20:40:32 +0200, Stefano Lattarini wrote:
> 
>>> -SUBDIRS = gnulib-local gettext-runtime gettext-tools
>>> +if ENABLE_TOOLS
>>> +TOOLS_SUBDIR = gettext-tools
>>> +else
>>> +TOOLS_SUBDIR =
>>> +endif
>>> +
>>> +SUBDIRS = gnulib-local gettext-runtime $(TOOLS_SUBDIR)
>>>
>> Or even simply:
>>
>>   -SUBDIRS = gnulib-local gettext-runtime gettext-tools
>>   +SUBDIRS = gnulib-local gettext-runtime
>>   +if ENABLE_TOOLS
>>   +SUBDIRS += gettext-tools
>>   +endif
>>
>> maybe?
> 
> I am by far not a Makefile.am expert, but I believe the += thing
> doesn't work.
>
It does --- it's not portable to *make*, but Automake handles it
correctly and spits out an output that is portable to POSIX make
(and more :-)

> Most of the Makefile.am I have seen do this trick of
> defining an intermediate variable.
>
I can't answer about that.  Maybe they have good reasons to do so,
or maybe they too have just copied the idiom from other makefiles,
that too have copied from other older makefiles, ...  You get the
picture.

> What I did is the way that is documented in the automake documentation:
> http://www.gnu.org/software/automake/manual/html_node/Subdirectories-with-AM_005fCONDITIONAL.html.
>
I think the example is in that form to minimize spurious difference
with the following example "Subdirectories with AC_SUBST":
http://www.gnu.org/software/automake/manual/html_node/Subdirectories-with-AC_005fSUBST.html

Anyway, what you did is *not wrong* -- I just suggested a briefer
idiom to obtain the same effect.  I have no strong feelings in the
issue, so if the Gettext maintainer can absolutely take your patch
if he prefers.  No objection at all from me!

> Thanks for your feedback,
> 
> Thomas

Regards,
  Stefano