bug#27781: LIBOBJS should depend on LIBOBJDIR

[Michael Haubenwallner]

On 08/23/2017 11:24 PM, Mathieu Lirzin wrote:
> 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.

Agreed.

> Let's first fix the without such optimization.

I'm going to look into the (other subthread's) AC_LIBOBJ thing,
for adding the rule per explicit object file instead.

>>
>>>>        # 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?
It took a while for me too to understand what's going on here:

Consider $var = 'LTLIBOBJS';

> 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;

here, unconditionally: $clean_files{'$(LTLIBOBJS)'} = 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//;

Crucial here is that trailing { if $var =~ s/^LT// }:
First, leading 'LT' eventually is dropped: $var = 'LIBOBJS';
If the leading 'LT' was actually found: $clean_files{'LIBOBJS'} = MOSTLY_CLEAN;

> --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.
> 

/haubi/