From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= Subject: Re: 02/09: guix: store: Make register-items transactional, register drv outputs Date: Sat, 09 Feb 2019 23:09:05 +0100 Message-ID: <87wom8pqbi.fsf@gnu.org> References: <20190204192241.15758.66035@vcs0.savannah.gnu.org> <20190204192243.D1BD820B84@vcs0.savannah.gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([209.51.188.92]:41145) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gsaoA-00069M-I9 for guix-devel@gnu.org; Sat, 09 Feb 2019 17:09:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gsao9-0001xr-Qd for guix-devel@gnu.org; Sat, 09 Feb 2019 17:09:10 -0500 Received: from hera.aquilenet.fr ([185.233.100.1]:58052) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gsao9-0001wu-G1 for guix-devel@gnu.org; Sat, 09 Feb 2019 17:09:09 -0500 In-Reply-To: <20190204192243.D1BD820B84@vcs0.savannah.gnu.org> (guix-commits's message of "Mon, 4 Feb 2019 14:22:43 -0500 (EST)") 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, Caleb Ristvedt Hi reepca, guix-commits@gnu.org skribis: > commit 91cbfa8da90d8d1723da753c171a378dae13cb40 > Author: Caleb Ristvedt > Date: Wed Jan 30 17:03:38 2019 -0600 > > guix: store: Make register-items transactional, register drv outputs >=20=20=20=20=20 > * guix/store/database.scm (SQLITE_BUSY, register-output-sql): new var= iables > (add-references): don't try finalizing after each use, only after a= ll the > uses. > (call-with-transaction): New procedure. > (register-items): Use call-with-transaction to prevent broken inter= mediate > states from being visible. Also if item is a derivation register its > outputs (the C++ registering does this). > ((guix derivations)): use it for read-derivation-from-file and > derivation-path? >=20=20=20=20=20 > * .dir-locals.el (call-with-transaction): indent it. That looks interesting! That=E2=80=99s really two changes: making a proper transaction, and registe= ring the outputs. Could you make it two separate commits? > +(define SQLITE_BUSY 5) Please add a comment stating that this is missing in Guile-SQLite3=C2=A00.1.0 (which we should consider fixing.) > +(define (call-with-transaction db proc) > + "Starts a transaction with DB (makes as many attempts as necessary) an= d runs > +PROC. If PROC exits abnormally, aborts the transaction, otherwise commit= s the > +transaction after it finishes." Please use imperative tense (=E2=80=9CStart=E2=80=9D, not =E2=80=9CStarts= =E2=80=9D) and two spaces after an end-of-sentence period. I realize I=E2=80=99m nitpicking but I think th= at can help make the process smoother as we go. > (define* (register-items items > #:key prefix state-directory > (deduplicate? #t) > @@ -305,6 +336,22 @@ Write a progress report to LOG-PORT." > (define real-file-name > (string-append store-dir "/" (basename (store-info-item item)))) >=20=20 > + (define (register-derivation-outputs) > + "Register all output paths of REAL-FILE-NAME as being produced by > +it (note this doesn't mean 'already produced by it', but rather just > +'associated with it'). This assumes REAL-FILE-NAME is a derivation!" > + (let ((drv (read-derivation-from-file real-file-name)) > + (stmt (sqlite-prepare db register-output-sql #:cache? #t))) > + (for-each (match-lambda > + ((outid . ($ path)) > + (sqlite-bind-arguments stmt > + #:drvpath to-register > + #:outid outid > + #:outpath path) > + (sqlite-fold noop #f stmt))) > + (derivation-outputs drv)) > + (sqlite-finalize stmt))) I don=E2=80=99t feel comfortable using (guix derivations) here because that= =E2=80=99s supposed to be a higher level of abstraction. I don=E2=80=99t see any way around it though. Could you add a test in tests/store-database.scm for this bit? > + (when (derivation-path? real-file-name) > + (register-derivation-outputs)) For clarity I=E2=80=99d make it: (register-derivation-outputs (read-derivation-from-file real-file-name)) Otherwise LGTM. I hope we can merge things piecemeal while preserving existing functionality. Could you send updated patches? Thanks, Ludo=E2=80=99.