bug#26264: [PATCH 0/1] Use '@' to separate name, version in package-full-name

[Alex Sassmannshausen <alex.sassmannshausen@gmail.com>]

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

Heya,

Following on from Tobias patch I will also submit my revised patch.  I'm
afraid I've been sitting on it for a while.

My patch takes the approach of refactoring ‘package-full-name‘ to accept
an optional argument, ‘separator’, which defaults to ’@’ as proposed by
Ludo.

On the bright side, it seems both me and Tobias are touching the same
files in our patches, and presumably have done some testing — so either
patch should work nicely.

I'm happy for either patch to be pushed.

Best wishes,

Alex


[-- Attachment #2: Patch --]
[-- Type: text/plain, Size: 3902 bytes --]

From 44c356f50058a7d0c60523c1411150847ac66578 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex@pompo.co>
Date: Mon, 10 Apr 2017 14:38:02 +0200
Subject: [PATCH] Use '@' as default separator in `package-full-name`.

* guix/packages.scm (package-full-name): New optional argument, 'separator'
  defaulting to '@'.
* gnu/packages/commencement.scm (gcc-boot0): Set use of '-' as
  `package-full-name` separator.
* tests/graph.scm: Update tests to account for new `package-full-name`.
* tests/profiles.scm: Update tests to account for new `package-full-name`.
---
 gnu/packages/commencement.scm | 2 +-
 guix/packages.scm             | 7 ++++---
 tests/graph.scm               | 2 +-
 tests/profiles.scm            | 7 ++++---
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 1b41feac1..552358d67 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -256,7 +256,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 "-")
                                         char-set:letter)
                                       ,(package-name lib)))
                           (list gmp-6.0 mpfr mpc))))
diff --git a/guix/packages.scm b/guix/packages.scm
index 8ead95073..92b3f034c 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -385,9 +385,10 @@ object."
   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 #:optional (separator "@"))
