Using 'system*' instead of 'system' in 'guix environment'

Using 'system*' instead of 'system' in 'guix environment'

[David Thompson]

Hello Guix hackers,

In an effort to finish up a patch to add a --container flag to 'guix
environment', I've encountered a serious problem.  The --exec flag
allows the user to pass an arbitrary command to be run using 'system'.
Unlike 'system*', 'system' spawns a command interpreter first and passes
the command string in.  This is very problematic when using a container,
because there's a very good chance that the command interpreter of the
running Guile process is not mounted inside the container.

So, I think we should switch to using 'system*' instead which will avoid
this hairy issue.  However, it's unclear to me how to make this happen.
I wanted to use 'system*' since I first wrote 'guix environment', but I
couldn't figure out how to make the command line syntax work since each
argument needs to be processed separately instead of being bunched up
into a string.

If the above explanation is confusing, the 'sudo' program provides a
good example of the UI I'm after:

    sudo guile -c '(do-root-things)'

But for now we're stuck with this:

    guix environment --ad-hoc guile -E "guile -c '(do-root-things)'"

Now, we can't actually do exactly what 'sudo' does because 'guix
environment' already recognizes operands as package names, not program
arguments.  Perhaps we can use '--' to separate the package list from
the command to run:

    guix environment --ad-hoc guile -- guile -c '(do-root-things)'

Does that look okay?  Any other ideas?

Thanks,

- Dave

Re: Using 'system*' instead of 'system' in 'guix environment'

[Ludovic Courtès]

Hi!

David Thompson <dthompson2@worcester.edu> skribis:

> In an effort to finish up a patch to add a --container flag to 'guix
> environment', I've encountered a serious problem.  The --exec flag
> allows the user to pass an arbitrary command to be run using 'system'.
> Unlike 'system*', 'system' spawns a command interpreter first and passes
> the command string in.  This is very problematic when using a container,
> because there's a very good chance that the command interpreter of the
> running Guile process is not mounted inside the container.

Oooh, good catch!

How about using something like:

  (system* (or (the-container-shell) (getenv "SHELL") "/bin/sh")
           "-c" the-string)

?

> If the above explanation is confusing, the 'sudo' program provides a
> good example of the UI I'm after:
>
>     sudo guile -c '(do-root-things)'

Or similarly: “ssh HOST some command and arguments”.

> But for now we're stuck with this:
>
>     guix environment --ad-hoc guile -E "guile -c '(do-root-things)'"
>
> Now, we can't actually do exactly what 'sudo' does because 'guix
> environment' already recognizes operands as package names, not program
> arguments.  Perhaps we can use '--' to separate the package list from
> the command to run:
>
>     guix environment --ad-hoc guile -- guile -c '(do-root-things)'
>
> Does that look okay?  Any other ideas?

I really like the UI that you propose; using -- to separate the
arguments sounds good.

I think it’s orthogonal to the question of whether to use ‘system’ or
not though.

Currently one can do things like:

  guix environment foo -E 'cd /bar ; frob'

and I think we should keep this capability, which means running the
command via /bin/sh -c (which is what ‘system’ does, but we can use
‘system*’ the way I wrote above to achieve that.)

So I think the new UI should essentially ‘string-join’ everything that
comes after --, and pass that to the procedure that invokes sh -c.

How does that sound?

Thanks for looking into it!

Ludo’.

Re: Using 'system*' instead of 'system' in 'guix environment'

[Thompson, David]

On Thu, Oct 8, 2015 at 3:53 AM, Ludovic Courtès <ludo@gnu.org> wrote:
> Hi!
>
> David Thompson <dthompson2@worcester.edu> skribis:
>
>> In an effort to finish up a patch to add a --container flag to 'guix
>> environment', I've encountered a serious problem.  The --exec flag
>> allows the user to pass an arbitrary command to be run using 'system'.
>> Unlike 'system*', 'system' spawns a command interpreter first and passes
>> the command string in.  This is very problematic when using a container,
>> because there's a very good chance that the command interpreter of the
>> running Guile process is not mounted inside the container.
>
> Oooh, good catch!
>
> How about using something like:
>
>   (system* (or (the-container-shell) (getenv "SHELL") "/bin/sh")
>            "-c" the-string)

