From mboxrd@z Thu Jan 1 00:00:00 1970 From: Danny Milosavljevic Subject: Re: pre-push signature hook error reporting [was Re: [PATCH v6] gnu: python-sphinx: Update to 1.4.8.] Date: Tue, 17 Jan 2017 12:34:28 +0100 Message-ID: <20170117123424.714a23b5@scratchpost.org> References: <20170105161431.9803-1-dannym@scratchpost.org> <878tqpjw93.fsf@kirby.i-did-not-set--mail-host-address--so-tickle-me> <8760lj5eb6.fsf@kirby.i-did-not-set--mail-host-address--so-tickle-me> <20170113152400.GA24322@jasmine> <20170117031414.GB23513@jasmine> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:53675) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cTS2C-0004I7-Mx for guix-devel@gnu.org; Tue, 17 Jan 2017 06:34:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cTS28-0007o2-P4 for guix-devel@gnu.org; Tue, 17 Jan 2017 06:34:40 -0500 Received: from dd1012.kasserver.com ([85.13.128.8]:49882) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cTS28-0007ku-HZ for guix-devel@gnu.org; Tue, 17 Jan 2017 06:34:36 -0500 In-Reply-To: <20170117031414.GB23513@jasmine> 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: Leo Famulari Cc: guix-devel@gnu.org Hi Leo, On Mon, 16 Jan 2017 22:14:14 -0500 Leo Famulari wrote: > In Git 2.11.0, it seems that `git verify-commit` can't tell the user > which commits failed verification: > > https://git.kernel.org/cgit/git/git.git/tree/builtin/verify-commit.c?h=v2.11.0 We should report that upstream and add the one line that does tell the user which commits failed verification upstream (for example print argv[i-1] in line 92). > With a warm cache and all the public keys on my machine, checking the > signature of all 17813 commits on the master branch takes ~40 seconds ... > Checking the commits one at a time takes ~105 seconds, using something > like this: > > for commit in $(git rev-list HEAD); do For minimal improvement (I don't even think it's measureable), try `git rev-list HEAD` (backquotes) - it prevents having to spawn a subshell. > We could make the hook do something like that. Thoughts? I think the > performance regression is worth the convenience of knowing why it > failed. Uhhh it's already very slow... so even slower doesn't matter anymore (HIG guideline maximum duration is 2 seconds, so we are way off anyhow). So I'd say do it your way for now and report it upstream for the future. Depending on whether we think it will fail more often than not we could also combine it: - first check the fast (40 s) path - if it fails, - print "Signature could not be verified to be correct. We are checking which failed..." info message - check the slow (105 s) path Do we think that failures are likely? Also, git seems to invoke the gpg executable for each and every commit. It would be interesting whether gpg-interface.c verify_signed_buffer could be adapted to either invoke gpg once or to just use a library instead (gpgme ?). Long term we could also cache the checking result - I think that's something more difficult in the face of keys that expire. It would have to store at least the expiration date, the public key and the list of commit hashes that were checked and validated successfully.