Subject: Re: bug#31088: Use '@' as version number separator in guix output

From: Chris Marusich <[email-masked]>
Date: Tue, 10 Apr 2018 21:41:23 -0700

Hi,

Here's a new version of the patch.  The only change is: I modified the
tests (tests/graph.scm and tests/profiles.scm) so that they also use the
package-full-name procedure's new #:delimiter keyword argument when
necessary.  I intended to do that from the start, but I forgot.

Alex Kost <[email-masked]> writes:

> Chris Marusich (2018-04-09 23:38 -0700) wrote:
>
> [...]
>> Because I changed the package-full-name procedure, this also fixes the
>> package names in other places, such as graphs produced by "guix graph".
>
> I recall there was some discussion about 'package-full-name' in the past
> but I remember nothing about it :-)
>
> Well, maybe it is not relevant at all, but I've found there was a
> proposition to use "@" in 'package-full-name' and it is still not used
> there.  Look at:
>
>   http://lists.gnu.org/archive/html/guix-devel/2016-04/msg01141.html

Yes, we need to be careful.  The most obvious problem I encountered was
that the Guix daemon prohibits "@" in store item names (which I learned
only after attempting to change the delimiter everywhere and then
running some tests).  However, my patch does not put the "@" symbol into
store item names.  In fact, to be extra clear, I've added a comment to
the package->bag procedure to explain why we can't use "@" there.

I think the risk is fairly low that something in Guix itself will
malfunction if we make this change, since I've looked at the call sites
for package-full-name (in the guix Git repo only), I've run "make check"
successfully, and I've verified manually that a handful of guix commands
still function correctly.  But you never really know until you try.  We
should probably still check in some other places before changing the
default delimiter from "-" to "@".  For example, we should probably
check at least the following places:

* Hydra
* Cuirass
* Emacs-Guix

Where else should we check?

If it's too risky, we can always make the default delimiter "-" and only
change it to "@" in those places where we really need to (e.g., the
package->recutils procedure in (guix ui), to fix this specific bug
report).  What do you think?

-- 
Chris
From 67165c95ff9b52b7ae34d7c07778138548013ccd Mon Sep 17 00:00:00 2001
From: Chris Marusich <[email-masked]>
Date: Sun, 8 Apr 2018 16:51:42 -0700
Subject: [PATCH] guix: Separate the package name and version with "@", not
 "-".

* guix/packages.scm (package-full-name): By default, use "@" to separate
  the package name and package version.  Add #:delimiter keyword
  argument so that there is still a way to explicitly use a different
  delimiter.
* gnu/packages/commencement.scm (gcc-boot0) <unpack-gmp&co>: Adjust
  accordingly.
* tests/graph.scm: Adjust accordingly.
* tests/profiles.scm: Adjust accordingly.
* NEWS: Mention the change.

Fixes: <https://bugs.gnu.org/31088>.
Reported by Pierre Neidhardt <[email-masked]>.
---
 NEWS                          |  1 +
 gnu/packages/commencement.scm |  2 +-
 guix/packages.go.134WZR       |  0
 guix/packages.scm             | 13 +++++++++----
 tests/graph.scm               |  3 ++-
 tests/profiles.scm            | 12 ++++++------
 6 files changed, 19 insertions(+), 12 deletions(-)
 create mode 100644 guix/packages.go.134WZR

diff --git a/NEWS b/NEWS
index 2c898e65f..9769bfc87 100644
--- a/NEWS
+++ b/NEWS
@@ -777,6 +777,7 @@ [email-masked]
 
 ** Programming interfaces
 
+*** package-full-name (guix packages) now uses "@" instead of "-" as its delimiter.
 *** New build systems: ‘font’, ‘meson’, ‘minify’, ‘scons’, ‘texlive’
 *** ‘cmake-build-system’ now supports cross-compilation
 *** Various improvements to ‘asdf-build-system’, ‘emacs-build-system’,
diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index fe9fbebcc..9506fbb2b 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -282,7 +282,7 @@
                             ;; Drop trailing letters, as gmp-6.0.0a unpacks
                             ;; into gmp-6.0.0.
                             `(symlink ,(string-trim-right
-                                        (package-full-name lib)
+                                        (package-full-name lib #:delimiter "-")
                                         char-set:letter)
                                       ,(package-name lib)))
                           (list gmp-6.0 mpfr mpc))))
diff --git a/guix/packages.go.134WZR b/guix/packages.go.134WZR
new file mode 100644
index 000000000..e69de29bb
diff --git a/guix/packages.scm b/guix/packages.scm
index b5c0b6044..38e6344bf 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -388,10 +388,11 @@ object."
 (define-condition-type &package-cross-build-system-error &package-error
   package-cross-build-system-error?)
 
-
-(define (package-full-name package)
-  "Return the full name of PACKAGE--i.e., `NAME-VERSION'."
-  (string-append (package-name package) "-" (package-version package)))
+(define* (package-full-name package #:key (delimiter "@"))
+  "Return the full name of PACKAGE--i.e., `NAME@VERSION'.  By specifying
+DELIMITER (a string), you can customize what will appear between the name and
+the version.  By default, DELIMITER is \"@\"."
+  (string-append (package-name package) delimiter (package-version package)))
 
 (define (%standard-patch-inputs)
   (let* ((canonical (module-ref (resolve-interface '(gnu packages base))
@@ -935,6 +936,10 @@ and return it."
               (($ <package> name version source build-system
                             args inputs propagated-inputs native-inputs
                             self-native-input? outputs)
+               ;; Even though we prefer to use "@" to separate the package
+               ;; name from the package version in various user-facing parts
+               ;; of Guix, checkStoreName (in nix/libstore/store-api.cc)
+               ;; prohibits the use of "@", so use "-" instead.
                (or (make-bag build-system (string-append name "-" version)
                              #:system system
                              #:target target
diff --git a/tests/graph.scm b/tests/graph.scm
index 5faa19298..a2941056d 100644
--- a/tests/graph.scm
+++ b/tests/graph.scm
@@ -134,7 +134,8 @@ edges."
                      (map (lambda (destination)
                             (list "p-0.drv"
                                   (string-append
-                                   (package-full-name destination)
+                                   (package-full-name destination
+                                                      #:delimiter "-")
                                    ".drv")))
                           implicit)))))))
 
diff --git a/tests/profiles.scm b/tests/profiles.scm
index 92eb08cb9..218bf5f8a 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -242,8 +242,9 @@
                                         #:hooks '()
                                         #:locales? #t
                                         #:target target)))
-    (define (find-input name)
-      (let ((name (string-append name ".drv")))
+    (define (find-input package)
+      (let ((name (string-append (package-full-name package #:delimiter "-")
+                                 ".drv")))
         (any (lambda (input)
                (let ((input (derivation-input-path input)))
                  (and (string-suffix? name input) input)))
@@ -252,12 +253,11 @@
     ;; The inputs for grep and sed should be cross-build derivations, but that
     ;; for the glibc-utf8-locales should be a native build.
     (return (and (string=? (derivation-system drv) (%current-system))
-                 (string=? (find-input (package-full-name packages:grep))
+                 (string=? (find-input packages:grep)
                            (derivation-file-name grep))
-                 (string=? (find-input (package-full-name packages:sed))
+                 (string=? (find-input packages:sed)
                            (derivation-file-name sed))
-                 (string=? (find-input
-                            (package-full-name packages:glibc-utf8-locales))
+                 (string=? (find-input packages:glibc-utf8-locales)
                            (derivation-file-name locales))))))
 
 (test-assert "package->manifest-entry defaults to \"out\""
-- 
2.17.0