Yes, that could work.  I've tried that but I don't love it.  More
about that below.

>> If the above explanation is confusing, the 'sudo' program provides a
>> good example of the UI I'm after:
>>
>>     sudo guile -c '(do-root-things)'
>
> Or similarly: “ssh HOST some command and arguments”.
>
>> But for now we're stuck with this:
>>
>>     guix environment --ad-hoc guile -E "guile -c '(do-root-things)'"
>>
>> Now, we can't actually do exactly what 'sudo' does because 'guix
>> environment' already recognizes operands as package names, not program
>> arguments.  Perhaps we can use '--' to separate the package list from
>> the command to run:
>>
>>     guix environment --ad-hoc guile -- guile -c '(do-root-things)'
>>
>> Does that look okay?  Any other ideas?
>
> I really like the UI that you propose; using -- to separate the
> arguments sounds good.
>
> I think it’s orthogonal to the question of whether to use ‘system’ or
> not though.
>
> Currently one can do things like:
>
>   guix environment foo -E 'cd /bar ; frob'
>
> and I think we should keep this capability, which means running the
> command via /bin/sh -c (which is what ‘system’ does, but we can use
> ‘system*’ the way I wrote above to achieve that.)
>
> So I think the new UI should essentially ‘string-join’ everything that
> comes after --, and pass that to the procedure that invokes sh -c.

I disagree, and here's why.  Going back to the sudo/ssh example, it's
not possible to do 'cd /bar; frob' naively because this...

    sudo cd /bar; frob

...is two commands.  And this doesn't work either because it's not a
valid string for exec:

    sudo 'cd /bar; frob'

However, we can just do the 'sh -c' trick!

    sudo sh -c 'cd /bar; frob'

This is essentially what you propose having built-in, but I think it
would be best to leave it out.  That way we can simply use 'system*'
and users that want to execute an inline Bash script can do so using
the method they most likely already know about from tools like sudo
and ssh.

    guix environment --ad-hoc guile -- guile -c '(frob)'

    guix environment --ad-hoc guile -- sh -c "cd bar/; guile -c '(frob)'"

This has the additional advantage that the first process created
inside containers will be PID 1, not 2.

Does this counter-proposal sound OK?

- Dave

Re: Using 'system*' instead of 'system' in 'guix environment'

[Ludovic Courtès]

"Thompson, David" <dthompson2@worcester.edu> skribis:

> I disagree, and here's why.  Going back to the sudo/ssh example, it's
> not possible to do 'cd /bar; frob' naively because this...
>
>     sudo cd /bar; frob
>
> ...is two commands.  And this doesn't work either because it's not a
> valid string for exec:
>
>     sudo 'cd /bar; frob'
>
> However, we can just do the 'sh -c' trick!
>
>     sudo sh -c 'cd /bar; frob'
>
> This is essentially what you propose having built-in, but I think it
> would be best to leave it out.  That way we can simply use 'system*'
> and users that want to execute an inline Bash script can do so using
> the method they most likely already know about from tools like sudo
> and ssh.
>
>     guix environment --ad-hoc guile -- guile -c '(frob)'
>
>     guix environment --ad-hoc guile -- sh -c "cd bar/; guile -c '(frob)'"
>
> This has the additional advantage that the first process created
> inside containers will be PID 1, not 2.
>
> Does this counter-proposal sound OK?

Yes it does; you win.  ;-)

I guess we must still support -E for compatibility.  Probably it should
do an implicit ‘sh -c’?

Thanks,
Ludo’.

Re: Using 'system*' instead of 'system' in 'guix environment'

[Thompson, David]

On Thu, Oct 8, 2015 at 10:59 AM, Ludovic Courtès <ludo@gnu.org> wrote:

> I guess we must still support -E for compatibility.  Probably it should
> do an implicit ‘sh -c’?

This introduces implementation issues.  What if a user provides both a
-E command *and* a command after '--'?  What's the sane thing to do?

I also don't feel strongly that we need to keep flags around for
compatibility this early in the game, given that we are alpha software
and such.

Thoughts?

- Dave

Re: Using 'system*' instead of 'system' in 'guix environment'

[Taylan Ulrich Bayırlı/Kammer]

"Thompson, David" <dthompson2@worcester.edu> writes:

