Hi Clément,
> Fixes <https://bugs.gnu.org/32190>. Woo! Thank you for this patch. > * src/cuirass/base.scm (evaluate): Don't add jobs to the Derivations table. I see that you’ve mentioned your changes to “build-packages” in a later email. I have two general questions about this: why was the change from “id” to “rowid” necessary? And: could you please also update the documentation so that is reflects the changes? > diff --git a/src/cuirass/database.scm b/src/cuirass/database.scm > index b4b1652..7788ac9 100644 > --- a/src/cuirass/database.scm > +++ b/src/cuirass/database.scm […] > (define (db-add-evaluation db eval) > (sqlite-exec db "\ > INSERT INTO Evaluations (specification, commits) VALUES (" > @@ -384,27 +356,39 @@ string." > (define (db-add-build db build) > "Store BUILD in database DB. BUILD eventual outputs are stored > in the OUTPUTS table." > - (let* ((build-exec > - (sqlite-exec db "\ > -INSERT INTO Builds (derivation, evaluation, log, status, timestamp, > starttime, stoptime)\ > - VALUES (" > - (assq-ref build #:derivation) ", " > - (assq-ref build #:eval-id) ", " > - (assq-ref build #:log) ", " > - (or (assq-ref build #:status) > - (build-status scheduled)) ", " > - (or (assq-ref build #:timestamp) 0) ", " > - (or (assq-ref build #:starttime) 0) ", " > - (or (assq-ref build #:stoptime) 0) ");")) > - (build-id (last-insert-rowid db))) > - (for-each (lambda (output) > - (match output > - ((name . path) > - (sqlite-exec db "\ > -INSERT INTO Outputs (build, name, path) VALUES (" > - build-id ", " name ", " path ");")))) > - (assq-ref build #:outputs)) > - build-id)) > + (catch 'sqlite-error > + (lambda () > + (sqlite-exec db " > +INSERT INTO Builds (derivation, evaluation, job_name, system, nix_name, log, > +status, timestamp, starttime, stoptime) > +VALUES (" > + (assq-ref build #:derivation) ", " > + (assq-ref build #:eval-id) ", " > + (assq-ref build #:job-name) ", " > + (assq-ref build #:system) ", " > + (assq-ref build #:nix-name) ", " > + (assq-ref build #:log) ", " > + (or (assq-ref build #:status) > + (build-status scheduled)) ", " > + (or (assq-ref build #:timestamp) 0) ", " > + (or (assq-ref build #:starttime) 0) ", " > + (or (assq-ref build #:stoptime) 0) ");") > + (let ((derivation (assq-ref build #:derivation))) > + (for-each (lambda (output) > + (match output > + ((name . path) > + (sqlite-exec db "\ > +INSERT INTO Outputs (derivation, name, path) VALUES (" > + derivation ", " name ", " path ");")))) > + (assq-ref build #:outputs)) > + derivation)) This procedure is called when a build is scheduled, isn’t it? The docstring says “BUILD eventual outputs are stored in the OUTPUTS table.” – does this mean the names of the *expected* output directories are recorded in Outputs? They don’t exist at this point, right? > + (lambda (key who code message . rest) > + ;; If we get a unique-constraint-failed error, that means we have > + ;; already inserted the same build. That happens when several jobs > + ;; produce the same derivation, and we can ignore it. > + (if (= code SQLITE_CONSTRAINT_PRIMARYKEY) > + #f > + (apply throw key who code rest))))) Okay. Can we prevent this from happening in the first place? I feel a little uncomfortable about performing an operation that we expect to cause primary key errors regularly. > diff --git a/src/schema.sql b/src/schema.sql > index eb0f7e9..0452495 100644 > --- a/src/schema.sql > +++ b/src/schema.sql […] > --- Builds are not in a one to one relationship with derivations in order to > --- keep track of non deterministic compilations. Is this comment still correct considering that the derivation is now the primary key of the Builds table? > CREATE TABLE Builds ( > - id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, > - derivation TEXT NOT NULL, > + derivation TEXT NOT NULL PRIMARY KEY, > evaluation INTEGER NOT NULL, > + job_name TEXT NOT NULL, > + system TEXT NOT NULL, > + nix_name TEXT NOT NULL, > log TEXT NOT NULL, > status INTEGER NOT NULL, > timestamp INTEGER NOT NULL, > starttime INTEGER NOT NULL, > stoptime INTEGER NOT NULL, > - FOREIGN KEY (derivation) REFERENCES Derivations (derivation), > FOREIGN KEY (evaluation) REFERENCES Evaluations (id) > ); Thanks again! -- Ricardo
