coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] build: do not require help2man at build-from-tarball time


From: Stefano Lattarini
Subject: Re: [PATCH] build: do not require help2man at build-from-tarball time
Date: Mon, 10 Sep 2012 12:37:08 +0200

Hi Jim.

On 09/10/2012 09:41 AM, Jim Meyering wrote:
> We can't have build-from-tarball requiring perl (via help2man)
> or regenerating a distributed .1 file when its .c file has not changed.
> 
> From 57da212a95054cc230ffb00f38ea655c798926f8 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Sun, 9 Sep 2012 19:27:25 +0200
> Subject: [PATCH] build: do not require help2man at build-from-tarball time
> 
> But do retain full dependencies when building from a git clone.
> We do this by converting the full dependency (of the .1 file on
> the binary we run with --help) into a dependency on the .c file.
> * Makefile.am (do-not-require-help2man): New rule.
> (dist-hook): depend on it.
> ---
>  Makefile.am | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 4999ca1..384b9e9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -92,11 +92,29 @@ BUILT_SOURCES = .version
>  .version:
>       $(AM_V_GEN)echo $(VERSION) > address@hidden && mv address@hidden $@
> 
> +# In general, we run help2man to build a man page from the binary's --help
> +# output, but when building from a just-unpacked distribution tarball, we
> +# must not do that, since help2man uses perl.  We don't want to depend on
> +# perl in that case.  In general, the .1 file does indeed depend on the
> +# binary.  I.e., for cat, we have this Makefile dependency:
> +#   man/cat.1: src/cat
> +# That means that once we build src/cat, we would trigger the .x.1
> +# rule which runs help2man.  The trick is simply to change the RHS to
> +# "src/cat.c" in the $(distdir) that we're about to tar and compress.
>
I believe this is incorrect.  Think of what happens if a user building from
the tarball modifies, say, 'src/ls.c', and then runs "make -j8" to rebuild
everything: make will see that it has to rebuild the man page 'man/ls.1'
(because its prerequisite 'src/ls.c' has changed), but won't see that it
has to rebuild 'src/ls' before re-running 'help2man' ro generate that man
page; so, if 'man/ls.1' is rebuilt before 'src/ls' (which can happen with
concurrent make), our user will get either a build error (if 'src/ls' was
non-existent) or, worse, a man page with an up-to-date timestamp but an
out-of-date content.  And what's even worse is that this problem will be
present also for users who have perl installed!

So we are breaking correctness for all tarball users only to accommodate
those few crippled systems lacking a decent perl.

I think the right way to go is "graceful degradation": we should keep the
correct dependency for man pages ("man/ls.1: src/ls"); and if perl is not
present, we should use, instead of help2man, a shell script that just
emit a "placeholder" man page stating that a real man page couldn't be
built due to lack of perl, and redirecting the user back to either the
info documentation or the '--help' output.

WDYT?

> +# Also handle the three exceptions corresponding to the three binaries
> +# for which there is no like-named .c file: dir, vdir, ginstall.
> +.PHONY: do-not-require-help2man
> +do-not-require-help2man:
> +     perl -pi -e 's,^(man/.+?\.1:\s*src/.+?)$$,$$1.c,;'              \
> +         -e  's,^(man/.+?\.1:\s*src)/ginstall\.c$$,$$1/install.c,;'  \
> +         -e  's,^(man/.+?\.1:\s*src)/v?dir\.c$$,$$1/ls.c,;'          \
> +          $(distdir)/Makefile.in
> +
>  # Arrange so that .tarball-version appears only in the distribution
>  # tarball, and never in a checked-out repository.
>  # The perl substitution is to change some key uses of "rm" to "/bin/rm".
>  # See the rm_subst comment for details.
> -dist-hook: gen-ChangeLog
> +dist-hook: gen-ChangeLog do-not-require-help2man
>       $(AM_V_GEN)echo $(VERSION) > $(distdir)/.tarball-version
>       $(AM_V_at)perl -pi -e '$(rm_subst)' $(distdir)/Makefile.in
> 
> --
> 1.7.12.289.g0ce9864
> 

Regards,
  Stefano



reply via email to

[Prev in Thread] Current Thread [Next in Thread]