Re: success! [Re: [PATCH 22/22] maint: avoid distcheck failure...

[Stefano Lattarini]

On 09/01/2012 06:53 PM, Jim Meyering wrote:
> Stefano Lattarini wrote:
>>> Do you feel like merging this into one of your patches?
> 
> No problem.
> I've merged that, as well as a couple of other fixes,
> into the commits they belong with.
> 
> Here's what I'd like to push pretty soon:
>
Here are my last nits:

Thanks,
  Stefano

-*-*-*-

> [PATCH 01/25] maint: add our 'bootstrap_post_import_hook' function
>
> This is in preparation of future changes.
>
> * bootstrap.conf (bootstrap_post_import_hook): New, will be executed
> by bootstrap after gnulib-tool abut before the autotools.
>
s/abut/but/

-*-*-*-

> [PATCH 02/25] build: refactor how lists of coreutils programs are defined
>
> This is in preparation of future changes.  Still, this patch
> leaves the build system in a better shape; true, with more
> indirections, but also with less convoluted and brittle hacks.
>
> Unfortunately, this commit also makes some rebuild rules
> incomplete; that will son be fixed by follow-up patches.
>
> * build-aux/gen-lists-of-programs.sh: New, generates autoconf
> and automake input fragments that define "lists" of all coreutils
> programs, with further distinctions about how and when these
> programs should be built (by default; if the system is capable
> enough; only if the user asks for them explicitly). This is
> useful to avoid duplicating the definitions of these list among
>
s/list/lists/

> several files (at least 'configure.ac' 'src/Makefile.am'); such
> duplication had proved a source of inconsistencies and bugs in
> the past.  And the pre-existing way to avoid such duplication,
> as implemented in 'configure.ac' before this patch, was overly
> complex and brittle.
>
> * Makefile.am (EXTRA_DIST): Distribute the new script.
> * bootstrap.conf (bootstrap_post_import_hook): Run the new script
> to generate 'm4/cu-progs.m4' and 'src/cu-progs.mk'.
> * .gitignore: Ignore those files.
> * configure.ac: Include 'm4/cu-progs.m4', and decidedly simplify
> most of the programs' lists definition and processing accordingly.
>
s/programs' lists/program lists/

> * src/Makefile.am: Similarly include 'src/cu-progs.mk', containing
> definition of variables $(default__progs), $(no_install__progs)
> and $(build_if_possible__progs). Accordingly ...
>
Missing space after dot.

> (no_install__progs, build_if_possible__progs): ... remove.
> (EXTRA_DIST): Adjust definition.
> Adjust a comment.


> [PATCH 03/25] build: don't use recursive make to build the 'src' subdirectory
>
> * Makefile.am (SUBDIRS): Remove 'src'.  Ensure '.' is listed before
> 'tests' and 'gnulib-tests'.
> (dist-hook): Adjust: we must now tweak the top-level Makefile.in
> in $(distdir), not the one in the 'src/' subdir (which is gone).
> (include): The '$(top_srcdir)/src/local.mk' file.
> * build-aux/gen-lists-of-programs.sh: Adjust the generation of the
> automake input fragment.
> * tests/Makefile.am (.built-programs): Adjust.
> * cfg.mk (all_programs): Remove this convenience rule; it's no
> longer needed, now that we can rely directly on the contents of
> $(all_programs).
> (sc_option_desc_uppercase, check-programs-vs-x:): Adjust list
>
s/list/lists/

>  of prerequisites accordingly.
> (all-progs-but-lbracket): Simplify definition accordingly.
> * configure.ac ($OPTIONAL_BIN_PROGS): Adjust definition.
> ($OPTIONAL_PKGLIB_PROGS): Likewise.
> ($NO_INSTALL_PROGS_DEFAULT): Tweak definition, for consistency.
> (AC_CONFIG_FILES): Remove 'src/Makefile'.
> * src/Makefile.am: Rename ...
> * src/local.mk: ... like this, with a lot of adjustments.  In
> addition ...
> (all_programs): ... remove this now-unneeded convenience target.


> [PATCH 04/25] build: fix generation of manpages for programs not built by 
> default

> * src/local.mk (dist-hook): Don't use this to ensure all the
> programs, even the ones disabled by default or by the user, are
> built (doing so is required to generate the distributed manpages
>
s/generate/ensure/

> are properly built).  This would build those programs too late
> ...
> [SNIP]
>

> [PATCH 05/25] maint: improve remake rules for maintainers
>
> This is a follow up on today's former commit "build: refactor
> how lists of coreutils programs are defined".
>
Once the series has stabilized, here we could add a reference to
the 'git-describe' output of that commit.  Probably overly picky
though; I leave it to you to decide whether such tweaking is worth
or not.

> * Makefile.am ($(top_srcdir)/m4/cu-progs.m4,
> $(srcdir)/src/cu-progs.mk): New, generate these files from the
> 'build-aux/gen-lists-of-programs.sh', the same way it's done
> from the bootstrap script.
> * bootstrap.conf (bootstrap_post_import_hook): Add comment about
> the necessity to keep those new rules synced with the commands
> here.  Enhance those commands so to that the generated files are
> set read-only.

> [PATCH 07/25] maint: fix and simplify maintainer checks
>
> Some of them can be simplified after the previous changes, some
> of them have been downright broken by them, and need fixing.
>
> * src/local.mk: Adjust some comments.
> (EXTRA_DIST): Avoid SPACE-TAB sequences.
> (src/dircolors.h, src/fs.h src/fs-is-local.h): Avoid 8-SPACES
> indentation.
> (_sc_check-AUTHORS): Move ...
> * cfg.mk (sc_check-AUTHORS): ... here (superseding the old rule
> with the same name, that was only
>
s/only/just/

> a recursive invocation to it).
> Adjust the paths of the invoked coreutils programs, to account
> for the fact that this rule now runs in the top-level build dir,
> not in the 'src/' subdir.  Other minor cosmetic adjustments.
> (ALL_RECURSIVE_TARGETS): Remove 'sc_option_desc_uppercase' and
> 'sc_man_file_correlation', since they no longer entails any
>
s/entails/entail/

> recursive make invocation.
>
> [SNIP]
>

> [PATCH 10/25] build: simplify: get rid of some indirection variables
>
> The code deciding which coreutils programs to build (depending on
> defaults, system capabilities, and user requests) is overly complex
> and rather confusing.  Let's begin simplifying it by removing some
> non-strictly-necessary indirection variables.
>
> * configure.ac: Adjust and improve few comments.
> (OPTIONAL_BIN_PROGS, OPTIONAL_PKGLIB_PROGS): Rename ...
> (bin_PROGRAMS, pkglibexec_PROGRAMS): ... like these, respectively.
> Ensure they aren't initialized in all Makefiles (which would lead
> to spurious errors), by calling AM_SUBST_NOTMAKE on them.
> * src/local.mk: Adjust and improve few comments.
> (bin_PROGRAMS, pkglibexec_PROGRAMS): Simply define
> to the corresponding '@substitution@'.  Not strictly required,
> but certainly clearer and safer.
>
False, it is strictly required since we call AM_SUBST_NOTMAKE on
them.  Just delete the last sentence.

