Re: [PATCH 00/22] Towards a non-recursive build system for coreutils?

[Jim Meyering]

Stefano Lattarini wrote:
> On 08/30/2012 02:16 PM, Jim Meyering wrote:
>>
>> I've merged those into the appropriate change sets, removed all
>> Signed-off-by: lines and made a few minor wording changes.
>> Please review the differences.  If you're happy with the result,
>> I'll push the lot.
>>
> I have few nits against myself then :-)  Inlined below.  Could you fix
> them before pushing?
>
> Thanks,
>   Stefano
>
>> From a5af26936f26bcde91c45d99c499d648ecc37af4 Mon Sep 17 00:00:00 2001
>> From: Stefano Lattarini <address@hidden>
>> Date: Wed, 29 Aug 2012 13:55:49 +0200
>> Subject: [PATCH 03/22] build: add extra *.texi files to coreutils_TEXINFOS,
>>  not EXTRA_DIST
>>
>> * doc/Makefile.am (coreutils_TEXINFO): List them here, instead of ...
>> (EXTRA_DIST): ... listing them here.  This ensure
>>
> s/ensure/ensures/

Done.

>> the rebuild rules will be more faithful.
>> ($(DVIS), $(INFO_DEPS)): No need to depend on $(EXTRA_DIST) now.
>> ---
>
>> From 78f9df627616976bf27777d5751d60f0fe841cac Mon Sep 17 00:00:00 2001
>> From: Stefano Lattarini <address@hidden>
>> Date: Wed, 29 Aug 2012 14:01:12 +0200
>> Subject: [PATCH 04/22] build: prefer '$(top_srcdir)/doc' over '$(srcdir)' in
>>  doc Makefile
>>
>> This is just a preparatory refactoring that will become useful in
>> a future change (where
>>
> s/where/when/ maybe.

I did s/where/in which/

>> the doc/Makefile.am makefile will be merged
>> with the top-level one).
>>
>> * doc/Makefile.am (doc_srcdir): New, defined to '$(top_srcdir)/doc'.
>>
> s/defined/define/.

Done

>> Use it throughout instead of "bare" '$(srcdir)'.
>> ---
>>  doc/Makefile.am | 28 +++++++++++++++-------------
>>  1 file changed, 15 insertions(+), 13 deletions(-)
>
>
>> From 88efe23e17882a42c167b9524858bb0883e18e70 Mon Sep 17 00:00:00 2001
>> From: Stefano Lattarini <address@hidden>
>> Date: Wed, 29 Aug 2012 16:57:41 +0200
>> Subject: [PATCH 15/22] build: provide convenience target 'all_programs' also
>>  at top-level
>>
>> This will be mostly useful in future changes.
>>
>> * Makefile.am (all_programs): New, simply the work delegating to
>>
> s/the work/work by/.

Done.

>> From 56a914537bd732b6c38ea64ec25c7e0477fbb890 Mon Sep 17 00:00:00 2001
>> From: Stefano Lattarini <address@hidden>
>> Date: Wed, 29 Aug 2012 21:31:57 +0200
>> Subject: [PATCH 16/22] build: rework some recipes in man/Makefile.am, for
>>  future changes
>>
>> This change is merely required to make future changes easier.
>>
>> In particular, since we are going to merge the contents of
>> 'man/Makefile.am' in
>>
> s/in/into/

Done.

>> the top-level Makefile, we need to avoid
>> conflicts with the rules and variables in 'dist-check.mk', and
>> to prepare for changes in the value of the '$*' variable in the
>>
> s/in the/as used in the/

Done.

