Re: [PATCH] Core sanity and taking build options from environment.

[Deck Pickard <deck.r.pickard@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3531 bytes --]

On 28 Nov 2014 22:52, "Ludovic Courtès" <ludo@gnu.org> wrote:
>
> Deck Pickard <deck.r.pickard@gmail.com> skribis:
>
> > From 3693753aefc27b5a68a2b762feeebc41320e79ef Mon Sep 17 00:00:00 2001
> > From: nebuli <nebu@kipple>
> > Date: Wed, 26 Nov 2014 19:51:37 +0100
> > Subject: [PATCH 1/2] guix: Default to daemon's default --cores setting
of 1.
>
> I think most of the time one would prefer to use all the available cores
> when building.  WDYT?
>

Perhaps for one-off 15 minutes worth of compiles it's a OK, but not for
hours of work. And you are creating more confusion by having different
defaults from guix-daemon. Lastly, some unsuspecting newb   may still
stumble into N^2 trap with '-M <my number of shiny cores>'...  ouch, 36
threads competing for silicon on a 6 core box...

> > From fa8738ff2cf48886c9ef8fbacfa806f547f3c3c8 Mon Sep 17 00:00:00 2001
> > From: nebuli <nebu@kipple>
> > Date: Thu, 27 Nov 2014 23:36:29 +0100
> > Subject: [PATCH 2/2] guix: scripts: Add handling of options from
GUIX_BUILD
> >  environmental variable.
>
> [...]
>
> > --- a/guix/scripts/build.scm
> > +++ b/guix/scripts/build.scm
> > @@ -401,7 +401,13 @@ arguments with packages that use the specified
source."
> >  (define (guix-build . args)
> >    (define (parse-options)
> >      ;; Return the alist of option values.
> > -    (args-fold* args %options
> > +    (args-fold* (append args
> > +                        (args-from-env "GUIX_BUILD"
> > +                                       (lambda (var opts)
> > +                                         (format (current-error-port)
> > +                                                 (_ "guix build: ~a:
~a~%")
> > +                                                 var opts))))
> > +                %options
>
> This sounds like a good idea.
>
> Some suggestions that come to mind:
>
>   • What about $GUIX_BUILD_OPTIONS?  (grep uses $GREP_OPTIONS, for
>     reference.)
>
>   • What about factorizing the above ‘args-from-env’ call like this:
>
>     (define (environment-build-options)
>       "Return additional build options passed as environment variables."
>       (args-from-env "GUIX_BUILD_OPTIONS"))
>
>     This procedure would go in (guix ui).
>
>   • I would leave out the second argument to ‘args-from-env’.  I don’t
>     think it would be convenient to have that extra line printed every
>     time.
>
> > +(define (args-from-env var . rest)
> > +  "Retrieve value of environment variable denoted by string VAR in the
form
> > +of a list of strings ('char-set:graphic' tokens) suitable for
consumption by
> > +the fold-arg family of functions.  If VAR is defined, call car of a
non-null
>
> s/fold-arg family of functions/'args-fold'/
>
> > +REST on the VAR and result, otherwise return an empty list."
> > +  (let ((env-opts (getenv variable)))
> > +    (if env-opts
> > +        (let ((opts (string-tokenize env-opts char-set:graphic)))
> > +          (and (not (null? rest))
> > +               (apply (car rest)
> > +                      (list variable opts)))
> > +          opts)
> > +        '())))
>
> Please write it like this:
>
>   (define (arguments-from-environment-variable variable)
>     (let ((env (getenv variable)))
>       ...))
>
> Could you also update guix.texi, near the end of “Invoking guix build”?
>
> WDYT?  Could you send an updated patch?
>

DONE,
Drp
-- 
(or ((,\ (x) `(,x x)) '(,\ (x) `(,x x))) (smth (that 'like)))

[-- Attachment #1.2: Type: text/html, Size: 4674 bytes --]

[-- Attachment #2: 0001-guix-scripts-Add-GUIX_BUILD_OPTIONS-environment-hand.patch --]
[-- Type: application/octet-stream, Size: 6671 bytes --]

From a4ec02e09f2bc1d4ad33e82ac9052439bdece6d0 Mon Sep 17 00:00:00 2001
From: nebuli <nebu@kipple>
Date: Sun, 30 Nov 2014 17:47:22 +0100
Subject: [PATCH] guix: scripts: Add GUIX_BUILD_OPTIONS environment handling.

* doc/guix.texi: Mention in the 'Invoking guix build' section.
* guix/scripts/archive.scm: (append args (environment-build-options)
* guix/scripts/build.scm: Ditto.
* guix/scripts/environment.scm: Ditto.
* guix/scripts/package.scm: Ditto.
* guix/scripts/system.scm: Ditto.
* guix/ui.scm (environment-build-options): New function.
* guix/utils.scm (arguments-from-environment-variable): New function.
---
 doc/guix.texi                | 12 ++++++++++++
 guix/scripts/archive.scm     |  3 ++-
 guix/scripts/build.scm       |  3 ++-
 guix/scripts/environment.scm |  3 ++-
 guix/scripts/package.scm     |  3 ++-
 guix/scripts/system.scm      |  3 ++-
 guix/ui.scm                  |  5 +++++
 guix/utils.scm               | 10 ++++++++++
 8 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 53bfb29..abfdc61 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -2781,6 +2781,18 @@ the @code{package-derivation} procedure of the @code{(guix packages)}
 module, and to the @code{build-derivations} procedure of the @code{(guix
 store)} module.
 
+Because @command{guix}, as an RPC client to @command{guix-daemon}
+(@pxref{Invoking guix-daemon}), uses its own set of default build options
+on each invocation, it is useful to export @code{GUIX_BUILD_OPTIONS}
+environmental variable, in the form of usual command-line arguments syntax:
+
+@example
+$ export GUIX_BUILD_OPTIONS="--no-substitutes -c 2 -L /foo/bar"
+@end example
+
+Those arguments will be appended to the ones passed via command-line to any
+guix command capable of handling build options.
+
 @node Invoking guix download
 @section Invoking @command{guix download}
 
diff --git a/guix/scripts/archive.scm b/guix/scripts/archive.scm
index 84904e2..29a3ad1 100644
--- a/guix/scripts/archive.scm
+++ b/guix/scripts/archive.scm
@@ -293,7 +293,8 @@ the input port."
 (define (guix-archive . args)
   (define (parse-options)
     ;; Return the alist of option values.
-    (args-fold* args %options
+    (args-fold* (append args (environment-build-options))
+                %options
                 (lambda (opt name arg result)
                   (leave (_ "~A: unrecognized option~%") name))
                 (lambda (arg result)
diff --git a/guix/scripts/build.scm b/guix/scripts/build.scm
index b4aa33b..76a743f 100644
--- a/guix/scripts/build.scm
+++ b/guix/scripts/build.scm
@@ -401,7 +401,8 @@ arguments with packages that use the specified source."
 (define (guix-build . args)
   (define (parse-options)
     ;; Return the alist of option values.
-    (args-fold* args %options
+    (args-fold* (append args (environment-build-options))
+                %options
                 (lambda (opt name arg result)
                   (leave (_ "~A: unrecognized option~%") name))
                 (lambda (arg result)
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 81bad96..a309dfa 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -213,7 +213,8 @@ packages."
 ;; Entry point.
 (define (guix-environment . args)
   (define (parse-options)
-    (args-fold* args %options
+    (args-fold* (append args (environment-build-options))
+                %options
                 (lambda (opt name arg result)
                   (leave (_ "~A: unrecognized option~%") name))
                 (lambda (arg result)
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 3a72053..9ff4d17 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -668,7 +668,8 @@ removed from MANIFEST."
 (define (guix-package . args)
   (define (parse-options)
     ;; Return the alist of option values.
-    (args-fold* args %options
+    (args-fold* (append args (environment-build-options))
+                %options
                 (lambda (opt name arg result arg-handler)
                   (leave (_ "~A: unrecognized option~%") name))
                 (lambda (arg result arg-handler)
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 398a5a3..8ea77e4 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -467,7 +467,8 @@ Build the operating system declared in FILE according to ACTION.\n"))
 (define (guix-system . args)
   (define (parse-options)
     ;; Return the alist of option values.
-    (args-fold* args %options
+    (args-fold* (append args (environment-build-options))
+                %options
                 (lambda (opt name arg result)
                   (leave (_ "~A: unrecognized option~%") name))
                 (lambda (arg result)
diff --git a/guix/ui.scm b/guix/ui.scm
index 69b073d..c77e041 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -64,6 +64,7 @@
             string->generations
             string->duration
             args-fold*
+            environment-build-options
             run-guix-command
             program-name
             guix-warning-port
@@ -712,6 +713,10 @@ reporting."
       (leave (_ "invalid argument: ~a~%")
              (apply format #f msg args)))))
 
+(define (environment-build-options)
+  "Return additional build options passed as environment variables."
+  (arguments-from-environment-variable "GUIX_BUILD_OPTIONS"))
+
 (define (show-guix-usage)
   (format (current-error-port)
           (_ "Try `guix --help' for more information.~%"))
diff --git a/guix/utils.scm b/guix/utils.scm
index 9b802b6..d0d2e8a 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -72,6 +72,7 @@
             package-name->name+version
             string-tokenize*
             string-replace-substring
+            arguments-from-environment-variable
             file-extension
             file-sans-extension
             call-with-temporary-output-file
@@ -627,6 +628,15 @@ REPLACEMENT."
                        (substring str start index)
                        pieces))))))))
 
+(define (arguments-from-environment-variable variable)
+  "Retrieve value of environment variable denoted by string VARIABLE in the
+form of a list of strings (`char-set:graphic' tokens) suitable for consumption
+by `args-fold', if VARIABLE is defined, otherwise return an empty list."
+  (let ((env (getenv variable)))
+    (if env
+        (string-tokenize env char-set:graphic)
+        '())))
+
 (define (call-with-temporary-output-file proc)
   "Call PROC with a name of a temporary file and open output port to that
 file; close the file and delete it when leaving the dynamic extent of this
-- 
2.1.2