> [PATCH 11/25] build: simplify and make more portable to non-GNU make
>
> The AC_SUBST'd variable '$(NO_INSTALL_PROGS_DEFAULT)' is only used in
> makefile expressions listing the
>
s/listing the/expanding to the list of/

> manual pages that are not built by
> default (but might need to be when a distribution tarball is created).
> Such expressions exploited a feature of make variable expansion --
> namely, $(VAR:%=dir/%.x) -- that, while seemingly quite portable in
> practice, is not POSIX-conforming, and could break on lesser vendor
> make implementations.  So kill two birds with one stone, by getting
> rid of the $(NO_INSTALL_PROGS_DEFAULT) intermediate variable and
> improving makefile portability in the process.
>
> While at it, we also clean up some other minor naming inconsistency
> and useless indirection,
>
Which the patch actually does ...

> and fix some dependency issue.
>
Huh?  The patch does no such thing.  Just delete this last part of
the sentence.

> * configure.ac (NO_INSTALL_PROGS_DEFAULT): Don't define or AC_SUBST
> anymore; instead ...
> (EXTRA_MANS): ... define and AC_SUBST these.
> * man/local.mk (extra_man_1): Rename ...
> (EXTRA_MANS): ... like this, explicitly making clear it's AC_SUBST'd.
> (extra_man_x): It's used only once, no need to define it; just inline
> its only expansion where needed.
> (EXTRA_DIST): Adjust.
(ALL_MANS): New, union of $(EXTRA_MANS) and $(dist_man1_MANS).
> * cfg.mk (check-x-vs-1, sc_option_desc_uppercase): Rely on $(ALL_MANS)
> rather than $(NO_INSTALL_PROGS_DEFAULT) and $(dist_man1_MANS).
>
s/than/than on/

> [PATCH 12/25] build: one less unneeded make variable
>
> * man/local.mk (man_aux): This was used only once, so inline its
> expansion at its only point of use ...
>
s/at its only point of use/in the only place where it is used/ maybe?

> (EXTRA_DIST): ... here.

>From 8624cc107691542cf0d10fc8934609c4d0d863ba Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Sat, 1 Sep 2012 01:46:50 +0200
Subject: [PATCH 13/25] build: rename dist_man1_MANS -> man1_MANS

> And list $(man1_MANS) directly in $(EXTRA_DIST) instead.

> This offers more similarity to what is done for $(EXTRA_MANS)
>
I'd substitute this sentence with:

    This is similar to what is done for $(EXTRA_MANS), thus
    improving consistency and readability.

> * man/local.mk (dist_man1_MANS): Rename ...
> (man1_MANS): ... like this.
> (EXTRA_DIST): Add its contents.
> * cfg.mk (check-x-vs-1): Fix a botched comment.

> [PATCH 14/25] build: simplify: get rid of yet some more indirection variables
>
> * configure.ac: Adjust and improve few comments.
> (MAN): Rename ...
> (man1_MANS): ... like this.
> Ensure it's aren't initialized in all Makefiles (which would lead
>
s/it's aren't/it isn't/

> to spurious errors), by calling AM_SUBST_NOTMAKE on it.
> Also call AM_SUBST_NOTMAKE on 'EXTRA_MANS', for consistency.
> * man/local.mk (man1_MANS): Simply define to '@man1_MANS@'.

> [PATCH 19/25] maint: port manpages generation to VPATH builds
>
> * man/local.mk (.x.1): Use '$(MKDIR_P)' rather than bare 'mkdir'
> where appropriate.
>
> Reported-by: Jim Meyering <address@hidden>
>
You might want to delete this 'Reported-by:', if I remember your
preferences correctly

> [PATCH 22/25] build: create src/ and man/ via configure
>
> * configure.ac: Create man/ and src/, since, now that there is no
> generated Makefile in each, the directory will not exist in a
> non-srcdir build.
>
This patch can be dropped AFAICS: 'man/' is created thanks to the
changes done by "[PATCH 19/25] maint: port manpages generation to
VPATH builds", while src/ should be created (through a proper
.distramp file) by the Automake-generated compilation rules.  Can
you confirm everything works also dropping this patch?