bug#31647: [core-updates] gtkglext fails in a weird way

bug#31647: [core-updates] gtkglext fails in a weird way

[Ricardo Wurmus]

Hi,

on “core-updates” the “gtkglext” package fails with an odd error
message:

--8<---------------cut here---------------start------------->8-
make[4]: Leaving directory '/tmp/guix-build-gtkglext-1.2.0.drv-0/gtkglext-1.2.0/gdk'
make[3]: *** [Makefile:558: all-recursive] Error 1
make[3]: Leaving directory '/tmp/guix-build-gtkglext-1.2.0.drv-0/gtkglext-1.2.0/gdk'
make[2]: *** [Makefile:428: all] Error 2
make[2]: Leaving directory '/tmp/guix-build-gtkglext-1.2.0.drv-0/gtkglext-1.2.0/gdk'
make[1]: *** [Makefile:363: all-recursive] Error 1
make[1]: Leaving directory '/tmp/guix-build-gtkglext-1.2.0.drv-0/gtkglext-1.2.0'
make: *** [Makefile:279: all] Error 2
Backtrace:
           4 (primitive-load "/gnu/store/spxlr67fxvsjyz489wwds5saa6p…")
In ice-9/eval.scm:
   191:35  3 (_ _)
In srfi/srfi-1.scm:
    640:9  2 (for-each #<procedure 95ddc0 at /gnu/store/f95ghy8mx00…> …)
In /gnu/store/f95ghy8mx00fc22nrvswvnpqlfdkf2nk-module-import/guix/build/gnu-build-system.scm:
   799:31  1 (_ _)
In /gnu/store/f95ghy8mx00fc22nrvswvnpqlfdkf2nk-module-import/guix/build/utils.scm:
    616:6  0 (invoke _ . _)

/gnu/store/f95ghy8mx00fc22nrvswvnpqlfdkf2nk-module-import/guix/build/utils.scm:616:6: In procedure invoke:
Throw to key `srfi-34' with args `(#<condition &invoke-error [program: "make" arguments: ("-j" "4") exit-status: 2 term-signal: #f stop-signal: #f] 9f5f80>)'.
builder for `/gnu/store/0ykv7qamqrk130j5wcg7hvs07gidhvkc-gtkglext-1.2.0.drv' failed with exit code 1
--8<---------------cut here---------------end--------------->8---

Note that it does not print that the build phase failed.

--
Ricardo

bug#31647: [core-updates] gtkglext fails in a weird way

[Ricardo Wurmus]

> on “core-updates” the “gtkglext” package fails with an odd error
> message: […]

It seems that this is not limited to “gtkglext”.  This behaviour seems
to affect any package using “invoke” where the invoked command fails.

Should the build system catch the exception and make sure that it gets
to print “build phase `foo' failed”?

-- 
Ricardo

bug#31647: [core-updates] gtkglext fails in a weird way

[Mark H Weaver]

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:

>> on “core-updates” the “gtkglext” package fails with an odd error
>> message: […]
>
> It seems that this is not limited to “gtkglext”.  This behaviour seems
> to affect any package using “invoke” where the invoked command fails.
>
> Should the build system catch the exception and make sure that it gets
> to print “build phase `foo' failed”?

In my opinion, it's not important for that message to be printed.  What
is being printed now is far more informative, and sometimes that extra
information is quite useful.

If we caught exceptions, we'd need to ensure that all of the relevant
information about the exception is printed.  The code to catch and print
those exceptions would need to be in (guix build ...), and it could
never be updated without forcing a full rebuild.
'call-with-error-handling' in (guix ui) might be suitable, but it would
need to be duplicated.

Also, even if we catch the exception and do a nice job printing it, we'd
likely lose the backtrace to the original error, which in some cases
might be quite useful.

In summary, although the new messages don't look as nice in common
cases, I think it's more important to ensure that we have the
information we need to debug the occasional non-obvious problem.  So, I
think we should leave it alone :)

What do you think?

      Mark

bug#31647: [core-updates] gtkglext fails in a weird way

[Ricardo Wurmus]

Mark H Weaver <mhw@netris.org> writes:

> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:
>
>>> on “core-updates” the “gtkglext” package fails with an odd error
>>> message: […]
>>
>> It seems that this is not limited to “gtkglext”.  This behaviour seems
>> to affect any package using “invoke” where the invoked command fails.
>>
>> Should the build system catch the exception and make sure that it gets
>> to print “build phase `foo' failed”?
>
> In my opinion, it's not important for that message to be printed.  What
> is being printed now is far more informative, and sometimes that extra
> information is quite useful.
[…]
> Also, even if we catch the exception and do a nice job printing it, we'd
> likely lose the backtrace to the original error, which in some cases
> might be quite useful.

Since the build system itself uses invoke to run “make” any error
encountered during the build phase looks something like this:

--8<---------------cut here---------------start------------->8---
Backtrace:
           4 (primitive-load "/gnu/store/vrv8gx4s940z0vaaq9a40jsq9xf…")
In ice-9/eval.scm:
   191:35  3 (_ _)
In srfi/srfi-1.scm:
    640:9  2 (for-each #<procedure a4bbc0 at /gnu/store/qzsljkcllc0…> …)
In /gnu/store/qzsljkcllc01dmdq9z0yrqri3ajam3vp-module-import/guix/build/gnu-build-system.scm:
   799:31  1 (_ _)
In /gnu/store/qzsljkcllc01dmdq9z0yrqri3ajam3vp-module-import/guix/build/utils.scm:
    616:6  0 (invoke _ . _)

/gnu/store/qzsljkcllc01dmdq9z0yrqri3ajam3vp-module-import/guix/build/utils.scm:616:6: In procedure invoke:
Throw to key `srfi-34' with args `(#<condition &invoke-error [program: "make" arguments: ("-j" "4") exit-status: 2 term-signal: #f stop-signal: #f] 9a61c0>)'.
builder for `/gnu/store/2yn2kz4vdfxrrm8lcxfkfbx2548rrmd6-powertabeditor-2.0.0-alpha10.drv' failed with exit code 1
--8<---------------cut here---------------end--------------->8---

It doesn’t seem like this information is actually useful.  I see here
that make failed, which is obvious because we are in the build phase.  I
don’t see why it failed.  I could do without the information that the
“gnu-build-system” uses invoke internally, because “invoke” is not at
fault here.

Previously, we had the phase return #f, which allowed us to cleanly
exit, inform the observant user about the phase that failed (this is
useful because the name of the phase may have long scrolled out of the
buffer), and exit.  The backtrace for invoke does not seem to add
anything of importance and it obscures the actual error.

> In summary, although the new messages don't look as nice in common
> cases, I think it's more important to ensure that we have the
> information we need to debug the occasional non-obvious problem.  So, I
> think we should leave it alone :)

I think we should strive to make the common case look good.  Can we
achieve this without making the exceptional case harder to debug?  Can
we caught the exception triggered by standard build phase invocations of
“make” but not those of custom “invoke” expressions in custom build
phases where the error message could be useful?

What do others think?

--
Ricardo

bug#31647: [core-updates] gtkglext fails in a weird way

[Mark H Weaver]

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:

> Mark H Weaver <mhw@netris.org> writes:
>
>> In summary, although the new messages don't look as nice in common
>> cases, I think it's more important to ensure that we have the
>> information we need to debug the occasional non-obvious problem.  So, I
>> think we should leave it alone :)
>
> I think we should strive to make the common case look good.  Can we
> achieve this without making the exceptional case harder to debug?  Can
> we caught the exception triggered by standard build phase invocations of
> “make” but not those of custom “invoke” expressions in custom build
> phases where the error message could be useful?

I appreciate your perspective on this, and you've made some good points.

How about this idea: in core-updates-next, we could add code to
'gnu-build' in (guix build gnu-build-system) which catches &invoke-error
exceptions thrown by the phase procedures.  This is a very common case,
and I agree with you that a backtrace is rarely (if ever) useful for
that particular exception type.  The program name and arguments included
in the condition object should be enough information.  We could use a
copy of the code from (guix ui) to print the invoke errors nicely:

            ((invoke-error? c)
             (leave (G_ "program exited\
~@[ with non-zero exit status ~a~]\
~@[ terminated by signal ~a~]\
~@[ stopped by signal ~a~]: ~s~%")
                    (invoke-error-exit-status c)
                    (invoke-error-term-signal c)
                    (invoke-error-stop-signal c)
                    (cons (invoke-error-program c)
                          (invoke-error-arguments c))))

However, I would prefer to catch *only* invoke errors, and to let most
exception types go unhandled by gnu-build.  If you can think of another
exception type that should be handled more gracefully, please let me
know.

What do you think?

      Mark

bug#31647: [core-updates] gtkglext fails in a weird way

[Mark H Weaver]

I wrote:
> However, I would prefer to catch *only* invoke errors, and to let most
> exception types go unhandled by gnu-build.

On second thought, I don't have a good justification for this.  What I
really care about is that all exceptions except for specific case(s)
like invoke-error should generate a full backtrace to the original
source of the exception, along with all information present in the
condition object or exception.  I see no reason not to let Guile's
generic exception reporting code handle these unusual cases, but if it's
important to you we could do the same thing from gnu-build, I suppose.

What do you think?

      Mark

bug#31647: [core-updates] gtkglext fails in a weird way

[Ricardo Wurmus]

Mark H Weaver <mhw@netris.org> writes:

> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:
>
>> Mark H Weaver <mhw@netris.org> writes:
>>
>>> In summary, although the new messages don't look as nice in common
>>> cases, I think it's more important to ensure that we have the
>>> information we need to debug the occasional non-obvious problem.  So, I
>>> think we should leave it alone :)
>>
>> I think we should strive to make the common case look good.  Can we
>> achieve this without making the exceptional case harder to debug?  Can
>> we caught the exception triggered by standard build phase invocations of
>> “make” but not those of custom “invoke” expressions in custom build
>> phases where the error message could be useful?
>
> I appreciate your perspective on this, and you've made some good points.
>
> How about this idea: in core-updates-next, we could add code to
> 'gnu-build' in (guix build gnu-build-system) which catches &invoke-error
> exceptions thrown by the phase procedures.  This is a very common case,
> and I agree with you that a backtrace is rarely (if ever) useful for
> that particular exception type.  The program name and arguments included
> in the condition object should be enough information.  We could use a
> copy of the code from (guix ui) to print the invoke errors nicely:
>
>             ((invoke-error? c)
>              (leave (G_ "program exited\
> ~@[ with non-zero exit status ~a~]\
> ~@[ terminated by signal ~a~]\
> ~@[ stopped by signal ~a~]: ~s~%")
>                     (invoke-error-exit-status c)
>                     (invoke-error-term-signal c)
>                     (invoke-error-stop-signal c)
>                     (cons (invoke-error-program c)
>                           (invoke-error-arguments c))))

This sounds good to me.

> However, I would prefer to catch *only* invoke errors, and to let most
> exception types go unhandled by gnu-build.  If you can think of another
> exception type that should be handled more gracefully, please let me
> know.
[…]
> On second thought, I don't have a good justification for this.  What I
> really care about is that all exceptions except for specific case(s)
> like invoke-error should generate a full backtrace to the original
> source of the exception, along with all information present in the
> condition object or exception.  I see no reason not to let Guile's
> generic exception reporting code handle these unusual cases, but if it's
> important to you we could do the same thing from gnu-build, I suppose.

I agree.  I only really care about the invoke errors, because they are
to be expected when there is anything at all wrong with the build.

Any exception other than those triggered by “invoke” could be reported
by Guile directly without us catching and reformatting them in
gnu-build.

--
Ricardo

bug#31647: [core-updates] gtkglext fails in a weird way

[Ludovic Courtès]

Hello,

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:

> Mark H Weaver <mhw@netris.org> writes:
>
>> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:
>>
>>> Mark H Weaver <mhw@netris.org> writes:
>>>
>>>> In summary, although the new messages don't look as nice in common
>>>> cases, I think it's more important to ensure that we have the
>>>> information we need to debug the occasional non-obvious problem.  So, I
>>>> think we should leave it alone :)
>>>
>>> I think we should strive to make the common case look good.  Can we
>>> achieve this without making the exceptional case harder to debug?  Can
>>> we caught the exception triggered by standard build phase invocations of
>>> “make” but not those of custom “invoke” expressions in custom build
>>> phases where the error message could be useful?
>>
>> I appreciate your perspective on this, and you've made some good points.
>>
>> How about this idea: in core-updates-next, we could add code to
>> 'gnu-build' in (guix build gnu-build-system) which catches &invoke-error
>> exceptions thrown by the phase procedures.  This is a very common case,
>> and I agree with you that a backtrace is rarely (if ever) useful for
>> that particular exception type.  The program name and arguments included
>> in the condition object should be enough information.  We could use a
>> copy of the code from (guix ui) to print the invoke errors nicely:
>>
>>             ((invoke-error? c)
>>              (leave (G_ "program exited\
>> ~@[ with non-zero exit status ~a~]\
>> ~@[ terminated by signal ~a~]\
>> ~@[ stopped by signal ~a~]: ~s~%")
>>                     (invoke-error-exit-status c)
>>                     (invoke-error-term-signal c)
>>                     (invoke-error-stop-signal c)
>>                     (cons (invoke-error-program c)
>>                           (invoke-error-arguments c))))
>
> This sounds good to me.
>
>> However, I would prefer to catch *only* invoke errors, and to let most
>> exception types go unhandled by gnu-build.  If you can think of another
>> exception type that should be handled more gracefully, please let me
>> know.
> […]
>> On second thought, I don't have a good justification for this.  What I
>> really care about is that all exceptions except for specific case(s)
>> like invoke-error should generate a full backtrace to the original
>> source of the exception, along with all information present in the
>> condition object or exception.  I see no reason not to let Guile's
>> generic exception reporting code handle these unusual cases, but if it's
>> important to you we could do the same thing from gnu-build, I suppose.
>
> I agree.  I only really care about the invoke errors, because they are
> to be expected when there is anything at all wrong with the build.
>
> Any exception other than those triggered by “invoke” could be reported
> by Guile directly without us catching and reformatting them in
> gnu-build.

I agree, we should do this in ‘core-updates-next’.

Ludo’.