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

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

[Pierre Neidhardt]

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


At the moment it seems that there is no syntactic different between the
name and the version number in the output of Guix.

The other day I tried to extract the version number of package
dependencies with "everything after the last '-'", until I stumbled on
the following:

	> guix package --show=ncdu | recsel -R dependencies
	ncurses-6.0-20170930

Here the version number contains a '-'.  Because of this it makes the
problem undecidable I think.

Would it be possible to separate the package name from the version
number with '@' instead?

--
Pierre Neidhardt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

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

[Chris Marusich]

[-- Attachment #1.1: Type: text/plain, Size: 2655 bytes --]

Hi Pierre and Mathieu,

Thank you for the bug report, Pierre!  Mathieu, can you take a look at
the patch I've attached to this email?  It should fix this bug, but
since you made similar changes in the past, I'd value your feedback.

Pierre Neidhardt <ambrevar@gmail.com> writes:

> Would it be possible to separate the package name from the version
> number with '@' instead?

Pierre's suggestion makes sense to me.  Starting with Mathieu's commit
1b846da8c372bee78851439fd9e72b2499115e5a about one year ago, we use "@"
to separate the package name from the version in commands like "guix
package -i ncdu@1.13" and also in exported scheme procedures like
specification->package (defined in (gnu packages)).  Before that, we
used "-" instead of "@".  When we made the change, it looks like we did
not also update the "dependencies" output from commands like "guix
package --show".

I've attached a patch that fixes the issue.  It applies cleanly to
df154c05dc65025eac2d14ab45699b34c7a1c7c8 (the tip of the master branch
at the time I drafted this email), and "make check" succeeds.  With the
change, the output now looks like this:

  $ ./pre-inst-env guix package --show=ncdu | recsel -p dependencies
  dependencies: ncurses@6.0-20170930

I was not able to run "make check-system" successfully, but there is no
reason to suspect that the attached patch causes any problem, since the
"check-system" target was already failing in the same way at commit
df154c05dc65025eac2d14ab45699b34c7a1c7c8.  I'll open up a separate bug
report for that separate issue.

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'm not sure how many package rebuilds (if any?) this will cause, and
I'd like to hear what Mathieu has to say about my patch, so I'll wait to
push it until after I get feedback from him or another interested
person.  We should probably also regenerate the images in
guix/doc/images (so that the delimiter used in the images is "@" instead
of "-", although I'm not sure yet how to regenerate the source dot
files.  Mathieu, please let me know what you think!

In the meantime, Pierre, you can work around the issue by writing a
Guile script that gets the package version using the Guix API like so:

  scheme@(guile-user)> ,use (gnu packages ncdu)
  scheme@(guile-user)> ,use (guix packages)
  scheme@(guile-user)> (package-version ncdu)
  $1 = "1.13"

If you aren't sure how to get started writing Guile scripts, please
refer to the section "Running Guile Scripts" in the Guile manual.

-- 
Chris


[-- Attachment #1.2: 0001-guix-Separate-the-package-name-and-version-with-not.patch --]
[-- Type: text/x-patch, Size: 6406 bytes --]

From 96160c457c852d88e9a584cb16eb7703b02d8de5 Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
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 <ambrevar@gmail.com>.
---
 NEWS                          |  1 +
 gnu/packages/commencement.scm |  2 +-
 guix/packages.go.134WZR       |  0
 guix/packages.scm             | 13 +++++++++----
 tests/graph.scm               |  4 ++--
 tests/profiles.scm            | 11 ++++++-----
 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 @@ zynaddsubfx@3.0.2
 
 ** 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..34bc55d2a 100644
--- a/tests/graph.scm
+++ b/tests/graph.scm
@@ -134,8 +134,8 @@ edges."
                      (map (lambda (destination)
                             (list "p-0.drv"
                                   (string-append
-                                   (package-full-name destination)
-                                   ".drv")))
+                                   (package-name destination) "-"
+                                   (package-version destination) ".drv")))
                           implicit)))))))
 
 (test-assert "bag DAG"                            ;a big town in Iraq
diff --git a/tests/profiles.scm b/tests/profiles.scm
index 92eb08cb9..dd0e19967 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-name package) "-"
+                                 (package-version package) ".drv")))
         (any (lambda (input)
                (let ((input (derivation-input-path input)))
                  (and (string-suffix? name input) input)))
@@ -252,12 +253,12 @@
     ;; 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))
