Re: [PATCH] avoid parallel build failures

[Stefano Lattarini]

Hi Jim, thanks for the patches.

On 04/12/2012 06:11 PM, Jim Meyering wrote:
>
>>> IMHO, there is only one way to work around this: don't distribute
>>> those two files, i.e., generate them at build time.
>>> That means either automake distributes the help2man script
>>> (as coreutils does) or it adds help2man to its list of build-time
>>> dependencies, as diffutils, cppi, libtool and others do.
>>> Given automake's position in the system build-dependency graph
>>> I have a slight preference to include help2man in the tarball.
>>>
>> That's seems the easiest and safest way out, especially considering
>> that Automake already requires per anyway.  Care to write a patch?
>> Otherwise I should be able to get to it by this evening or tomorrow
>> (as more serious issues with Vala support requires urgent attention
>> now).
> 
> Here's the retitled first patch, and a second one implementing
> the above suggestion.  It passed "make distcheck" on master,
> but then I realized I should use a newer help2man than the copy
> that was in coreutils -- and besides, the coreutils one had
> a few small changes that were not appropriate here.
> 
> At first, I took the latest help2man (1.40.8) but saw that that would
> raise the minimum required perl version to 5.008 from automake's 5.006.
>
Bummer, I was hoping we could get the latest help2man version as part
of "make fetch" :-(  Oh well, maybe for a later time, once we can assume
perl 5.8 in Automake as well.

> Instead, I found that the latest requiring a lesser version of perl is
> help2man-1.37.1, which should be fine since coreutils is using the
> even-older help2man-1.35.
> 

-*-*-*-

> From 3429be48a67bd2381e150486490063e8aba77973 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Wed, 11 Apr 2012 21:25:48 +0200
> Subject: [PATCH 1/2] build: avoid parallel build failures
> 
> A parallel build would fail when two concurrent sub-make processes
> tried to build lib/Automake/Config.pm.  The loser would complain that
>   grep: lib/Automake/Config.pm-t: No such file or directory
>   chmod: cannot access `lib/Automake/Config.pm-t': No such file or\
>     directory
>   make[1]: *** [lib/Automake/Config.pm] Error 1
> * Makefile.am (update_mans): Don't build lib/Automake/Config.pm here.
> Instead, depend on it from the two rules that use it:
> ($(srcdir)/doc/aclocal-$(APIVERSION).1): Depend on it.
> ($(srcdir)/doc/automake-$(APIVERSION).1): Likewise.
> 
> However, that was not enough, since even then, a parallel build
> would still fail, now with this:
> 
>   help2man: can't get `--help' info from automake-1.11a
>   Try `--no-discard-stderr' if option outputs to stderr
>   make: *** [doc/automake-1.11a.1] Error 1
> 
> a subsequent "make -j3" would create the missing file.
> That was because help2man would invoke t/wrap/aclocal.in and
> t/wrap/automake.in, each of which would require aclocal and
> automake, yet those two files weren't guaranteed to be created.
> Add explicit dependencies:
> ($(srcdir)/doc/aclocal-$(APIVERSION).1): Depend on aclocal.
> ($(srcdir)/doc/automake-$(APIVERSION).1): Depend on automake.
>
Here I'd say explicitly this is not truly correct, since we are making
distributed files to depend on distributed ones; but that this issue
will be solved in a follow-up change (the one you posted below).  WDYT?

> ---
>  Makefile.am |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 991de4c..438ffe6 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -459,7 +459,6 @@ MAINTAINERCLEANFILES += $(dist_man1_MANS)
> 
>  update_mans = \
>    $(AM_V_GEN): \
> -    && $(MAKE) $(AM_MAKEFLAGS) lib/Automake/Config.pm \
>      && PATH="$(abs_builddir)/t/wrap$(PATH_SEPARATOR)$$PATH" \
>      && export PATH \
>      && $(HELP2MAN) --output=$@
> @@ -469,9 +468,9 @@ $(srcdir)/doc/aclocal.1 $(srcdir)/doc/automake.1:
>         && f=`echo $@ | sed 's|.*/||; s|\.1$$||; $(transform)'` \
>         && echo ".so man1/$$f-$(APIVERSION).1" > $@
> 
> -$(srcdir)/doc/aclocal-$(APIVERSION).1: $(srcdir)/aclocal.in
> +$(srcdir)/doc/aclocal-$(APIVERSION).1: $(srcdir)/aclocal.in aclocal 
> lib/Automake/Config.pm
>       $(update_mans) aclocal-$(APIVERSION)
> -$(srcdir)/doc/automake-$(APIVERSION).1: $(srcdir)/automake.in
> +$(srcdir)/doc/automake-$(APIVERSION).1: $(srcdir)/automake.in automake 
> lib/Automake/Config.pm
>       $(update_mans) automake-$(APIVERSION)
> 
> 
> --
> 1.7.10.128.g7945c
>
ACK with the nit above addressed.

-*-*-*-

> From cc0d0cdb3e8ea2ececc4732d03e05ec4482af74d Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Thu, 12 Apr 2012 15:07:19 +0200
> Subject: [PATCH 2/2] build: generate doc/*.1 files; include help2man
>
Here, I'd like to read a brief explanation of why this change is needed
(i.e., avoiding to have distributed files to depend on non-distributed
ones).

> * doc/help2man: New file, version 1.37.1.
> * Makefile.am (EXTRA): Add doc/help2man.
>
s/EXTRA/EXTRA_DIST/.

> (man1_MANS): Rename from $(dist_man1_MANS).  Remove $(srcdir) prefix.
> (DISTCLEANFILES): Add these files here, rather than to
> $(MAINTAINERCLEANFILES), since we are no longer distributing them.
>
Why not adding them to $(CLEANFILES)?  After all, they depend on the
generated automake and aclocal scripts, which are removed by "make clean",
not by "make distclean".

> (update_mans): Use doc/help2man, not $(HELP2MAN).
> * configure.ac: Don't test for help2man, now that we bundle it.
> ---
>  Makefile.am  |   25 ++-
>  configure.ac |    3 -
>  doc/help2man |  656 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 670 insertions(+), 14 deletions(-)
>  create mode 100755 doc/help2man
> 
> diff --git a/Makefile.am b/Makefile.am
> index 438ffe6..7b58023 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -447,30 +447,33 @@ info_TEXINFOS = doc/automake.texi 
> doc/automake-history.texi
>  doc_automake_TEXINFOS = doc/fdl.texi
>  doc_automake_history_TEXINFOS = doc/fdl.texi
> 
> -dist_man1_MANS = \
> -  $(srcdir)/doc/aclocal.1 \
> -  $(srcdir)/doc/automake.1 \
> -  $(srcdir)/doc/aclocal-$(APIVERSION).1 \
> -  $(srcdir)/doc/automake-$(APIVERSION).1
> +man1_MANS = \
> +  doc/aclocal.1 \
> +  doc/automake.1 \
> +  doc/aclocal-$(APIVERSION).1 \
> +  doc/automake-$(APIVERSION).1
> 
> -$(dist_man1_MANS): $(srcdir)/configure.ac
> +$(man1_MANS): $(srcdir)/configure.ac
> 
> -MAINTAINERCLEANFILES += $(dist_man1_MANS)
> +DISTCLEANFILES += $(man1_MANS)
> +EXTRA_DIST += doc/help2man
> 
>  update_mans = \
>    $(AM_V_GEN): \
> +    && mkdir -p doc \
>
s/mkdir -p/$(MKDIR_P)/

>      && PATH="$(abs_builddir)/t/wrap$(PATH_SEPARATOR)$$PATH" \
>      && export PATH \
> -    && $(HELP2MAN) --output=$@
> +    && $(srcdir)/doc/help2man --output=$@
>
Missing '$(PERL)' in front of '$(srcdir)/doc/help2man'.

> -$(srcdir)/doc/aclocal.1 $(srcdir)/doc/automake.1:
> +doc/aclocal.1 doc/automake.1:
>       $(AM_V_GEN): \
> +       && mkdir -p doc \
>
s/mkdir -p/$(MKDIR_P)/

>         && f=`echo $@ | sed 's|.*/||; s|\.1$$||; $(transform)'` \
>         && echo ".so man1/$$f-$(APIVERSION).1" > $@
> 
> -$(srcdir)/doc/aclocal-$(APIVERSION).1: $(srcdir)/aclocal.in aclocal 
> lib/Automake/Config.pm
> +doc/aclocal-$(APIVERSION).1: aclocal.in aclocal lib/Automake/Config.pm
>       $(update_mans) aclocal-$(APIVERSION)
> -$(srcdir)/doc/automake-$(APIVERSION).1: $(srcdir)/automake.in automake 
> lib/Automake/Config.pm
> +doc/automake-$(APIVERSION).1: automake.in automake lib/Automake/Config.pm
>       $(update_mans) automake-$(APIVERSION)
> 
> 
> diff --git a/configure.ac b/configure.ac
> index 81cb303..f4582d2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -124,9 +124,6 @@ AM_RUN_LOG([$TEX --version </dev/null])
>  AC_CHECK_PROGS([YACC], [yacc byacc 'bison -y'], [false])
>  AC_CHECK_PROGS([LEX], [lex flex], [false])
> 
> -# Generate man pages.
> -AM_MISSING_PROG([HELP2MAN], [help2man])
> -
>  # Test for Autoconf.  We run Autoconf in a subdirectory to ease
>  # deletion of any files created (such as those added to
>  # autom4te.cache).  We used to perform only the last of the three
> diff --git a/doc/help2man b/doc/help2man
> new file mode 100755
> index 0000000..96896f6
> --- /dev/null
> +++ b/doc/help2man
> @@ -0,0 +1,656 @@
> +#!/usr/bin/perl -w
>
[SNIP]

ACK with the nits above addressed.

Thanks,
  Stefano