From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark H Weaver Subject: Regarding "Use Invoke" or "Return a boolean from phase" commits Date: Thu, 31 May 2018 14:58:33 -0400 Message-ID: <87bmcvfy92.fsf@netris.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:55947) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fOSns-0005iN-4i for guix-devel@gnu.org; Thu, 31 May 2018 15:00:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fOSno-00077R-5L for guix-devel@gnu.org; Thu, 31 May 2018 15:00:04 -0400 Received: from world.peace.net ([64.112.178.59]:43192) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fOSnn-0006zh-UK for guix-devel@gnu.org; Thu, 31 May 2018 15:00:00 -0400 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: guix-devel@gnu.org --=-=-= Content-Type: text/plain Hello Guix, Many thanks to those of you who have been contributing to the effort to clean up our error reporting from phases, with the eventual goal of ignoring the return value from phases and snippets, and relying on exceptions as the sole method to report errors. I just wanted to clarify that what's really important when looking at a package is that its phases and snippets should always return #t, and that errors generate exceptions. Fixing phases to merely "returning a boolean" is a good thing to do, but not enough to bring us to the ultimate goal stated above. For that, we need to return #t. Similarly, fixing a package to "use invoke" is also not, in itself, enough to ensure this goal. That is merely a convenient way to achieve the goal in phases or snippets that end with (zero? (system* ...)). So, although I've used both of these phrases in my commit logs in the past, now I see that it's be more useful to say "Return #t from all phases", because that's what we really need. Furthermore, it will be important for a static code analyzer to be able to infer that phases and snippets always return #t, so let's try not to be too clever about it. We'll need a static analyzer to gain confidence that we can start ignoring the return values without losing failure reports. Quite a while ago, I hacked up a preliminary static analyzer to do this. Obviously it would be good to integrate something like this into our linter, but I haven't gotten around to it yet. In the meantime, I've attached my preliminary code below. Here's how it's used: --8<---------------cut here---------------start------------->8--- mhw@jojen ~/guix$ ./pre-inst-env guile GNU Guile 2.2.3 Copyright (C) 1995-2017 Free Software Foundation, Inc. Guile comes with ABSOLUTELY NO WARRANTY; for details type `,show w'. This program is free software, and you are welcome to redistribute it under certain conditions; type `,show c' for details. Enter `,help' for help. scheme@(guile-user)> (load "/home/mhw/src/returns-t-checker.scm") scheme@(guile-user)> (define builder-probs (possible-builder-problem-pkgs)) scheme@(guile-user)> ,pp (sort builder-probs package-name #) scheme@(guile-user)> (define phase-probs (possible-phase-problem-pkgs)) scheme@(guile-user)> ,pp (sort phase-probs package-name # # # # # # # # [...] # # # # # # # # # # #) scheme@(guile-user)> --8<---------------cut here---------------end--------------->8--- Comments and suggestions welcome. Mark --=-=-= Content-Type: text/plain; charset=utf-8 Content-Disposition: inline; filename=returns-t-checker.scm Content-Transfer-Encoding: quoted-printable Content-Description: Preliminary "returns-t-checker" static analyzer ;;; GNU Guix --- Functional package management for GNU ;;; Copyright =C2=A9 2018 Mark H Weaver ;;; ;;; This file is part of GNU Guix. ;;; ;;; GNU Guix is free software; you can redistribute it and/or modify it ;;; under the terms of the GNU General Public License as published by ;;; the Free Software Foundation; either version 3 of the License, or (at ;;; your option) any later version. ;;; ;;; GNU Guix is distributed in the hope that it will be useful, but ;;; WITHOUT ANY WARRANTY; without even the implied warranty of ;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ;;; GNU General Public License for more details. ;;; ;;; You should have received a copy of the GNU General Public License ;;; along with GNU Guix. If not, see . ;;; This is very preliminary work. (use-modules (guix) (guix derivations) (gnu) (ice-9 match) (srfi srfi-1)) (define proc-returns-t? (match-lambda (((or 'lambda 'lambda*) formals bodies ... last) (returns-t? last)) (((or 'let 'let* 'letrec 'letrec* 'with-fluids) (bindings ...) bodies ... last) (proc-returns-t? last)) (('begin exprs ... last) (proc-returns-t? last)) (('const expr) (returns-t? expr)) ('invoke #t) (_ #f))) (define returns-t? (match-lambda (#t #t) (('begin exprs ... last) (returns-t? last)) (((or 'let 'let* 'letrec 'letrec* 'with-fluids) (bindings ...) bodies ... last) (returns-t? last)) (('match expr (pattern bodies ... last) ...) (every returns-t? last)) (('with-directory-excursion dir bodies ... last) (returns-t? last)) (((or 'call-with-input-file 'call-with-output-file 'with-input-to-file 'with-output-to-file 'with-atomic-file-replacement) file-name proc) (proc-returns-t? proc)) (('apply proc args ... tail) (proc-returns-t? proc)) ((proc args ...) (proc-returns-t? proc)) (_ #f))) (define mod-spec-returns-t? (match-lambda (((or 'add-before 'add-after) _ _ proc) (proc-returns-t? proc)) (('replace _ proc) (proc-returns-t? proc)) (('delete _) #t))) (define phases-return-t? (match-lambda (#f #t) ('%standard-phases #t) (('modify-phases orig mod-specs ...) (and (every mod-spec-returns-t? mod-specs) (phases-return-t? orig))) (((or 'alist-cons-before 'alist-cons-after) _ _ proc orig) (and (proc-returns-t? proc) (phases-return-t? orig))) (('alist-replace _ proc orig) (and (proc-returns-t? proc) (phases-return-t? orig))) (('alist-delete _ orig) (phases-return-t? orig)) (_ #f))) (define (package-snippet pkg) (and=3D> (package-source pkg) origin-snippet)) (define (snippet-returns-t? snippet) (or (not snippet) (returns-t? snippet))) (define (possible-snippet-problem-pkgs) (fold-packages cons '() #:select? (negate (compose snippet-returns-t? package-snippet)))) (define (arguments->phases arguments) (apply (lambda* (#:key phases #:allow-other-keys) phases) arguments)) (define (package-phases pkg) (and=3D> (package-arguments pkg) arguments->phases)) (define (possible-phase-problem-pkgs) (fold-packages cons '() #:select? (negate (compose phases-return-t? package-phases)))) (define (arguments->builder arguments) (apply (lambda* (#:key builder #:allow-other-keys) builder) arguments)) (define (package-builder pkg) (and=3D> (package-arguments pkg) arguments->builder)) (define (builder-returns-t? builder) (or (not builder) (returns-t? builder))) (define (possible-builder-problem-pkgs) (fold-packages cons '() #:select? (negate (compose builder-returns-t? package-builder)))) (define (package-namephases arguments)= )) (not (phases-return-t? phases)))) (_ #f))))) (_ #f))))) drv-files)) --=-=-=--