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 <[email protected]> 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 <[email protected]> > 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?
