Re: [PATCH 06/13] build: Generate man pages after compiling guile objects.

[Mathieu Lirzin <mthl@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]

Eric Bavier <ericbavier@openmailbox.org> writes:

> On Thu, 28 Jan 2016 18:00:35 +0100
> Mathieu Lirzin <mthl@gnu.org> wrote:
>
>> OTOH I must admit that for maintainability reasons I would prefer
>> avoiding complexity in Makefiles as much as possible.  This means
>> using Automake's abstract concepts as much as possible, having a
>> simple dependency graph on top of that, and put things that don't fit
>> well in this scheme in individual "build-aux/" scripts.  For that
>> reason I still prefer the embed help2man solution.  but otherwise it
>> looks nice.  :)
>
> I sympathize with the concerns for maintainability.  In this case, it
> seems to me that a few lines of make/shell would be preferable to 20k
> lines of perl.  Perhaps the simpler attached patch is better?

A few lines of Make/Shell seems more maintainable than a full perl
program.  But in this case the maintenance is handled upstream, so it
only consists in updating the script periodically (which is still not
ideal).

Your patch is definitely better now since the hack is localized to a
specific target.  Let's use this solution!  I have attached an updated
version which simplifies how silent rules are handled and integrates
changes from my initial patch.  


[-- Attachment #2: 0001-build-Generate-man-pages-after-compiling-Guile-objec.patch --]
[-- Type: text/x-diff, Size: 4744 bytes --]

From ab46507a6c2ddcfb3113ed31b1d38bc96c2f1679 Mon Sep 17 00:00:00 2001
From: Eric Bavier <bavier@member.fsf.org>
Date: Wed, 27 Jan 2016 20:31:04 -0600
Subject: [PATCH] build: Generate man pages after compiling Guile objects.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This improves compilation speed since scripts are not compiled twice.
When building with ‘make -j4‘ from a clean repository on an Intel
i5-2540M, the time of compilation gets from:

real	4m48.708s
user	17m29.892s
sys	0m9.596s

down to:

real	2m23.399s
user	6m48.604s
sys	0m6.420s

* doc.am (subcommand-manual-target): Delete function.
(SUBCOMMANDS): Delete variable.
(sub_command_mans, gen_man): New variables.
(dist_man1_MANS): Add $(sub_commands_mans).
(doc/guix-%.1): New target for sub-commands man pages.  It ensures that
man pages are built only after compiling Guile objects.
(doc/guix-daemon.1): Use $(gen_man).  Build only if BUILD_DAEMON.
(doc/guix.1): Use $(gen_man).  Add $(sub_command_mans) prerequisite.

Co-authored-by: Mathieu Lirzin <mthl@gnu.org>
---
 doc.am | 91 ++++++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 47 insertions(+), 44 deletions(-)

diff --git a/doc.am b/doc.am
index ad59aa5..d1fd096 100644
--- a/doc.am
+++ b/doc.am
@@ -1,7 +1,9 @@
 # GNU Guix --- Functional package management for GNU
+# Copyright © 2016 Eric Bavier <bavier@member.fsf.org>
 # Copyright © 2012, 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 # Copyright © 2013 Andreas Enge <andreas@enge.fr>
 # Copyright © 2016 Taylan Ulrich Bayırlı/Kammer <taylanbayirli@gmail.com>
+# Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
 #
 # This file is part of GNU Guix.
 #
@@ -91,55 +93,56 @@ ps-local: $(DOT_FILES=%.dot=$(top_srcdir)/%.eps)		\
 	  $(top_srcdir)/doc/images/coreutils-size-map.eps
 dvi-local: ps-local
 
-\f
-# Manual pages.
-
-doc/guix.1: $(SUBCOMMANDS:%=guix/scripts/%.scm)
-	-$(AM_V_HELP2MAN)LANGUAGE= $(top_builddir)/pre-inst-env		\
-	  $(HELP2MAN) --output="$@" guix
-
-# Note: Do not depend on 'guix-daemon' since that would trigger a rebuild even
-# for people building from a tarball.
-doc/guix-daemon.1: nix/nix-daemon/guix-daemon.cc
-	-$(AM_V_HELP2MAN)LANGUAGE= $(top_builddir)/pre-inst-env		\
-	  $(HELP2MAN) --output="$@" guix-daemon
-
-define subcommand-manual-target
-
-doc/guix-$(1).1: guix/scripts/$(1).scm
-	-$$(AM_V_HELP2MAN)LANGUAGE= $(top_builddir)/pre-inst-env	\
-	  $(HELP2MAN) --output="$$@" "guix $(1)"
-
-endef
-
-SUBCOMMANDS :=					\
-  archive					\
-  build						\
-  challenge					\
-  download					\
-  edit						\
-  environment					\
-  gc						\
-  hash						\
-  import					\
-  lint						\
-  package					\
-  publish					\
-  pull						\
-  refresh					\
-  size						\
-  system
-
-$(eval $(foreach subcommand,$(SUBCOMMANDS),			\
-          $(call subcommand-manual-target,$(subcommand))))
+## ----------- ##
+##  Man pages. ##
+## ----------- ##
+
+sub_commands_mans =				\
+  doc/guix-archive.1				\
+  doc/guix-build.1				\
+  doc/guix-challenge.1				\
+  doc/guix-download.1				\
+  doc/guix-edit.1				\
+  doc/guix-environment.1			\
+  doc/guix-gc.1					\
+  doc/guix-hash.1				\
+  doc/guix-import.1				\
+  doc/guix-lint.1				\
+  doc/guix-package.1				\
+  doc/guix-publish.1				\
+  doc/guix-pull.1				\
+  doc/guix-refresh.1				\
+  doc/guix-size.1				\
+  doc/guix-system.1
 
 dist_man1_MANS =				\
   doc/guix.1					\
-  $(SUBCOMMANDS:%=doc/guix-%.1)
+  $(sub_commands_mans)
+
+# Man pages are generated using GNU help2man.
+gen_man = LANGUAGE= $(top_builddir)/pre-inst-env $(HELP2MAN)
+
+# Note: Do not depend on 'scripts/guix' since that would trigger a rebuild
+# even for people building from a tarball.
+doc/guix.1: $(sub_commands_mans)
+	-$(AM_V_HELP2MAN)$(gen_man) --output="$@" `basename "$@" .1`
+
+# Note: The dependency on $(GOBJECTS) is to force these docs to be made only
+# after all guile modules have been compiled.  The 'case' ensures the manpages
+# are only generated if the corresponding script source has been changed.
+doc/guix-%.1: guix/scripts/%.scm $(GOBJECTS)
+	-$(AM_V_HELP2MAN)case '$?' in \
+	  *$<*) $(gen_man) --output="$@" "guix `basename "$<" .scm`";; \
+	  *)    : ;; \
+	esac
 
 if BUILD_DAEMON
 
-dist_man1_MANS +=				\
-  doc/guix-daemon.1
+# Note: Do not depend on 'guix-daemon' since that would trigger a rebuild even
+# for people building from a tarball.
+doc/guix-daemon.1: nix/nix-daemon/guix-daemon.cc
+	-$(AM_V_HELP2MAN)$(gen_man) --output="$@" `basename "$@" .1`
+
+dist_man1_MANS += doc/guix-daemon.1
 
 endif
-- 
2.6.3


[-- Attachment #3: Type: text/plain, Size: 122 bytes --]

Are you OK with it?

Thanks,

--
Mathieu Lirzin

ps: I was speaking of 20KB (790 lines of perl) not 20K lines of code.
:)