bug#59913: [tentative PATCH] Failure to guix pull on aarch64 since recent make-linux-libre*

bug#59913: [tentative PATCH] Failure to guix pull on aarch64 since recent make-linux-libre*

[Pierre Langlois]

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

Hi Guix!

I've been getting errors while running `guix pull' on an aarch64 system,
during the final guix-package-cache step:

--8<---------------cut here---------------start------------->8---
(repl-version 0 1 1)
Generating package cache for '/gnu/store/m8in1imi93snq711d7568dj9hlrx4diz-profile'...

Backtrace:
In ice-9/boot-9.scm:
  1747:15 19 (with-exception-handler #<procedure af1570 at ice-9/bo?> ?)
  1752:10 18 (with-exception-handler _ _ #:unwind? _ # _)
In guix/repl.scm:
    99:21 17 (_)
In unknown file:
          16 (_ #<procedure 82fd00 at guix/repl.scm:100:25 ()> #<pr?> ?)
          15 (primitive-load "/gnu/store/3x6g541ixbmdjav4ky6dp1ryj4l?")
In ice-9/boot-9.scm:
  1752:10 14 (with-exception-handler _ _ #:unwind? _ # _)
In gnu/packages.scm:
   438:11 13 (generate-package-cache _)
In srfi/srfi-1.scm:
   460:18 12 (fold #<procedure expand-cache expr> _ _)
In gnu/packages.scm:
    390:9 11 (expand-cache . _)
In guix/packages.scm:
  1317:17 10 (supported-package? #<package linux-libre@4.14.300 gnu?> ?)
In guix/memoization.scm:
    101:0  9 (_ #<hash-table 31605e0 13974/28099> #<package linux-l?> ?)
In guix/packages.scm:
  1295:37  8 (_)
  1555:16  7 (package->bag _ _ _ #:graft? _)
  1660:43  6 (thunk)
In gnu/packages/linux.scm:
   986:37  5 (arguments #<package linux-libre@4.14.300 gnu/packages/?>)
In guix/gexp.scm:
   460:52  4 (%local-file #f #<promise #<procedure 4df2660 at gnu/p?> ?)
In unknown file:
           3 (basename #f #<undefined>)
In ice-9/boot-9.scm:
  1685:16  2 (raise-exception _ #:continuable? _)
  1780:13  1 (_ #<&compound-exception components: (#<&assertion-fail?>)
In unknown file:
           0 (backtrace #<undefined>)

(exception wrong-type-arg (value "scm_to_utf8_stringn") (value "Wrong type argument in position ~A (expecting ~A): ~S") (value (1 "string" #f)) (value (#f)))
--8<---------------cut here---------------end--------------->8---

I was able to decipher the backtrace to *maybe* put together a fix, but
I'm unsure why the problem started. My best guess is that it started
with commit dfc6957a5af7d179d4618eb19d4f555c519bc6f2, even though I
can't find where the issue actually is, it looks fine to me!

What seems to happen is that the `kernel-config' function now receive an
`arch' argument for an architecture that isn't actually supported by
that kernel, as is the case for linux-libre@4.14.300.  And, correctly,
the function should not expect to ever get such arch value to begin
with, so we get a `(local-file #f)'.

--8<---------------cut here---------------start------------->8---
(define* (kernel-config arch #:key variant)
  "Return a file-like object of the Linux-Libre build configuration file for
ARCH and optionally VARIANT, or #f if there is no such configuration."
  (let* ((name (string-append (if variant (string-append variant "-") "")
                              (if (string=? "i386" arch) "i686" arch) ".conf"))
         (file (string-append "linux-libre/" name)))
    (local-file (search-auxiliary-file file))))
--8<---------------cut here---------------end--------------->8---

I think it's fair for that function expect the arch to be valid (why
would you ask the config for an unsupported arch?).

I think it should be possible to fix this by checking the arch is
supported at the call site:


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-make-linux-libre-Do-not-get-config-for-unsupport.patch --]
[-- Type: text/x-patch, Size: 1168 bytes --]

From 77829140f14928e30cbe4e53c625be3ba2f5895f Mon Sep 17 00:00:00 2001
From: Pierre Langlois <pierre.langlois@gmx.com>
Date: Thu, 8 Dec 2022 23:41:40 +0000
Subject: [PATCH] gnu: make-linux-libre*: Do not get config for unsupported
 systems.

* gnu/packages/linux.scm (make-linux-libre*)[phases] <configure>: Check
arch is in supported-systems before calling configuration-file.
---
 gnu/packages/linux.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 5ae6366593..87fc9fe94c 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -983,6 +983,7 @@ (define* (make-linux-libre* version gnu-revision source supported-systems
                                             (or (%current-target-system)
                                                 (%current-system))))))
                                 (and configuration-file arch
+                                     (member arch supported-systems)
                                      (configuration-file
                                       arch
                                       #:variant (version-major+minor version))))
-- 
2.38.1


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


But I'm not quite sure why this is happening, some quirk from moving
things over gexps?

I'm currently trying this fix to make sure it does solve the problem
(guix pull takes so long without substitutes :-) ). Will report back in
5-10 minutes.

Thanks!
Pierre

bug#59913: [tentative PATCH] Failure to guix pull on aarch64 since recent make-linux-libre*

[Pierre Langlois]

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


Pierre Langlois <pierre.langlois@gmx.com> writes:

(snip)

> What seems to happen is that the `kernel-config' function now receive an
> `arch' argument for an architecture that isn't actually supported by
> that kernel, as is the case for linux-libre@4.14.300.  And, correctly,
> the function should not expect to ever get such arch value to begin
> with, so we get a `(local-file #f)'.
>
> (define* (kernel-config arch #:key variant)
>   "Return a file-like object of the Linux-Libre build configuration file for
> ARCH and optionally VARIANT, or #f if there is no such configuration."

I pasted this bit of code but didn't read it properly, it specificaly
says that it should return #f if the config doesn't exist. I guess
that's now a bug? We could either fix the docstring and apply the fix I
proposed, or returning #f probably works.

>   (let* ((name (string-append (if variant (string-append variant "-") "")
>                               (if (string=? "i386" arch) "i686" arch) ".conf"))
>          (file (string-append "linux-libre/" name)))
>     (local-file (search-auxiliary-file file))))
>
> I think it's fair for that function expect the arch to be valid (why
> would you ask the config for an unsupported arch?).
>
> I think it should be possible to fix this by checking the arch is
> supported at the call site:
>
> [[End of PGP Signed Part]]
> From 77829140f14928e30cbe4e53c625be3ba2f5895f Mon Sep 17 00:00:00 2001
> From: Pierre Langlois <pierre.langlois@gmx.com>
> Date: Thu, 8 Dec 2022 23:41:40 +0000
> Subject: [PATCH] gnu: make-linux-libre*: Do not get config for unsupported
>  systems.
>
> * gnu/packages/linux.scm (make-linux-libre*)[phases] <configure>: Check
> arch is in supported-systems before calling configuration-file.
> ---
>  gnu/packages/linux.scm | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 5ae6366593..87fc9fe94c 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -983,6 +983,7 @@ (define* (make-linux-libre* version gnu-revision source supported-systems
>                                              (or (%current-target-system)
>                                                  (%current-system))))))
>                                  (and configuration-file arch
> +                                     (member arch supported-systems)
>                                       (configuration-file
>                                        arch
>                                        #:variant (version-major+minor version))))


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

bug#59913: [tentative PATCH] Failure to guix pull on aarch64 since recent make-linux-libre*

[Pierre Langlois]

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


Pierre Langlois <pierre.langlois@gmx.com> writes:

(snip)

> I'm currently trying this fix to make sure it does solve the problem
> (guix pull takes so long without substitutes :-) ). Will report back in
> 5-10 minutes.

Yeey the fixed worked!

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

bug#59913: [tentative PATCH] Failure to guix pull on aarch64 since recent make-linux-libre*

[Maxim Cournoyer]

Hi Pierre,

Pierre Langlois <pierre.langlois@gmx.com> writes:

> Hi Guix!
>
> I've been getting errors while running `guix pull' on an aarch64 system,
> during the final guix-package-cache step:
>
> (repl-version 0 1 1)
> Generating package cache for '/gnu/store/m8in1imi93snq711d7568dj9hlrx4diz-profile'...
>
> Backtrace:
> In ice-9/boot-9.scm:
>   1747:15 19 (with-exception-handler #<procedure af1570 at ice-9/bo?> ?)
>   1752:10 18 (with-exception-handler _ _ #:unwind? _ # _)
> In guix/repl.scm:
>     99:21 17 (_)
> In unknown file:
>           16 (_ #<procedure 82fd00 at guix/repl.scm:100:25 ()> #<pr?> ?)
>           15 (primitive-load "/gnu/store/3x6g541ixbmdjav4ky6dp1ryj4l?")
> In ice-9/boot-9.scm:
>   1752:10 14 (with-exception-handler _ _ #:unwind? _ # _)
> In gnu/packages.scm:
>    438:11 13 (generate-package-cache _)
> In srfi/srfi-1.scm:
>    460:18 12 (fold #<procedure expand-cache expr> _ _)
> In gnu/packages.scm:
>     390:9 11 (expand-cache . _)
> In guix/packages.scm:
>   1317:17 10 (supported-package? #<package linux-libre@4.14.300 gnu?> ?)
> In guix/memoization.scm:
>     101:0  9 (_ #<hash-table 31605e0 13974/28099> #<package linux-l?> ?)
> In guix/packages.scm:
>   1295:37  8 (_)
>   1555:16  7 (package->bag _ _ _ #:graft? _)
>   1660:43  6 (thunk)
> In gnu/packages/linux.scm:
>    986:37  5 (arguments #<package linux-libre@4.14.300 gnu/packages/?>)
> In guix/gexp.scm:
>    460:52  4 (%local-file #f #<promise #<procedure 4df2660 at gnu/p?> ?)
> In unknown file:
>            3 (basename #f #<undefined>)
> In ice-9/boot-9.scm:
>   1685:16  2 (raise-exception _ #:continuable? _)
>   1780:13  1 (_ #<&compound-exception components: (#<&assertion-fail?>)
> In unknown file:
>            0 (backtrace #<undefined>)
>
> (exception wrong-type-arg (value "scm_to_utf8_stringn") (value "Wrong type argument in position ~A (expecting ~A): ~S") (value (1 "string" #f)) (value (#f)))
>
>
> I was able to decipher the backtrace to *maybe* put together a fix, but
> I'm unsure why the problem started. My best guess is that it started
> with commit dfc6957a5af7d179d4618eb19d4f555c519bc6f2, even though I
> can't find where the issue actually is, it looks fine to me!
>
> What seems to happen is that the `kernel-config' function now receive an
> `arch' argument for an architecture that isn't actually supported by
> that kernel, as is the case for linux-libre@4.14.300.  And, correctly,
> the function should not expect to ever get such arch value to begin
> with, so we get a `(local-file #f)'.
>
> (define* (kernel-config arch #:key variant)
>   "Return a file-like object of the Linux-Libre build configuration file for
> ARCH and optionally VARIANT, or #f if there is no such configuration."
>   (let* ((name (string-append (if variant (string-append variant "-") "")
>                               (if (string=? "i386" arch) "i686" arch) ".conf"))
>          (file (string-append "linux-libre/" name)))
>     (local-file (search-auxiliary-file file))))
>
> I think it's fair for that function expect the arch to be valid (why
> would you ask the config for an unsupported arch?).
>
> I think it should be possible to fix this by checking the arch is
> supported at the call site:
>
>>From 77829140f14928e30cbe4e53c625be3ba2f5895f Mon Sep 17 00:00:00 2001
> From: Pierre Langlois <pierre.langlois@gmx.com>
> Date: Thu, 8 Dec 2022 23:41:40 +0000
> Subject: [PATCH] gnu: make-linux-libre*: Do not get config for unsupported
>  systems.
>
> * gnu/packages/linux.scm (make-linux-libre*)[phases] <configure>: Check
> arch is in supported-systems before calling configuration-file.
> ---
>  gnu/packages/linux.scm | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 5ae6366593..87fc9fe94c 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -983,6 +983,7 @@ (define* (make-linux-libre* version gnu-revision source supported-systems
>                                              (or (%current-target-system)
>                                                  (%current-system))))))
>                                  (and configuration-file arch
> +                                     (member arch supported-systems)
>                                       (configuration-file
>                                        arch
>                                        #:variant (version-major+minor version))))

Thanks for investigating the issue.  I'm hitting the same kind of
problem (it seems) trying to migrate make-u-boot-package to gexps, so
I'm revisiting this.

It seems to me that the produced backtrace is overly cryptic; perhaps
configuration-file should throw an exception with a more detailed error
"unsupported architecture -- not generating configuration file" or
similar?

Or should the generate-package-cache machinery catch these #f itself?
That's the same place where it fails in my case with this patch applied:
https://issues.guix.gnu.org/60224#30, and with the reproducer:

--8<---------------cut here---------------start------------->8---
(package-transitive-supported-systems u-boot-malta (%current-system))
--8<---------------cut here---------------end--------------->8---

I'm still digging into the issue to get a better understanding.

-- 
Thanks,
Maxim

bug#59913: branch master updated: Revert "gnu: make-linux-libre*: Remove input labels."

[Pierre Langlois]

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

Hi Maxim,

guix-commits@gnu.org writes:

> This is an automated email from the git hooks/post-receive script.
>
> apteryx pushed a commit to branch master
> in repository guix.
>
> The following commit(s) were added to refs/heads/master by this push:
>      new 0ffa501f2b Revert "gnu: make-linux-libre*: Remove input labels."
> 0ffa501f2b is described below
>
> commit 0ffa501f2b3e83ae56e9c2bd31418439090e869a
> Author: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> AuthorDate: Fri Dec 9 08:16:07 2022 -0500
>
>     Revert "gnu: make-linux-libre*: Remove input labels."
>     
>     This reverts commit dfc6957a5af7d179d4618eb19d4f555c519bc6f2.  git-bisect
>     found it broke guix pull on non-x86_64 systems like powerpc64le, for reasons
>     to investigate.

In case you've not seen, I noticed this as well and I /think/ worked out
what went wrong on https://issues.guix.gnu.org/59913.

Hopefully the report is helpful and you didn't spend too much time on it
if you've not seen it!

Thanks,
Pierre

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

bug#59913: branch master updated: Revert "gnu: make-linux-libre*: Remove input labels."

[Maxim Cournoyer]

Hi Pierre,

Pierre Langlois <pierre.langlois@gmx.com> writes:

> Hi Maxim,
>
> guix-commits@gnu.org writes:
>
>> This is an automated email from the git hooks/post-receive script.
>>
>> apteryx pushed a commit to branch master
>> in repository guix.
>>
>> The following commit(s) were added to refs/heads/master by this push:
>>      new 0ffa501f2b Revert "gnu: make-linux-libre*: Remove input labels."
>> 0ffa501f2b is described below
>>
>> commit 0ffa501f2b3e83ae56e9c2bd31418439090e869a
>> Author: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> AuthorDate: Fri Dec 9 08:16:07 2022 -0500
>>
>>     Revert "gnu: make-linux-libre*: Remove input labels."
>>     
>>     This reverts commit dfc6957a5af7d179d4618eb19d4f555c519bc6f2.  git-bisect
>>     found it broke guix pull on non-x86_64 systems like powerpc64le, for reasons
>>     to investigate.
>
> In case you've not seen, I noticed this as well and I /think/ worked out
> what went wrong on https://issues.guix.gnu.org/59913.
>
> Hopefully the report is helpful and you didn't spend too much time on it
> if you've not seen it!

Thanks for finding a solution!  I was quite puzzled by the failure and
didn't have the time to investigate.  We could rebase it on master with
a patch reverting the revert, and post it here to see what QA thinks
about it.

-- 
Thanks,
Maxim

bug#59913: branch master updated: Revert "gnu: make-linux-libre*: Remove input labels."

[Pierre Langlois]

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


Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Pierre,
>
> Pierre Langlois <pierre.langlois@gmx.com> writes:
>
>> Hi Maxim,
>>
>> guix-commits@gnu.org writes:
>>
>>> This is an automated email from the git hooks/post-receive script.
>>>
>>> apteryx pushed a commit to branch master
>>> in repository guix.
>>>
>>> The following commit(s) were added to refs/heads/master by this push:
>>>      new 0ffa501f2b Revert "gnu: make-linux-libre*: Remove input labels."
>>> 0ffa501f2b is described below
>>>
>>> commit 0ffa501f2b3e83ae56e9c2bd31418439090e869a
>>> Author: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>>> AuthorDate: Fri Dec 9 08:16:07 2022 -0500
>>>
>>>     Revert "gnu: make-linux-libre*: Remove input labels."
>>>     
>>>     This reverts commit dfc6957a5af7d179d4618eb19d4f555c519bc6f2.  git-bisect
>>>     found it broke guix pull on non-x86_64 systems like powerpc64le, for reasons
>>>     to investigate.
>>
>> In case you've not seen, I noticed this as well and I /think/ worked out
>> what went wrong on https://issues.guix.gnu.org/59913.
>>
>> Hopefully the report is helpful and you didn't spend too much time on it
>> if you've not seen it!
>
> Thanks for finding a solution!  I was quite puzzled by the failure and
> didn't have the time to investigate.  We could rebase it on master with
> a patch reverting the revert, and post it here to see what QA thinks
> about it.

I'm not sure I follow, I'd suggest to revert the revert and then apply a
fix in the same commit, that way it can easily be reverted again if it's
problematic, that's probably what you meant already?

I think the following fix should do it, just to make sure the
`kernel-config' function returns #f on unsupported systems, as per its
docstring:

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 5ae6366593..2d0d1aa29f 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -781,8 +781,10 @@ (define* (kernel-config arch #:key variant)
 ARCH and optionally VARIANT, or #f if there is no such configuration."
   (let* ((name (string-append (if variant (string-append variant "-") "")
                               (if (string=? "i386" arch) "i686" arch) ".conf"))
-         (file (string-append "linux-libre/" name)))
-    (local-file (search-auxiliary-file file))))
+         (file (string-append "linux-libre/" name))
+         (config (search-auxiliary-file file)))
+    (and config
+         (local-file config))))
 
 (define %default-extra-linux-options
   `(;; Make the kernel config available at /proc/config.gz

WDYT?

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

bug#59913: [tentative PATCH] Failure to guix pull on aarch64 since recent make-linux-libre*

[Ludovic Courtès]

Hi,

Pierre Langlois <pierre.langlois@gmx.com> skribis:

> I'm not sure I follow, I'd suggest to revert the revert and then apply a
> fix in the same commit, that way it can easily be reverted again if it's
> problematic, that's probably what you meant already?

Sounds good to me.  The commit log can be similar to the original one
(rather than “Revert: "Revert: "whatever"”), with a couple of lines
like:

  This restores commit XYZ, with an additional fix for …

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

> I think the following fix should do it, just to make sure the
> `kernel-config' function returns #f on unsupported systems, as per its
> docstring:
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 5ae6366593..2d0d1aa29f 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -781,8 +781,10 @@ (define* (kernel-config arch #:key variant)
>  ARCH and optionally VARIANT, or #f if there is no such configuration."
>    (let* ((name (string-append (if variant (string-append variant "-") "")
>                                (if (string=? "i386" arch) "i686" arch) ".conf"))
> -         (file (string-append "linux-libre/" name)))
> -    (local-file (search-auxiliary-file file))))
> +         (file (string-append "linux-libre/" name))
> +         (config (search-auxiliary-file file)))
> +    (and config
> +         (local-file config))))

LGTM!

Thanks,
Ludo’.

bug#59913: [tentative PATCH] Failure to guix pull on aarch64 since recent make-linux-libre*

[Maxim Cournoyer]

Hello,

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

> Hi,
>
> Pierre Langlois <pierre.langlois@gmx.com> skribis:
>
>> I'm not sure I follow, I'd suggest to revert the revert and then apply a
>> fix in the same commit, that way it can easily be reverted again if it's
>> problematic, that's probably what you meant already?
>
> Sounds good to me.  The commit log can be similar to the original one
> (rather than “Revert: "Revert: "whatever"”), with a couple of lines
> like:
>
>   This restores commit XYZ, with an additional fix for …
>
>   Fixes <https://issues.guix.gnu.org/59913>.

Commit restored as 4913ac74915c4229aeb3ca26a5f9920c759fb6a3, with
Pierre's fix (thanks!)

Closing.

-- 
Thanks,
Maxim