Re: [Automake-NG] [PATCH 01/14] [ng] deptrack: remove an obsolete comment

[Stefano Lattarini]

On 06/21/2012 08:37 PM, Dave Hart wrote:
> On Thu, Jun 21, 2012 at 18:29 UTC, Stefano Lattarini wrote:
>> Hi Dave, thanks for the review.
>>
>> On 06/21/2012 07:16 PM, Dave Hart wrote:
>>> On Thu, Jun 21, 2012 at 10:32 AM, Stefano Lattarini wrote:
>>>> * lib/am/depend.am: Since in Automake-NG the generated Makefile calls
>>>> '-include' (not 'include') on the dependency tracking '.Po' files, we
>>>> can remove them at any time without causing any 'make' call to fail.
>>>
>>> I assume you already understand the nuances, Stefano, but I don't like
>>> the suggestion "we can remove them at any time without causing any
>>> 'make' call to fail."
>>>
>>> I agree one can remove *.Po files using Automake-NG without causing
>>> make to fail due to missing .Po include file.  However, there are many
>>> other ways for any 'make' call to fail -- such as due to incorrect
>>> dependency tracking caused by too-aggressive cleaning or stubbing of
>>> *.Po files.
>>>
>>> The natural intuition of the end user building with 'make' is to
>>> assume deleting files created after the first 'make' is making the
>>> source tree cleaner and thereby increase the odds of a subsequent
>>> 'make' succeeding and producing correct resulting outputs.  That
>>> intuition misleads when it comes to .deps/*.Po files, which should not
>>> be removed unless all of the dependent *.o files are removed first.
>>>
>> A most sensible invariant indeed -- which is respected in the current
>> code, because the '.deps' directory is only cleaned by "make distclean",
>> while the '*.o' files are removed by "make mostlyclean".  So the
>> '.deps' directories should only be removed when (or after) all the
>> compiled objects are removed -- no problems there.
>>
>> Seen in another perspective, since "make distclean" is meant to reset
>> the status of a build tree to that of a just extracted tarball (and
>> this invariant is checked by the "distcheck" target, we can be sure
>> that such a status is consistent too (assuming the status of the
>> release tarball was, of course).
>>
>> Does this reasoning dispels your misgivings?
>>
>>> This understandable error caused recurring friction between myself and
>>> another developer for years before I discovered why her incremental
>>> builds of my commits failed to link due to incorrect dependency
>>> tracking all too often.  She was using a script which updated source
>>> from the VCS, then "cleaned" all the .deps dirs, then stubbed the *.Po
>>> files using config.status (so they existed devoid of any
>>> dependencies), and finally invoked configure and make.
>>>
>> But you agree that this is just an user error that is in no way Automake's
>> fault, right?
>>
>>> Given the nonintuitive effect of "cleaning" .deps/*.Po files, I would
>>> prefer if you try to avoid appearing to minimize the possibility of
>>> "make" failing as a result.
>>>
>> Consider that the '*.Po' files should only be deleted upon "make distcheck",
>> not upon "make check" (for the excellent reasons you've stated), no such
>> issue should be possible (the extensive test cases 't/depcomp-*.tap' should
>> offer coverage in this respect).
>>
>> But maybe I am misunderstanding you, and you are just objecting to my
>> commit message?  In which case, feel free to suggest improvements, and
>> I'll gladly incorporate them.
> 
> Yes, my only concern is the commit message could be misread to suggest
> removing *.Po files is now safe at any time.
> 
> How about:
> 
> * lib/am/depend.am: Since in Automake-NG the generated Makefile calls
> '-include' (not 'include') on the dependency tracking '.Po' files, 'make' will
> never fail simply because a '.Po' file is not found.  Remove a comment
> regarding the now-impossible include failure.
>
Much better; consider yourself co-opted as a co-author of this patch :-)

Thanks,
  Stefano