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

[Stefano Lattarini]

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/

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

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

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

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

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

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

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

> (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/

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

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

> 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 :-)

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

Thanks,
  Stefano