GNU bug report logs - #43627
[PATCH core-updates]: Add a 'append-separator?' field to the <search-path-specification> record.

Previous Next

Package: guix-patches;

Reported by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

Date: Sat, 26 Sep 2020 06:10:02 UTC

Severity: normal

Tags: patch

Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


Message #38 received at 43627 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 43627 <at> debbugs.gnu.org, Leo Prikler <leo.prikler <at> student.tugraz.at>
Subject: Re: [PATCH core-updates v2 1/2] guix: Add an append-separator?
 field to the <search-path-specification> record.
Date: Sat, 10 Apr 2021 22:42:18 +0200
Hi,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> This new field allows to specify in a search-path-specification that a
> trailing separator should be added to the computed value of the environment
> variable.  A trailing separator sometimes has the meaning that the usual
> builtin locations should be looked up as well as the ones explicitly
> specified.
>
> One use case is to specify the Emacs library paths using EMACSLOADPATH.  This
> allows to not embed the Emacs version in its search path specification, which
> has been shown to cause issues when upgrading a profile or when defining
> variant Emacs packages of different versions.

Got it now.  :-)

Leos patch series seems to be addressing the same issue:

  https://issues.guix.gnu.org/47661

Should we hold on until weve reviewed and acted upon #47661?  Or does
it have known uses apart from Emacs?

> * guix/search-paths.scm (searh-path-specification): Add an APPEND-SEPARATOR?
> field.
> (search-path-specification->sexp): Adjust accordingly.
> (sexp->search-path-specification): Likewise.
> (evaluate-search-paths): Append a separator to the search path value when both
> `separator' and `append-separator?' are #t.  Document the new behavior.
> * guix/scripts/environment.scm (create-environment): Adjust the logic used to
> merge search-path values when creating an environment profile.
> * guix/build/gnu-build-system.scm (set-paths): Adjust accordingly.

Overall LGTM.  Id suggest maybe replacing append-separator? by
trailing-separator?, which I find a bit clearer.

Could you add a test or two in tests/search-paths.scm, for the corner
cases?

> -             ((env-var (files ...) separator type pattern)
> +             ((env-var (files ...) separator type pattern append-sep)
>                (set-path-environment-variable env-var files
>                                               input-directories
>                                               #:separator separator
>                                               #:type type
> -                                             #:pattern pattern)))
> +                                             #:pattern pattern
> +                                             #:append-separator? append-sep)))
>              search-paths)
>  
>    (when native-search-paths
>      ;; Search paths for native inputs, when cross building.
>      (for-each (match-lambda
> -               ((env-var (files ...) separator type pattern)
> +               ((env-var (files ...) separator type pattern append-sep)
>                  (set-path-environment-variable env-var files

Id fully spell out the variable name, like append-separator?,
append?, trailing?, as per our coding style.

> +++ b/guix/build/profiles.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2015, 2017, 2018, 2019, 2020, 2021 Ludovic Court¨s <ludo <at> gnu.org>
> +;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -20,6 +21,7 @@
>    #:use-module (guix build union)
>    #:use-module (guix build utils)
>    #:use-module (guix search-paths)
> +  #:use-module (srfi srfi-1)
>    #:use-module (srfi srfi-26)
>    #:use-module (ice-9 ftw)
>    #:use-module (ice-9 match)
> @@ -51,10 +53,13 @@ user-friendly name of the profile is, for instance ~/.guix-profile rather than
>           ((? string? separator)
>            (let ((items (string-tokenize* value separator)))
>              (cons search-path
> -                  (string-join (map (lambda (str)
> -                                      (string-append replacement (crop str)))
> -                                    items)
> -                               separator)))))))))
> +                  (string-join
> +                   (map (lambda (str)
> +                          (string-append replacement (crop str)))
> +                        ;; When APPEND-SEPARATOR? is #t, the trailing
> +                        ;; separator causes an empty string item.  Remove it.
> +                        (remove string-null? items))
> +                   separator)))))))))

If we remove the empty string, dont we lose the trailing separator?

> +++ b/guix/build/utils.scm
> @@ -573,7 +573,8 @@ for under the directories designated by FILES.  For example:
>                                          #:key
>                                          (separator ":")
>                                          (type 'directory)
> -                                        pattern)
> +                                        pattern
> +                                        (append-separator? #f))
>    "Look for each of FILES of the given TYPE (a symbol as returned by
>  'stat:type') in INPUT-DIRS.  Set ENV-VAR to a SEPARATOR-separated path
>  accordingly.  Example:
> @@ -590,11 +591,16 @@ denoting file names to look for under the directories designated by FILES:
>                                   (list docbook-xml docbook-xsl)
>                                   #:type 'regular
>                                   #:pattern \"^catalog\\\\.xml$\")
> -"
> +
> +When both SEPARATOR and APPEND-SEPARATOR? are true, a separator is appended to
> +the value of the environment variable."
>    (let* ((path  (search-path-as-list files input-dirs
>                                       #:type type
>                                       #:pattern pattern))
> -         (value (list->search-path-as-string path separator)))
> +         (value (list->search-path-as-string path separator))
> +         (value (if append-separator?
> +                          (string-append value separator)
> +                          value)))

Indentation is off.

Thats it.  Thanks and apologies for the delay!

Ludo.




This bug report was last modified 4 years ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.