> On Thu, Oct 8, 2015 at 10:59 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> I guess we must still support -E for compatibility.  Probably it should
>> do an implicit ‘sh -c’?
>
> This introduces implementation issues.  What if a user provides both a
> -E command *and* a command after '--'?  What's the sane thing to do?
>
> I also don't feel strongly that we need to keep flags around for
> compatibility this early in the game, given that we are alpha software
> and such.
>
> Thoughts?

I thought it would be nice to keep also for convenience...

-E 'foo' is somewhat nicer than -- sh -c 'foo'.

Taylan

Re: Using 'system*' instead of 'system' in 'guix environment'

[Thompson, David]

On Thu, Oct 8, 2015 at 12:05 PM, Taylan Ulrich Bayırlı/Kammer
<taylanbayirli@gmail.com> wrote:
> "Thompson, David" <dthompson2@worcester.edu> writes:
>
>> On Thu, Oct 8, 2015 at 10:59 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>>
>>> I guess we must still support -E for compatibility.  Probably it should
>>> do an implicit ‘sh -c’?
>>
>> This introduces implementation issues.  What if a user provides both a
>> -E command *and* a command after '--'?  What's the sane thing to do?
>>
>> I also don't feel strongly that we need to keep flags around for
>> compatibility this early in the game, given that we are alpha software
>> and such.
>>
>> Thoughts?
>
> I thought it would be nice to keep also for convenience...
>
> -E 'foo' is somewhat nicer than -- sh -c 'foo'.

But this is not a very common case (citing my own personal experience
and sudo, ssh, and other programs that use this pattern), and now we
have to deal with precedence rules in the argument parser.  If we have
to keep -E, then I would rather not implement the '--' stuff.

- Dave

Re: Using 'system*' instead of 'system' in 'guix environment'

[Taylan Ulrich Bayırlı/Kammer]

"Thompson, David" <dthompson2@worcester.edu> writes:

> On Thu, Oct 8, 2015 at 12:05 PM, Taylan Ulrich Bayırlı/Kammer
> <taylanbayirli@gmail.com> wrote:
>> "Thompson, David" <dthompson2@worcester.edu> writes:
>>
>>> On Thu, Oct 8, 2015 at 10:59 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>>>
>>>> I guess we must still support -E for compatibility.  Probably it should
>>>> do an implicit ‘sh -c’?
>>>
>>> This introduces implementation issues.  What if a user provides both a
>>> -E command *and* a command after '--'?  What's the sane thing to do?
>>>
>>> I also don't feel strongly that we need to keep flags around for
>>> compatibility this early in the game, given that we are alpha software
>>> and such.
>>>
>>> Thoughts?
>>
>> I thought it would be nice to keep also for convenience...
>>
>> -E 'foo' is somewhat nicer than -- sh -c 'foo'.
>
> But this is not a very common case (citing my own personal experience
> and sudo, ssh, and other programs that use this pattern), and now we
> have to deal with precedence rules in the argument parser.  If we have
> to keep -E, then I would rather not implement the '--' stuff.

Well never mind then, I don't have a strong opinion.

I used -E 'make && make check' frequently in the recent past but meh,
I'll set up some aliases in worst case, or finally integrate M-x compile
with a dir-local compile-command into my workflow.

Taylan

Re: Using 'system*' instead of 'system' in 'guix environment'

[Ludovic Courtès]

"Thompson, David" <dthompson2@worcester.edu> skribis:

> On Thu, Oct 8, 2015 at 10:59 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> I guess we must still support -E for compatibility.  Probably it should
>> do an implicit ‘sh -c’?
>
> This introduces implementation issues.  What if a user provides both a
> -E command *and* a command after '--'?  What's the sane thing to do?

I’d consider this a bug in the user’s mind ;-) and would do whatever is
easiest.

> I also don't feel strongly that we need to keep flags around for
> compatibility this early in the game, given that we are alpha software
> and such.

I think it’s neither black nor white.

For instance, I use it at work for continuous integration.  I can
definitely migrate the scripts to the new syntax, but it’s best if it
doesn’t break overnight.

So we could remove -E from ‘--help’ and from the manual, but still keep
it around for a while.

WDYT?

Ludo’.

