bug#48225: Wrong result of package-name->name+version

bug#48225: Wrong result of package-name->name+version

[Guillaume Le Vaillant]

Hi,

The 'package-name->name+version' function defined in
"guix/build/utils.scm" returns a wrong result if there is a '-'
followed by a number in the package name:

--8<---------------cut here---------------start------------->8---
(use-modules ((guix build utils)))
(package-name->name+version "sbcl-3d-vectors-3.1.0-1.29bb968")

$1 = "sbcl"
$2 = "3d-vectors-3.1.0-1.29bb968"
--8<---------------cut here---------------end--------------->8---

It should be:

--8<---------------cut here---------------start------------->8---
$1 = "sbcl-3d-vectors"
$2 = "3.1.0-1.29bb968"
--8<---------------cut here---------------end--------------->8---

Can someone think of an elegant modification for
'package-name->name+version' so that it finds where the version is
even if there are several hyphens before of after it (as in
"sbcl-3d-vectors-3.1.0-1.29bb968" or "nyxt-2-pre-release-6")?

This is related to issue #48208, and also probably to issue #41437.

bug#48225: Wrong result of package-name->name+version

[Leo Prikler]

Am Dienstag, den 04.05.2021, 13:35 +0000 schrieb Guillaume Le Vaillant:
> Hi,
> 
> The 'package-name->name+version' function defined in
> "guix/build/utils.scm" returns a wrong result if there is a '-'
> followed by a number in the package name:
> 
> --8<---------------cut here---------------start------------->8---
> (use-modules ((guix build utils)))
> (package-name->name+version "sbcl-3d-vectors-3.1.0-1.29bb968")
> 
> $1 = "sbcl"
> $2 = "3d-vectors-3.1.0-1.29bb968"
> --8<---------------cut here---------------end--------------->8---
> 
> It should be:
> 
> --8<---------------cut here---------------start------------->8---
> $1 = "sbcl-3d-vectors"
> $2 = "3.1.0-1.29bb968"
> --8<---------------cut here---------------end--------------->8---
> 
> Can someone think of an elegant modification for
> 'package-name->name+version' so that it finds where the version is
> even if there are several hyphens before of after it (as in
> "sbcl-3d-vectors-3.1.0-1.29bb968" or "nyxt-2-pre-release-6")?
> 
> This is related to issue #48208, and also probably to issue #41437.
I don't think there's any way to cleverly match this.  For all we know,
3d could be a version, we have 2019c, 2021a, 1a, 9d, 9100h and 063a
already.  Perhaps we should forward name and version as keyword
arguments, so that we don't have to reconstruct them, or alternatively
use a different delimiter (e.g. @)

I'm honestly surprised, that many other stuff "works fine" despite the
fact, that they'd probably also suffer from this bug.  Can anyone tell
me why emacs-2048-game builds?

bug#48225: Wrong result of package-name->name+version

[Ludovic Courtès]

Hi Guillaume,

Guillaume Le Vaillant <glv@posteo.net> skribis:

> The 'package-name->name+version' function defined in
> "guix/build/utils.scm" returns a wrong result if there is a '-'
> followed by a number in the package name:
>
> (use-modules ((guix build utils)))
> (package-name->name+version "sbcl-3d-vectors-3.1.0-1.29bb968")
>
> $1 = "sbcl"
> $2 = "3d-vectors-3.1.0-1.29bb968"
>
>
> It should be:
>
> $1 = "sbcl-3d-vectors"
> $2 = "3.1.0-1.29bb968"
>
> Can someone think of an elegant modification for
> 'package-name->name+version' so that it finds where the version is
> even if there are several hyphens before of after it (as in
> "sbcl-3d-vectors-3.1.0-1.29bb968" or "nyxt-2-pre-release-6")?

It’s implements a heuristic meant to work for most packages.  It’s hard
to tweak that without breaking something else instead.  (Plus,
“nyxt-2-pre-release-6” looks really bogus to me.)

A better fix would be to not guess and instead pass #:name and #:version
to all the build phases, with the value taken from the <package> object.
(That’s a world-rebuild fix though.)

WDYT?

> This is related to issue #48208, and also probably to issue #41437.

Perhaps we can find a workaround for these?

Thanks,
Ludo’.

bug#48225: Simple workaround

[Sharlatan Hellseher]

Hi,

If chaining  `package-name->name+version` function  may affect a large
layer of infrastructure here is could a quick adhoc workaround:

