[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[Maxime Devos]

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

Hi guix,

This patch improves the error message generated by "guix time-machine"
and presumably "guix pull" as well when the guix derivation fails to
compute.

Before: something like <https://issues.guix.gnu.org/50232>.
After: something like

[...]
substitute: lijst van substitutes van ?[...]? aan het bijwerken... 100.0%
/gnu/store/4yx3vms0rfpzd1430za3g4pa6jvjsfw9-nowhere.tar.bz2.drv bouwen...
/error: build of `/gnu/store/6s0cmkfa4hq7d9w8g08mv2rhq0cr7wqc-subversion-1.14.1.drv' failed
guix time-machine: fout: You found a bug: the program '/gnu/store/j07lizpjf7v65shdgxyxddz171g6vjvx-compute-guix-derivation'
failed to compute the derivation for Guix (version: "9fc551e089c25f510f918a2c52917ba4e842f1c3"; system: "x86_64-linux";
host version: "75a3413b4e5c1f7443eb944a36ff364f4c4085f4"; pull-version: 1).
Please report it by email to <bug-guix@gnu.org

To test, apply the patch, and introduce an error in the hash of subversion:

;;; (gnu packages version-control)
(define-public subversion
  (package
    (name "subversion")
    (version "1.14.1")
    (source (origin
             (method url-fetch)
             ;; Oops!
             (uri "http://localhost/nowhere.tar.bz2")
             #;(string-append "mirror://apache/subversion/"
                                 "subversion-" version ".tar.bz2")
             (sha256
              (base32
               ;; Oops! 1->0
               #;"1ag1hvcm9q92kgalzbbgcsq9clxnzmbj9nciz9lmabjx4lyajp9c"
               "0ag1hvcm9q92kgalzbbgcsq9clxnzmbj9nciz9lmabjx4lyajp9c"))))
    (build-system gnu-build-system)
    [...]

and commit that mistake.  Then run
  guix time-machine --branch=master --url=$PWD --disable-authentication

Repeat without the error in subversion.

Greetings,
Maxime

[-- Attachment #1.2: 0001-build-self-Try-printing-nicer-error-messages.patch --]
[-- Type: text/x-patch, Size: 3636 bytes --]

From 9dbcd5cedeb861c27aec2b41fcbe6aba92de6c2e Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Sat, 28 Aug 2021 14:42:32 +0200
Subject: [PATCH] build-self: Try printing nicer error messages.

This prevents daunting backtraces like in
<https://issues.guix.gnu.org/50232> by only printing
the relevant information: that 'subversion' fails to
build.

* build-aux/self.scm (build-program): Wrap the 'run-with-store'
  in a 'with-error-handling'.
---
 build-aux/build-self.scm | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/build-aux/build-self.scm b/build-aux/build-self.scm
index f100ff4aae..1dbaf52061 100644
--- a/build-aux/build-self.scm
+++ b/build-aux/build-self.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -280,6 +281,7 @@ interface (FFI) of Guile.")
 
                            ,@(source-module-closure `((guix store)
                                                       (guix self)
+                                                      (guix ui)
                                                       (guix derivations)
                                                       (gnu packages bootstrap))
                                                     (list source)
@@ -316,6 +318,7 @@ interface (FFI) of Guile.")
                         (read-disable 'positions))
 
                       (use-modules (guix store)
+                                   (guix ui)
                                    (guix self)
                                    (guix derivations)
                                    (srfi srfi-1))
@@ -347,14 +350,18 @@ interface (FFI) of Guile.")
                              (parameterize ((current-warning-port
                                              (%make-void-port "w"))
                                             (current-build-output-port sock))
-                               (run-with-store store
-                                 (guix-derivation source version
-                                                  #$guile-version
-                                                  #:channel-metadata
-                                                  '#$channel-metadata
-                                                  #:pull-version
-                                                  #$pull-version)
-                                 #:system system))
+                               ;; Use 'with-error-handling' to prevent scary
+                               ;; backtraced like
+                               ;; <https://issues.guix.gnu.org/50232>.
+                               (with-error-handling
+                                 (run-with-store store
+                                   (guix-derivation source version
+                                                    #$guile-version
+                                                    #:channel-metadata
+                                                    '#$channel-metadata
+                                                    #:pull-version
+                                                    #$pull-version)
+                                   #:system system)))
                              derivation-file-name))))))
                   #:module-path (list source))))
 

base-commit: b4d132f98e03fae559db832e88897f1e166c4d47
prerequisite-patch-id: 91a26ba19372112a11a0eea2b066d2f63641deb1
-- 
2.33.0


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[Ludovic Courtès]

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> This patch improves the error message generated by "guix time-machine"
> and presumably "guix pull" as well when the guix derivation fails to
> compute.
>
> Before: something like <https://issues.guix.gnu.org/50232>.
> After: something like
>
> [...]
> substitute: lijst van substitutes van ?[...]? aan het bijwerken... 100.0%
> /gnu/store/4yx3vms0rfpzd1430za3g4pa6jvjsfw9-nowhere.tar.bz2.drv bouwen...
> /error: build of `/gnu/store/6s0cmkfa4hq7d9w8g08mv2rhq0cr7wqc-subversion-1.14.1.drv' failed
> guix time-machine: fout: You found a bug: the program '/gnu/store/j07lizpjf7v65shdgxyxddz171g6vjvx-compute-guix-derivation'
> failed to compute the derivation for Guix (version: "9fc551e089c25f510f918a2c52917ba4e842f1c3"; system: "x86_64-linux";
> host version: "75a3413b4e5c1f7443eb944a36ff364f4c4085f4"; pull-version: 1).
> Please report it by email to <bug-guix@gnu.org

I sympathize with the goal but unsure about the implementation.

To me, compute-guix-derivation should remain as simple as possible and
it shouldn't take care of UI concerns.  Having said that, I don't know
how to address the problem above while sticking to this principle.

WDYT?

Ludo'.

[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[zimoun]

Hi,

On ven., 24 sept. 2021 at 13:59, Ludovic Courtès <ludo@gnu.org> wrote:

> I sympathize with the goal but unsure about the implementation.

On one hand, I agree that the import of '(guix ui)' here feels "hum, it
seems an incorrect location", but...

> To me, compute-guix-derivation should remain as simple as possible and
> it shouldn't take care of UI concerns.  Having said that, I don't know
> how to address the problem above while sticking to this principle.

...on the other hand, where 'build-program' is called, i.e, 'build,
there is:

--8<---------------cut here---------------start------------->8---
        (format (current-error-port) "Computing Guix derivation for '~a'...  "
                system)
--8<---------------cut here---------------end--------------->8---

which is UI, no?  Idem for the message "You found a bug".


Well, I am unsure about the addition of '(guix ui)' and by
'with-error-handlng' in 'build-program'.  However, maybe the exception
handling could happen in the 'build' part.  WDYT?


Cheers,
simon

[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[Maxime Devos]

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

zimoun schreef op di 08-03-2022 om 20:22 [+0100]:
> Well, I am unsure about the addition of '(guix ui)' and by
> 'with-error-handlng' in 'build-program'.  However, maybe the exception
> handling could happen in the 'build' part.  WDYT?

Keep in mind that the result of the '(build-program ...)' is a program
that will be invoked as a separate program.  Exceptions do not
propagate accross process boundaries,  so 'build' cannot catch the
exceptions happening inside, so 'build' cannot catch the exceptions
happening inside 'build-program'.  Hence, the error handling bits need
to be inside the 'compute-guix-derivation'.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[Maxime Devos]

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

> I sympathize with the goal but unsure about the implementation.
> 
> To me, compute-guix-derivation should remain as simple as possible
> and it shouldn't take care of UI concerns.  Having said that, I don't
> know how to address the problem above while sticking to this
> principle.
>
> WDYT?

*Something* should take care of UI concerns, and 'build-program' is in
a good position to do so.  I don't really see another way to do it
(except by serialising the exception and letting the Guix that is doing
the pulling report it, but that's even more fragile).  Where else could
the UI bits be done?

Additionally, while importing (guix ui) may seem rather daunting,
the error handling bits are only run when there's actually an error to
handle, so I expect it to be fine in practice --- it seems only
possible for this to break "guix pull" if "guix pull" would have failed
anyway.

I have one concern though: (guix ui) used with-exception-handler which
is new-ish with Guile 3.0 IIUC.  Do we need to support pulling from a
Guix of version Guile <2.2?  If so, a little compatibility code may
be necessary -- e.g., not doing the 'with-error-handling' when guile-
version=2.2.

I think the little bit of extra non-simplicity is worth the clarity
(not very much, but still useful) it brings to people on bug-guix and
people reporting the ‘guix pull: error: You found a bug: ...’ issues --
-- see the long list at
<https://issues.guix.gnu.org/search?query=%22you+found+a+bug%22>, and
compare it with the two extra lines of code.

In other terms, if we do a risk-benefit analysis, there is some risk,
but due to the large number of reports there would be much benefit.
And even though there is some risk (see e.g. the Guile 2.2/3.0 thing
above), whenever such an accident happens, it could quickly addressed
by pushing a new commit.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages --- verifying if it doesn't break anything

[Maxime Devos]

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

To make sure the patch doesn't break "guix pull", I've begun testing
using "guix time-machine" to go to an older version of Guix and then go
to the latest version of Guix (with the patch applied).

Older commits tested so far:

  * 1.3.0

    # 1.3.0 --> master + the patch applied
    guix time-machine --url=$PWD --commit=a0178d34f582b50e9bdbb0403943129ae5b560ff -- time-machine --url=$PWD --commit=47ffd21fa177e300df5346f275da1759870540b3 --disable-authentication -- describe

    (succeeds!)

1.3.0 is not a good test though because it's relatively recent, I'll
try some older versions as well.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages --- verifying if it doesn't break anything

[Maxime Devos]

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

Maxime Devos schreef op za 19-03-2022 om 10:13 [+0100]:
> 1.3.0 is not a good test though because it's relatively recent, I'll
> try some older versions as well.

I tried from 1.1.0, seems like it fails because guile@2.2 does not have
with-exception-handler:

Backtrace:
           5 (primitive-load "/gnu/store/jdf2z2vl4gc6yzl5gglixd5jpflcgikm-compute-guix-derivation")
In ice-9/eval.scm:
    155:9  4 (_ #(#(#(#(#(#(#(#(#(#(#(#(#(#(#(#(#<directory (guile-u?> ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?))
    159:9  3 (_ #(#(#(#(#(#(#(#(#(#(#(#(#(#(#(#(#<directory (guile-u?> ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?) ?))
In ice-9/boot-9.scm:
    152:2  2 (with-fluid* _ _ _)
    152:2  1 (with-fluid* _ _ _)
In srfi/srfi-34.scm:
     38:0  0 (with-exception-handler #<procedure 7f3a57d078b8 at ./guix/ui.scm:694:2 (c)> #<procedure 7f3a582457c0 ?> ?)

srfi/srfi-34.scm:38:0: In procedure with-exception-handler:
Wrong number of arguments to #<procedure with-exception-handler (handler thunk)>
Computing Guix derivation for 'x86_64-linux'...  guix time-machine: error: You found a bug: the program '/gnu/store/jdf2z2vl4gc6yzl5gglixd5jpflcgikm-compute-guix-derivation'
failed to compute the derivation for Guix (version: "aac29b952e558e20ac97a95713b15ac453e59742"; system: "x86_64-linux";
host version: "1.1.0rc2-1.9d0d27f"; pull-version: 1).
Please report the COMPLETE output above by email to <bug-guix@gnu.org>.

I guess the simplest solution is to only do 'with-error-handler' wrapping
when guile-version>=3.0 ...


Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages --- verifying if it doesn't break anything

[Maxime Devos]

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

Maxime Devos schreef op di 22-03-2022 om 21:46 [+0100]:
> I tried from 1.1.0, seems like it fails because guile@2.2 does not
> have
> with-exception-handler:

Actually, it does, but it does not support #:unwind?.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages --- verifying if it doesn't break anything

[Maxime Devos]

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

Maxime Devos schreef op di 22-03-2022 om 21:46 [+0100]:
> I tried from 1.1.0, seems like it fails because guile@2.2 does not
> have
> with-exception-handler:

Actually, it looks like it exists since 2.0, but #:unwind is new.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH v2] build-self: Try printing nicer error messages

[Maxime Devos]

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

The revised patch allows time travelling from v1.1.0 to
(master + patch), by not always setting #:unwind?.  Next step: test
some older versions (v1.0.0, v0.2?, ...?)?

Greetings,
Maxime.

[-- Attachment #1.2: 0001-build-self-Try-printing-nicer-error-messages.patch --]
[-- Type: text/x-patch, Size: 5089 bytes --]

From 78d8f1c69f39103f23e72f438f23a708d5388a77 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Sat, 28 Aug 2021 14:42:32 +0200
Subject: [PATCH] build-self: Try printing nicer error messages.

This prevents daunting backtraces like in
<https://issues.guix.gnu.org/50232> by only printing
the relevant information: that 'subversion' fails to
build.

* build-aux/self.scm (build-program): Wrap the 'run-with-store'
  in a 'with-error-handling'.
* guix/ui.scm (guard*): Don't explode on old Guiles.
---
 build-aux/build-self.scm | 23 +++++++++++++++--------
 guix/ui.scm              | 16 +++++++++++-----
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/build-aux/build-self.scm b/build-aux/build-self.scm
index 02822a2ee8..25a7b1e618 100644
--- a/build-aux/build-self.scm
+++ b/build-aux/build-self.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -280,6 +281,7 @@ interface (FFI) of Guile.")
 
                            ,@(source-module-closure `((guix store)
                                                       (guix self)
+                                                      (guix ui)
                                                       (guix derivations)
                                                       (gnu packages bootstrap))
                                                     (list source)
@@ -316,6 +318,7 @@ interface (FFI) of Guile.")
                         (read-disable 'positions))
 
                       (use-modules (guix store)
+                                   (guix ui)
                                    (guix self)
                                    (guix derivations)
                                    (srfi srfi-1))
@@ -347,14 +350,18 @@ interface (FFI) of Guile.")
                              (parameterize ((current-warning-port
                                              (%make-void-port "w"))
                                             (current-build-output-port sock))
-                               (run-with-store store
-                                 (guix-derivation source version
-                                                  #$guile-version
-                                                  #:channel-metadata
-                                                  '#$channel-metadata
-                                                  #:pull-version
-                                                  #$pull-version)
-                                 #:system system))
+                               ;; Use 'with-error-handling' to prevent scary
+                               ;; backtraced like
+                               ;; <https://issues.guix.gnu.org/50232>.
+                               (with-error-handling
+                                 (run-with-store store
+                                   (guix-derivation source version
+                                                    #$guile-version
+                                                    #:channel-metadata
+                                                    '#$channel-metadata
+                                                    #:pull-version
+                                                    #$pull-version)
+                                   #:system system)))
                              derivation-file-name))))))
                   #:module-path (list source))))
 
diff --git a/guix/ui.scm b/guix/ui.scm
index 238952723e..0afe75bbad 100644
--- a/guix/ui.scm
+++ b/guix/ui.scm
@@ -17,6 +17,7 @@
 ;;; Copyright © 2020 Arun Isaac <arunisaac@systemreboot.net>
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2018 Steve Sprang <scs@stevesprang.com>
+;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -677,11 +678,16 @@ or remove one of them from the profile.")
 (define-syntax-rule (guard* (var clauses ...) exp ...)
   "This variant of SRFI-34 'guard' does not unwind the stack before
 evaluating the tests and bodies of CLAUSES."
-  (with-exception-handler
-      (lambda (var)
-        (cond clauses ... (else (raise var))))
-    (lambda () exp ...)
-    #:unwind? #f))
+  (apply with-exception-handler
+         (lambda (var)
+           (cond clauses ... (else (raise var))))
+         (lambda () exp ...)
+         ;; Some old Guiles used by old versions of Guix don't support
+         ;; #:unwind.  Avoid breaking "guix pull" from an old Guix to
+         ;; a new Guix, see <https://issues.guix.gnu.org/50238>.
+         (if (>= (string->number (major-version)) 3)
+             '(#:unwind? #f)
+             '())))
 
 (define (call-with-error-handling thunk)
   "Call THUNK within a user-friendly error handler."

base-commit: 29091731a0c6cb649cdfd72297575fe2bb2a9591
prerequisite-patch-id: e2faf5cdf72f293aca0aff5c89cc1f0dd874d29c
-- 
2.30.2


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH v2] build-self: Try printing nicer error messages

[Maxime Devos]

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

Maxime Devos schreef op di 22-03-2022 om 23:24 [+0100]:
> The revised patch allows time travelling from v1.1.0 to
> (master + patch), by not always setting #:unwind?.  Next step: test
> some older versions (v1.0.0, v0.2?, ...?)?
> 
> Greetings,
> Maxime.

I think the current patch is good enough as-is.  WDYT?

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[Maxime Devos]

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

Seems required for effectively debugging #55279.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[Maxime Devos]

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

Would presumably have improved the error message at


https://issues.guix.gnu.org/39849

and

https://issues.guix.gnu.org/55932

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[bug#50238] Bug in compute-guix-derivation

[Maxime Devos via Guix-patches]

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

block 56466 with 50238

thanks


This error message isn't informative, maybe the patch at

https://issues.guix.gnu.org/50238 would give some context, though not really sure, it was written with other kind of errors in mind.


Greetings,

Maxime.

[-- Attachment #2: Type: text/html, Size: 722 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 241 bytes --]

This (reviewed since 5 months, at least the previous versions) patch 
would be helpful for debugging 
<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56836> and many other 
"guix pull: You found a bug" reports.

Greetings,
Maxime.



[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 931 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[bug#50238] [PATCH] build-self: Try printing nicer error messages.

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 93 bytes --]

Would be useful for debugging <https://issues.guix.gnu.org/57177>.

Greetings,
Maxime.


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]