Re: Using 'system*' instead of 'system' in 'guix environment'

[Thompson, David]

On Thu, Oct 8, 2015 at 12:53 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> "Thompson, David" <dthompson2@worcester.edu> skribis:
>
>> On Thu, Oct 8, 2015 at 10:59 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>>
>>> I guess we must still support -E for compatibility.  Probably it should
>>> do an implicit ‘sh -c’?
>>
>> This introduces implementation issues.  What if a user provides both a
>> -E command *and* a command after '--'?  What's the sane thing to do?
>
> I’d consider this a bug in the user’s mind ;-) and would do whatever is
> easiest.

Okay.  Will do.

>> I also don't feel strongly that we need to keep flags around for
>> compatibility this early in the game, given that we are alpha software
>> and such.
>
> I think it’s neither black nor white.
>
> For instance, I use it at work for continuous integration.  I can
> definitely migrate the scripts to the new syntax, but it’s best if it
> doesn’t break overnight.

I didn't know that you were one the users in question. ;)

> So we could remove -E from ‘--help’ and from the manual, but still keep
> it around for a while.

That sounds good.

- Dave

Re: Using 'system*' instead of 'system' in 'guix environment'

[Thompson, David]

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

On Thu, Oct 8, 2015 at 12:53 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> "Thompson, David" <dthompson2@worcester.edu> skribis:
>
>> On Thu, Oct 8, 2015 at 10:59 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>>
>>> I guess we must still support -E for compatibility.  Probably it should
>>> do an implicit ‘sh -c’?
>>
>> This introduces implementation issues.  What if a user provides both a
>> -E command *and* a command after '--'?  What's the sane thing to do?
>
> I’d consider this a bug in the user’s mind ;-) and would do whatever is
> easiest.
>
>> I also don't feel strongly that we need to keep flags around for
>> compatibility this early in the game, given that we are alpha software
>> and such.
>
> I think it’s neither black nor white.
>
> For instance, I use it at work for continuous integration.  I can
> definitely migrate the scripts to the new syntax, but it’s best if it
> doesn’t break overnight.
>
> So we could remove -E from ‘--help’ and from the manual, but still keep
> it around for a while.

Here is the patch that does this.

Thanks,

- Dave