>> recipe of the '.x -> .1' suffix rule.
>>
>> * man/Makefile.am (t, mapped_name): Delete, inlining their use ...
>> (.1.x): ... in the recipe of this suffix rule.  Other adjustments
>> to prepare to changes in the value of the '$*' automatic variable.
>> While at it, made more resilient about unlikely but possible failure.
>> Adjust and reorder few comments.
>> ---
>
>>
>> From f6696dd246d96ce3449119c4d2f2365bac156de1 Mon Sep 17 00:00:00 2001
>> From: Stefano Lattarini <address@hidden>
>> Date: Wed, 29 Aug 2012 16:37:24 +0200
>> Subject: [PATCH 17/22] build: don't use recursive make to build the 'man'
>>  subdirectory
>>
>> * Makefile.am: Include 'man/local.mk'.
>> (SUBDIRS): Remove 'man'.
>> * configure.ac ($MAN): Adjust to have a leading 'man/' component.
>>
> s/to have/so that all its entries have/

I have a slight preference for using "each" over "all"

  * configure.ac ($MAN): Adjust so that each of its entries has a leading
  'man/' component.

>> (AC_CONFIG_FILES): Remove 'man/Makefile'.
>> * man/Makefile.am: Rename ...
>> * man/local.mk: ... like this.  With further adjustments: each 'foo.1'
>> target renamed like 'man/foo.1', each '../src/foo.c' dependency as
>> 'src/foo.c', and each '$(srcdir)' usage as '$(srcdir)/man'.  Also ...
>> (mandep): Adjust, removing the leading '../' component.
>> Several whitespace adjustments wile at it.
>>
> s/wile/while/.

Done

>> (ASSORT): Remove, it's already defined in the top-level Makefile.am.
>> * cfg.mk (sc_option_desc_uppercase, sc_man_file_correlation): Remove
>> the associated recipes, they are now directly available from the
>> included 'man/local.mk'.  Actually, the other changes in this commit
>> have made these recipes instable and not completely correct, bu that
>>
> s/bu/but/

Done

>> will be fixed in later changes.
>> ---
>
>> From aaa9d080a4d0d60a6d5498cb01204115c8fc9b7d Mon Sep 17 00:00:00 2001
>> From: Stefano Lattarini <address@hidden>
>> Date: Wed, 29 Aug 2012 17:37:35 +0200
>> Subject: [PATCH 18/22] maint: move man-related syntax checks in cfg.mk
>>
>> This is more natural, now that the top-level Makefile has access to
>> all the variables and rules once defined only in 'man/Makefile.am'
>
>> (that was before the previous commit).
>>
> This aside seems redundant now that I re-read it with a cold mind.  I'd
> say we just remove it.

Done.

>> * man/local.mk (all_programs, sc_option_desc_uppercase,
>> sc_man_file_correlation check-x-vs-1, check-programs-vs-x): Move
>> from here ...
>> * cfg.mk: ... to here.  Adjust some comments in the process.
>> ---
>>
>> From 963dc11f81e569b971fda7eb73ffed91ef18a7ec Mon Sep 17 00:00:00 2001
>> From: Stefano Lattarini <address@hidden>
>> Date: Wed, 29 Aug 2012 17:57:45 +0200
>> Subject: [PATCH 20/22] maint: adjust syntax check 'sc_option_desc_uppercase'
>>
>> * cfg.mk (sc_option_desc_uppercase): Here, by grafting the 'man/'
>> prefix to the manpages obtained from the $(NO_INSTALL_PROGS_DEFAULT)
>>
> The last "the" can be safely removed IMHO.

Done.

>> and listed as prerequisites for this rule.
>
>
>> From 0aec09e9850fb107fc939c782642d33c8b91f188 Mon Sep 17 00:00:00 2001
>> From: Stefano Lattarini <address@hidden>
>>
> I think this change should have you as the author, as you've completely
> changed my original patch (making it much better, I admit :-)

Hmm... I guess so.
Thanks.

>> Date: Wed, 29 Aug 2012 17:59:48 +0200
>> Subject: [PATCH 21/22] build: factor out a little more re list of *.texi
>>  files
>>
>> We may well want to switch from checking all *.texi to
>> checking only version-controlled .texi files, so encapsulate
>> this concept in one place.
>>
>> * doc/local.mk (doc_srcdir): Delete.  Use this instead:
>> (texi_files): Define.  All usages adjusted.

Ok?