sbcl-3d-vectors -> sbcl-cl3d-vectors
sbcl-3d-vectors -> sbcl-three-d-vectors
sbcl-3d-vectors -> sbcl-iiid-vectors

Or use any predictable common prefix which could be use and replaced
after the function is reviewed.
-- 
… наш разум - превосходная объяснительная машина которая способна
найти смысл почти в чем угодно, истолковать любой феномен, но
совершенно не в состоянии принять мысль о непредсказуемости.

bug#48225: Wrong result of package-name->name+version

[Guillaume Le Vaillant]

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

Ludovic Courtès <ludo@gnu.org> skribis:

> [...]
> A better fix would be to not guess and instead pass #:name and #:version
> to all the build phases, with the value taken from the <package> object.
> (That’s a world-rebuild fix though.)
>
> WDYT?
>
>> This is related to issue #48208, and also probably to issue #41437.
>
> Perhaps we can find a workaround for these?

Sharlatan Hellseher <sharlatanus@gmail.com> skribis:

> If chaining  `package-name->name+version` function  may affect a large
> layer of infrastructure here is could a quick adhoc workaround:
>
> sbcl-3d-vectors -> sbcl-cl3d-vectors
> sbcl-3d-vectors -> sbcl-three-d-vectors
> sbcl-3d-vectors -> sbcl-iiid-vectors
>
> Or use any predictable common prefix which could be use and replaced
> after the function is reviewed.

I agree that having '#:name' and '#:version' available in the build
phases would be ideal.
Meanwhile I found a workaround for the asdf-build-system that fixes
bug#41437 and allows building the 'sbcl-3d-vectors' and
'sbcl-3d-matrices' packages of bug#48208 without having to mangle the
package names. It rebuilds all the Common Lisp packages, but that
doesn't take too much time.
WDYT?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-build-system-asdf-Work-around-package-name-name-vers.patch --]
[-- Type: text/x-patch, Size: 3190 bytes --]

From 1e37a89b943a818b5274c1d5f31143ca48bad40a Mon Sep 17 00:00:00 2001
From: Guillaume Le Vaillant <glv@posteo.net>
Date: Thu, 6 May 2021 10:32:56 +0200
Subject: [PATCH] build-system: asdf: Work around package-name->name+version
 bug.

This patch modifies how the name of the main Common Lisp system is extracted
from the full Guix package name to work around bug#48225 concerning the
'package-name->name+version' function.

Fixes <https://issues.guix.gnu.org/41437>.

* guix/build-system/asdf.scm (asdf-build): Fix 'systems' function.
* guix/build/asdf-build-system.scm (main-system-name): Fix it.
---
 guix/build-system/asdf.scm       | 15 +++++++--------
 guix/build/asdf-build-system.scm | 12 ++++++------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/guix/build-system/asdf.scm b/guix/build-system/asdf.scm
index 28403a1960..8f9d63337f 100644
--- a/guix/build-system/asdf.scm
+++ b/guix/build-system/asdf.scm
@@ -291,16 +291,15 @@ set up using CL source package conventions."
                   (imported-modules %asdf-build-system-modules)
                   (modules %asdf-build-modules))
 
