Message ID | b8eaa03230227033135d919aaafe76ad5af1eefc.camel@telenet.be |
---|---|
State | New |
Headers | show |
Series | [bug#50238] build-self: Try printing nicer error messages. | expand |
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 |
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'.
> 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.
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
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.
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.
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.
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?.
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.
Seems required for effectively debugging #55279.
Would presumably have improved the error message at https://issues.guix.gnu.org/39849 and https://issues.guix.gnu.org/55932
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.
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.
Would be useful for debugging <https://issues.guix.gnu.org/57177>. Greetings, Maxime.
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