Hi reepca, guix-comm...@gnu.org skribis:
> commit 91cbfa8da90d8d1723da753c171a378dae13cb40 > Author: Caleb Ristvedt <caleb.ristv...@cune.org> > Date: Wed Jan 30 17:03:38 2019 -0600 > > guix: store: Make register-items transactional, register drv outputs > > * guix/store/database.scm (SQLITE_BUSY, register-output-sql): new > variables > (add-references): don't try finalizing after each use, only after all > the > uses. > (call-with-transaction): New procedure. > (register-items): Use call-with-transaction to prevent broken > intermediate > 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? > > * .dir-locals.el (call-with-transaction): indent it. That looks interesting! That’s really two changes: making a proper transaction, and registering 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 0.1.0 (which we should consider fixing.) > +(define (call-with-transaction db proc) > + "Starts a transaction with DB (makes as many attempts as necessary) and > runs > +PROC. If PROC exits abnormally, aborts the transaction, otherwise commits the > +transaction after it finishes." Please use imperative tense (“Start”, not “Starts”) and two spaces after an end-of-sentence period. I realize I’m nitpicking but I think that 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)))) > > + (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 . ($ <derivation-output> 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’t feel comfortable using (guix derivations) here because that’s supposed to be a higher level of abstraction. I don’t 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’d 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’.