-    ;; FIXME: The definition of 'systems' is pretty hacky.
-    ;; Is there a more elegant way to do it?
     (define systems
       (if (null? (cadr asd-systems))
-          `(quote
-            ,(list
-              (string-drop
-               ;; NAME is the value returned from `package-full-name'.
-               (hyphen-separated-name->name+version name)
-               (1+ (string-length lisp-type))))) ; drop the "<lisp>-" prefix.
+          (let* ((lisp-prefix (string-append lisp-type "-"))
+                 (package-name (hyphen-separated-name->name+version
+                                (if (string-prefix? lisp-prefix name)
+                                    (string-drop name
+                                                 (string-length lisp-prefix))
+                                    name))))
+            `(quote ,(list package-name)))
           asd-systems))
 
     (define builder
diff --git a/guix/build/asdf-build-system.scm b/guix/build/asdf-build-system.scm
index 6ad855cab2..5a4fc44aef 100644
--- a/guix/build/asdf-build-system.scm
+++ b/guix/build/asdf-build-system.scm
@@ -52,12 +52,12 @@
   (string-append %source-install-prefix "/systems"))
 
 (define (main-system-name output)
-  (let ((package-name (package-name->name+version
-                       (strip-store-file-name output)))
-        (lisp-prefix (string-append (%lisp-type) "-")))
-    (if (string-prefix? lisp-prefix package-name)
-        (string-drop package-name (string-length lisp-prefix))
-        package-name)))
+  (let* ((full-name (strip-store-file-name output))
+         (lisp-prefix (string-append (%lisp-type) "-"))
+         (package-name (if (string-prefix? lisp-prefix full-name)
+                           (string-drop full-name (string-length lisp-prefix))
+                           full-name)))
+    (package-name->name+version package-name)))
 
 (define (lisp-source-directory output name)
   (string-append output (%lisp-source-install-prefix) "/" name))
-- 
2.31.1


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

bug#48225: Wrong result of package-name->name+version

[Ludovic Courtès]

Hi Guillaume,

Guillaume Le Vaillant <glv@posteo.net> skribis:

> From 1e37a89b943a818b5274c1d5f31143ca48bad40a Mon Sep 17 00:00:00 2001
> From: Guillaume Le Vaillant <glv@posteo.net>
> Date: Thu, 6 May 2021 10:32:56 +0200
> Subject: [PATCH] build-system: asdf: Work around package-name->name+version
>  bug.
>
> This patch modifies how the name of the main Common Lisp system is extracted
> from the full Guix package name to work around bug#48225 concerning the
> 'package-name->name+version' function.
>
> Fixes <https://issues.guix.gnu.org/41437>.
>
> * guix/build-system/asdf.scm (asdf-build): Fix 'systems' function.
> * guix/build/asdf-build-system.scm (main-system-name): Fix it.

If it works for you, sounds good to me.  Please do rebuild as many CL
packages, with different CL implementations, to make sure we do not
overlook any corner case.

> +          (let* ((lisp-prefix (string-append lisp-type "-"))
> +                 (package-name (hyphen-separated-name->name+version
> +                                (if (string-prefix? lisp-prefix name)
> +                                    (string-drop name
> +                                                 (string-length lisp-prefix))
> +                                    name))))
> +            `(quote ,(list package-name)))

I’d like to see a FIXME in there: this is all guesswork and we should
eventually replace guesses with known-good info.

What would it take to pass the right package name and implementation
name upfront from the package?

Thanks,
Ludo’.

bug#48225: Wrong result of package-name->name+version

[Guillaume Le Vaillant]

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

Ludovic Courtès <ludo@gnu.org> skribis:

> Hi Guillaume,
>
> Guillaume Le Vaillant <glv@posteo.net> skribis:
>
>> From 1e37a89b943a818b5274c1d5f31143ca48bad40a Mon Sep 17 00:00:00 2001
>> From: Guillaume Le Vaillant <glv@posteo.net>
>> Date: Thu, 6 May 2021 10:32:56 +0200
>> Subject: [PATCH] build-system: asdf: Work around package-name->name+version
>>  bug.
>>
>> This patch modifies how the name of the main Common Lisp system is extracted
>> from the full Guix package name to work around bug#48225 concerning the
>> 'package-name->name+version' function.
>>
>> Fixes <https://issues.guix.gnu.org/41437>.
>>
>> * guix/build-system/asdf.scm (asdf-build): Fix 'systems' function.
>> * guix/build/asdf-build-system.scm (main-system-name): Fix it.
>
> If it works for you, sounds good to me.  Please do rebuild as many CL
> packages, with different CL implementations, to make sure we do not
> overlook any corner case.
>
>> +          (let* ((lisp-prefix (string-append lisp-type "-"))
>> +                 (package-name (hyphen-separated-name->name+version
>> +                                (if (string-prefix? lisp-prefix name)
>> +                                    (string-drop name
>> +                                                 (string-length lisp-prefix))
>> +                                    name))))
>> +            `(quote ,(list package-name)))
>
> I’d like to see a FIXME in there: this is all guesswork and we should
> eventually replace guesses with known-good info.
>
> What would it take to pass the right package name and implementation
> name upfront from the package?
>
> Thanks,
> Ludo’.

I tried rebuilding all the sbcl-*, cl-* and ecl-* packages, as well as
stumpwm, uglify-js and nyxt, and I didn't see new failures.
I pushed the patch as 2fa8fd4af59af0de392352915fa50fc21a4cf98a.

When 'package-name->name+version' returns a bad result leading to an
incorrect default Lisp system name being computed, it can be overridden
using the '#:asd-systems' parameter of 'asdf-build-system', which should
work around the problem in almost all cases.

However I suppose other build systems could have issues if they make use
of 'package-name->name+version' on a package with a name like
"abc-123-def-1.2.3" (depending how they use the result).

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