+                            packages:glibc-utf8-locales)
                            (derivation-file-name locales))))))
 
 (test-assert "package->manifest-entry defaults to \"out\""
-- 
2.17.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

[Pierre Neidhardt]

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


Thank you very much for the hard work!

-- 
Pierre Neidhardt

We totally deny the allegations, and we're trying to identify the allegators.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

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

[Chris Marusich]

[-- Attachment #1.1: Type: text/plain, Size: 2194 bytes --]

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 <alezost@gmail.com> 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

[-- Attachment #1.2: 0001-guix-Separate-the-package-name-and-version-with-not.patch --]
[-- Type: text/x-patch, Size: 6332 bytes --]

From 67165c95ff9b52b7ae34d7c07778138548013ccd Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
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 <ambrevar@gmail.com>.
---
 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 @@ zynaddsubfx@3.0.2
 
 ** 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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

[Ludovic Courtès]

Hello Chris,

Thanks for digging into this.

Chris Marusich <cmmarusich@gmail.com> skribis:

> From 67165c95ff9b52b7ae34d7c07778138548013ccd Mon Sep 17 00:00:00 2001
> From: Chris Marusich <cmmarusich@gmail.com>
> 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 <ambrevar@gmail.com>.

[...]

> diff --git a/NEWS b/NEWS
> index 2c898e65f..9769bfc87 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -777,6 +777,7 @@ zynaddsubfx@3.0.2
>  
>  ** Programming interfaces
>  
> +*** package-full-name (guix packages) now uses "@" instead of "-" as its delimiter.

This is under “Changes in 0.14.0 (since 0.13.0)”, which is probably not
what you want.  :-)

> +(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 \"@\"."

I’d prefer #:optional instead of #:key, it’d be less verbose.

Other than that, please make sure “make check” passes, and make sure
this doesn’t trigger a full rebuild (from the patch it looks you already
checked both of these, but who knows ;-)).

OK with this.

Thank you!

Ludo’.

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

[Chris Marusich]

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

Hi everyone!

I've committed this as ede121de426f9c56820852888a0b370f0ccbce49 on the
master branch.  If anything breaks on Hydra or elsewhere, please don't
hesitate to revert it.

ludo@gnu.org (Ludovic Courtès) writes:

>> diff --git a/NEWS b/NEWS
>> index 2c898e65f..9769bfc87 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -777,6 +777,7 @@ zynaddsubfx@3.0.2
>>  
>>  ** Programming interfaces
>>  
>> +*** package-full-name (guix packages) now uses "@" instead of "-" as its delimiter.
>
> This is under “Changes in 0.14.0 (since 0.13.0)”, which is probably not
> what you want.  :-)

Ah!  Good catch.  I've added a new section for 0.15.0 - which I'm sure
will be released soon!  ;-)

>> +(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 \"@\"."
>
> I’d prefer #:optional instead of #:key, it’d be less verbose.

Sounds good.  I've implemented your suggestion.

> Other than that, please make sure “make check” passes, and make sure
> this doesn’t trigger a full rebuild (from the patch it looks you already
> checked both of these, but who knows ;-)).

I've confirmed the following:

1) It builds, and "make check" passes [1].

2) It does not trigger a full rebuild.  I tested this by first invoking
"./pre-inst-env guix system build gnu/system/examples/desktop.tmpl"
before and after the change.

3) I reconfigured my own system a few days ago, too, using this change.
So far, I have observed no problems.

> OK with this.

Thank you for the review!  And thank you, Pierre, for reporting the bug!

Footnotes: 
[1]  tests/guix-system.sh fails, but it was already failing on the
master branch.  I will open a bug report for that if there is not
already one open.

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

[Ludovic Courtès]

Hey Chris,

Chris Marusich <cmmarusich@gmail.com> skribis:

> I've committed this as ede121de426f9c56820852888a0b370f0ccbce49 on the
> master branch.  If anything breaks on Hydra or elsewhere, please don't
> hesitate to revert it.

Awesome, thank you!

Ludo’.