Re: [PATCH] Core sanity and taking build options from environment.
Deck Pickard skribis: > On 28 Nov 2014 22:52, "Ludovic Courtès" wrote: >> >> Deck Pickard skribis: >> >> > From 3693753aefc27b5a68a2b762feeebc41320e79ef Mon Sep 17 00:00:00 2001 >> > From: nebuli >> > 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, I’ve always used as many cores as available, and it’s definitely been OK, FWIW. > but not for hours of work. And you are creating more confusion by > having different defaults from guix-daemon. Right, I’ve changed guix-daemon’s default to 0, which at least will avoid confusion. > From a4ec02e09f2bc1d4ad33e82ac9052439bdece6d0 Mon Sep 17 00:00:00 2001 > From: nebuli > 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. I tweaked guix.texi, updated test-env.in, added a test, and pushed it. Thanks! Ludo’.
Re: [PATCH] Core sanity and taking build options from environment.
On 28 Nov 2014 22:52, "Ludovic Courtès" wrote:
>
> Deck Pickard skribis:
>
> > From 3693753aefc27b5a68a2b762feeebc41320e79ef Mon Sep 17 00:00:00 2001
> > From: nebuli
> > 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 '... ouch, 36
threads competing for silicon on a 6 core box...
> > From fa8738ff2cf48886c9ef8fbacfa806f547f3c3c8 Mon Sep 17 00:00:00 2001
> > From: nebuli
> > 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)))
0001-guix-scripts-Add-GUIX_BUILD_OPTIONS-environment-hand.patch
Description: Binary data
Re: [PATCH] Core sanity and taking build options from environment.
On Fri, Nov 28, 2014 at 10:52:48PM +0100, Ludovic Courtès wrote: > I think most of the time one would prefer to use all the available cores > when building. WDYT? I agree. Andreas
Re: [PATCH] Core sanity and taking build options from environment.
Deck Pickard skribis:
> From 3693753aefc27b5a68a2b762feeebc41320e79ef Mon Sep 17 00:00:00 2001
> From: nebuli
> 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
> 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’.
[PATCH] Core sanity and taking build options from environment.
Perhaps GUIX_BUILD (suffix with _OPTS or some such?) is not the best name and I'm not certain if it shouldn't be stdout (why are we using error port for "normal" communication? Emacs?), but I think it might prove useful if user keeps being reminded his environment is polluted or not as his intentions might have been, or it could go "only" into guix|guix --help... To doc or not to doc, Drp -- .sig place holder 0001-guix-Default-to-daemon-s-default-cores-setting-of-1.patch Description: Binary data 0002-guix-scripts-Add-handling-of-options-from-GUIX_BUILD.patch Description: Binary data
