From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark H Weaver Subject: Re: Why should build phases not return unspecified values? Date: Tue, 19 Dec 2017 16:35:34 -0500 Message-ID: <8737462yft.fsf@netris.org> References: Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:59748) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eRPYX-000386-0l for guix-devel@gnu.org; Tue, 19 Dec 2017 16:36:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eRPYL-0008IO-Bx for guix-devel@gnu.org; Tue, 19 Dec 2017 16:36:08 -0500 Received: from world.peace.net ([50.252.239.5]:42898) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eRPYK-0008HB-Uk for guix-devel@gnu.org; Tue, 19 Dec 2017 16:35:57 -0500 In-Reply-To: (Arun Isaac's message of "Sun, 17 Dec 2017 04:58:15 +0530") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Arun Isaac Cc: guix-devel@gnu.org Arun Isaac 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.