Cuirass: add support for multiple inputs

  • Done
  • quality assurance status badge
Details
2 participants
  • Clément Lassieur
  • Ludovic Courtès
Owner
unassigned
Submitted by
Clément Lassieur
Severity
normal
C
C
C
Clément Lassieur wrote on 11 Jul 2018 01:02
[PATCH 1/5] base: Compile CHECKOUT in the fiber.
(address . 32121@debbugs.gnu.org)
20180710230247.16639-1-clement@lassieur.org
Because it may take time and thus prevent PROCESS-SPECS to run every INTERVAL
seconds.

* src/cuirass/base.scm (process-specs): move the COMPILE invocation inside
SPAWN-FIBER's thunk. Add log message.
---
src/cuirass/base.scm | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

Toggle diff (30 lines)
diff --git a/src/cuirass/base.scm b/src/cuirass/base.scm
index 9985fd6..de54f72 100644
--- a/src/cuirass/base.scm
+++ b/src/cuirass/base.scm
@@ -3,6 +3,7 @@
;;; Copyright © 2016, 2017 Mathieu Lirzin <mthl@gnu.org>
;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;; Copyright © 2017 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
;;;
;;; This file is part of Cuirass.
;;;
@@ -631,12 +632,11 @@ procedure is meant to be called at startup."
;; Immediately mark COMMIT as being processed so we don't spawn
;; a concurrent evaluation of that same commit.
(db-add-stamp db spec commit)
-
- (when compile?
- (non-blocking (compile checkout)))
-
(spawn-fiber
(lambda ()
+ (when compile?
+ (log-message "compiling '~a' with commit ~s" name commit)
+ (non-blocking (compile checkout)))
(guard (c ((evaluation-error? c)
(log-message "failed to evaluate spec '~s'"
(evaluation-error-spec-name c))
--
2.18.0
C
C
Clément Lassieur wrote on 11 Jul 2018 01:02
[PATCH 2/5] utils: Reset the Fiber dynamic environment in %NON-BLOCKING.
(address . 32121@debbugs.gnu.org)
20180710230247.16639-2-clement@lassieur.org
* src/cuirass/utils.scm (%non-blocking): Wrap body in PARAMETERIZE form that
clears CURRENT-FIBER.

So that PUT-MESSAGE doesn't try to suspend itself within CALL-WITH-NEW-THREAD.
---
src/cuirass/utils.scm | 34 ++++++++++++++++++----------------
1 file changed, 18 insertions(+), 16 deletions(-)

Toggle diff (54 lines)
diff --git a/src/cuirass/utils.scm b/src/cuirass/utils.scm
index bbecfb6..d219a3e 100644
--- a/src/cuirass/utils.scm
+++ b/src/cuirass/utils.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2012, 2013, 2016, 2018 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2015 David Thompson <davet@gnu.org>
;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
+;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
;;;
;;; This file is part of Cuirass.
;;;
@@ -122,22 +123,23 @@ VARS... are bound to the arguments of the critical section."
(lambda (vars ...) exp ...)))
(define (%non-blocking thunk)
- (let ((channel (make-channel)))
- (call-with-new-thread
- (lambda ()
- (catch #t
- (lambda ()
- (call-with-values thunk
- (lambda values
- (put-message channel `(values ,@values)))))
- (lambda args
- (put-message channel `(exception ,@args))))))
-
- (match (get-message channel)
- (('values . results)
- (apply values results))
- (('exception . args)
- (apply throw args)))))
+ (parameterize (((@@ (fibers internal) current-fiber) #f))
+ (let ((channel (make-channel)))
+ (call-with-new-thread
+ (lambda ()
+ (catch #t
+ (lambda ()
+ (call-with-values thunk
+ (lambda values
+ (put-message channel `(values ,@values)))))
+ (lambda args
+ (put-message channel `(exception ,@args))))))
+
+ (match (get-message channel)
+ (('values . results)
+ (apply values results))
+ (('exception . args)
+ (apply throw args))))))
(define-syntax-rule (non-blocking exp ...)
"Evalaute EXP... in a separate thread so that it doesn't block the execution
--
2.18.0
C
C
Clément Lassieur wrote on 11 Jul 2018 01:02
[PATCH 3/5] database: Add support for database upgrades.
(address . 32121@debbugs.gnu.org)
20180710230247.16639-3-clement@lassieur.org
* Makefile.am: Copy SQL files into their data directory.
* doc/cuirass.texi (Database schema): Document the change.
* src/cuirass/database.scm (%package-sql-dir): New parameter.
(db-load, db-get-version, db-set-version, get-target-version,
get-upgrade-file, db-upgrade): New procedures.
(db-init): Set version corresponding to the existing upgrade-n.sql files.
(db-open): If database exists, upgrade it.
* src/schema.sql: New file.
* src/sql/upgrade-1.sql: New file.
---
Makefile.am | 3 +++
doc/cuirass.texi | 16 ++++++++++---
src/cuirass/database.scm | 50 +++++++++++++++++++++++++++++++++++++---
src/schema.sql | 5 ++++
src/sql/upgrade-1.sql | 7 ++++++
5 files changed, 75 insertions(+), 6 deletions(-)
create mode 100644 src/sql/upgrade-1.sql

Toggle diff (201 lines)
diff --git a/Makefile.am b/Makefile.am
index d372b9e..00954b8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3,6 +3,7 @@
# Copyright © 1995-2016 Free Software Foundation, Inc.
# Copyright © 2016, 2017 Mathieu Lirzin <mthl@gnu.org>
# Copyright © 2018 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2018 Clément Lassieur <clement@lassieur.org>
#
# This file is part of Cuirass.
#
@@ -32,6 +33,7 @@ pkgmoduledir = $(guilesitedir)/$(PACKAGE)
pkgobjectdir = $(guileobjectdir)/$(PACKAGE)
webmoduledir = $(guilesitedir)/web/server
webobjectdir = $(guileobjectdir)/web/server
+sqldir = $(pkgdatadir)/sql
dist_pkgmodule_DATA = \
src/cuirass/base.scm \
@@ -55,6 +57,7 @@ nodist_webobject_DATA = \
$(dist_webmodule_DATA:.scm=.go)
dist_pkgdata_DATA = src/schema.sql
+dist_sql_DATA = src/sql/upgrade-*.sql
TEST_EXTENSIONS = .scm .sh
AM_TESTS_ENVIRONMENT = \
diff --git a/doc/cuirass.texi b/doc/cuirass.texi
index b5b27e8..38eb0b0 100644
--- a/doc/cuirass.texi
+++ b/doc/cuirass.texi
@@ -12,7 +12,8 @@ server.
Copyright @copyright{} 2016, 2017 Mathieu Lirzin@*
Copyright @copyright{} 2017 Mathieu Othacehe@*
-Copyright @copyright{} 2018 Ludovic Courtès
+Copyright @copyright{} 2018 Ludovic Courtès@*
+Copyright @copyright{} 2018 Clément Lassieur
@quotation
Permission is granted to copy, distribute and/or modify this document
@@ -228,8 +229,8 @@ Cuirass uses a SQLite database to store information about jobs and past
build results, but also to coordinate the execution of jobs.
The database contains the following tables: @code{Specifications},
-@code{Stamps}, @code{Evaluations}, @code{Derivations}, and
-@code{Builds}. The purpose of each of these tables is explained below.
+@code{Stamps}, @code{Evaluations}, @code{Derivations}, @code{Builds} and
+@code{SchemaVersion}. The purpose of each of these tables is explained below.
@section Specifications
@cindex specifications, database
@@ -412,6 +413,15 @@ This text field holds the path of the output.
@end table
+@section SchemaVersion
+@cindex version, database
+
+This table keeps track of the schema version. During the initialization, the
+version @code{v} is compared to the highest @code{n} of the
+@code{sql/upgrade-n.sql} files, so that if that @code{n} is higher than the
+schema version, files @code{sql/upgrade-(v+1).sql} to @code{sql/upgrade-n.sql}
+are loaded and the version is updated.
+
@c *********************************************************************
@node Web API
@chapter Web API
diff --git a/src/cuirass/database.scm b/src/cuirass/database.scm
index a1398bc..188b9a8 100644
--- a/src/cuirass/database.scm
+++ b/src/cuirass/database.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2016, 2017 Mathieu Lirzin <mthl@gnu.org>
;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;; Copyright © 2018 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
;;;
;;; This file is part of Cuirass.
;;;
@@ -23,10 +24,13 @@
#:use-module (cuirass utils)
#:use-module (ice-9 match)
#:use-module (ice-9 format)
+ #:use-module (ice-9 ftw)
#:use-module (ice-9 rdelim)
+ #:use-module (ice-9 regex)
#:use-module (srfi srfi-1)
#:use-module (srfi srfi-19)
#:use-module (srfi srfi-26)
+ #:use-module (srfi srfi-42)
#:use-module (sqlite3)
#:export (;; Procedures.
db-init
@@ -126,6 +130,12 @@ question marks matches the number of arguments to bind."
(string-append %datadir "/" %package))
"/schema.sql")))
+(define %package-sql-dir
+ ;; Define to the directory containing the SQL files.
+ (make-parameter (string-append (or (getenv "CUIRASS_DATADIR")
+ (string-append %datadir "/" %package))
+ "/sql")))
+
(define (read-sql-file file-name)
"Return a list of string containing SQL instructions from FILE-NAME."
(call-with-input-file file-name
@@ -153,6 +163,30 @@ question marks matches the number of arguments to bind."
db)
+(define (db-load db schema)
+ (for-each (cut sqlite-exec db <>)
+ (read-sql-file schema)))
+
+(define (db-get-version db)
+ (if (pair? (sqlite-exec db "SELECT name FROM sqlite_master WHERE \
+type='table' AND name='SchemaVersion';"))
+ (vector-ref
+ (car (sqlite-exec db "SELECT MAX(version) FROM SchemaVersion;")) 0)
+ 0))
+
+(define (db-set-version db version)
+ (sqlite-exec db "INSERT INTO SchemaVersion (version) VALUES (" version
+ ");"))
+
+(define (get-target-version)
+ (apply max
+ (map string->number
+ (map (cut match:substring <> 1)
+ (filter regexp-match?
+ (map (cut string-match
+ "^upgrade-([0-9]+)\\.sql$" <>)
+ (scandir (%package-sql-dir))))))))
+
(define* (db-init #:optional (db-name (%package-database))
#:key (schema (%package-schema-file)))
"Open the database to store and read jobs and builds informations. Return a
@@ -162,10 +196,20 @@ database object."
(delete-file db-name))
(let ((db (sqlite-open db-name (logior SQLITE_OPEN_CREATE
SQLITE_OPEN_READWRITE))))
- (for-each (lambda (sql) (sqlite-exec db sql))
- (read-sql-file schema))
+ (db-load db schema)
+ (db-set-version db (get-target-version))
db))
+(define (get-upgrade-file version)
+ (in-vicinity (%package-sql-dir) (format #f "upgrade-~a.sql" version)))
+
+(define (db-upgrade db)
+ (do-ec (:range version (db-get-version db) (get-target-version))
+ (let ((intermediate-version (1+ version)))
+ (db-load db (get-upgrade-file intermediate-version))
+ (db-set-version db intermediate-version)))
+ db)
+
(define* (db-open #:optional (db (%package-database)))
"Open database to store or read jobs and builds informations. Return a
database object."
@@ -173,7 +217,7 @@ database object."
;; avoid SQLITE_LOCKED errors when we have several readers:
;; <https://www.sqlite.org/wal.html>.
(set-db-options (if (file-exists? db)
- (sqlite-open db SQLITE_OPEN_READWRITE)
+ (db-upgrade (sqlite-open db SQLITE_OPEN_READWRITE))
(db-init db))))
(define (db-close db)
diff --git a/src/schema.sql b/src/schema.sql
index 65aebbd..a3f14eb 100644
--- a/src/schema.sql
+++ b/src/schema.sql
@@ -1,5 +1,10 @@
BEGIN TRANSACTION;
+-- Singleton table to keep track of the schema version.
+CREATE TABLE SchemaVersion (
+ version integer not null
+);
+
CREATE TABLE Specifications (
repo_name TEXT NOT NULL PRIMARY KEY,
url TEXT NOT NULL,
diff --git a/src/sql/upgrade-1.sql b/src/sql/upgrade-1.sql
new file mode 100644
index 0000000..8f561da
--- /dev/null
+++ b/src/sql/upgrade-1.sql
@@ -0,0 +1,7 @@
+BEGIN TRANSACTION;
+
+CREATE TABLE SchemaVersion (
+ version integer not null
+);
+
+COMMIT;
--
2.18.0
C
C
Clément Lassieur wrote on 11 Jul 2018 01:02
[PATCH 4/5] database: Call a specification 'jobset' instead of 'project'.
(address . 32121@debbugs.gnu.org)
20180710230247.16639-4-clement@lassieur.org
This removes the possibility to filter specifications by branch, because
branches were previously called 'jobset'. But it doesn't matter because later
on, specifications will have as many branches as inputs. And people should
filter by specification name instead.

* doc/cuirass.texi (Build Information, Latest builds): Remove 'jobset',
replace 'project' with 'jobset'.
* src/cuirass/http.scm (build->hydra-build): Idem.
* tests/database.scm (db-get-builds): Idem.
* tests/http.scm (build-query-result, /api/latestbuilds?nr=1&jobset=guix,
/api/latestbuilds?nr=1&jobset=gnu): Idem.
* src/cuirass/database.scm (db-format-build, db-get-builds): Don't associate
builds with branches (which were called 'jobset' afterwards).
(db-get-builds): Remove the #:project filter.
---
doc/cuirass.texi | 15 ++++-----------
src/cuirass/database.scm | 20 ++++++++------------
src/cuirass/http.scm | 4 ++--
tests/database.scm | 6 ++----
tests/http.scm | 12 ++++++------
5 files changed, 22 insertions(+), 35 deletions(-)

Toggle diff (234 lines)
diff --git a/doc/cuirass.texi b/doc/cuirass.texi
index 38eb0b0..5c8c23f 100644
--- a/doc/cuirass.texi
+++ b/doc/cuirass.texi
@@ -448,8 +448,7 @@ $ curl -s "http://localhost:8080/build/2" | jq
@{
"id": 2,
- "project": "guix",
- "jobset": "master",
+ "jobset": "guix",
"job": "acpica-20150410-job",
"timestamp": 1501347493,
"starttime": 1501347493,
@@ -487,11 +486,8 @@ hereafter.
@item id
The unique build id.
-@item project
-The associated specification name, as a string.
-
@item jobset
-The associated specification branch, as a string.
+The associated specification name, as a string.
@item job
The associated job-name, as a string.
@@ -586,9 +582,6 @@ This request accepts a mandatory parameter and multiple optional ones.
@item nr
Limit query result to nr elements. This parameter is @emph{mandatory}.
-@item project
-Filter query result to builds with the given @code{project}.
-
@item jobset
Filter query result to builds with the given @code{jobset}.
@@ -606,10 +599,10 @@ For example, to ask for the ten last builds:
$ curl "http://localhost:8080/api/latestbuilds?nr=10"
@end example
-or the five last builds where project is ``guix'' and jobset ``master'':
+or the five last builds where jobset ``guix'':
@example
-$ curl "http://localhost:8080/api/latestbuilds?nr=5&project=guix&jobset=master"
+$ curl "http://localhost:8080/api/latestbuilds?nr=5&jobset=guix"
@end example
If no builds matching given parameters are found, an empty JSON array is
diff --git a/src/cuirass/database.scm b/src/cuirass/database.scm
index 188b9a8..f38dcd4 100644
--- a/src/cuirass/database.scm
+++ b/src/cuirass/database.scm
@@ -405,7 +405,7 @@ log file for DRV."
(define (db-format-build db build)
(match build
(#(id timestamp starttime stoptime log status derivation job-name system
- nix-name repo-name branch)
+ nix-name repo-name)
`((#:id . ,id)
(#:timestamp . ,timestamp)
(#:starttime . ,starttime)
@@ -417,13 +417,12 @@ log file for DRV."
(#:system . ,system)
(#:nix-name . ,nix-name)
(#:repo-name . ,repo-name)
- (#:outputs . ,(db-get-outputs db id))
- (#:branch . ,branch)))))
+ (#:outputs . ,(db-get-outputs db id))))))
(define (db-get-builds db filters)
"Retrieve all builds in database DB which are matched by given FILTERS.
-FILTERS is an assoc list which possible keys are 'project | 'jobset | 'job |
-'system | 'nr | 'order | 'status."
+FILTERS is an assoc list which possible keys are 'jobset | 'job | 'system |
+'nr | 'order | 'status."
;; XXX Change caller and remove
(define (assqx-ref filters key)
@@ -467,7 +466,7 @@ Assumes that if group id stays the same the group headers stay the same."
(define (finish-group)
(match repeated-row
(#(timestamp starttime stoptime log status derivation job-name system
- nix-name repo-name branch)
+ nix-name repo-name)
`((#:id . ,repeated-builds-id)
(#:timestamp . ,timestamp)
(#:starttime . ,starttime)
@@ -479,8 +478,7 @@ Assumes that if group id stays the same the group headers stay the same."
(#:system . ,system)
(#:nix-name . ,nix-name)
(#:repo-name . ,repo-name)
- (#:outputs . ,outputs)
- (#:branch . ,branch)))))
+ (#:outputs . ,outputs)))))
(define (same-group? builds-id)
(= builds-id repeated-builds-id))
@@ -520,22 +518,20 @@ Assumes that if group id stays the same the group headers stay the same."
(stmt-text (format #f "\
SELECT Builds.id, Outputs.name, Outputs.path, Builds.timestamp, Builds.starttime, Builds.stoptime, Builds.log, Builds.status, Builds.derivation,\
Derivations.job_name, Derivations.system, Derivations.nix_name,\
-Specifications.repo_name, Specifications.branch \
+Specifications.repo_name \
FROM Builds \
INNER JOIN Derivations ON Builds.derivation = Derivations.derivation AND Builds.evaluation = Derivations.evaluation \
INNER JOIN Evaluations ON Derivations.evaluation = Evaluations.id \
INNER JOIN Specifications ON Evaluations.specification = Specifications.repo_name \
LEFT JOIN Outputs ON Outputs.build = Builds.id \
WHERE (:id IS NULL OR (:id = Builds.id)) \
-AND (:project IS NULL OR (:project = Specifications.repo_name)) \
-AND (:jobset IS NULL OR (:jobset = Specifications.branch)) \
+AND (:jobset IS NULL OR (:jobset = Specifications.repo_name)) \
AND (:job IS NULL OR (:job = Derivations.job_name)) \
AND (:system IS NULL OR (:system = Derivations.system)) \
AND (:status IS NULL OR (:status = 'done' AND Builds.status >= 0) OR (:status = 'pending' AND Builds.status < 0)) \
ORDER BY ~a, Builds.id ASC LIMIT :nr;" order))
(stmt (sqlite-prepare db stmt-text #:cache? #t)))
(sqlite-bind-arguments stmt #:id (assqx-ref filters 'id)
- #:project (assqx-ref filters 'project)
#:jobset (assqx-ref filters 'jobset)
#:job (assqx-ref filters 'job)
#:system (assqx-ref filters 'system)
diff --git a/src/cuirass/http.scm b/src/cuirass/http.scm
index e911b9b..a45e6b1 100644
--- a/src/cuirass/http.scm
+++ b/src/cuirass/http.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
;;; Copyright © 2018 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
;;;
;;; This file is part of Cuirass.
;;;
@@ -45,8 +46,7 @@
(build-status started)))))
`((#:id . ,(assq-ref build #:id))
- (#:project . ,(assq-ref build #:repo-name))
- (#:jobset . ,(assq-ref build #:branch))
+ (#:jobset . ,(assq-ref build #:repo-name))
(#:job . ,(assq-ref build #:job-name))
;; Hydra's API uses "timestamp" as the time of the last useful event for
diff --git a/tests/database.scm b/tests/database.scm
index 847c8a6..e71c7f7 100644
--- a/tests/database.scm
+++ b/tests/database.scm
@@ -2,6 +2,7 @@
;;;
;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
;;; Copyright © 2018 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
;;;
;;; This file is part of Cuirass.
;;;
@@ -156,7 +157,6 @@ INSERT INTO Evaluations (specification, revision) VALUES (3, 3);")
#(((1 "/foo.drv") (2 "/bar.drv") (3 "/baz.drv")) ;ascending order
((3 "/baz.drv") (2 "/bar.drv") (1 "/foo.drv")) ;descending order
((3 "/baz.drv") (2 "/bar.drv") (1 "/foo.drv")) ;ditto
- ((3 "/baz.drv") (2 "/bar.drv") (1 "/foo.drv")) ;ditto
((3 "/baz.drv")) ;nr = 1
((2 "/bar.drv") (1 "/foo.drv") (3 "/baz.drv"))) ;status+submission-time
(with-temporary-database db
@@ -185,9 +185,7 @@ INSERT INTO Evaluations (specification, revision) VALUES (3, 3);")
(assq-ref alist #:derivation)))))
(vector (map summarize (db-get-builds db '((nr 3) (order build-id))))
(map summarize (db-get-builds db '()))
- (map summarize (db-get-builds db '((project "guix"))))
- (map summarize (db-get-builds db '((project "guix")
- (jobset "master"))))
+ (map summarize (db-get-builds db '((jobset "guix"))))
(map summarize (db-get-builds db '((nr 1))))
(map summarize
(db-get-builds db '((order status+submission-time))))))))
diff --git a/tests/http.scm b/tests/http.scm
index 9d460b2..ba53887 100644
--- a/tests/http.scm
+++ b/tests/http.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
;;; Copyright © 2017, 2018 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
;;;
;;; This file is part of Cuirass.
;;;
@@ -76,8 +77,7 @@
(define build-query-result
'((#:id . 1)
- (#:project . "guix")
- (#:jobset . "master")
+ (#:jobset . "guix")
(#:job . "fake-job")
(#:timestamp . 1501347493)
(#:starttime . 1501347493)
@@ -226,13 +226,13 @@
500
(response-code (http-get (test-cuirass-uri "/api/latestbuilds"))))
- (test-assert "/api/latestbuilds?nr=1&project=guix&jobset=master"
+ (test-assert "/api/latestbuilds?nr=1&jobset=guix"
(let ((hash-list
(call-with-input-string
(utf8->string
(http-get-body
(test-cuirass-uri
- "/api/latestbuilds?nr=1&project=guix&jobset=master")))
+ "/api/latestbuilds?nr=1&jobset=guix")))
json->scm)))
(and (= (length hash-list) 1)
(hash-table=?
@@ -241,14 +241,14 @@
(object->json-string build-query-result)
json->scm)))))
- (test-assert "/api/latestbuilds?nr=1&project=gnu"
+ (test-assert "/api/latestbuilds?nr=1&jobset=gnu"
;; The result should be an empty JSON array.
(let ((hash-list
(call-with-input-string
(utf8->string
(http-get-body
(test-cuirass-uri
- "/api/latestbuilds?nr=1&project=gnu")))
+ "/api/latestbuilds?nr=1&jobset=gnu")))
json->scm)))
(= (length hash-list) 0)))
--
2.18.0
C
C
Clément Lassieur wrote on 11 Jul 2018 01:02
[PATCH 5/5] Add support for multiple inputs.
(address . 32121@debbugs.gnu.org)
20180710230247.16639-5-clement@lassieur.org
* bin/evaluate.in (absolutize, find-checkout, get-proc-source, get-load-path,
get-guix-package-path, format-checkouts, append-paths): New procedures.
(%not-colon): Remove variable.
(main): Take the load path, package path and PROC from the checkouts that
result from the inputs. Format the checkouts before sending them to the
procedure.
* doc/cuirass.texi (Overview, Database schema): Document the changes.
* examples/{guix-jobs.scm, hello-git.scm, hello-singleton.scm,
hello-subset.scm, random.scm}: Adapt to the new specification format.
* examples/guix-track-git.scm (package->spec): Rename to PACKAGE->INPUT.
(package->git-tracked): Replace FETCH-REPOSITORY with FETCH-INPUT and handle
the new format of its return value.
* examples/random-jobs.scm (make-random-jobs): Rename RANDOM to CHECKOUT.
Rename the checkout from 'random (which is a specification) to 'cuirass (which
is a checkout resulting from an input).
* src/cuirass/base.scm (fetch-repository): Rename to fetch-input. Rename SPEC
to INPUT. Return a checkout object instead of returning two values.
(evaluate): Take a list of CHECKOUTS and COMMITS as arguments, instead of
SOURCE. Remove TOKENIZE and LOAD-PATH. Pass the CHECKOUTS instead of the
SOURCE to "evaluate". Build the EVAL object instead of getting it from
"evaluate".
(compile?, fetch-inputs, compile-checkouts): New procedures.
(process-specs): Fetch all inputs instead of only fetching one repository.
The result of that fetching operation is a list of CHECKOUTS whose COMMITS are
used as a STAMP.
* src/cuirass/database.scm (db-add-input, db-get-inputs): New procedures.
(db-add-specification, db-get-specifications): Adapt to the new specification
format. Add/get all inputs as well.
(db-add-evaluation): Rename REVISION to COMMITS. Store COMMITS as space
separated commit hashes.
(db-get-builds): Rename REPO_NAME to NAME.
(db-get-stamp): Rename COMMIT to STAMP. Return #f when there is no STAMP.
(db-add-stamp): Rename COMMIT to STAMP. Deal with DB-GET-STAMP's new return
value.
(db-get-evaluations): Rename REVISION to COMMITS. Tokenize COMMITS.
* src/cuirass/utils.scm (%non-blocking): Export it.
* src/schema.sql (Inputs): New table that refers to the Specifications table.
(Specifications): Move input related fields to the Inputs table. Rename
REPO_NAME to NAME. Rename ARGUMENTS to PROC_ARGS. Rename FILE to PROC_PATH.
Add LOAD_PATH_INPUTS, PACKAGE_PATH_INPUTS and PROC_INPUT fields that refer to
the Inputs table.
(Stamps): Rename REPO_NAME to NAME.
(Evaluations): Rename REPO_NAME to NAME. Rename REVISION to COMMITS.
(Specifications_index): Replace with Inputs_index.
* src/sql/upgrade-2.sql: New file.
* tests/database.scm (example-spec, make-dummy-eval, sqlite-exec): Adapt to
the new specifications format. Rename REVISION to COMMITS.
* tests/http.scm (evaluations-query-result, fill-db): Idem.
---
bin/evaluate.in | 119 +++++++++++++++-------
doc/cuirass.texi | 147 +++++++++++++++++----------
examples/guix-jobs.scm | 38 ++++---
examples/guix-track-git.scm | 26 ++---
examples/hello-git.scm | 55 +++++------
examples/hello-singleton.scm | 28 +++---
examples/hello-subset.scm | 39 +++++---
examples/random-jobs.scm | 7 +-
examples/random.scm | 17 ++--
src/cuirass/base.scm | 186 ++++++++++++++++++++---------------
src/cuirass/database.scm | 115 ++++++++++++++--------
src/cuirass/utils.scm | 1 +
src/schema.sql | 28 ++++--
src/sql/upgrade-2.sql | 78 +++++++++++++++
tests/database.scm | 39 +++++---
tests/http.scm | 26 ++---
16 files changed, 613 insertions(+), 336 deletions(-)
create mode 100644 src/sql/upgrade-2.sql

Toggle diff (389 lines)
diff --git a/bin/evaluate.in b/bin/evaluate.in
index 86d0e83..14ff52f 100644
--- a/bin/evaluate.in
+++ b/bin/evaluate.in
@@ -27,37 +27,99 @@ exec ${GUILE:-@GUILE@} --no-auto-compile -e main -s "$0" "$@"
;; Note: Do not use any Guix modules (see below).
(use-modules (ice-9 match)
- (ice-9 pretty-print))
+ (ice-9 pretty-print)
+ (srfi srfi-1)
+ (srfi srfi-26))
(define (ref module name)
"Dynamically link variable NAME under MODULE and return it."
(let ((m (resolve-interface module)))
(module-ref m name)))
-(define %not-colon
- (char-set-complement (char-set #\:)))
+(define (absolutize directory load-path)
+ (if (string-prefix? "/" load-path)
+ load-path
+ (string-append directory "/" load-path)))
+
+(define (find-checkout checkouts input-name)
+ (find (lambda (checkout)
+ (string=? (assq-ref checkout #:name)
+ input-name))
+ checkouts))
+
+(define (get-proc-source spec checkouts)
+ (let* ((input-name (assq-ref spec #:proc-input))
+ (checkout (find-checkout checkouts input-name)))
+ (assq-ref checkout #:directory)))
+
+(define (get-load-path spec checkouts)
+ (map (lambda (input-name)
+ (let* ((checkout (find-checkout checkouts input-name))
+ (directory (assq-ref checkout #:directory))
+ (load-path (assq-ref checkout #:load-path)))
+ (absolutize directory load-path)))
+ (assq-ref spec #:load-path-inputs)))
+
+(define (get-guix-package-path spec checkouts)
+ (let* ((input-names (assq-ref spec #:package-path-inputs))
+ (checkouts (map (cut find-checkout checkouts <>) input-names)))
+ (string-join
+ (map
+ (lambda (checkout)
+ (let ((directory (assq-ref checkout #:directory))
+ (load-path (assq-ref checkout #:load-path)))
+ (absolutize directory load-path)))
+ checkouts)
+ ":")))
+
+(define (format-checkouts checkouts)
+ "Format checkouts the way Hydra does: #:NAME becomes the key as a symbol,
+#:DIRECTORY becomes FILE-NAME and #:COMMIT becomes REVISION. The other
+entries are added because they could be useful during the evaluation."
+ (map
+ (lambda (checkout)
+ (let loop ((in checkout)
+ (out '())
+ (name #f))
+ (match in
+ (()
+ (cons name out))
+ (((#:name . val) . rest)
+ (loop rest out (string->symbol val)))
+ (((#:directory . val) . rest)
+ (loop rest (cons `(file-name . ,val) out) name))
+ (((#:commit . val) . rest)
+ (loop rest (cons `(revision . ,val) out) name))
+ (((keyword . val) . rest)
+ (loop rest (cons `(,(keyword->symbol keyword) . ,val) out) name)))))
+ checkouts))
+
+(define (append-paths . paths)
+ (string-join (delete "" paths) ":"))
(define* (main #:optional (args (command-line)))
(match args
- ((command load-path guix-package-path source specstr)
- ;; Load FILE, a Scheme file that defines Hydra jobs.
+ ((command static-guix-package-path specstr checkoutsstr)
+ ;; Load PROC-FILE, a Scheme file that defines Hydra jobs.
;;
- ;; Until FILE is loaded, we must *not* load any Guix module because
- ;; SOURCE may be providing its own, which could differ from ours--this is
- ;; the case when SOURCE is a Guix checkout. The 'ref' procedure helps us
- ;; achieve this.
- (let ((%user-module (make-fresh-user-module))
- (spec (with-input-from-string specstr read))
- (stdout (current-output-port))
- (stderr (current-error-port))
- (load-path (string-tokenize load-path %not-colon)))
- (unless (string-null? guix-package-path)
- (setenv "GUIX_PACKAGE_PATH" guix-package-path))
+ ;; Until PROC-FILE is loaded, we must *not* load any Guix module because
+ ;; the user may be providing its own with #:LOAD-PATH-INPUTS, which could
+ ;; differ from ours. The 'ref' procedure helps us achieve this.
+ (let* ((%user-module (make-fresh-user-module))
+ (spec (with-input-from-string specstr read))
+ (checkouts (with-input-from-string checkoutsstr read))
+ (proc-source (get-proc-source spec checkouts))
+ (load-path (get-load-path spec checkouts))
+ (guix-package-path (get-guix-package-path spec checkouts))
+ (stdout (current-output-port))
+ (stderr (current-error-port)))
+ (setenv "GUIX_PACKAGE_PATH"
+ (append-paths static-guix-package-path guix-package-path))
;; Since we have relative file name canonicalization by default, better
- ;; change to SOURCE to make sure things like 'include' with relative
- ;; file names work as expected.
- (chdir source)
+ ;; change to PROC-SOURCE to make sure things like 'include' with
+ ;; relative file names work as expected.
+ (chdir proc-source)
;; Change '%load-path' once and for all. We need it to be effective
;; both when we load SPEC's #:file and when we later call the thunks.
@@ -66,7 +128,7 @@ exec ${GUILE:-@GUILE@} --no-auto-compile -e main -s "$0" "$@"
(save-module-excursion
(lambda ()
(set-current-module %user-module)
- (primitive-load (assq-ref spec #:file))))
+ (primitive-load (assq-ref spec #:proc-path))))
;; From there on we can access Guix modules.
@@ -93,22 +155,13 @@ building things during evaluation~%")
(apply real-build-things store args))))
;; Call the entry point of FILE and print the resulting job sexp.
- ;; Among the arguments, always pass 'file-name' and 'revision' like
- ;; Hydra does.
(let* ((proc-name (assq-ref spec #:proc))
(proc (module-ref %user-module proc-name))
- (commit (assq-ref spec #:current-commit))
- (name (assq-ref spec #:name))
- (args `((guix
- (revision . ,commit)
- (file-name . ,source))
- ,@(or (assq-ref spec #:arguments) '())))
- (thunks (proc store args))
- (eval `((#:specification . ,name)
- (#:revision . ,commit))))
+ (args `(,@(format-checkouts checkouts)
+ ,@(or (assq-ref spec #:proc-args) '())))
+ (thunks (proc store args)))
(pretty-print
- `(evaluation ,eval
- ,(map (lambda (thunk) (thunk))
+ `(evaluation ,(map (lambda (thunk) (thunk))
thunks))
stdout)))))
((command _ ...)
diff --git a/doc/cuirass.texi b/doc/cuirass.texi
index 5c8c23f..308518e 100644
--- a/doc/cuirass.texi
+++ b/doc/cuirass.texi
@@ -105,10 +105,10 @@ basis of the @dfn{Continuous integration} practice.
@chapter Overview
@command{cuirass} acts as a daemon polling @acronym{VCS, version control
-system} repositories for changes, and evaluating a derivation when
-something has changed (@pxref{Derivations, Derivations,, guix, Guix}).
-As a final step the derivation is realized and the result of that build
-allows you to know if the job succeeded or not.
+system} repositories (called @code{inputs}) for changes, and evaluating a
+derivation when an @code{input} has changed (@pxref{Derivations, Derivations,,
+guix, Guix}). As a final step the derivation is realized and the result of
+that build allows you to know if the job succeeded or not.
What is actually done by @command{cuirass} is specified in a @dfn{job
specification} which is represented as an association list which is a
@@ -116,20 +116,40 @@ basic and traditional Scheme data structure. Here is an example of what
a specification might look like:
@lisp
- `((#:name . "hello")
- (#:url . "git://git.savannah.gnu.org/guix.git")
- (#:branch . "master")
- (#:no-compile? . #t)
- (#:load-path . ".")
+ '((#:name . "foo-master")
+ (#:load-path-inputs . ("guix"))
+ (#:package-path-inputs . ("packages"))
+ (#:proc-input . "conf")
+ (#:proc-path . "drv-list.scm")
(#:proc . cuirass-jobs)
- (#:file . "/tmp/drv-file.scm")
- (#:arguments (subset . "hello")))
+ (#:proc-args (subset . "foo"))
+ (#:inputs . (((#:name . "guix")
+ (#:url . "git://git.savannah.gnu.org/guix.git")
+ (#:load-path . ".")
+ (#:branch . "master")
+ (#:no-compile? . #t))
+ ((#:name . "conf")
+ (#:url . "git://my-personal-conf.git")
+ (#:load-path . ".")
+ (#:branch . "master")
+ (#:no-compile? . #t))
+ ((#:name . "packages")
+ (#:url . "git://my-custom-packages.git")
+ (#:load-path . ".")
+ (#:branch . "master")
+ (#:no-compile? . #t)))))
@end lisp
In this specification the keys are Scheme keywords which have the nice
property of being self evaluating. This means that they can't refer to
another value like symbols do.
+There are three @code{inputs}: one tracking the Guix repository, one tracking
+the repository containing the @code{proc}, and one tracking the repository
+containing the custom packages (see @code{GUIX_PACKAGE_PATH}).
+@code{#:load-path-inputs}, @code{#:package-path-inputs} and
+@code{#:proc-input} refer to these inputs by their name.
+
@quotation Note
@c This refers to
@c <https://github.com/libgit2/libgit2sharp/issues/1094#issuecomment-112306072>.
@@ -229,47 +249,70 @@ Cuirass uses a SQLite database to store information about jobs and past
build results, but also to coordinate the execution of jobs.
The database contains the following tables: @code{Specifications},
-@code{Stamps}, @code{Evaluations}, @code{Derivations}, @code{Builds} and
-@code{SchemaVersion}. The purpose of each of these tables is explained below.
+@code{Inputs}, @code{Stamps}, @code{Evaluations}, @code{Derivations},
+@code{Builds} and @code{SchemaVersion}. The purpose of each of these tables
+is explained below.
@section Specifications
@cindex specifications, database
-This table stores specifications describing the repository from whence
+This table stores specifications describing the repositories from whence
Cuirass fetches code and the environment in which it will be processed.
Entries in this table must have values for the following text fields:
@table @code
-@item repo_name
-This field holds the name of the repository. This field is also the
-primary key of this table. Although this field is called
-@code{repo_name} in the database, it's called @code{name} in the
-specification itself.
-
-@item url
-The URL of the repository.
+@item name
+This field holds the name of the specification. This field is also the
+primary key of this table.
-@item load_path
-This field holds a colon-separated list of directories that are
-prepended to the Guile load path when evaluating @code{file} (see
-below.)
+@item load_path_inputs
+This field holds a list of input names whose load path is prepended to Guile's
+@code{%load-path} when evaluating @code{proc_path}.
-Each entry that is not an absolute file name is interpreted relative to
-the source code checkout. Often, @code{load_path} has just one entry,
-@code{"."}.
+@item package_path_inputs
+This field holds a list of input names whose load path is prepended to
+@code{GUIX_PACKAGE_PATH} when evaluating @code{proc_path}.
-When @code{load_path} is empty, the load path is left unchanged.
+@item proc_input
+The name of the input containing @code{proc}.
-@item file
-The absolute name of the Scheme file containing PROC.
+@item proc_path
+The path of the Scheme file containing @code{proc}, relative to
+@code{proc_input}.
@item proc
-This text field holds the name of the procedure in the Scheme file FILE
-that produces a list of jobs.
+This text field holds the name of the procedure in the Scheme file
+@code{proc_path} that produces a list of jobs.
+
+@item proc_args
+A list of arguments to be passed to @code{proc}. This can be used to produce
+a different set of jobs using the same @code{proc}.
+@end table
+
+@section Inputs
+@cindex inputs, database
+
+This table stores the data related to the repositories that are periodically
+fetched by Cuirass. Entries in this table must have values for the following
+text fields:
+
+@table @code
+@item specification
+This field holds the name of the specification from the @code{Specifications}
+table associated with the input. Every input belongs to a specification, and
+that specification can refer to its inputs.
+
+@item name
+This field holds the name of the input. That name can be used as a key by the
+@code{proc} if it needs access to its resulting checkout.
+
+@item url
+The URL of the repository.
+
+@item load_path
+Used by a specification when it refers to an input's load path. See
+@code{load_path_inputs} and @code{package_path_inputs}.
-@item arguments
-A list of arguments to be passed to PROC. This can be used to produce a
-different set of jobs using the same PROC.
@end table
The following columns are optional:
@@ -280,13 +323,12 @@ This text field determines which branch of the repository Cuirass should
check out.
@item tag
-This text field is an alternative to using BRANCH or REVISION. It tells
-Cuirass to check out the repository at the specified tag.
+This text field is an alternative to using @code{branch} or @code{revision}.
+It tells Cuirass to check out the repository at the specified tag.
@item revision
-This text field is an alternative to using BRANCH or TAG. It tells
-Cuirass to check out the repository at a particular revision. In the
-case of a git repository this would be a commit hash.
+This text field is an alternative to using @code{branch} or @code{tag}. It
+tells Cuirass to check out the repository at a particular commit.
@item no_compile_p
When this integer field holds the value @code{1} Cuirass will skip
@@ -296,14 +338,13 @@ compilation for the specified repository.
@section Stamps
@cindex stamps, database
-When a specification is processed, the repository must be downloaded at
-a certain revision as specified. The @code{Stamps} table stores the
-current revision for every specification when it is being processed.
+When a specification is processed, the repositories must be downloaded at a
+certain revision as specified. The @code{Stamps} table stores the current
+revisions for every specification when it is being processed.
-The table only has two text columns: @code{specification}, which
-references a specification from the @code{Specifications} table via the
-field @code{repo_name}, and @code{stamp}, which holds the revision
-(e.g. a commit hash).
+The table only has two text columns: @code{specification}, which references a
+specification from the @code{Specifications} table via the field @code{name},
+and @code{stamp}, which holds the revisions (space separated commit hashes).
@section Evaluations
@cindex evaluations, database
@@ -319,12 +360,12 @@ The @code{Evaluations} table has the following columns:
This is an automatically incrementing numeric identifier.
@item specification
-This field holds the @code{repo_name} of a specification from the
+This field holds the @code{name} of a specification from the
@code{Specifications} table.
-@item revision
-This text field holds the revision string (e.g. a git commit) of the
-repository specified in the related specification.
+@item commits
+This text field holds the revisions (space separated commit hashes) of the
+repositories specified as inputs of the related specification.
@end table
@section Derivations
diff --git a/examples/guix-jobs.scm b/examples/guix-jobs.scm
index 862cff7..4a01b66 100644
--- a/examples/guix-jobs.scm
+++ b/examples/guix-jobs.scm
@@ -1,5 +1,6 @@
;;; guix-jobs.scm -- job specification test for Guix
;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
+;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
;;;
;;; This file is part o
This message was truncated. Download the full message here.
L
L
Ludovic Courtès wrote on 13 Jul 2018 10:32
Re: [bug#32121] [PATCH 1/5] base: Compile CHECKOUT in the fiber.
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 32121@debbugs.gnu.org)
87in5jmt7a.fsf@gnu.org
Morning!

Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (6 lines)
> Because it may take time and thus prevent PROCESS-SPECS to run every INTERVAL
> seconds.
>
> * src/cuirass/base.scm (process-specs): move the COMPILE invocation inside
> SPAWN-FIBER's thunk. Add log message.

[...]

Toggle quote (9 lines)
> - (when compile?
> - (non-blocking (compile checkout)))
> -
> (spawn-fiber
> (lambda ()
> + (when compile?
> + (log-message "compiling '~a' with commit ~s" name commit)
> + (non-blocking (compile checkout)))

I think this doesn’t bring anything compared to the existing
‘non-blocking’ call.

The ‘non-blocking’ procedure evaluates its argument in a separate
thread; the calling fiber then “waits” for a message from that thread,
which it gets when the computation is over. The ‘get-message’ is
non-blocking though: the calling fiber is simply unscheduled until the
message has arrived.

Does that make sense?

Ludo’.
L
L
Ludovic Courtès wrote on 13 Jul 2018 10:35
Re: [bug#32121] [PATCH 2/5] utils: Reset the Fiber dynamic environment in %NON-BLOCKING.
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 32121@debbugs.gnu.org)
87bmbbmt2x.fsf@gnu.org
Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (6 lines)
> * src/cuirass/utils.scm (%non-blocking): Wrap body in PARAMETERIZE form that
> clears CURRENT-FIBER.
>
> So that PUT-MESSAGE doesn't try to suspend itself within CALL-WITH-NEW-THREAD.
> See https://lists.gnu.org/archive/html/guile-devel/2018-07/msg00009.html.

Good catch!

Toggle quote (3 lines)
> + (parameterize (((@@ (fibers internal) current-fiber) #f))
> + (let ((channel (make-channel)))

Instead of using @@, I think you can add an explicit:

#:use-module ((fibers internal) #:select (current-fiber))

at the top.

OK with this change!

Could you also report the issue to Andy (there’s a GitHub thing or you
can email guile-user I guess)?

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 13 Jul 2018 10:47
Re: [bug#32121] [PATCH 3/5] database: Add support for database upgrades.
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 32121@debbugs.gnu.org)
87sh4nldxv.fsf@gnu.org
Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (10 lines)
> * Makefile.am: Copy SQL files into their data directory.
> * doc/cuirass.texi (Database schema): Document the change.
> * src/cuirass/database.scm (%package-sql-dir): New parameter.
> (db-load, db-get-version, db-set-version, get-target-version,
> get-upgrade-file, db-upgrade): New procedures.
> (db-init): Set version corresponding to the existing upgrade-n.sql files.
> (db-open): If database exists, upgrade it.
> * src/schema.sql: New file.
> * src/sql/upgrade-1.sql: New file.

Awesome!

What follows is nitpicking, but the patch otherwise LGTM!

For Makefile.am, please specify the new variables explicitly in the
commit log.

Toggle quote (3 lines)
> dist_pkgdata_DATA = src/schema.sql
> +dist_sql_DATA = src/sql/upgrade-*.sql

This won’t really work; you have to explicitly list the files (or use
$(wildcard …), but I have a slight preference for an explicit list.)

Toggle quote (1 lines)
> +@section SchemaVersion
^
Please add a space here…

Toggle quote (2 lines)
> +This table keeps track of the schema version. During the initialization, the

… and here s/This table/The @code{SchemaVersion} table/

Toggle quote (2 lines)
> +(define (db-get-version db)

Rather ‘db-schema-version’? Also, please consider adding docstrings to
top-level procedures.

Toggle quote (2 lines)
> +(define (db-set-version db version)

Likewise: ‘db-set-schema-version’.

Toggle quote (2 lines)
> +(define (get-target-version)

‘latest-db-schema-version’ maybe? :-)

Toggle quote (8 lines)
> + (apply max
> + (map string->number
> + (map (cut match:substring <> 1)
> + (filter regexp-match?
> + (map (cut string-match
> + "^upgrade-([0-9]+)\\.sql$" <>)
> + (scandir (%package-sql-dir))))))))

I think you can write it along these lines:

(reduce max 0
(map (compose string->number (cut match:substring <> 1))
(filter-map (cut string-match …) (scandir …))))

Toggle quote (2 lines)
> +(define (get-upgrade-file version)

‘schema-upgrade-file’?

Toggle quote (3 lines)
> +(define (db-upgrade db)
> + (do-ec (:range version (db-get-version db) (get-target-version))

I would rather avoid SRFI-42, not just because I can’t parse it ;-), but
also to maintain consistency and make the code possibly more accessible.

In this case I think we could use a simple loop or (for-each … (iota n))
and that wouldn’t be bad.

WDYT?

Thank you!

Ludo’.
L
L
Ludovic Courtès wrote on 13 Jul 2018 10:51
Re: [bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'.
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 32121@debbugs.gnu.org)
87muuvldrv.fsf@gnu.org
Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (15 lines)
> This removes the possibility to filter specifications by branch, because
> branches were previously called 'jobset'. But it doesn't matter because later
> on, specifications will have as many branches as inputs. And people should
> filter by specification name instead.
>
> * doc/cuirass.texi (Build Information, Latest builds): Remove 'jobset',
> replace 'project' with 'jobset'.
> * src/cuirass/http.scm (build->hydra-build): Idem.
> * tests/database.scm (db-get-builds): Idem.
> * tests/http.scm (build-query-result, /api/latestbuilds?nr=1&jobset=guix,
> /api/latestbuilds?nr=1&jobset=gnu): Idem.
> * src/cuirass/database.scm (db-format-build, db-get-builds): Don't associate
> builds with branches (which were called 'jobset' afterwards).
> (db-get-builds): Remove the #:project filter.

To make sure I understand correctly: it’ll still be possible to have,
say, a “guix” job or a “modular” job built with several different
branches, right?

I think we should try to keep the HTTP API compatible with Hydra so we
don’t break guix-hydra.el and possibly Tatiana’s work.

WDYT?

Ludo’.
C
C
Clément Lassieur wrote on 13 Jul 2018 10:55
Re: [bug#32121] [PATCH 1/5] base: Compile CHECKOUT in the fiber.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 32121@debbugs.gnu.org)
87y3ef1pmo.fsf@lassieur.org
Morning :-)

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (31 lines)
> Morning!
>
> Clément Lassieur <clement@lassieur.org> skribis:
>
>> Because it may take time and thus prevent PROCESS-SPECS to run every INTERVAL
>> seconds.
>>
>> * src/cuirass/base.scm (process-specs): move the COMPILE invocation inside
>> SPAWN-FIBER's thunk. Add log message.
>
> [...]
>
>> - (when compile?
>> - (non-blocking (compile checkout)))
>> -
>> (spawn-fiber
>> (lambda ()
>> + (when compile?
>> + (log-message "compiling '~a' with commit ~s" name commit)
>> + (non-blocking (compile checkout)))
>
> I think this doesn’t bring anything compared to the existing
> ‘non-blocking’ call.
> The ‘non-blocking’ procedure evaluates its argument in a separate
> thread; the calling fiber then “waits” for a message from that thread,
> which it gets when the computation is over. The ‘get-message’ is
> non-blocking though: the calling fiber is simply unscheduled until the
> message has arrived.
>
> Does that make sense?

Well, no :-)

My understanding is that non-blocking is, actually... blocking, because
get-message is blocking. (It doesn't block the scheduler because it's
in another thread, but that's not the problem here.)

What I wanted to fix here is the fact that if the build takes one hour,
we will block for one hour in the COMPILE call, and process-spec won't
return for one hour. If it doesn't return for one hour, that means we
can't evaluate anything else for all that time.

With my change, the one-hour call will be in the fiber, which means that
process-spec can return, and other evaluations can be processed.

But this is untested (because compilation doesn't work IIRC), so I can't
be sure.

Clément
L
L
Ludovic Courtès wrote on 13 Jul 2018 11:28
Re: [bug#32121] [PATCH 5/5] Add support for multiple inputs.
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 32121@debbugs.gnu.org)
87va9jjxgr.fsf@gnu.org
Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (49 lines)
> * bin/evaluate.in (absolutize, find-checkout, get-proc-source, get-load-path,
> get-guix-package-path, format-checkouts, append-paths): New procedures.
> (%not-colon): Remove variable.
> (main): Take the load path, package path and PROC from the checkouts that
> result from the inputs. Format the checkouts before sending them to the
> procedure.
> * doc/cuirass.texi (Overview, Database schema): Document the changes.
> * examples/{guix-jobs.scm, hello-git.scm, hello-singleton.scm,
> hello-subset.scm, random.scm}: Adapt to the new specification format.
> * examples/guix-track-git.scm (package->spec): Rename to PACKAGE->INPUT.
> (package->git-tracked): Replace FETCH-REPOSITORY with FETCH-INPUT and handle
> the new format of its return value.
> * examples/random-jobs.scm (make-random-jobs): Rename RANDOM to CHECKOUT.
> Rename the checkout from 'random (which is a specification) to 'cuirass (which
> is a checkout resulting from an input).
> * src/cuirass/base.scm (fetch-repository): Rename to fetch-input. Rename SPEC
> to INPUT. Return a checkout object instead of returning two values.
> (evaluate): Take a list of CHECKOUTS and COMMITS as arguments, instead of
> SOURCE. Remove TOKENIZE and LOAD-PATH. Pass the CHECKOUTS instead of the
> SOURCE to "evaluate". Build the EVAL object instead of getting it from
> "evaluate".
> (compile?, fetch-inputs, compile-checkouts): New procedures.
> (process-specs): Fetch all inputs instead of only fetching one repository.
> The result of that fetching operation is a list of CHECKOUTS whose COMMITS are
> used as a STAMP.
> * src/cuirass/database.scm (db-add-input, db-get-inputs): New procedures.
> (db-add-specification, db-get-specifications): Adapt to the new specification
> format. Add/get all inputs as well.
> (db-add-evaluation): Rename REVISION to COMMITS. Store COMMITS as space
> separated commit hashes.
> (db-get-builds): Rename REPO_NAME to NAME.
> (db-get-stamp): Rename COMMIT to STAMP. Return #f when there is no STAMP.
> (db-add-stamp): Rename COMMIT to STAMP. Deal with DB-GET-STAMP's new return
> value.
> (db-get-evaluations): Rename REVISION to COMMITS. Tokenize COMMITS.
> * src/cuirass/utils.scm (%non-blocking): Export it.
> * src/schema.sql (Inputs): New table that refers to the Specifications table.
> (Specifications): Move input related fields to the Inputs table. Rename
> REPO_NAME to NAME. Rename ARGUMENTS to PROC_ARGS. Rename FILE to PROC_PATH.
> Add LOAD_PATH_INPUTS, PACKAGE_PATH_INPUTS and PROC_INPUT fields that refer to
> the Inputs table.
> (Stamps): Rename REPO_NAME to NAME.
> (Evaluations): Rename REPO_NAME to NAME. Rename REVISION to COMMITS.
> (Specifications_index): Replace with Inputs_index.
> * src/sql/upgrade-2.sql: New file.
> * tests/database.scm (example-spec, make-dummy-eval, sqlite-exec): Adapt to
> the new specifications format. Rename REVISION to COMMITS.
> * tests/http.scm (evaluations-query-result, fill-db): Idem.

Wow, that’s intimidating. :-)

Toggle quote (7 lines)
> (define* (main #:optional (args (command-line)))
> (match args
> - ((command load-path guix-package-path source specstr)
> - ;; Load FILE, a Scheme file that defines Hydra jobs.
> + ((command static-guix-package-path specstr checkoutsstr)
> + ;; Load PROC-FILE, a Scheme file that defines Hydra jobs.

There’s no “proc-file”; should it be “proc-source”?

Toggle quote (25 lines)
> - ;; Until FILE is loaded, we must *not* load any Guix module because
> - ;; SOURCE may be providing its own, which could differ from ours--this is
> - ;; the case when SOURCE is a Guix checkout. The 'ref' procedure helps us
> - ;; achieve this.
> - (let ((%user-module (make-fresh-user-module))
> - (spec (with-input-from-string specstr read))
> - (stdout (current-output-port))
> - (stderr (current-error-port))
> - (load-path (string-tokenize load-path %not-colon)))
> - (unless (string-null? guix-package-path)
> - (setenv "GUIX_PACKAGE_PATH" guix-package-path))
> + ;; Until PROC-FILE is loaded, we must *not* load any Guix module because
> + ;; the user may be providing its own with #:LOAD-PATH-INPUTS, which could
> + ;; differ from ours. The 'ref' procedure helps us achieve this.
> + (let* ((%user-module (make-fresh-user-module))
> + (spec (with-input-from-string specstr read))
> + (checkouts (with-input-from-string checkoutsstr read))
> + (proc-source (get-proc-source spec checkouts))
> + (load-path (get-load-path spec checkouts))
> + (guix-package-path (get-guix-package-path spec checkouts))
> + (stdout (current-output-port))
> + (stderr (current-error-port)))
> + (setenv "GUIX_PACKAGE_PATH"
> + (append-paths static-guix-package-path guix-package-path))

Do I get it write that inputs do not necessarily contribute to
GUIX_PACKAGE_PATH?

Some inputs may provide code (to be in %load-path) while not provide any
package definition (so nothing to add to GUIX_PACKAGE_PATH.)

Toggle quote (8 lines)
> ;; Since we have relative file name canonicalization by default, better
> - ;; change to SOURCE to make sure things like 'include' with relative
> - ;; file names work as expected.
> - (chdir source)
> + ;; change to PROC-SOURCE to make sure things like 'include' with
> + ;; relative file names work as expected.
> + (chdir proc-source)

As a rule of thumb, identifiers for local variables should, IMO, almost
always be a single word or at most two words. Long names like
‘static-guix-package-path’ in local scope tend to make code harder to
read; ‘proc-source’ here should probably be ‘source’ because we know
what it is we’re talking about.

Toggle quote (6 lines)
> (save-module-excursion
> (lambda ()
> (set-current-module %user-module)
> - (primitive-load (assq-ref spec #:file))))
> + (primitive-load (assq-ref spec #:proc-path))))

Nitpick: in GNU “path” means “search path” (a list of directories), so
here I think it should be “file” or “file name”, not “path”.

Toggle quote (7 lines)
> @command{cuirass} acts as a daemon polling @acronym{VCS, version control
> -system} repositories for changes, and evaluating a derivation when
> -something has changed (@pxref{Derivations, Derivations,, guix, Guix}).
> -As a final step the derivation is realized and the result of that build
> -allows you to know if the job succeeded or not.
> +system} repositories (called @code{inputs}) for changes, and evaluating a

s/@code/@dfn/

Toggle quote (2 lines)
> +derivation when an @code{input} has changed (@pxref{Derivations, Derivations,,

s/@code//

@code is to refer to identifiers in the code, things like that.

Toggle quote (2 lines)
> +There are three @code{inputs}: one tracking the Guix repository, one tracking

s/@code//

Toggle quote (21 lines)
> +(define (compile-checkouts spec all-checkouts)
> + (let* ((checkouts (filter compile? all-checkouts))
> + (thunks
> + (map
> + (lambda (checkout)
> + (lambda ()
> + (log-message "compiling input '~a' of spec '~a' (commit ~s)"
> + (assq-ref checkout #:name)
> + (assq-ref spec #:name)
> + (assq-ref checkout #:commit))
> + (compile checkout)))
> + checkouts))
> + (results (par-map %non-blocking thunks)))
> + (map (lambda (checkout)
> + (log-message "compiled input '~a' of spec '~a' (commit ~s)"
> + (assq-ref checkout #:name)
> + (assq-ref spec #:name)
> + (assq-ref checkout #:commit))
> + checkout)
> + results)))

Since the return value is unused, we could perhaps make it:

(define (compile-checkouts spec checkouts)
(for-each (lambda (checkout)
(log-message …)
(non-blocking (compile checkout)))
checkouts))

and move the ‘filter’ call to the call site (the job of
‘compile-checkouts’, one might think, is to compile what it’s given, not
to filter things.)

I think that’s about it.

The size of reviews is often inversely proportional to the size of the
change, and I think this one is no exception. :-)

I’m not fully up-to-speed on all the changes but I’ll guess we’ll see it
live when we upgrade Cuirass on berlin.

Thank you!

Ludo’.
C
C
Clément Lassieur wrote on 13 Jul 2018 11:35
Re: [bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 32121@debbugs.gnu.org)
87wotz1nso.fsf@lassieur.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (21 lines)
> Clément Lassieur <clement@lassieur.org> skribis:
>
>> This removes the possibility to filter specifications by branch, because
>> branches were previously called 'jobset'. But it doesn't matter because later
>> on, specifications will have as many branches as inputs. And people should
>> filter by specification name instead.
>>
>> * doc/cuirass.texi (Build Information, Latest builds): Remove 'jobset',
>> replace 'project' with 'jobset'.
>> * src/cuirass/http.scm (build->hydra-build): Idem.
>> * tests/database.scm (db-get-builds): Idem.
>> * tests/http.scm (build-query-result, /api/latestbuilds?nr=1&jobset=guix,
>> /api/latestbuilds?nr=1&jobset=gnu): Idem.
>> * src/cuirass/database.scm (db-format-build, db-get-builds): Don't associate
>> builds with branches (which were called 'jobset' afterwards).
>> (db-get-builds): Remove the #:project filter.
>
> To make sure I understand correctly: it’ll still be possible to have,
> say, a “guix” job or a “modular” job built with several different
> branches, right?

Yes, you can have a specification "guix-modular-master" whose Guix
input's branch will be "master", and a specification
"guix-modular-core-updates" whose Guix input's branch will be
"core-updates".

Toggle quote (3 lines)
> I think we should try to keep the HTTP API compatible with Hydra so we
> don’t break guix-hydra.el and possibly Tatiana’s work.

This will somehow break a minor part of Tatiana's work because the main
page will look like

Toggle snippet (8 lines)
Projects/Specifications

| Name | Branch |
|--------------+--------------|
| guix-modular | master |
| guix-modular | core-updates |

instead of

Toggle snippet (8 lines)
Projects/Specifications

| Name |
|---------------------------|
| guix-modular-master |
| guix-modular-core-updates |

But to me it's not a problem. The branch is an implementation detail
and it's hidden. Instead the information is in the name of the
specification.

Note that we will have control over the specification name, which wasn't
possible before because it was used by the evaluator. Now the evaluator
uses the input's name.

It's not possible to keep the exact same API as hydra because we don't
have projects. We could put everything under the same static project,
but it wouldn't really make sense.

However, we could still be able to bind a specification to a branch, but
that would require adding a 'guix-input' specification field, so that
the specification knows which input is the one whose branch should be
displayed. I doubt it's worth it though. Or we could replace the
'load-path-inputs' field with a 'guix-input' field. That was kind of
the point of the 3rd part of my initial message[1]. Or, we could
automate things: find out from which input the Guix modules come. That
would be a bit tricky.

WDYT?

Clément

C
C
Clément Lassieur wrote on 13 Jul 2018 11:43
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 32121@debbugs.gnu.org)
87va9j1nep.fsf@lassieur.org
Clément Lassieur <clement@lassieur.org> writes:

Toggle quote (3 lines)
> This will somehow break a minor part of Tatiana's work because the main
> page will look like

Correction:

It used to look like:

Toggle quote (11 lines)
> --8<---------------cut here---------------start------------->8---
> Projects/Specifications
>
> | Name | Branch |
> |--------------+--------------|
> | guix-modular | master |
> | guix-modular | core-updates |
> --8<---------------cut here---------------end--------------->8---
>
> instead of

And it would look like:.

Toggle quote (9 lines)
> --8<---------------cut here---------------start------------->8---
> Projects/Specifications
>
> | Name |
> |---------------------------|
> | guix-modular-master |
> | guix-modular-core-updates |
> --8<---------------cut here---------------end--------------->8---

instead.
L
L
Ludovic Courtès wrote on 13 Jul 2018 13:50
Re: [bug#32121] [PATCH 1/5] base: Compile CHECKOUT in the fiber.
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 32121@debbugs.gnu.org)
87fu0njqwy.fsf@gnu.org
Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (44 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Morning!
>>
>> Clément Lassieur <clement@lassieur.org> skribis:
>>
>>> Because it may take time and thus prevent PROCESS-SPECS to run every INTERVAL
>>> seconds.
>>>
>>> * src/cuirass/base.scm (process-specs): move the COMPILE invocation inside
>>> SPAWN-FIBER's thunk. Add log message.
>>
>> [...]
>>
>>> - (when compile?
>>> - (non-blocking (compile checkout)))
>>> -
>>> (spawn-fiber
>>> (lambda ()
>>> + (when compile?
>>> + (log-message "compiling '~a' with commit ~s" name commit)
>>> + (non-blocking (compile checkout)))
>>
>> I think this doesn’t bring anything compared to the existing
>> ‘non-blocking’ call.
>> The ‘non-blocking’ procedure evaluates its argument in a separate
>> thread; the calling fiber then “waits” for a message from that thread,
>> which it gets when the computation is over. The ‘get-message’ is
>> non-blocking though: the calling fiber is simply unscheduled until the
>> message has arrived.
>>
>> Does that make sense?
>
> Well, no :-)
>
> My understanding is that non-blocking is, actually... blocking, because
> get-message is blocking. (It doesn't block the scheduler because it's
> in another thread, but that's not the problem here.)
>
> What I wanted to fix here is the fact that if the build takes one hour,
> we will block for one hour in the COMPILE call, and process-spec won't
> return for one hour. If it doesn't return for one hour, that means we
> can't evaluate anything else for all that time.

Oh, I see. However we have to wait for compilation to complete before
continuing anyway, no?

Toggle quote (6 lines)
> With my change, the one-hour call will be in the fiber, which means that
> process-spec can return, and other evaluations can be processed.
>
> But this is untested (because compilation doesn't work IIRC), so I can't
> be sure.

Yeah, what about this plan: let’s forget about this patch, and let’s
remove support for compilation altogether in a future patch.

WDYT?

Ludo’.
L
L
Ludovic Courtès wrote on 13 Jul 2018 13:56
Re: [bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'.
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 32121@debbugs.gnu.org)
87y3efic2b.fsf@gnu.org
Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (20 lines)
> This will somehow break a minor part of Tatiana's work because the main
> page will look like
>
> Projects/Specifications
>
> | Name | Branch |
> |--------------+--------------|
> | guix-modular | master |
> | guix-modular | core-updates |
>
>
> instead of
>
> Projects/Specifications
>
> | Name |
> |---------------------------|
> | guix-modular-master |
> | guix-modular-core-updates |

So we’d be moving the project/branch structure to naming conventions.

In a way, that’s not great, because as users we like to think in terms
of branches to answer questions like “how many packages failed in branch
X of the Savannah repo?”.

However, this can probably be addressed at the UI level: the web UI and
guix-hydra.el could list (shortened) repo URLs and branch names instead
of this ‘name’ field. Eventually, we could remove this ‘name’ field
altogether and instead have an automatically-assigned numerical ID.

WDYT?

(This does not affect this patch series, I’m thinking about what we can
do eventually.)

Toggle quote (9 lines)
> However, we could still be able to bind a specification to a branch, but
> that would require adding a 'guix-input' specification field, so that
> the specification knows which input is the one whose branch should be
> displayed. I doubt it's worth it though. Or we could replace the
> 'load-path-inputs' field with a 'guix-input' field. That was kind of
> the point of the 3rd part of my initial message[1]. Or, we could
> automate things: find out from which input the Guix modules come. That
> would be a bit tricky.

Oh right, since we now have multiple inputs, what I wrote above is not
quite true; there can be several repo URLs/branches that would need to
be shown on the UI. Hmm, maybe we need to keep the ‘name’, but only as
a hint and not as a key.

Thanks,
Ludo’.
C
C
Clément Lassieur wrote on 13 Jul 2018 13:57
Re: [bug#32121] [PATCH 1/5] base: Compile CHECKOUT in the fiber.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 32121@debbugs.gnu.org)
87r2k71h6w.fsf@lassieur.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (49 lines)
> Clément Lassieur <clement@lassieur.org> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Morning!
>>>
>>> Clément Lassieur <clement@lassieur.org> skribis:
>>>
>>>> Because it may take time and thus prevent PROCESS-SPECS to run every INTERVAL
>>>> seconds.
>>>>
>>>> * src/cuirass/base.scm (process-specs): move the COMPILE invocation inside
>>>> SPAWN-FIBER's thunk. Add log message.
>>>
>>> [...]
>>>
>>>> - (when compile?
>>>> - (non-blocking (compile checkout)))
>>>> -
>>>> (spawn-fiber
>>>> (lambda ()
>>>> + (when compile?
>>>> + (log-message "compiling '~a' with commit ~s" name commit)
>>>> + (non-blocking (compile checkout)))
>>>
>>> I think this doesn’t bring anything compared to the existing
>>> ‘non-blocking’ call.
>>> The ‘non-blocking’ procedure evaluates its argument in a separate
>>> thread; the calling fiber then “waits” for a message from that thread,
>>> which it gets when the computation is over. The ‘get-message’ is
>>> non-blocking though: the calling fiber is simply unscheduled until the
>>> message has arrived.
>>>
>>> Does that make sense?
>>
>> Well, no :-)
>>
>> My understanding is that non-blocking is, actually... blocking, because
>> get-message is blocking. (It doesn't block the scheduler because it's
>> in another thread, but that's not the problem here.)
>>
>> What I wanted to fix here is the fact that if the build takes one hour,
>> we will block for one hour in the COMPILE call, and process-spec won't
>> return for one hour. If it doesn't return for one hour, that means we
>> can't evaluate anything else for all that time.
>
> Oh, I see. However we have to wait for compilation to complete before
> continuing anyway, no?

Yes, for continuing that specific evaluation. But other evaluations
would happen in the meantime.

Toggle quote (11 lines)
>> With my change, the one-hour call will be in the fiber, which means that
>> process-spec can return, and other evaluations can be processed.
>>
>> But this is untested (because compilation doesn't work IIRC), so I can't
>> be sure.
>
> Yeah, what about this plan: let’s forget about this patch, and let’s
> remove support for compilation altogether in a future patch.
>
> WDYT?

Agreed!
C
C
Clément Lassieur wrote on 14 Jul 2018 14:13
Re: [bug#32121] [PATCH 2/5] utils: Reset the Fiber dynamic environment in %NON-BLOCKING.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 32121@debbugs.gnu.org)
87k1pyf20x.fsf@lassieur.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (17 lines)
> Clément Lassieur <clement@lassieur.org> skribis:
>
>> * src/cuirass/utils.scm (%non-blocking): Wrap body in PARAMETERIZE form that
>> clears CURRENT-FIBER.
>>
>> So that PUT-MESSAGE doesn't try to suspend itself within CALL-WITH-NEW-THREAD.
>> See https://lists.gnu.org/archive/html/guile-devel/2018-07/msg00009.html.
>
> Good catch!
>
>> + (parameterize (((@@ (fibers internal) current-fiber) #f))
>> + (let ((channel (make-channel)))
>
> Instead of using @@, I think you can add an explicit:
>
> #:use-module ((fibers internal) #:select (current-fiber))

That doesn't work because it would select the exported variable (as '@'
would have done), that is: the value of the parameter. What I need is
the parameter itself, which is hidden.

See (fibers internal):

Toggle snippet (13 lines)
#:export ;; Low-level interface: schedulers and threads.

[...]

(current-fiber/public . current-fiber)
[...]

(define current-fiber (make-parameter #f))
(define (current-fiber/public)
"Return the current fiber, or @code{#f} if no fiber is current."
(current-fiber))

Toggle quote (7 lines)
> at the top.
>
> OK with this change!
>
> Could you also report the issue to Andy (there’s a GitHub thing or you
> can email guile-user I guess)?

Sure!

Thank you,
Clément
L
L
Ludovic Courtès wrote on 14 Jul 2018 15:45
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 32121@debbugs.gnu.org)
87in5iaq21.fsf@gnu.org
Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (37 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Clément Lassieur <clement@lassieur.org> skribis:
>>
>>> * src/cuirass/utils.scm (%non-blocking): Wrap body in PARAMETERIZE form that
>>> clears CURRENT-FIBER.
>>>
>>> So that PUT-MESSAGE doesn't try to suspend itself within CALL-WITH-NEW-THREAD.
>>> See https://lists.gnu.org/archive/html/guile-devel/2018-07/msg00009.html.
>>
>> Good catch!
>>
>>> + (parameterize (((@@ (fibers internal) current-fiber) #f))
>>> + (let ((channel (make-channel)))
>>
>> Instead of using @@, I think you can add an explicit:
>>
>> #:use-module ((fibers internal) #:select (current-fiber))
>
> That doesn't work because it would select the exported variable (as '@'
> would have done), that is: the value of the parameter. What I need is
> the parameter itself, which is hidden.
>
> See (fibers internal):
>
> #:export ;; Low-level interface: schedulers and threads.
>
> [...]
>
> (current-fiber/public . current-fiber)
> [...]
>
> (define current-fiber (make-parameter #f))
> (define (current-fiber/public)
> "Return the current fiber, or @code{#f} if no fiber is current."
> (current-fiber))

Oh I see. Thanks for explaining!

Ludo’.
C
C
Clément Lassieur wrote on 14 Jul 2018 17:00
Re: [bug#32121] [PATCH 3/5] database: Add support for database upgrades.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 32121@debbugs.gnu.org)
87in5hg8ut.fsf@lassieur.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (14 lines)
>> + (apply max
>> + (map string->number
>> + (map (cut match:substring <> 1)
>> + (filter regexp-match?
>> + (map (cut string-match
>> + "^upgrade-([0-9]+)\\.sql$" <>)
>> + (scandir (%package-sql-dir))))))))
>
> I think you can write it along these lines:
>
> (reduce max 0
> (map (compose string->number (cut match:substring <> 1))
> (filter-map (cut string-match …) (scandir …))))

Very nice! Now it returns 0 if the list is empty (which shouldn't
happen) but it makes more sense.

Toggle quote (6 lines)
> I would rather avoid SRFI-42, not just because I can’t parse it ;-), but
> also to maintain consistency and make the code possibly more accessible.
>
> In this case I think we could use a simple loop or (for-each … (iota n))
> and that wouldn’t be bad.

iota... Exactly what I was looking for! But for some reason I thought
it would be named something like "range" :-)

Thank you so much!
Clément
C
C
Clément Lassieur wrote on 14 Jul 2018 17:32
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 32121@debbugs.gnu.org)
87h8l1g7eo.fsf@lassieur.org
Clément Lassieur <clement@lassieur.org> writes:

Toggle quote (11 lines)
> +(define (db-get-version db)
> + (if (pair? (sqlite-exec db "SELECT name FROM sqlite_master WHERE \
> +type='table' AND name='SchemaVersion';"))
> + (vector-ref
> + (car (sqlite-exec db "SELECT MAX(version) FROM SchemaVersion;")) 0)
> + 0))
> +
> +(define (db-set-version db version)
> + (sqlite-exec db "INSERT INTO SchemaVersion (version) VALUES (" version
> + ");"))

Actually, there is:

Toggle snippet (9 lines)
PRAGMA schema.user_version;
PRAGMA schema.user_version = integer ;

The user_version pragma will to get or set the value of the user-version
integer at offset 60 in the database header. The user-version is an
integer that is available to applications to use however they
want. SQLite makes no use of the user-version itself.

Better use them than creating an ad-hoc table I guess, WDYT?
C
C
Clément Lassieur wrote on 14 Jul 2018 21:57
Re: [bug#32121] [PATCH 4/5] database: Call a specification 'jobset' instead of 'project'.
(address . 32121@debbugs.gnu.org)
87fu0lfv40.fsf@lassieur.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (2 lines)
> So we’d be moving the project/branch structure to naming conventions.

Yes.

Toggle quote (28 lines)
> In a way, that’s not great, because as users we like to think in terms
> of branches to answer questions like “how many packages failed in branch
> X of the Savannah repo?”.
>
> However, this can probably be addressed at the UI level: the web UI and
> guix-hydra.el could list (shortened) repo URLs and branch names instead
> of this ‘name’ field. Eventually, we could remove this ‘name’ field
> altogether and instead have an automatically-assigned numerical ID.
>
> WDYT?
>
> (This does not affect this patch series, I’m thinking about what we can
> do eventually.)
>
>> However, we could still be able to bind a specification to a branch, but
>> that would require adding a 'guix-input' specification field, so that
>> the specification knows which input is the one whose branch should be
>> displayed. I doubt it's worth it though. Or we could replace the
>> 'load-path-inputs' field with a 'guix-input' field. That was kind of
>> the point of the 3rd part of my initial message[1]. Or, we could
>> automate things: find out from which input the Guix modules come. That
>> would be a bit tricky.
>
> Oh right, since we now have multiple inputs, what I wrote above is not
> quite true; there can be several repo URLs/branches that would need to
> be shown on the UI. Hmm, maybe we need to keep the ‘name’, but only as
> a hint and not as a key.

Yes. As you said above, we could display a preview of all the inputs
(name, URL, branch) in that 'name' field. And call it 'inputs', maybe.
:-)

Cc'ing Tatiana, as this is a UI thing. Tatiana, you can follow the
whole conversation at:
and
C
C
Clément Lassieur wrote on 15 Jul 2018 10:25
Re: [bug#32121] [PATCH 5/5] Add support for multiple inputs.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 32121@debbugs.gnu.org)
87efg4gb2b.fsf@lassieur.org
Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (11 lines)
> Clément Lassieur <clement@lassieur.org> skribis:
>
>> (define* (main #:optional (args (command-line)))
>> (match args
>> - ((command load-path guix-package-path source specstr)
>> - ;; Load FILE, a Scheme file that defines Hydra jobs.
>> + ((command static-guix-package-path specstr checkoutsstr)
>> + ;; Load PROC-FILE, a Scheme file that defines Hydra jobs.
>
> There’s no “proc-file”; should it be “proc-source”?

It was #:PROC-PATH, but I'll bind it to FILE and use FILE in the
comment, as it was initially.

Toggle quote (3 lines)
> Do I get it write that inputs do not necessarily contribute to
> GUIX_PACKAGE_PATH?

Yes! Only inputs in package-path-inputs contribute to
GUIX_PACKAGE_PATH.

Toggle quote (3 lines)
> Some inputs may provide code (to be in %load-path) while not provide any
> package definition (so nothing to add to GUIX_PACKAGE_PATH.)

Indeed. And some inputs can contribute to both %load-path and
GUIX_PACKAGE_PATH. It's flexible.

Toggle quote (14 lines)
>> ;; Since we have relative file name canonicalization by default, better
>> - ;; change to SOURCE to make sure things like 'include' with relative
>> - ;; file names work as expected.
>> - (chdir source)
>> + ;; change to PROC-SOURCE to make sure things like 'include' with
>> + ;; relative file names work as expected.
>> + (chdir proc-source)
>
> As a rule of thumb, identifiers for local variables should, IMO, almost
> always be a single word or at most two words. Long names like
> ‘static-guix-package-path’ in local scope tend to make code harder to
> read; ‘proc-source’ here should probably be ‘source’ because we know
> what it is we’re talking about.

Okay! Well I'll just remove static-guix-package-path (you know, the
--load-path argument to the cuirass command), because it's better to use
inputs instead. And it'll simplify the code.

I'll also rename my GET-something procedures.

Toggle quote (9 lines)
>> (save-module-excursion
>> (lambda ()
>> (set-current-module %user-module)
>> - (primitive-load (assq-ref spec #:file))))
>> + (primitive-load (assq-ref spec #:proc-path))))
>
> Nitpick: in GNU “path” means “search path” (a list of directories), so
> here I think it should be “file” or “file name”, not “path”.

Ok I'll change it everywhere else too.

Toggle quote (15 lines)
>> @command{cuirass} acts as a daemon polling @acronym{VCS, version control
>> -system} repositories for changes, and evaluating a derivation when
>> -something has changed (@pxref{Derivations, Derivations,, guix, Guix}).
>> -As a final step the derivation is realized and the result of that build
>> -allows you to know if the job succeeded or not.
>> +system} repositories (called @code{inputs}) for changes, and evaluating a
>
> s/@code/@dfn/
>
>> +derivation when an @code{input} has changed (@pxref{Derivations, Derivations,,
>
> s/@code//
>
> @code is to refer to identifiers in the code, things like that.

Got it :-)

Toggle quote (33 lines)
>> +There are three @code{inputs}: one tracking the Guix repository, one tracking
>
> s/@code//
>
>> +(define (compile-checkouts spec all-checkouts)
>> + (let* ((checkouts (filter compile? all-checkouts))
>> + (thunks
>> + (map
>> + (lambda (checkout)
>> + (lambda ()
>> + (log-message "compiling input '~a' of spec '~a' (commit ~s)"
>> + (assq-ref checkout #:name)
>> + (assq-ref spec #:name)
>> + (assq-ref checkout #:commit))
>> + (compile checkout)))
>> + checkouts))
>> + (results (par-map %non-blocking thunks)))
>> + (map (lambda (checkout)
>> + (log-message "compiled input '~a' of spec '~a' (commit ~s)"
>> + (assq-ref checkout #:name)
>> + (assq-ref spec #:name)
>> + (assq-ref checkout #:commit))
>> + checkout)
>> + results)))
>
> Since the return value is unused, we could perhaps make it:
>
> (define (compile-checkouts spec checkouts)
> (for-each (lambda (checkout)
> (log-message …)
> (non-blocking (compile checkout)))
> checkouts))

I use par-map because it's better to build them in parallel. Also, the
return value is used to display a log message.

Toggle quote (4 lines)
> and move the ‘filter’ call to the call site (the job of
> ‘compile-checkouts’, one might think, is to compile what it’s given, not
> to filter things.)

Right!

Thank you for the review, I'm learning a lot ;-).
Clément
L
L
Ludovic Courtès wrote on 16 Jul 2018 15:17
Re: [bug#32121] [PATCH 3/5] database: Add support for database upgrades.
(name . Clément Lassieur)(address . clement@lassieur.org)(address . 32121@debbugs.gnu.org)
87efg3wc8u.fsf@gnu.org
Hello,

Clément Lassieur <clement@lassieur.org> skribis:

Toggle quote (25 lines)
> Clément Lassieur <clement@lassieur.org> writes:
>
>> +(define (db-get-version db)
>> + (if (pair? (sqlite-exec db "SELECT name FROM sqlite_master WHERE \
>> +type='table' AND name='SchemaVersion';"))
>> + (vector-ref
>> + (car (sqlite-exec db "SELECT MAX(version) FROM SchemaVersion;")) 0)
>> + 0))
>> +
>> +(define (db-set-version db version)
>> + (sqlite-exec db "INSERT INTO SchemaVersion (version) VALUES (" version
>> + ");"))
>
> Actually, there is:
>
> PRAGMA schema.user_version;
> PRAGMA schema.user_version = integer ;
>
> The user_version pragma will to get or set the value of the user-version
> integer at offset 60 in the database header. The user-version is an
> integer that is available to applications to use however they
> want. SQLite makes no use of the user-version itself.
>
> Better use them than creating an ad-hoc table I guess, WDYT?

Sounds good, yes.

Thanks,
Ludo’.
C
C
Clément Lassieur wrote on 16 Jul 2018 22:13
Re: [bug#32121] [PATCH 5/5] Add support for multiple inputs.
(name . Ludovic Courtès)(address . ludo@gnu.org)
87sh4jhrcf.fsf@lassieur.org
Hi Ludo,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (8 lines)
> I think that’s about it.
>
> The size of reviews is often inversely proportional to the size of the
> change, and I think this one is no exception. :-)
>
> I’m not fully up-to-speed on all the changes but I’ll guess we’ll see it
> live when we upgrade Cuirass on berlin.

I pushed everything. If you need my help for the Berlin migration,
don't hesitate to ask. The database migration should go smoothly of
course, but... you never know :-)

Clément
Closed
?