Subject: Re: [bug#30147] Simplify “scripts/guix”

From: [email-masked] (Ludovic Courtès)
Date: Tue, 23 Jan 2018 10:13:57 +0100

Hello,

Mathieu Lirzin <[email-masked]> skribis:

> From 342444897673d5f9d9a475986e76ca2e912f6674 Mon Sep 17 00:00:00 2001
> From: Mathieu Lirzin <[email-masked]>
> Date: Wed, 17 Jan 2018 17:14:24 +0100
> Subject: [PATCH 1/2] =?UTF-8?q?build:=20Expand=20=E2=80=98scripts/guix?=
>  =?UTF-8?q?=E2=80=99=20at=20Make=20time.?=
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> This moves the complexity of Autotools variable expansion outside of the
> application code.
>
> * scripts/guix.in (config-lookup): Delete.
> (maybe-augment-load-paths!, run-guix-main): Use fully expanded variables
> instead of calling ‘config-lookup’.
> * configure.ac: Don't use AC_CONFIG_FILES for ‘scripts/guix’.
> * Makefile.am (scripts/guix): New rule.
> (do_subst): New variable.
> (CLEANFILES, EXTRA_DIST): Adapt.

That’s a good idea.  I applied it with the changes below, mostly to
account for Eric’s suggestions.

diff --git a/Makefile.am b/Makefile.am
index 5e36dbf44..9bafdab49 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -29,17 +29,17 @@
 bin_SCRIPTS = scripts/guix
 
 # Handle substitution of fully-expanded Autoconf variables.
-do_subst = sed \
-  -e 's,[@]GUILE[@],$(GUILE),g' \
-  -e 's,[@]guilemoduledir[@],$(guilemoduledir),g' \
-  -e 's,[@]guileobjectdir[@],$(guileobjectdir),g' \
+do_subst = $(SED)					\
+  -e 's,[@]GUILE[@],$(GUILE),g'				\
+  -e 's,[@]guilemoduledir[@],$(guilemoduledir),g'	\
+  -e 's,[@]guileobjectdir[@],$(guileobjectdir),g'	\
   -e 's,[@]localedir[@],$(localedir),g'
 
 scripts/guix: scripts/guix.in Makefile
-	$(AM_V_GEN)rm -f $@ $@-t \
-	  && $(MKDIR_P) $(@D) \
-	  && $(do_subst) <$(srcdir)/$@.in >$@-t \
-	  && chmod a+x,a-w $@-t && mv -f $@-t $@
+	$(AM_V_at)rm -f $@ $@-t
+	$(AM_V_at)$(MKDIR_P) "$(@D)"
+	$(AM_V_GEN)$(do_subst) < "$(srcdir)/$@.in" > "$@-t"
+	$(AM_V_at)chmod a+x,a-w "$@-t" && mv -f "$@-t" "$@"
 
 nodist_noinst_SCRIPTS =				\
   pre-inst-env					\
diff --git a/configure.ac b/configure.ac
index c50dcaf2e..f69f79648 100644
--- a/configure.ac
+++ b/configure.ac
@@ -124,6 +124,8 @@ dnl Make sure we don't suffer from the bug in 'equal?' wrt. syntax objects
 dnl found in 2.2.1.  See <https://bugs.gnu.org/29903>.
 GUIX_ASSERT_SYNTAX_OBJECT_EQUAL
 
+AC_PROG_SED
+
 dnl Decompressors, for use by the substituter and other modules.
 AC_PATH_PROG([GZIP], [gzip])
 AC_PATH_PROG([BZIP2], [bzip2])

> From b6f8331455da1ffc4896b06cd2ee98e09b05be43 Mon Sep 17 00:00:00 2001
> From: Mathieu Lirzin <[email-masked]>
> Date: Wed, 17 Jan 2018 19:55:49 +0100
> Subject: [PATCH 2/2] guix: Refactor script.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> * scripts/guix.in: Use ‘and-let*’ and remove empty surrounding ‘let’.
> (run-guix-main, maybe-augment-load-paths!): Inline them.

This is entirely subjective but I prefer the current style (in fact I
never use SRFI-2), so I’d rather skip this patch.  WDYT?  :-)

Thank you!

Ludo’.