[-- Attachment #2: 0001-scripts-environment-Use-system-instead-of-system.patch --]
[-- Type: text/x-diff, Size: 9408 bytes --]

From 4be0c2bfd2e2e9a03d860cfb2ff92aa66cbfaa70 Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Thu, 8 Oct 2015 21:23:09 -0400
Subject: [PATCH] scripts: environment: Use system* instead of system.

This allows for direct program invokation without needing a shell to act
as a command interpreter.

* guix/scripts/environment.scm (%default-shell): New variable.
  (show-help): Adjust description.  Remove '--exec' reference.
  (%default-options): Use '%default-shell'.
  (%options): Adjust '--exec' to run command via the default shell.
  (parse-args): New procedure.
  (guix-environment): Use 'parse-args'.  Use 'system*' instead of
  'system'.
* guix/utils.scm (split): New procedure.
* tests/guix-environment.sh: Adjust tests to use '--' instead of
  '--exec'.
* tests/utils.scm: Add tests for 'split'.
* doc/guix.texi ("Invoking guix environment"): Use new syntax.  Remove
  '--exec' documentation.
---
 doc/guix.texi                | 16 ++++++----------
 guix/scripts/environment.scm | 38 ++++++++++++++++++++++++++------------
 guix/utils.scm               | 18 ++++++++++++++++++
 tests/guix-environment.sh    |  4 ++--
 tests/utils.scm              | 14 ++++++++++++++
 5 files changed, 66 insertions(+), 24 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 6da7281..39b76c7 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -4538,11 +4538,12 @@ and Emacs are available:
 guix environment guile emacs
 @end example
 
-Sometimes an interactive shell session is not desired.  The
-@code{--exec} option can be used to specify the command to run instead.
+Sometimes an interactive shell session is not desired.  An arbitrary
+command may be invoked by placing the @code{--} token to separate the
+command from the rest of the arguments:
 
 @example
-guix environment guile --exec=make
+guix environment guile -- make -j4
 @end example
 
 In other situations, it is more convenient to specify the list of
@@ -4551,7 +4552,7 @@ runs @command{python} from an environment containing Python@tie{}2.7 and
 NumPy:
 
 @example
-guix environment --ad-hoc python2-numpy python-2.7 -E python
+guix environment --ad-hoc python2-numpy python-2.7 -- python
 @end example
 
 The available options are summarized below.
@@ -4582,11 +4583,6 @@ As an example, @var{file} might contain a definition like this
 @verbatiminclude environment-gdb.scm
 @end example
 
-
-@item --exec=@var{command}
-@item -E @var{command}
-Execute @var{command} in the new environment.
-
 @item --ad-hoc
 Include all specified packages in the resulting environment, as if an
 @i{ad hoc} package were defined with them as inputs.  This option is
@@ -4596,7 +4592,7 @@ package expression to contain the desired inputs.
 For instance, the command:
 
 @example
-guix environment --ad-hoc guile guile-sdl -E guile
+guix environment --ad-hoc guile guile-sdl -- guile
 @end example
 
 runs @command{guile} in an environment where Guile and Guile-SDL are
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 7aa52e8..d35ab18 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -57,6 +57,9 @@ OUTPUT) tuples."
 (define %precious-variables
   '("HOME" "USER" "LOGNAME" "DISPLAY" "TERM" "TZ" "PAGER"))
 
+(define %default-shell
+  (or (getenv "SHELL") "/bin/sh"))
+
 (define (purify-environment)
   "Unset almost all environment variables.  A small number of variables such
 as 'HOME' and 'USER' are left untouched."
@@ -103,9 +106,9 @@ existing environment variables with additional search paths."
     ,@(package-transitive-propagated-inputs package)))
 
 (define (show-help)
-  (display (_ "Usage: guix environment [OPTION]... PACKAGE...
-Build an environment that includes the dependencies of PACKAGE and execute a
-shell command in that environment.\n"))
+  (display (_ "Usage: guix environment [OPTION]... PACKAGE... [-- COMMAND...]
+Build an environment that includes the dependencies of PACKAGE and execute
+COMMAND or an interactive shell in that environment.\n"))
   (display (_ "
   -e, --expression=EXPR  create environment for the package that EXPR
                          evaluates to"))
@@ -113,8 +116,6 @@ shell command in that environment.\n"))
   -l, --load=FILE        create environment for the package that the code within
                          FILE evaluates to"))
   (display (_ "
-  -E, --exec=COMMAND     execute COMMAND in new environment"))
-  (display (_ "
       --ad-hoc           include all specified packages in the environment instead
                          of only their inputs"))
   (display (_ "
@@ -135,7 +136,7 @@ shell command in that environment.\n"))
 
 (define %default-options
   ;; Default to opening a new shell.
-  `((exec . ,(or (getenv "SHELL") "/bin/sh"))
+  `((exec . (,%default-shell))
     (system . ,(%current-system))
     (substitutes? . #t)
     (max-silent-time . 3600)
@@ -155,7 +156,7 @@ shell command in that environment.\n"))
                    (alist-cons 'pure #t result)))
          (option '(#\E "exec") #t #f
                  (lambda (opt name arg result)
-                   (alist-cons 'exec arg result)))
+                   (alist-cons 'exec (list %default-shell "-c" arg) result)))
          (option '("search-paths") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'search-paths #t result)))
@@ -230,14 +231,24 @@ OUTPUT) tuples, using the build options in OPTS."
                (built-derivations derivations)
                (return derivations))))))))
 
-;; Entry point.
-(define (guix-environment . args)
+(define (parse-args args)
+  "Parse the list of command line arguments ARGS."
   (define (handle-argument arg result)
     (alist-cons 'package arg result))
 
+  ;; The '--' token is used to separate the command to run from the rest of
+  ;; the operands.
+  (let-values (((args command) (split args "--")))
+    (let ((opts (parse-command-line args %options (list %default-options)
+                                    #:argument-handler handle-argument)))
+      (if (null? command)
+          opts
+          (alist-cons 'exec command opts)))))
+
+;; Entry point.
+(define (guix-environment . args)
   (with-error-handling
-    (let* ((opts     (parse-command-line args %options (list %default-options)
-                                         #:argument-handler handle-argument))
+    (let* ((opts     (parse-args args))
            (pure?    (assoc-ref opts 'pure))
            (ad-hoc?  (assoc-ref opts 'ad-hoc?))
            (command  (assoc-ref opts 'exec))
@@ -282,4 +293,7 @@ OUTPUT) tuples, using the build options in OPTS."
                      (return #t))
                     (else
                      (create-environment inputs paths pure?)
-                     (return (exit (status:exit-val (system command)))))))))))))
+                     (return
+                      (exit
+                       (status:exit-val
+                        (apply system* command)))))))))))))
diff --git a/guix/utils.scm b/guix/utils.scm
index 1d4b2ff..070f804 100644
--- a/guix/utils.scm
+++ b/guix/utils.scm
@@ -79,6 +79,7 @@
             fold2
             fold-tree
             fold-tree-leaves
+            split
 
             filtered-port
             compressed-port
@@ -684,6 +685,23 @@ are connected to NODE in the tree, or '() or #f if NODE is a leaf node."
        (else result)))
    init children roots))
 
+(define (split lst e)
+  "Return two values, a list containing the elements of the list LST that
+appear before the first occurence of the object E and a list containing the
+elements after E."
+  (define (same? x)
+    (equal? e x))
+
+  (let loop ((rest lst)
+             (acc '()))
+    (match rest
+      (()
+       (values lst '()))
+      (((? same?) . tail)
+       (values (reverse acc) tail))
+      ((head . tail)
+       (loop tail (cons head acc))))))
+
 \f
 ;;;
 ;;; Source location.
diff --git a/tests/guix-environment.sh b/tests/guix-environment.sh
index 32faf71..279692f 100644
--- a/tests/guix-environment.sh
+++ b/tests/guix-environment.sh
@@ -40,7 +40,7 @@ test "`wc -l < "$tmpdir/a"`" = 1
 cmp "$tmpdir/a" "$tmpdir/b"
 
 # Make sure the exit value is preserved.
-if guix environment --ad-hoc guile-bootstrap --pure -E 'guile -c "(exit 42)"'
+if guix environment --ad-hoc guile-bootstrap --pure -- guile -c '(exit 42)'
 then
     false
 else
@@ -66,7 +66,7 @@ then
     # as returned by '--search-paths'.
     guix environment -e '(@@ (gnu packages commencement) gnu-make-boot0)'	\
 	 --no-substitutes --pure						\
-         --exec='echo $PATH $CPATH $LIBRARY_PATH' > "$tmpdir/b"
+         -- /bin/sh -c 'echo $PATH $CPATH $LIBRARY_PATH' > "$tmpdir/b"
     ( . "$tmpdir/a" ; echo $PATH $CPATH $LIBRARY_PATH ) > "$tmpdir/c"
     cmp "$tmpdir/b" "$tmpdir/c"
 
diff --git a/tests/utils.scm b/tests/utils.scm
index 115868c..b65d6d2 100644
--- a/tests/utils.scm
+++ b/tests/utils.scm
@@ -121,6 +121,20 @@
                '(0 1 2 3)))
     list))
 
+(test-equal "split, element is in list"
+  '((foo) (baz))
+  (call-with-values
+      (lambda ()
+        (split '(foo bar baz) 'bar))
+    list))
+
+(test-equal "split, element is not in list"
+  '((foo bar baz) ())
+  (call-with-values
+      (lambda ()
+        (split '(foo bar baz) 'quux))
+    list))
+
 (test-equal "strip-keyword-arguments"
   '(a #:b b #:c c)
   (strip-keyword-arguments '(#:foo #:bar #:baz)
-- 
2.5.0

Re: Using 'system*' instead of 'system' in 'guix environment'

[Ludovic Courtès]

"Thompson, David" <dthompson2@worcester.edu> skribis:

> From 4be0c2bfd2e2e9a03d860cfb2ff92aa66cbfaa70 Mon Sep 17 00:00:00 2001
> From: David Thompson <dthompson2@worcester.edu>
> Date: Thu, 8 Oct 2015 21:23:09 -0400
> Subject: [PATCH] scripts: environment: Use system* instead of system.
>
> This allows for direct program invokation without needing a shell to act
> as a command interpreter.
>
> * guix/scripts/environment.scm (%default-shell): New variable.
>   (show-help): Adjust description.  Remove '--exec' reference.
>   (%default-options): Use '%default-shell'.
>   (%options): Adjust '--exec' to run command via the default shell.
>   (parse-args): New procedure.
>   (guix-environment): Use 'parse-args'.  Use 'system*' instead of
>   'system'.
> * guix/utils.scm (split): New procedure.
> * tests/guix-environment.sh: Adjust tests to use '--' instead of
>   '--exec'.
> * tests/utils.scm: Add tests for 'split'.
> * doc/guix.texi ("Invoking guix environment"): Use new syntax.  Remove
>   '--exec' documentation.

Looks good to me.  Could you just add ‘split’ in a separate commit?

>           (option '(#\E "exec") #t #f

Add a “deprecated” comment.

>  # Make sure the exit value is preserved.
> -if guix environment --ad-hoc guile-bootstrap --pure -E 'guile -c "(exit 42)"'
> +if guix environment --ad-hoc guile-bootstrap --pure -- guile -c '(exit 42)'

Could you keep the example with -E, in addition to the new one, with a
comment noting that this is the deprecated syntax?  We’ll remove it when
we finally remove -E.

OK with these changes, thank you!

Ludo’.

Re: Using 'system*' instead of 'system' in 'guix environment'

[Thompson, David]

On Fri, Oct 9, 2015 at 8:23 AM, Ludovic Courtès <ludo@gnu.org> wrote:
> "Thompson, David" <dthompson2@worcester.edu> skribis:
>
>> From 4be0c2bfd2e2e9a03d860cfb2ff92aa66cbfaa70 Mon Sep 17 00:00:00 2001
>> From: David Thompson <dthompson2@worcester.edu>
>> Date: Thu, 8 Oct 2015 21:23:09 -0400
>> Subject: [PATCH] scripts: environment: Use system* instead of system.
>>
>> This allows for direct program invokation without needing a shell to act
>> as a command interpreter.
>>
>> * guix/scripts/environment.scm (%default-shell): New variable.
>>   (show-help): Adjust description.  Remove '--exec' reference.
>>   (%default-options): Use '%default-shell'.
>>   (%options): Adjust '--exec' to run command via the default shell.
>>   (parse-args): New procedure.
>>   (guix-environment): Use 'parse-args'.  Use 'system*' instead of
>>   'system'.
>> * guix/utils.scm (split): New procedure.
>> * tests/guix-environment.sh: Adjust tests to use '--' instead of
>>   '--exec'.
>> * tests/utils.scm: Add tests for 'split'.
>> * doc/guix.texi ("Invoking guix environment"): Use new syntax.  Remove
>>   '--exec' documentation.
>
> Looks good to me.  Could you just add ‘split’ in a separate commit?
>
>>           (option '(#\E "exec") #t #f
>
> Add a “deprecated” comment.
>
>>  # Make sure the exit value is preserved.
>> -if guix environment --ad-hoc guile-bootstrap --pure -E 'guile -c "(exit 42)"'
>> +if guix environment --ad-hoc guile-bootstrap --pure -- guile -c '(exit 42)'
>
> Could you keep the example with -E, in addition to the new one, with a
> comment noting that this is the deprecated syntax?  We’ll remove it when
> we finally remove -E.
>
> OK with these changes, thank you!

Thanks, fixed and pushed 2 commits.

- Dave

Re: Using 'system*' instead of 'system' in 'guix environment'

[Taylan Ulrich Bayırlı/Kammer]

"Thompson, David" <dthompson2@worcester.edu> writes:

> guix environment --ad-hoc guile -- sh -c "cd bar/; guile -c '(frob)'"

AFAIUI that's equivalent to

  guix environment --ad-hoc guile -E "cd bar/; guile -c '(frob)'"

(Or was the intent to deprecate -E?)

Therefore I'd agree that doing no string join in the '--' case is good,
since the string joining variant is already covered, for convenience, by
the '-E' variant.

However, the '(or (container-shell) "/bin/sh")' thing could be done for
the '-E' variant, no?

So we'd have the pure system* variant with '--', and the string joining
variant with '-E' which might pass the joined string to sh or to some
other shell.  (Though those shells will need to agree on the '-c' bit.)


Taylan