+  "Return the full name of PACKAGE, separated by SEPARATOR--i.e.,
+`NAME@VERSION' by default."
+  (string-append (package-name package) separator (package-version package)))
 
 (define (%standard-patch-inputs)
   (let* ((canonical (module-ref (resolve-interface '(gnu packages base))
diff --git a/tests/graph.scm b/tests/graph.scm
index 6431c482f..a48fbfd51 100644
--- a/tests/graph.scm
+++ b/tests/graph.scm
@@ -130,7 +130,7 @@ edges."
                      (map (lambda (destination)
                             (list "p-0.drv"
                                   (string-append
-                                   (package-full-name destination)
+                                   (package-full-name destination "-")
                                    ".drv")))
                           implicit)))))))
 
diff --git a/tests/profiles.scm b/tests/profiles.scm
index 093422792..ea2111355 100644
--- a/tests/profiles.scm
+++ b/tests/profiles.scm
@@ -239,12 +239,13 @@
     ;; 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 (package-full-name packages:grep "-"))
                            (derivation-file-name grep))
-                 (string=? (find-input (package-full-name packages:sed))
+                 (string=? (find-input (package-full-name packages:sed "-"))
                            (derivation-file-name sed))
                  (string=? (find-input
-                            (package-full-name packages:glibc-utf8-locales))
+                            (package-full-name packages:glibc-utf8-locales
+                                               "-"))
                            (derivation-file-name locales))))))
 
 (test-assert "package->manifest-entry defaults to \"out\""
-- 
2.12.2


[-- Attachment #3: Type: text/plain, Size: 6002 bytes --]


Tobias Geerinckx-Rice writes:

> Guix,
>
> On 28/03/17 17:19, Tobias Geerinckx-Rice wrote:
>> I wouldn't mind giving this a try, or not.
>
> More revenge from the pre-new-job-mailbag.
>
> I'm sticking to my original patch, and changed the problematic callers
> to not use ‘package-full-name’ at all. I don't think it's the right
> abstraction in any of those cases. See the overly verbose commit messages.
>
> Kind regards,
>
> T G-R
>
> From 43892525fc981533445e60a649425791cc315d0a Mon Sep 17 00:00:00 2001
> From: Tobias Geerinckx-Rice <me@tobias.gr>
> Date: Sat, 6 May 2017 14:32:12 +0200
> Subject: [PATCH 4/4] packages: Use "@" as a version separator.
>
> * guix/packages.scm (package-full-name): Use "@" instead of "-" to separate
> PACKAGE-NAME and PACKAGE-VERSION.
> ---
>  guix/packages.scm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/guix/packages.scm b/guix/packages.scm
> index 44f2c32fb..802405102 100644
> --- a/guix/packages.scm
> +++ b/guix/packages.scm
> @@ -385,8 +385,8 @@ object."
>  
>  
>  (define (package-full-name package)
> -  "Return the full name of PACKAGE--i.e., `NAME-VERSION'."
> -  (string-append (package-name package) "-" (package-version package)))
> +  "Return the full name of PACKAGE--i.e., `NAME@VERSION'."
> +  (string-append (package-name package) "@" (package-version package)))
>  
>  (define (%standard-patch-inputs)
>    (let* ((canonical (module-ref (resolve-interface '(gnu packages base))
> -- 
> 2.12.2
>
>
> From 0325e536cf557ff48d885948bf5fab8f59bfc444 Mon Sep 17 00:00:00 2001
> From: Tobias Geerinckx-Rice <me@tobias.gr>
> Date: Sat, 6 May 2017 14:32:06 +0200
> Subject: [PATCH 3/4] profiles: Don't use PACKAGE-FULL-NAME.
>
> The non-hash parts of store paths aren't constructed according to
> PACKAGE-FULL-NAME rules.  They just happened to match in the past.
>
> * tests/profile.scm ("profile-derivation, cross-compilation"): Use
> PACKAGE-NAME and PACKAGE-VERSION directly.
> ---
>  tests/profiles.scm | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tests/profiles.scm b/tests/profiles.scm
> index d0b1e14a8..68e73c4ca 100644
> --- a/tests/profiles.scm
> +++ b/tests/profiles.scm
> @@ -230,15 +230,18 @@
>                   (and (string-suffix? name input) input)))
>               (derivation-inputs drv))))
>  
> +    (define (package-store-suffix package)
> +      (string-append (package-name package) "-" (package-version package)))
> +
>      ;; 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 (package-store-suffix packages:grep))
>                             (derivation-file-name grep))
> -                 (string=? (find-input (package-full-name packages:sed))
> +                 (string=? (find-input (package-store-suffix packages:sed))
>                             (derivation-file-name sed))
>                   (string=? (find-input
> -                            (package-full-name packages:glibc-utf8-locales))
> +                            (package-store-suffix packages:glibc-utf8-locales))
>                             (derivation-file-name locales))))))
>  
>  (test-assert "package->manifest-entry defaults to \"out\""
> -- 
> 2.12.2
>
>
> From f10c4fb9d269b85f9c388356a17c2b8b2fc54bd5 Mon Sep 17 00:00:00 2001
> From: Tobias Geerinckx-Rice <me@tobias.gr>
> Date: Sat, 6 May 2017 14:31:56 +0200
> Subject: [PATCH 2/4] gnu: gcc-boot0: Don't use PACKAGE-FULL-NAME.
>
> Don't use Guix's naming conventions where a different one is expected.
>
> * gnu/packages/commencement.scm (gcc-boot0)[arguments]: Use PACKAGE-NAME
> and PACKAGE-VERSION directly.
> ---
>  gnu/packages/commencement.scm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
> index 92f6e6c2e..dd3261e37 100644
> --- a/gnu/packages/commencement.scm
> +++ b/gnu/packages/commencement.scm
> @@ -255,7 +255,8 @@
>                              ;; Drop trailing letters, as gmp-6.0.0a unpacks
>                              ;; into gmp-6.0.0.
>                              `(symlink ,(string-trim-right
> -                                        (package-full-name lib)
> +                                        (string-append (package-name lib) "-"
> +                                                       (package-version lib))
>                                          char-set:letter)
>                                        ,(package-name lib)))
>                            (list gmp-6.0 mpfr mpc))))
> -- 
> 2.12.2
>
>
> From c090e526e21a960f34f0f02f9904757952d5a36e Mon Sep 17 00:00:00 2001
> From: Tobias Geerinckx-Rice <me@tobias.gr>
> Date: Sat, 6 May 2017 14:31:48 +0200
> Subject: [PATCH 1/4] graph: Don't use PACKAGE-FULL-NAME.
>
> Derivation files aren't named according to PACKAGE-FULL-NAME rules.
> We already forfeit any supposed abstraction by manually adding ".drv".
>
> * tests/graph.scm ("bag-emerged DAG"): Use PACKAGE-NAME and PACKAGE-VERSION
> directly.
> ---
>  tests/graph.scm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tests/graph.scm b/tests/graph.scm
> index 6431c482f..53475597a 100644
> --- a/tests/graph.scm
> +++ b/tests/graph.scm
> @@ -130,7 +130,9 @@ edges."
>                       (map (lambda (destination)
>                              (list "p-0.drv"
>                                    (string-append
> -                                   (package-full-name destination)
> +                                   (package-name destination)
> +                                   "-"
> +                                   (package-version destination)
>                                     ".drv")))
>                            implicit)))))))