diff mbox series

[bug#60341,Home] home: services: environment-variables: Add support for literal strings.

Message ID 20221226181938.30003-1-ludo@gnu.org
State New
Headers show
Series [bug#60341,Home] home: services: environment-variables: Add support for literal strings. | expand

Commit Message

Ludovic Courtès Dec. 26, 2022, 6:19 p.m. UTC
* gnu/home/services.scm (<literal-string>): New record type.
(environment-variable-shell-definitions): Split 'shell-quote' into
'quote-string' and 'shell-double-quote'.  Add 'shell-single-quote'.
Add clause for 'literal-string' records.
* tests/guix-home.sh: Test it.
* doc/guix.texi (Essential Home Services): Document it.
---
 doc/guix.texi         | 14 ++++++++++---
 gnu/home/services.scm | 48 ++++++++++++++++++++++++++++++++-----------
 tests/guix-home.sh    |  4 +++-
 3 files changed, 50 insertions(+), 16 deletions(-)

Hi!

I found that it’s occasionally useful to be able to define shell variables
with a value that is to be taken literally, without shell expansion.  This
is what this patch implements.

Thoughts?

Ludo’.


base-commit: 9369c1ccf47d9bf6f2e28a9454c1c329a2044f19

Comments

Andrew Tropin Dec. 27, 2022, 3:02 a.m. UTC | #1
On 2022-12-26 19:19, Ludovic Courtès wrote:

> * gnu/home/services.scm (<literal-string>): New record type.
> (environment-variable-shell-definitions): Split 'shell-quote' into
> 'quote-string' and 'shell-double-quote'.  Add 'shell-single-quote'.
> Add clause for 'literal-string' records.
> * tests/guix-home.sh: Test it.
> * doc/guix.texi (Essential Home Services): Document it.
> ---
>  doc/guix.texi         | 14 ++++++++++---
>  gnu/home/services.scm | 48 ++++++++++++++++++++++++++++++++-----------
>  tests/guix-home.sh    |  4 +++-
>  3 files changed, 50 insertions(+), 16 deletions(-)
>
> Hi!
>
> I found that it’s occasionally useful to be able to define shell variables
> with a value that is to be taken literally, without shell expansion.  This
> is what this patch implements.
>
> Thoughts?

Hi Ludo,

the code looks good.

I'm a little conserned that one need to learn a new abstraction to
properly quote the expression instead of just using " or ' inside value
string, but we already have af4c103595a725194318f40fc5aba110772ff417 and
with the current state of the code this change looks rational and
idiomatic.  Also, it makes the interface more high-level, which can be
good in some circumstances.

>
> Ludo’.
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index efd281d9b0..44361b481a 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -40972,13 +40972,15 @@ The easiest way to extend a service type, without defining a new service
>  type is to use the @code{simple-service} helper from @code{(gnu
>  services)}.
>  
> +@findex literal-string
>  @lisp
>  (simple-service 'some-useful-env-vars-service
>  		home-environment-variables-service-type
>  		`(("LESSHISTFILE" . "$XDG_CACHE_HOME/.lesshst")
>                    ("SHELL" . ,(file-append zsh "/bin/zsh"))
>                    ("USELESS_VAR" . #f)
> -                  ("_JAVA_AWT_WM_NONREPARENTING" . #t)))
> +                  ("_JAVA_AWT_WM_NONREPARENTING" . #t)
> +                  ("LITERAL_VALUE" . ,(literal-string "$@{abc@}"))))
>  @end lisp
>  
>  If you include such a service in you home environment definition, it
> @@ -40986,11 +40988,17 @@ will add the following content to the @file{setup-environment} script
>  (which is expected to be sourced by the login shell):
>  
>  @example
> -export LESSHISTFILE=$XDG_CACHE_HOME/.lesshst
> -export SHELL=/gnu/store/2hsg15n644f0glrcbkb1kqknmmqdar03-zsh-5.8/bin/zsh
> +export LESSHISTFILE="$XDG_CACHE_HOME/.lesshst"
> +export SHELL="/gnu/store/2hsg15n644f0glrcbkb1kqknmmqdar03-zsh-5.8/bin/zsh"
>  export _JAVA_AWT_WM_NONREPARENTING
> +export LITERAL_VALUE='$@{abc@}'
>  @end example
>  
> +Notice that @code{literal-string} above lets us declare that a value is
> +to be interpreted as a @dfn{literal string}, meaning that ``special
> +characters'' such as the dollar sign will not be interpreted by the
> +shell.
> +
>  @quotation Note
>  Make sure that module @code{(gnu packages shells)} is imported with
>  @code{use-modules} or any other way, this namespace contains the
> diff --git a/gnu/home/services.scm b/gnu/home/services.scm
> index 99035686f1..2c1f58fddf 100644
> --- a/gnu/home/services.scm
> +++ b/gnu/home/services.scm
> @@ -1,6 +1,7 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
>  ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
> +;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -33,6 +34,7 @@ (define-module (gnu home services)
>    #:use-module (guix i18n)
>    #:use-module (guix modules)
>    #:use-module (srfi srfi-1)
> +  #:use-module (srfi srfi-9)
>    #:use-module (ice-9 match)
>    #:use-module (ice-9 vlist)
>  
> @@ -47,6 +49,10 @@ (define-module (gnu home services)
>              home-run-on-change-service-type
>              home-provenance-service-type
>  
> +            literal-string
> +            literal-string?
> +            literal-string-value
> +
>              environment-variable-shell-definitions
>              home-files-directory
>              xdg-configuration-files-directory
> @@ -171,32 +177,50 @@ (define home-profile-service-type
>  configuration files that the user has declared in their
>  @code{home-environment} record.")))
>  
> +;; Representation of a literal string.
> +(define-record-type <literal-string>
> +  (literal-string str)
> +  literal-string?
> +  (str literal-string-value))
> +
>  (define (environment-variable-shell-definitions variables)
>    "Return a gexp that evaluates to a list of POSIX shell statements defining
>  VARIABLES, a list of environment variable name/value pairs.  The returned code
>  ensures variable values are properly quoted."
> -  #~(let ((shell-quote
> -           (lambda (value)
> -             ;; Double-quote VALUE, leaving dollar sign as is.
> -             (let ((quoted (list->string
> -                            (string-fold-right
> +  #~(let* ((quote-string
> +            (lambda (value quoted-chars)
> +              (list->string (string-fold-right
>                               (lambda (chr lst)
> -                               (case chr
> -                                 ((#\" #\\)
> -                                  (append (list chr #\\) lst))
> -                                 (else (cons chr lst))))
> +                               (if (memq chr quoted-chars)
> +                                   (append (list chr #\\) lst)
> +                                   (cons chr lst)))
>                               '()
>                               value))))
> -               (string-append "\"" quoted "\"")))))
> +           (shell-double-quote
> +            (lambda (value)
> +              ;; Double-quote VALUE, leaving dollar sign as is.
> +              (string-append "\"" (quote-string value '(#\" #\\))
> +                             "\"")))
> +           (shell-single-quote
> +            (lambda (value)
> +              ;; Single-quote VALUE to enter a literal string.
> +              (string-append "'" (quote-string value '(#\' #\\))
> +                             "'"))))
>        (string-append
>         #$@(map (match-lambda
>                   ((key . #f)
>                    "")
>                   ((key . #t)
>                    #~(string-append "export " #$key "\n"))
> -                 ((key . value)
> +                 ((key . (? string? value))
>                    #~(string-append "export " #$key "="
> -                                   (shell-quote #$value) "\n")))
> +                                   (shell-double-quote #$value)
> +                                   "\n"))
> +                 ((key . (? literal-string? value))
> +                  #~(string-append "export " #$key "="
> +                                   (shell-single-quote
> +                                    #$(literal-string-value value))
> +                                   "\n")))
>                 variables))))
>  
>  (define (environment-variables->setup-environment-script vars)
> diff --git a/tests/guix-home.sh b/tests/guix-home.sh
> index d5e2dadbb5..423ebf6f33 100644
> --- a/tests/guix-home.sh
> +++ b/tests/guix-home.sh
> @@ -81,7 +81,8 @@ trap 'chmod -Rf +w "$test_directory"; rm -rf "$test_directory"' EXIT
>  
>     (simple-service 'add-environment-variable
>                     home-environment-variables-service-type
> -                   '(("TODAY" . "26 messidor")))
> +                   `(("TODAY" . "26 messidor")
> +                     ("LITERAL" . ,(literal-string "${abc}"))))
>  
>     (simple-service 'home-bash-service-extension-test
>                     home-bash-service-type
> @@ -149,6 +150,7 @@ EOF
>      grep -q "the content of ~/.config/test.conf" "${HOME}/.config/test.conf"
>      grep '^export PS1="\$GUIX_ENVIRONMENT λ "$' "${HOME}/.bash_profile"
>      ( . "${HOME}/.guix-home/setup-environment"; test "$TODAY" = "26 messidor" )
> +    ( . "${HOME}/.guix-home/setup-environment"; test "$LITERAL" = '${abc}' )
>  
>      # This one should still be here.
>      grep "stay around" "$HOME/.config/random-file"
>
> base-commit: 9369c1ccf47d9bf6f2e28a9454c1c329a2044f19
Ludovic Courtès Jan. 3, 2023, 10:52 p.m. UTC | #2
Hi,

Andrew Tropin <andrew@trop.in> skribis:

> On 2022-12-26 19:19, Ludovic Courtès wrote:

[...]

>> I found that it’s occasionally useful to be able to define shell variables
>> with a value that is to be taken literally, without shell expansion.  This
>> is what this patch implements.
>>
>> Thoughts?
>
> Hi Ludo,
>
> the code looks good.
>
> I'm a little conserned that one need to learn a new abstraction to
> properly quote the expression instead of just using " or ' inside value
> string, but we already have af4c103595a725194318f40fc5aba110772ff417 and
> with the current state of the code this change looks rational and
> idiomatic.  Also, it makes the interface more high-level, which can be
> good in some circumstances.

Yeah, the shell/Scheme mixture is a bit weird; it’s sort of in-between
because you can’t completely ignore that it’s shell code under the hood.

Looking at <https://issues.guix.gnu.org/54469>, I think we should strive
to not generate non-working shell snippets.  Automatic quoting and this
‘literal-string’ construct are a way to achieve that.

Ludo’.
Andrew Tropin Jan. 4, 2023, 5:24 a.m. UTC | #3
On 2023-01-03 23:52, Ludovic Courtès wrote:

> Hi,
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>> On 2022-12-26 19:19, Ludovic Courtès wrote:
>
> [...]
>
>>> I found that it’s occasionally useful to be able to define shell variables
>>> with a value that is to be taken literally, without shell expansion.  This
>>> is what this patch implements.
>>>
>>> Thoughts?
>>
>> Hi Ludo,
>>
>> the code looks good.
>>
>> I'm a little conserned that one need to learn a new abstraction to
>> properly quote the expression instead of just using " or ' inside value
>> string, but we already have af4c103595a725194318f40fc5aba110772ff417 and
>> with the current state of the code this change looks rational and
>> idiomatic.  Also, it makes the interface more high-level, which can be
>> good in some circumstances.
>
> Yeah, the shell/Scheme mixture is a bit weird; it’s sort of in-between
> because you can’t completely ignore that it’s shell code under the hood.
>
> Looking at <https://issues.guix.gnu.org/54469>, I think we should strive
> to not generate non-working shell snippets.  Automatic quoting and this
> ‘literal-string’ construct are a way to achieve that.

Yep, I think this is a way to go.
Ludovic Courtès Jan. 5, 2023, 2:18 p.m. UTC | #4
Andrew Tropin <andrew@trop.in> skribis:

> On 2023-01-03 23:52, Ludovic Courtès wrote:

[...]

>> Yeah, the shell/Scheme mixture is a bit weird; it’s sort of in-between
>> because you can’t completely ignore that it’s shell code under the hood.
>>
>> Looking at <https://issues.guix.gnu.org/54469>, I think we should strive
>> to not generate non-working shell snippets.  Automatic quoting and this
>> ‘literal-string’ construct are a way to achieve that.
>
> Yep, I think this is a way to go.

Alright, pushed as 73684dc90e013f2f0cca1097b0c944bb9aa88709.

Thanks!

Ludo’.
Trevor Richards Jan. 8, 2023, 2:29 a.m. UTC | #5
I always knew my `environment-variables` were a mess.  This patch broke
my guix home config and send me down a confusing rabbit hole to this
submission.  My environment-variables form looks so nice now ;P
diff mbox series

Patch

diff --git a/doc/guix.texi b/doc/guix.texi
index efd281d9b0..44361b481a 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -40972,13 +40972,15 @@  The easiest way to extend a service type, without defining a new service
 type is to use the @code{simple-service} helper from @code{(gnu
 services)}.
 
+@findex literal-string
 @lisp
 (simple-service 'some-useful-env-vars-service
 		home-environment-variables-service-type
 		`(("LESSHISTFILE" . "$XDG_CACHE_HOME/.lesshst")
                   ("SHELL" . ,(file-append zsh "/bin/zsh"))
                   ("USELESS_VAR" . #f)
-                  ("_JAVA_AWT_WM_NONREPARENTING" . #t)))
+                  ("_JAVA_AWT_WM_NONREPARENTING" . #t)
+                  ("LITERAL_VALUE" . ,(literal-string "$@{abc@}"))))
 @end lisp
 
 If you include such a service in you home environment definition, it
@@ -40986,11 +40988,17 @@  will add the following content to the @file{setup-environment} script
 (which is expected to be sourced by the login shell):
 
 @example
-export LESSHISTFILE=$XDG_CACHE_HOME/.lesshst
-export SHELL=/gnu/store/2hsg15n644f0glrcbkb1kqknmmqdar03-zsh-5.8/bin/zsh
+export LESSHISTFILE="$XDG_CACHE_HOME/.lesshst"
+export SHELL="/gnu/store/2hsg15n644f0glrcbkb1kqknmmqdar03-zsh-5.8/bin/zsh"
 export _JAVA_AWT_WM_NONREPARENTING
+export LITERAL_VALUE='$@{abc@}'
 @end example
 
+Notice that @code{literal-string} above lets us declare that a value is
+to be interpreted as a @dfn{literal string}, meaning that ``special
+characters'' such as the dollar sign will not be interpreted by the
+shell.
+
 @quotation Note
 Make sure that module @code{(gnu packages shells)} is imported with
 @code{use-modules} or any other way, this namespace contains the
diff --git a/gnu/home/services.scm b/gnu/home/services.scm
index 99035686f1..2c1f58fddf 100644
--- a/gnu/home/services.scm
+++ b/gnu/home/services.scm
@@ -1,6 +1,7 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
+;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -33,6 +34,7 @@  (define-module (gnu home services)
   #:use-module (guix i18n)
   #:use-module (guix modules)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-9)
   #:use-module (ice-9 match)
   #:use-module (ice-9 vlist)
 
@@ -47,6 +49,10 @@  (define-module (gnu home services)
             home-run-on-change-service-type
             home-provenance-service-type
 
+            literal-string
+            literal-string?
+            literal-string-value
+
             environment-variable-shell-definitions
             home-files-directory
             xdg-configuration-files-directory
@@ -171,32 +177,50 @@  (define home-profile-service-type
 configuration files that the user has declared in their
 @code{home-environment} record.")))
 
+;; Representation of a literal string.
+(define-record-type <literal-string>
+  (literal-string str)
+  literal-string?
+  (str literal-string-value))
+
 (define (environment-variable-shell-definitions variables)
   "Return a gexp that evaluates to a list of POSIX shell statements defining
 VARIABLES, a list of environment variable name/value pairs.  The returned code
 ensures variable values are properly quoted."
-  #~(let ((shell-quote
-           (lambda (value)
-             ;; Double-quote VALUE, leaving dollar sign as is.
-             (let ((quoted (list->string
-                            (string-fold-right
+  #~(let* ((quote-string
+            (lambda (value quoted-chars)
+              (list->string (string-fold-right
                              (lambda (chr lst)
-                               (case chr
-                                 ((#\" #\\)
-                                  (append (list chr #\\) lst))
-                                 (else (cons chr lst))))
+                               (if (memq chr quoted-chars)
+                                   (append (list chr #\\) lst)
+                                   (cons chr lst)))
                              '()
                              value))))
-               (string-append "\"" quoted "\"")))))
+           (shell-double-quote
+            (lambda (value)
+              ;; Double-quote VALUE, leaving dollar sign as is.
+              (string-append "\"" (quote-string value '(#\" #\\))
+                             "\"")))
+           (shell-single-quote
+            (lambda (value)
+              ;; Single-quote VALUE to enter a literal string.
+              (string-append "'" (quote-string value '(#\' #\\))
+                             "'"))))
       (string-append
        #$@(map (match-lambda
                  ((key . #f)
                   "")
                  ((key . #t)
                   #~(string-append "export " #$key "\n"))
-                 ((key . value)
+                 ((key . (? string? value))
                   #~(string-append "export " #$key "="
-                                   (shell-quote #$value) "\n")))
+                                   (shell-double-quote #$value)
+                                   "\n"))
+                 ((key . (? literal-string? value))
+                  #~(string-append "export " #$key "="
+                                   (shell-single-quote
+                                    #$(literal-string-value value))
+                                   "\n")))
                variables))))
 
 (define (environment-variables->setup-environment-script vars)
diff --git a/tests/guix-home.sh b/tests/guix-home.sh
index d5e2dadbb5..423ebf6f33 100644
--- a/tests/guix-home.sh
+++ b/tests/guix-home.sh
@@ -81,7 +81,8 @@  trap 'chmod -Rf +w "$test_directory"; rm -rf "$test_directory"' EXIT
 
    (simple-service 'add-environment-variable
                    home-environment-variables-service-type
-                   '(("TODAY" . "26 messidor")))
+                   `(("TODAY" . "26 messidor")
+                     ("LITERAL" . ,(literal-string "${abc}"))))
 
    (simple-service 'home-bash-service-extension-test
                    home-bash-service-type
@@ -149,6 +150,7 @@  EOF
     grep -q "the content of ~/.config/test.conf" "${HOME}/.config/test.conf"
     grep '^export PS1="\$GUIX_ENVIRONMENT λ "$' "${HOME}/.bash_profile"
     ( . "${HOME}/.guix-home/setup-environment"; test "$TODAY" = "26 messidor" )
+    ( . "${HOME}/.guix-home/setup-environment"; test "$LITERAL" = '${abc}' )
 
     # This one should still be here.
     grep "stay around" "$HOME/.config/random-file"