all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#30147] Simplify “scripts/guix”
@ 2018-01-17 19:25 Mathieu Lirzin
  2018-01-22 18:19 ` Eric Bavier
  2018-01-23  9:13 ` bug#30147: " Ludovic Courtès
  0 siblings, 2 replies; 8+ messages in thread
From: Mathieu Lirzin @ 2018-01-17 19:25 UTC (permalink / raw)
  To: 30147

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

Hi,

Here are some improvements for “scripts/guix”.


[-- Attachment #2: 0001-build-Expand-scripts-guix-at-Make-time.patch --]
[-- Type: text/x-patch, Size: 5779 bytes --]

From 342444897673d5f9d9a475986e76ca2e912f6674 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <mthl@gnu.org>
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.
---
 Makefile.am     | 20 +++++++++++++++++---
 configure.ac    |  1 -
 scripts/guix.in | 31 ++++++-------------------------
 3 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index aebd3b1eb..5e36dbf44 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2,7 +2,7 @@
 # Copyright © 2012, 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
 # Copyright © 2013 Andreas Enge <andreas@enge.fr>
 # Copyright © 2015, 2017 Alex Kost <alezost@gmail.com>
-# Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
+# Copyright © 2016, 2018 Mathieu Lirzin <mthl@gnu.org>
 # Copyright © 2016, 2017 Mark H Weaver <mhw@netris.org>
 # Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 # Copyright © 2017 Leo Famulari <leo@famulari.name>
@@ -26,8 +26,20 @@
 # You should have received a copy of the GNU General Public License
 # along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
-bin_SCRIPTS =					\
-  scripts/guix
+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' \
+  -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 $@
 
 nodist_noinst_SCRIPTS =				\
   pre-inst-env					\
@@ -437,6 +449,7 @@ EXTRA_DIST =						\
   TODO							\
   CODE-OF-CONDUCT					\
   .dir-locals.el					\
+  bin/guix.in						\
   build-aux/build-self.scm				\
   build-aux/compile-all.scm				\
   build-aux/hydra/evaluate.scm				\
@@ -473,6 +486,7 @@ endif !BUILD_DAEMON_OFFLOAD
 
 
 CLEANFILES =					\
+  $(bin_SCRIPTS)				\
   $(GOBJECTS)					\
   $(SCM_TESTS:tests/%.scm=%.log)
 
diff --git a/configure.ac b/configure.ac
index 1e3912248..c50dcaf2e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -272,7 +272,6 @@ AC_CONFIG_FILES([Makefile
                  po/packages/Makefile.in
 		 guix/config.scm])
 
-AC_CONFIG_FILES([scripts/guix], [chmod +x scripts/guix])
 AC_CONFIG_FILES([test-env:build-aux/test-env.in], [chmod +x test-env])
 AC_CONFIG_FILES([pre-inst-env:build-aux/pre-inst-env.in],
   [chmod +x pre-inst-env])
diff --git a/scripts/guix.in b/scripts/guix.in
index e20c27424..af50a782b 100644
--- a/scripts/guix.in
+++ b/scripts/guix.in
@@ -3,6 +3,7 @@
 !#
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2018 Mathieu Lirzin <mthl@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -22,35 +23,15 @@
 ;; IMPORTANT: We must avoid loading any modules from Guix here,
 ;; because we need to adjust the guile load paths first.
 ;; It's okay to import modules from core Guile though.
-(use-modules (ice-9 regex)
-             (srfi srfi-26))
+(use-modules (srfi srfi-26))
 
 (let ()
   (define-syntax-rule (push! elt v) (set! v (cons elt v)))
 
-  (define config-lookup
-    (let ((config '(("prefix"         . "@prefix@")
-                    ("exec_prefix"    . "@exec_prefix@")
-                    ("datarootdir"    . "@datarootdir@")
-                    ("guilemoduledir" . "@guilemoduledir@")
-                    ("guileobjectdir" . "@guileobjectdir@")
-                    ("localedir"      . "@localedir@")))
-          (var-ref-regexp (make-regexp "\\$\\{([a-z_]+)\\}")))
-      (define (expand-var-ref match)
-        (lookup (match:substring match 1)))
-      (define (expand str)
-        (regexp-substitute/global #f var-ref-regexp str
-                                  'pre expand-var-ref 'post))
-      (define (lookup name)
-        (expand (assoc-ref config name)))
-      lookup))
-
   (define (maybe-augment-load-paths!)
     (unless (getenv "GUIX_UNINSTALLED")
-      (let ((module-dir (config-lookup "guilemoduledir"))
-            (object-dir (config-lookup "guileobjectdir")))
-        (push! module-dir %load-path)
-        (push! object-dir %load-compiled-path))
+      (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")))
@@ -64,8 +45,8 @@
   (define (run-guix-main)
     (let ((guix-main (module-ref (resolve-interface '(guix ui))
                                  'guix-main)))
-      (bindtextdomain "guix" (config-lookup "localedir"))
-      (bindtextdomain "guix-packages" (config-lookup "localedir"))
+      (bindtextdomain "guix" "@localedir@")
+      (bindtextdomain "guix-packages" "@localedir@")
       (apply guix-main (command-line))))
 
   (maybe-augment-load-paths!)
-- 
2.15.1


[-- Attachment #3: 0002-guix-Refactor-script.patch --]
[-- Type: text/x-patch, Size: 3307 bytes --]

From b6f8331455da1ffc4896b06cd2ee98e09b05be43 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <mthl@gnu.org>
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.
---
 scripts/guix.in | 45 +++++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/scripts/guix.in b/scripts/guix.in
index af50a782b..f971398e8 100644
--- a/scripts/guix.in
+++ b/scripts/guix.in
@@ -23,36 +23,33 @@
 ;; IMPORTANT: We must avoid loading any modules from Guix here,
 ;; because we need to adjust the guile load paths first.
 ;; 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)))
+(use-modules (srfi srfi-2)
+             (srfi srfi-26))
 
-  (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-syntax-rule (push! elt v) (set! v (cons elt v)))
 
-  (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))))
+(unless (getenv "GUIX_UNINSTALLED")
+  ;; Add installed modules to load-path.
+  (push! "@guilemoduledir@" %load-path)
+  (push! "@guileobjectdir@" %load-compiled-path)
 
-  (maybe-augment-load-paths!)
+  ;; Add modules fetched by 'guix pull' to load-path.
+  (and-let* ((conf-dir (or (getenv "XDG_CONFIG_HOME")
+                           (and=> (getenv "HOME")
+                                  (cut string-append <> "/.config"))))
+             (updates-dir (string-append conf-dir "/guix/latest"))
+             ((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)))
 
+;; Run Guix.
+(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 (run-guix-main))
   ;; but since the 'guix' command is not updated by 'guix pull', we cannot
   ;; really do it now.
-  (run-guix-main))
+  (apply guix-main (command-line)))
-- 
2.15.1


[-- Attachment #4: Type: text/plain, Size: 85 bytes --]


Thanks.

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#30147] Simplify “scripts/guix”
  2018-01-17 19:25 [bug#30147] Simplify “scripts/guix” Mathieu Lirzin
@ 2018-01-22 18:19 ` Eric Bavier
  2018-01-23 11:06   ` Mathieu Lirzin
  2018-01-23  9:13 ` bug#30147: " Ludovic Courtès
  1 sibling, 1 reply; 8+ messages in thread
From: Eric Bavier @ 2018-01-22 18:19 UTC (permalink / raw)
  To: Mathieu Lirzin; +Cc: 30147

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

On Wed, 17 Jan 2018 20:25:14 +0100
Mathieu Lirzin <mthl@gnu.org> wrote:

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

FWIW this is what my Joy compiler does for its entry script.  It works
well.

https://notabug.org/bavier/joy-in-the-morning.git

> +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 $@

I think. since there's no state variables that needs to carry accross
commands, that this would read better without the line continuations and
appropriate use of $(AM_V_at).

While we're looking at it: I'm not certain, but I think Guix currently
work if one overrides the "prefix" variable when invoking 'make
install', which is a use-case suggested by the GNU Coding Standards
(Section 7.2.5).  I bring it up because adding '$(prefix)/bin/guix' to
the above rule might be a start.

> +;; Run Guix.
> +(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 (run-guix-main))
>    ;; but since the 'guix' command is not updated by 'guix pull', we cannot
>    ;; really do it now.
> -  (run-guix-main))
> +  (apply guix-main (command-line)))

If we're going to be patching scripts/guix.in, would now be the time to
add the '(exit ...)'?

Otherwise LGTM!

`~Eric

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#30147: Simplify “scripts/guix”
  2018-01-17 19:25 [bug#30147] Simplify “scripts/guix” Mathieu Lirzin
  2018-01-22 18:19 ` Eric Bavier
@ 2018-01-23  9:13 ` Ludovic Courtès
  2018-01-23 12:29   ` [bug#30147] " Mathieu Lirzin
  1 sibling, 1 reply; 8+ messages in thread
From: Ludovic Courtès @ 2018-01-23  9:13 UTC (permalink / raw)
  To: Mathieu Lirzin; +Cc: 30147-done

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

Hello,

Mathieu Lirzin <mthl@gnu.org> skribis:

> From 342444897673d5f9d9a475986e76ca2e912f6674 Mon Sep 17 00:00:00 2001
> From: Mathieu Lirzin <mthl@gnu.org>
> 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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1434 bytes --]

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])

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


> From b6f8331455da1ffc4896b06cd2ee98e09b05be43 Mon Sep 17 00:00:00 2001
> From: Mathieu Lirzin <mthl@gnu.org>
> 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’.

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#30147] Simplify “scripts/guix”
  2018-01-22 18:19 ` Eric Bavier
@ 2018-01-23 11:06   ` Mathieu Lirzin
  0 siblings, 0 replies; 8+ messages in thread
From: Mathieu Lirzin @ 2018-01-23 11:06 UTC (permalink / raw)
  To: Eric Bavier; +Cc: 30147

Eric Bavier <ericbavier@centurylink.net> writes:

> On Wed, 17 Jan 2018 20:25:14 +0100
> Mathieu Lirzin <mthl@gnu.org> wrote:
>
>> 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.
>
> FWIW this is what my Joy compiler does for its entry script.  It works
> well.
>
> https://notabug.org/bavier/joy-in-the-morning.git

Thanks for sharing your Joy. :-)

>> +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 $@
>
> I think. since there's no state variables that needs to carry accross
> commands, that this would read better without the line continuations and
> appropriate use of $(AM_V_at).
>
> While we're looking at it: I'm not certain, but I think Guix currently
> work if one overrides the "prefix" variable when invoking 'make
> install', which is a use-case suggested by the GNU Coding Standards
> (Section 7.2.5).

I had this in mind too, but I am not sure how to achieve that.
Generating scripts that depend on $(prefix) at make time is indeed a
first start in conforming to that GCS requirement.  However in practice
I don't know any GNU package that conform to it.

> I bring it up because adding '$(prefix)/bin/guix' to the above rule
> might be a start.

If you mean:

--8<---------------cut here---------------start------------->8---
$(prefix)/bin/guix scripts/guix: scripts/guix.in Makefile
	...
--8<---------------cut here---------------end--------------->8---

I don't think it would work.  Installed files are not part of the
dependency graph, so I don't think this rule would be triggered when
doing ‘make install’.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [bug#30147] Simplify “scripts/guix”
  2018-01-23  9:13 ` bug#30147: " Ludovic Courtès
@ 2018-01-23 12:29   ` Mathieu Lirzin
  2018-01-23 14:01     ` Ludovic Courtès
  0 siblings, 1 reply; 8+ messages in thread
From: Mathieu Lirzin @ 2018-01-23 12:29 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 30147

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

Hello,

ludo@gnu.org (Ludovic Courtès) writes:

> Mathieu Lirzin <mthl@gnu.org> skribis:
>
>> From 342444897673d5f9d9a475986e76ca2e912f6674 Mon Sep 17 00:00:00 2001
>> From: Mathieu Lirzin <mthl@gnu.org>
>> 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 <mthl@gnu.org>
>> 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.


[-- Attachment #2: 0001-guix-Refactor-script.patch --]
[-- Type: text/x-patch, Size: 3857 bytes --]

From 77379bbf2642762927c01cc7f10eb5761626f172 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <mthl@gnu.org>
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


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


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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#30147] Simplify “scripts/guix”
  2018-01-23 12:29   ` [bug#30147] " Mathieu Lirzin
@ 2018-01-23 14:01     ` Ludovic Courtès
  2018-01-24 11:45       ` Mathieu Lirzin
  0 siblings, 1 reply; 8+ messages in thread
From: Ludovic Courtès @ 2018-01-23 14:01 UTC (permalink / raw)
  To: Mathieu Lirzin; +Cc: 30147-done

Mathieu Lirzin <mthl@gnu.org> skribis:

>>> From b6f8331455da1ffc4896b06cd2ee98e09b05be43 Mon Sep 17 00:00:00 2001
>>> From: Mathieu Lirzin <mthl@gnu.org>
>>> 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 <mthl@gnu.org>
> 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!'.

Works for me.  Applied and pushed, thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [bug#30147] Simplify “scripts/guix”
  2018-01-23 14:01     ` Ludovic Courtès
@ 2018-01-24 11:45       ` Mathieu Lirzin
  2018-01-24 13:46         ` Ludovic Courtès
  0 siblings, 1 reply; 8+ messages in thread
From: Mathieu Lirzin @ 2018-01-24 11:45 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 30147-done

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

Hello again,

ludo@gnu.org (Ludovic Courtès) writes:

> Mathieu Lirzin <mthl@gnu.org> skribis:
>
>> From 77379bbf2642762927c01cc7f10eb5761626f172 Mon Sep 17 00:00:00 2001
>> From: Mathieu Lirzin <mthl@gnu.org>
>> 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!'.
>
> Works for me.  Applied and pushed, thanks!

Thanks for applying this.

I didn't notice that Emacs stopped setting ‘scheme-mode’ automatically
due to the extra line implied by the use of the Guile meta switch.

Here is a fix to please Emacs.


[-- Attachment #2: 0001-guix-Let-Emacs-detect-scripts-guix.in-appropriate-mo.patch --]
[-- Type: text/x-patch, Size: 1265 bytes --]

From a798d6b7fa3b71faf85b4d415de99abccbfe7aab Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <mthl@gnu.org>
Date: Wed, 24 Jan 2018 12:29:17 +0100
Subject: [PATCH] =?UTF-8?q?guix:=20Let=20Emacs=20detect=20=E2=80=9Cscripts?=
 =?UTF-8?q?/guix.in=E2=80=9D=20appropriate=20mode.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since commit 6f774d481839f87178c5895ac2d661e141f879b8 which introduces the use
of Guile's meta switch in “scripts/guix.in”, Emacs was not using ‘scheme-mode’
for this file.

* scripts/guix.in: Replace "-*- scheme -*-" with a local variable.
---
 scripts/guix.in | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/guix.in b/scripts/guix.in
index e0f0ae7e0..d1c12eae5 100644
--- a/scripts/guix.in
+++ b/scripts/guix.in
@@ -1,6 +1,5 @@
 #!@GUILE@ \
 --no-auto-compile -e main -s
--*- scheme -*-
 !#
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013 Mark H Weaver <mhw@netris.org>
@@ -57,3 +56,7 @@
     ;; but since the 'guix' command is not updated by 'guix pull', we cannot
     ;; really do it now.
     (apply guix-main args)))
+
+;;; Local Variables:
+;;; mode: scheme
+;;; End:
-- 
2.16.0


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


Sorry for the mess.

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#30147] Simplify “scripts/guix”
  2018-01-24 11:45       ` Mathieu Lirzin
@ 2018-01-24 13:46         ` Ludovic Courtès
  0 siblings, 0 replies; 8+ messages in thread
From: Ludovic Courtès @ 2018-01-24 13:46 UTC (permalink / raw)
  To: Mathieu Lirzin; +Cc: 30147-done

Mathieu Lirzin <mthl@gnu.org> skribis:

> I didn't notice that Emacs stopped setting ‘scheme-mode’ automatically
> due to the extra line implied by the use of the Guile meta switch.
>
> Here is a fix to please Emacs.

Perfect, thanks!

Ludo'.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-01-24 13:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-17 19:25 [bug#30147] Simplify “scripts/guix” Mathieu Lirzin
2018-01-22 18:19 ` Eric Bavier
2018-01-23 11:06   ` Mathieu Lirzin
2018-01-23  9:13 ` bug#30147: " Ludovic Courtès
2018-01-23 12:29   ` [bug#30147] " Mathieu Lirzin
2018-01-23 14:01     ` Ludovic Courtès
2018-01-24 11:45       ` Mathieu Lirzin
2018-01-24 13:46         ` Ludovic Courtès

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.