diff mbox series

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

Message ID b8eaa03230227033135d919aaafe76ad5af1eefc.camel@telenet.be
State New
Headers show
Series [bug#50238] build-self: Try printing nicer error messages. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

M Aug. 28, 2021, 2:36 p.m. UTC
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

Comments

Ludovic Courtès Sept. 24, 2021, 11:59 a.m. UTC | #1
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'.
M March 8, 2022, 6:58 p.m. UTC | #2
> 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.
Simon Tournier March 8, 2022, 7:22 p.m. UTC | #3
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
M March 9, 2022, 1:18 p.m. UTC | #4
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.
M March 19, 2022, 9:13 a.m. UTC | #5
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.
M March 22, 2022, 8:46 p.m. UTC | #6
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.
M March 22, 2022, 8:55 p.m. UTC | #7
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?.
M March 22, 2022, 8:57 p.m. UTC | #8
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.
M May 6, 2022, 12:48 p.m. UTC | #9
Seems required for effectively debugging #55279.
M June 13, 2022, 2:03 a.m. UTC | #10
Would presumably have improved the error message at


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

and

https://issues.guix.gnu.org/55932
Maxime Devos July 9, 2022, 8:39 p.m. UTC | #11
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.
M July 30, 2022, 11:18 a.m. UTC | #12
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.
M Aug. 13, 2022, 7:38 p.m. UTC | #13
Would be useful for debugging <https://issues.guix.gnu.org/57177>.

Greetings,
Maxime.
diff mbox series

Patch

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