guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ludovic Courtès
Subject: Re: [PATCH] Core sanity and taking build options from environment.
Date: Fri, 28 Nov 2014 22:52:48 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4 (gnu/linux)

Deck Pickard <address@hidden> skribis:

> From 3693753aefc27b5a68a2b762feeebc41320e79ef Mon Sep 17 00:00:00 2001
> From: nebuli <address@hidden>
> 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?

> From fa8738ff2cf48886c9ef8fbacfa806f547f3c3c8 Mon Sep 17 00:00:00 2001
> From: nebuli <address@hidden>
> 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?

Thank you,
Ludo’.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]