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
