guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Why should build phases not return unspecified values?


From: Mark H Weaver
Subject: Re: Why should build phases not return unspecified values?
Date: Tue, 19 Dec 2017 16:35:34 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)

Arun Isaac <address@hidden> writes:

> Whenever we have a build phase that ends with a call (for example, to
> substitute, chdir, symlink, etc) that returns an unspecified value, we
> append a #t so that the return value is a boolean. However, the build
> system, as it stands currently, does not mind an unspecified value, and
> treats it as a success. As a result, forgetting to add a #t at the end
> of custom phases is a common mistake. To fix this, I have submitted a
> patch at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29745 that
> modifies the build system to reject unspecified values as
> failures.
>
> However, IMO, the addition of #t at the end of certain phases, does not
> contribute anything of value and we should simply be at peace with
> phases returning unspecified values. Am I missing something here?

I don't think we should rely on every "unspecified value" being treated
as #true in future versions of Guile.  That is merely an accident of the
current implementation.

However, I also agree that the current situation is a mess in need of
cleaning up.

My preference would be to deprecate the practice of returning explicit
boolean results from phases and snippets, and transition to reporting
errors exclusively using exceptions.

We would create a variant of the 'system*' procedure that raises an
exception in case of a non-zero status code.  This would eliminate the
many awkward code segments where we need to check multiple 'system*'
calls and manually handle propagating the errors upward, e.g. this one
from our 'sicp' package:

       (and (zero?
             (system* "makeinfo" "--output"
                      (string-append info-dir "/sicp.info")
                      (string-append source "/sicp-pocket.texi")))
            (every zero?
                   (map (cut system* "gzip" "-9n" <>)
                        (find-files info-dir))))

Most of the Guix veterans have surely seen or written bits of code like
this, and in my opinion it's an embarrassment.  We're using a language
that supports exceptions, and the overwhelming majority of our errors
are already being reported that way.  However, for historical reasons
we've ended up with this hybrid error reporting strategy where we
awkwardly use both exceptions and explicit error plumbing.  In addition
to the awkwardness, we have a great many bugs related to this.  We
surely still have a great many packages that are relying on "unspecified
value" being treated as #true.  We also surely have cases where the
results of 'system*' are not being checked at all.  This entire way of
doing things is error-prone.

Let's clean this up! [*]

Here's a transition plan: We could start by making the new
exception-throwing 'system*' variant, and switching existing packages to
use it, while removing the related error-code plumbing.  Once that work
is done, we could change the code that calls snippets or phase
procedures to ignore the result of those calls.  Finally, we could
remove the trailing #t's.

What do you think?

      Mark


[*] Incidentally, the clean up proposed here would not be possible if we
    had frozen our existing internal APIs to support external package
    repositories.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]