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

From: Mathieu Lirzin <[email-masked]>
Date: Tue, 23 Jan 2018 13:29:17 +0100

Hello,

[email-masked] (Ludovic Courtès) writes:

> 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.

Thanks.

>> 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?  :-)

I tried to avoid it, but the pipelining of checks for #f makes it very
tempting to use it.  But indeed this is a matter of style, so let's not
use it.

Here is an alternative patch that beside the pedantic issue of replacing

   (and updates-dir (file-exists? updates-dir))

with

   (and=> updates-dir file-exists?)

removes the surrounding empty 'let' that doesn't make sense to me.
Additionaly the compilation of the script is now possible which is
convenient for basic syntax checks.  This is done by using the ‘-e main
-s’ command-line switches.

>From 77379bbf2642762927c01cc7f10eb5761626f172 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <[email-masked]>
Date: Tue, 23 Jan 2018 12:52:33 +0100
Subject: [PATCH] guix: Refactor script.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* scripts/guix.in: Remove empty surrounding ‘let’.  Define 'main' as the
procedure called when running the script.
(maybe-augment-load-paths!): Rename to ...
(augment-load-paths!): ... this.  Use 'and=>' for 'file-exists?'.
(run-guix-main): Rename to ...
(main): ... this.  Call 'augment-load-paths!'.
---
 scripts/guix.in | 57 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/scripts/guix.in b/scripts/guix.in
index af50a782b..e0f0ae7e0 100644
--- a/scripts/guix.in
+++ b/scripts/guix.in
@@ -1,4 +1,5 @@
-#!@GUILE@ --no-auto-compile
+#!@GUILE@ \
+--no-auto-compile -e main -s
 -*- scheme -*-
 !#
 ;;; GNU Guix --- Functional package management for GNU
@@ -25,34 +26,34 @@
 ;; It's okay to import modules from core Guile though.
 (use-modules (srfi srfi-26))
 
-(let ()
-  (define-syntax-rule (push! elt v) (set! v (cons elt v)))
+(define-syntax-rule (push! elt v) (set! v (cons elt v)))
 
-  (define (maybe-augment-load-paths!)
-    (unless (getenv "GUIX_UNINSTALLED")
-      (push! "@guilemoduledir@" %load-path)
-      (push! "@guileobjectdir@" %load-compiled-path)
-      (let ((updates-dir (and=> (or (getenv "XDG_CONFIG_HOME")
-                                    (and=> (getenv "HOME")
-                                           (cut string-append <> "/.config")))
-                                (cut string-append <> "/guix/latest"))))
-        (when (and updates-dir (file-exists? updates-dir))
-          ;; XXX: Currently 'guix pull' puts both .scm and .go files in
-          ;; UPDATES-DIR.
-          (push! updates-dir %load-path)
-          (push! updates-dir %load-compiled-path)))))
+(define (augment-load-paths!)
+  ;; Add installed modules to load-path.
+  (push! "@guilemoduledir@" %load-path)
+  (push! "@guileobjectdir@" %load-compiled-path)
 
-  (define (run-guix-main)
-    (let ((guix-main (module-ref (resolve-interface '(guix ui))
-                                 'guix-main)))
-      (bindtextdomain "guix" "@localedir@")
-      (bindtextdomain "guix-packages" "@localedir@")
-      (apply guix-main (command-line))))
+  ;; Add modules fetched by 'guix pull' to load-path.
+  (let ((updates-dir (and=> (or (getenv "XDG_CONFIG_HOME")
+                                (and=> (getenv "HOME")
+                                       (cut string-append <> "/.config")))
+                            (cut string-append <> "/guix/latest"))))
+    (when (and=> updates-dir file-exists?)
+      ;; XXX: Currently 'guix pull' puts both .scm and .go files in
+      ;; UPDATES-DIR.
+      (push! updates-dir %load-path)
+      (push! updates-dir %load-compiled-path))))
 
-  (maybe-augment-load-paths!)
+(define* (main #:optional (args (command-line)))
+  (unless (getenv "GUIX_UNINSTALLED")
+    (augment-load-paths!))
 
-  ;; XXX: It would be more convenient to change it to:
-  ;;   (exit (run-guix-main))
-  ;; but since the 'guix' command is not updated by 'guix pull', we cannot
-  ;; really do it now.
-  (run-guix-main))
+  (let ((guix-main (module-ref (resolve-interface '(guix ui))
+                               'guix-main)))
+    (bindtextdomain "guix" "@localedir@")
+    (bindtextdomain "guix-packages" "@localedir@")
+    ;; XXX: It would be more convenient to change it to:
+    ;;   (exit (apply guix-main (command-line)))
+    ;; but since the 'guix' command is not updated by 'guix pull', we cannot
+    ;; really do it now.
+    (apply guix-main args)))
-- 
2.16.0

-- 
Mathieu Lirzin
GPG: F2A3 8D7E EB2B 6640 5761  070D 0ADE E100 9460 4D37