bug#27781: LIBOBJS should depend on LIBOBJDIR

[Mathieu Lirzin]

Michael Haubenwallner <address@hidden> writes:

> On 08/22/2017 12:40 AM, Mathieu Lirzin wrote:
>> Michael Haubenwallner <address@hidden> writes:
>> 
>>> In this case let me come up with attached patch for now
>>> (without deeper knowledge of automake internals though).
>> 
>> I have tested this patch, and confirm that it fixes the bug.  However I
>> have on question regarding the added code.
>> 
>>> From c3d51adb53400fc8bf65c0e003d810b2e7166d0d Mon Sep 17 00:00:00 2001
>>> From: Michael Haubenwallner <address@hidden>
>>> Date: Wed, 16 Aug 2017 18:16:12 +0200
>>> Subject: [PATCH] automake: Depend on LIBOBJDIR for LIBOBJS.
>>>
>>> This change fixes automake bug#27781.
>>>
>>> * bin/automake.in: Add Makefile dependency on LIBOBJDIR/dirstamp for
>>> each LIBOBJS/ALLOCA variable used.
>>> ---
>>>  bin/automake.in | 11 ++++++++++-
>>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/bin/automake.in b/bin/automake.in
>>> index 9c4cb86..b82e8c5 100644
>>> --- a/bin/automake.in
>>> +++ b/bin/automake.in
>>> @@ -2153,10 +2153,19 @@ sub handle_LIBOBJS_or_ALLOCA
>>>        $dir = backname ($relative_dir) . "/$dir"
>>>          if $relative_dir ne '.';
>>>        define_variable ('LIBOBJDIR', "$dir", INTERNAL);
>>> +      my $dirstamp = '';
>>> +      # Abusing $clean_files{"$(VAR)"} as indicator for whether
>>> +      # we have added the "$(VAR): $dirstamp" dependency already.
>> 
>> I am not sure to understand what this comment really means.  Can you
>> explain it in other words?
>>         
>>> +      $dirstamp = require_build_directory ($dir)
>
> Thing is, $dirstamp is calculated only when ...
>
>>> +   if ! defined $clean_files{"\$($var)"};
>
> ... $clean_files{} was defined, which is done ...
>
>>>        $clean_files{"\$($var)"} = MOSTLY_CLEAN;
>
> ... here. Subsequently, $output_rules is extended only when $dirstamp was
> calculated:
>
>>> +      $output_rules .= "\$($var): $dirstamp\n" if ($dirstamp);
>
> But the name "clean_files" feels somewhat unrelated to the dirstamp dependency
> (as in causing a different output by itself), because I use it to make sure
> the dirstamp rule is added only once to $output_rules:
> Within flex, I've seen handle_LIBOBJS_or_ALLOCA to be called thrice, but this
> dirstamp rule is enough once, as "\$($var)" holds all three object files.
> While I have also thought about adding this dependency for each explicit
> object file separately - which would not need the "clean_files" indicator,
> I've failed to identify where to get the correct object file name from.

OK so this is mostly an optimization, since having multiple time the
same line shouldn't hurt.  Let's first fix the without such optimization.

>
>>>        # If LTLIBOBJS is used, we must also clear LIBOBJS (which might
>>>        # be created by libtool as a side-effect of creating LTLIBOBJS).
>>> -      $clean_files{"\$($var)"} = MOSTLY_CLEAN if $var =~ s/^LT//;
>>> +      if ($var =~ s/^LT//) {
>>> +   $clean_files{"\$($var)"} = MOSTLY_CLEAN;
>>> +   $output_rules .= "\$($var): $dirstamp\n" if ($dirstamp);
>>> +      }
>>>      }
>>>  
>>>    return $dir;
>

Regarding that part of the code even before you made it seems to me that
the conditional second '$clean_files{"\$($var)"} = MOSTLY_CLEAN'
statement is useless.  Am I overlooking something?  Here is the snippet
of the code before your changed.

--8<---------------cut here---------------start------------->8---
          define_variable ('LIBOBJDIR', "$dir", INTERNAL);
          $clean_files{"\$($var)"} = MOSTLY_CLEAN;
          # If LTLIBOBJS is used, we must also clear LIBOBJS (which might
          # be created by libtool as a side-effect of creating LTLIBOBJS).
          $clean_files{"\$($var)"} = MOSTLY_CLEAN if $var =~ s/^LT//;
--8<---------------cut here---------------end--------------->8---

> Another thought about the final "$(LIBOBJS): .../.dirstamp" Makefile line:
> If I remember correctly, there have been (non-GNU) make implementations that
> choke on this rule when LIBOBJS is an empty variable:
> Am I wrong here, or is GNU make required anyway these days?

GNU make is not required for current Automake version.  I didn't find
any reference of this issue with empty variable target in Autoconf
"Portable Make programming" section [1] but if it is actually the case
then it would be nice to find a solution for that.

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37

[1] 
https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/Portable-Make.html