[PATCH 0/3] Cuirass service config options work

  • Done
  • quality assurance status badge
Details
2 participants
  • Ahmad Draidi
  • Ludovic Courtès
Owner
unassigned
Submitted by
Ahmad Draidi
Severity
normal
A
A
Ahmad Draidi wrote on 8 Apr 2024 19:48
(address . guix-patches@gnu.org)(name . Ahmad Draidi)(address . a.r.draidi@redscript.org)
cover.1712576504.git.a.r.draidi@redscript.org
Hello Guix,

I found the extra-options issue while trying to change the ttl for my cuirass
instance, and thought the service could use some updates.

I tested this locally and it looks OK.

Ahmad Draidi (3):
services: cuirass: Use separate extra options for cuirass web process.
services: cuirass: Add new configuration options.
services: cuirass: Remove use-substitutes? configuration option.

doc/guix.texi | 15 ++++++++++-----
gnu/services/cuirass.scm | 23 +++++++++++++++++------
2 files changed, 27 insertions(+), 11 deletions(-)


base-commit: cd45294d576975a3bff2f755764a3f46f09ea6f9
--
2.41.0
A
A
Ahmad Draidi wrote on 8 Apr 2024 19:54
[PATCH 1/3] services: cuirass: Use separate extra options for cuirass web process.
(address . 70285@debbugs.gnu.org)(name . Ahmad Draidi)(address . a.r.draidi@redscript.org)
ececea8f2ef40baea66330e41638c9c44b4cef9b.1712576504.git.a.r.draidi@redscript.org
Currently, "extra-options" is passed to both "cuirass register" and "cuirass
web" processes. This makes it impractical since the extra parameters have to
be intended for and supported by both processes.

* gnu/services/cuirass.scm (<cuirass-configuration>): Add a web-extra-options
field.
(cuirass-shepherd-service): Replace extra-options with web-extra-options for
cuirass web.
* doc/guix.texi (Continuous Integration): Document the changes.

Change-Id: Iba79c559ea8267aaf8f25248f3d18ed7b352cb60
---
doc/guix.texi | 5 ++++-
gnu/services/cuirass.scm | 7 +++++--
2 files changed, 9 insertions(+), 3 deletions(-)

Toggle diff (50 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 5827e0de14..77e17cb69c 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34406,7 +34406,10 @@ Continuous Integration
packages locally.
@item @code{extra-options} (default: @code{'()})
-Extra options to pass when running the Cuirass processes.
+Extra options to pass when running the @code{cuirass register} process.
+
+@item @code{web-extra-options} (default: @code{'()})
+Extra options to pass when running the @code{cuirass web} process.
@end table
@end deftp
diff --git a/gnu/services/cuirass.scm b/gnu/services/cuirass.scm
index bcdbffa2f3..b09d5601d4 100644
--- a/gnu/services/cuirass.scm
+++ b/gnu/services/cuirass.scm
@@ -120,6 +120,8 @@ (define-record-type* <cuirass-configuration>
(fallback? cuirass-configuration-fallback? ;boolean
(default #f))
(extra-options cuirass-configuration-extra-options
+ (default '()))
+ (web-extra-options cuirass-configuration-web-extra-options
(default '())))
(define (cuirass-shepherd-service config)
@@ -142,7 +144,8 @@ (define (cuirass-shepherd-service config)
(use-substitutes? (cuirass-configuration-use-substitutes? config))
(one-shot? (cuirass-configuration-one-shot? config))
(fallback? (cuirass-configuration-fallback? config))
- (extra-options (cuirass-configuration-extra-options config)))
+ (extra-options (cuirass-configuration-extra-options config))
+ (web-extra-options (cuirass-configuration-web-extra-options config)))
`(,(shepherd-service
(documentation "Run Cuirass.")
(provision '(cuirass))
@@ -192,7 +195,7 @@ (define (cuirass-shepherd-service config)
"--parameters="
parameters))
'())
- #$@extra-options)
+ #$@web-extra-options)
#:user #$user
#:group #$group
--
2.41.0
A
A
Ahmad Draidi wrote on 8 Apr 2024 19:54
[PATCH 2/3] services: cuirass: Add new configuration options.
(address . 70285@debbugs.gnu.org)(name . Ahmad Draidi)(address . a.r.draidi@redscript.org)
7ccac410c5f7d0c9b27c828bc088c485f22884a8.1712576504.git.a.r.draidi@redscript.org
* gnu/services/cuirass.scm (<cuirass-configuration>): Add ttl and threads
fields.
(cuirass-shepherd-service): Handle the new options.
* doc/guix.texi (Continuous Integration): Document them.

Change-Id: I4387fbd7bf6766f4a54801f35c7ed82ce4eb9645
---
doc/guix.texi | 6 ++++++
gnu/services/cuirass.scm | 12 ++++++++++++
2 files changed, 18 insertions(+)

Toggle diff (56 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 77e17cb69c..90a6135882 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34368,6 +34368,12 @@ Continuous Integration
Number of seconds between the poll of the repositories followed by the
Cuirass jobs.
+@item @code{ttl} (default: @code{"30d"})
+Duration to keep build results' GC roots alive.
+
+@item @code{threads} (default: @code{#f})
+Number of kernel threads to use for Cuirass. The default value should be appropriate for most cases.
+
@item @code{parameters} (default: @code{#f})
Read parameters from the given @var{parameters} file. The supported
parameters are described here (@pxref{Parameters,,, cuirass, Cuirass}).
diff --git a/gnu/services/cuirass.scm b/gnu/services/cuirass.scm
index b09d5601d4..ac086dedb9 100644
--- a/gnu/services/cuirass.scm
+++ b/gnu/services/cuirass.scm
@@ -101,6 +101,10 @@ (define-record-type* <cuirass-configuration>
(default "cuirass"))
(interval cuirass-configuration-interval ;integer (seconds)
(default 60))
+ (ttl cuirass-configuration-ttl ;string
+ (default "30d"))
+ (threads cuirass-configuration-threads ;integer
+ (default #f))
(parameters cuirass-configuration-parameters ;string
(default #f))
(remote-server cuirass-configuration-remote-server
@@ -133,6 +137,8 @@ (define (cuirass-shepherd-service config)
(user (cuirass-configuration-user config))
(group (cuirass-configuration-group config))
(interval (cuirass-configuration-interval config))
+ (ttl (cuirass-configuration-ttl config))
+ (threads (cuirass-configuration-threads config))
(parameters (cuirass-configuration-parameters config))
(remote-server (cuirass-configuration-remote-server config))
(database (cuirass-configuration-database config))
@@ -159,6 +165,12 @@ (define (cuirass-shepherd-service config)
"--specifications" #$config-file
"--database" #$database
"--interval" #$(number->string interval)
+ "--ttl" #$ttl
+ #$@(if threads
+ (list (string-append
+ "--threads="
+ (number->string threads)))
+ '())
#$@(if parameters
(list (string-append
"--parameters="
--
2.41.0
A
A
Ahmad Draidi wrote on 8 Apr 2024 19:54
[PATCH 3/3] services: cuirass: Remove use-substitutes? configuration option.
(address . 70285@debbugs.gnu.org)(name . Ahmad Draidi)(address . a.r.draidi@redscript.org)
4b85490a37a0e08146f31e544b671865bb5927a0.1712576504.git.a.r.draidi@redscript.org
As of Cuirass commit 9a1452ee021c9f773424961cfeef47ca0b7c5c5a, this option
seems to be unused and kept for back compatibility there.

* gnu/services/cuirass.scm (<cuirass-configuration>): Remove use-substitutes?
field.
(cuirass-shepherd-service): Remove the option.
* doc/guix.texi (Continuous Integration): Remove option documentation.

Change-Id: I933550ce76eecdf918b07891aa8212fd30a7c87e
---
doc/guix.texi | 4 ----
gnu/services/cuirass.scm | 4 ----
2 files changed, 8 deletions(-)

Toggle diff (46 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 90a6135882..cc55f48aea 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34400,10 +34400,6 @@ Continuous Integration
specifications records. The specification record is described in the
Cuirass manual (@pxref{Specifications,,, cuirass, Cuirass}).
-@item @code{use-substitutes?} (default: @code{#f})
-This allows using substitutes to avoid building every dependencies of a job
-from source.
-
@item @code{one-shot?} (default: @code{#f})
Only evaluate specifications and build derivations once.
diff --git a/gnu/services/cuirass.scm b/gnu/services/cuirass.scm
index ac086dedb9..0684c149c1 100644
--- a/gnu/services/cuirass.scm
+++ b/gnu/services/cuirass.scm
@@ -117,8 +117,6 @@ (define-record-type* <cuirass-configuration>
(default "localhost"))
(specifications cuirass-configuration-specifications)
;gexp that evaluates to specification-alist
- (use-substitutes? cuirass-configuration-use-substitutes? ;boolean
- (default #f))
(one-shot? cuirass-configuration-one-shot? ;boolean
(default #f))
(fallback? cuirass-configuration-fallback? ;boolean
@@ -147,7 +145,6 @@ (define (cuirass-shepherd-service config)
(config-file (scheme-file
"cuirass-specs.scm"
(cuirass-configuration-specifications config)))
- (use-substitutes? (cuirass-configuration-use-substitutes? config))
(one-shot? (cuirass-configuration-one-shot? config))
(fallback? (cuirass-configuration-fallback? config))
(extra-options (cuirass-configuration-extra-options config))
@@ -177,7 +174,6 @@ (define (cuirass-shepherd-service config)
parameters))
'())
#$@(if remote-server '("--build-remote") '())
- #$@(if use-substitutes? '("--use-substitutes") '())
#$@(if one-shot? '("--one-shot") '())
#$@(if fallback? '("--fallback") '())
#$@extra-options)
--
2.41.0
L
L
Ludovic Courtès wrote on 29 Apr 2024 23:33
Re: [bug#70285] [PATCH 2/3] services: cuirass: Add new configuration options.
(name . Ahmad Draidi)(address . a.r.draidi@redscript.org)(address . 70285@debbugs.gnu.org)
87sez3zwes.fsf@gnu.org
Hi Ahmad,

Ahmad Draidi <a.r.draidi@redscript.org> skribis:

Toggle quote (5 lines)
> * gnu/services/cuirass.scm (<cuirass-configuration>): Add ttl and threads
> fields.
> (cuirass-shepherd-service): Handle the new options.
> * doc/guix.texi (Continuous Integration): Document them.

[...]

Toggle quote (3 lines)
> + (ttl cuirass-configuration-ttl ;string
> + (default "30d"))

For consistency with <guix-publish-configuration>, I would make it an
integer (number of seconds).

If that’s fine with you, could you send an updated patch or let me know
if I should make this change on your behalf?

The rest of the patch series looks good to me!

Thanks,
Ludo’.
A
A
Ahmad Draidi wrote on 2 May 2024 03:36
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 70285@debbugs.gnu.org)
ae8dbc08-4243-4648-9643-171472bd365a@redscript.org
Hello Ludo,


Thanks for the review. I'll send in the updated patch in a bit.
A
A
Ahmad Draidi wrote on 2 May 2024 03:37
[PATCH v2 1/3] services: cuirass: Use separate extra options for cuirass web process.
(address . 70285@debbugs.gnu.org)
afc3d0e937cd07def1c13fd1f2a687f8e50b7faa.1714613869.git.a.r.draidi@redscript.org
Currently, "extra-options" is passed to both "cuirass register" and "cuirass
web" processes. This makes it impractical since the extra parameters have to
be intended for and supported by both processes.

* gnu/services/cuirass.scm (<cuirass-configuration>): Add a web-extra-options
field.
(cuirass-shepherd-service): Replace extra-options with web-extra-options for
cuirass web.
* doc/guix.texi (Continuous Integration): Document the changes.

Change-Id: Iba79c559ea8267aaf8f25248f3d18ed7b352cb60
---
v2 changes: None

doc/guix.texi | 5 ++++-
gnu/services/cuirass.scm | 7 +++++--
2 files changed, 9 insertions(+), 3 deletions(-)

Toggle diff (52 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 3f5d4e7f0d..f93fe7e474 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34465,7 +34465,10 @@ Continuous Integration
packages locally.
@item @code{extra-options} (default: @code{'()})
-Extra options to pass when running the Cuirass processes.
+Extra options to pass when running the @code{cuirass register} process.
+
+@item @code{web-extra-options} (default: @code{'()})
+Extra options to pass when running the @code{cuirass web} process.
@end table
@end deftp
diff --git a/gnu/services/cuirass.scm b/gnu/services/cuirass.scm
index bcdbffa2f3..b09d5601d4 100644
--- a/gnu/services/cuirass.scm
+++ b/gnu/services/cuirass.scm
@@ -120,6 +120,8 @@ (define-record-type* <cuirass-configuration>
(fallback? cuirass-configuration-fallback? ;boolean
(default #f))
(extra-options cuirass-configuration-extra-options
+ (default '()))
+ (web-extra-options cuirass-configuration-web-extra-options
(default '())))
(define (cuirass-shepherd-service config)
@@ -142,7 +144,8 @@ (define (cuirass-shepherd-service config)
(use-substitutes? (cuirass-configuration-use-substitutes? config))
(one-shot? (cuirass-configuration-one-shot? config))
(fallback? (cuirass-configuration-fallback? config))
- (extra-options (cuirass-configuration-extra-options config)))
+ (extra-options (cuirass-configuration-extra-options config))
+ (web-extra-options (cuirass-configuration-web-extra-options config)))
`(,(shepherd-service
(documentation "Run Cuirass.")
(provision '(cuirass))
@@ -192,7 +195,7 @@ (define (cuirass-shepherd-service config)
"--parameters="
parameters))
'())
- #$@extra-options)
+ #$@web-extra-options)
#:user #$user
#:group #$group

base-commit: 8bf41c80ef13ea57e834f4e23d649bd99a3e08fc
--
2.41.0
A
A
Ahmad Draidi wrote on 2 May 2024 03:37
[PATCH v2 2/3] services: cuirass: Add new configuration options.
(address . 70285@debbugs.gnu.org)
1fc81fcc6ca5915993d08f889a6ed7735d785d0d.1714613869.git.a.r.draidi@redscript.org
* gnu/services/cuirass.scm (<cuirass-configuration>): Add ttl and threads
fields.
(cuirass-shepherd-service): Handle the new options.
* doc/guix.texi (Continuous Integration): Document them.

Change-Id: I4387fbd7bf6766f4a54801f35c7ed82ce4eb9645
---
v2 changes: Switch ttl field to seconds integer

doc/guix.texi | 6 ++++++
gnu/services/cuirass.scm | 17 +++++++++++++++++
2 files changed, 23 insertions(+)

Toggle diff (61 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index f93fe7e474..1f97a2796e 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34427,6 +34427,12 @@ Continuous Integration
Number of seconds between the poll of the repositories followed by the
Cuirass jobs.
+@item @code{ttl} (default: @code{2592000})
+Duration to keep build results' GC roots alive, in seconds.
+
+@item @code{threads} (default: @code{#f})
+Number of kernel threads to use for Cuirass. The default value should be appropriate for most cases.
+
@item @code{parameters} (default: @code{#f})
Read parameters from the given @var{parameters} file. The supported
parameters are described here (@pxref{Parameters,,, cuirass, Cuirass}).
diff --git a/gnu/services/cuirass.scm b/gnu/services/cuirass.scm
index b09d5601d4..f430c38a69 100644
--- a/gnu/services/cuirass.scm
+++ b/gnu/services/cuirass.scm
@@ -101,6 +101,10 @@ (define-record-type* <cuirass-configuration>
(default "cuirass"))
(interval cuirass-configuration-interval ;integer (seconds)
(default 60))
+ (ttl cuirass-configuration-ttl ;integer
+ (default 2592000))
+ (threads cuirass-configuration-threads ;integer
+ (default #f))
(parameters cuirass-configuration-parameters ;string
(default #f))
(remote-server cuirass-configuration-remote-server
@@ -133,6 +137,8 @@ (define (cuirass-shepherd-service config)
(user (cuirass-configuration-user config))
(group (cuirass-configuration-group config))
(interval (cuirass-configuration-interval config))
+ (ttl (cuirass-configuration-ttl config))
+ (threads (cuirass-configuration-threads config))
(parameters (cuirass-configuration-parameters config))
(remote-server (cuirass-configuration-remote-server config))
(database (cuirass-configuration-database config))
@@ -159,6 +165,17 @@ (define (cuirass-shepherd-service config)
"--specifications" #$config-file
"--database" #$database
"--interval" #$(number->string interval)
+ #$@(if ttl
+ (list (string-append
+ "--ttl="
+ (number->string ttl)
+ "s"))
+ '())
+ #$@(if threads
+ (list (string-append
+ "--threads="
+ (number->string threads)))
+ '())
#$@(if parameters
(list (string-append
"--parameters="
--
2.41.0
A
A
Ahmad Draidi wrote on 2 May 2024 03:37
[PATCH v2 3/3] services: cuirass: Remove use-substitutes? configuration option.
(address . 70285@debbugs.gnu.org)
703c8515cdde9c643d687e527a1f4b0e568d7889.1714613869.git.a.r.draidi@redscript.org
As of Cuirass commit 9a1452ee021c9f773424961cfeef47ca0b7c5c5a, this option
seems to be unused and kept for back compatibility there.

* gnu/services/cuirass.scm (<cuirass-configuration>): Remove use-substitutes?
field.
(cuirass-shepherd-service): Remove the option.
* doc/guix.texi (Continuous Integration): Remove option documentation.

Change-Id: I933550ce76eecdf918b07891aa8212fd30a7c87e
---
v2 changes: None

doc/guix.texi | 4 ----
gnu/services/cuirass.scm | 4 ----
2 files changed, 8 deletions(-)

Toggle diff (46 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index 1f97a2796e..0ddf75693a 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34459,10 +34459,6 @@ Continuous Integration
specifications records. The specification record is described in the
Cuirass manual (@pxref{Specifications,,, cuirass, Cuirass}).
-@item @code{use-substitutes?} (default: @code{#f})
-This allows using substitutes to avoid building every dependencies of a job
-from source.
-
@item @code{one-shot?} (default: @code{#f})
Only evaluate specifications and build derivations once.
diff --git a/gnu/services/cuirass.scm b/gnu/services/cuirass.scm
index f430c38a69..f68b4dc5a2 100644
--- a/gnu/services/cuirass.scm
+++ b/gnu/services/cuirass.scm
@@ -117,8 +117,6 @@ (define-record-type* <cuirass-configuration>
(default "localhost"))
(specifications cuirass-configuration-specifications)
;gexp that evaluates to specification-alist
- (use-substitutes? cuirass-configuration-use-substitutes? ;boolean
- (default #f))
(one-shot? cuirass-configuration-one-shot? ;boolean
(default #f))
(fallback? cuirass-configuration-fallback? ;boolean
@@ -147,7 +145,6 @@ (define (cuirass-shepherd-service config)
(config-file (scheme-file
"cuirass-specs.scm"
(cuirass-configuration-specifications config)))
- (use-substitutes? (cuirass-configuration-use-substitutes? config))
(one-shot? (cuirass-configuration-one-shot? config))
(fallback? (cuirass-configuration-fallback? config))
(extra-options (cuirass-configuration-extra-options config))
@@ -182,7 +179,6 @@ (define (cuirass-shepherd-service config)
parameters))
'())
#$@(if remote-server '("--build-remote") '())
- #$@(if use-substitutes? '("--use-substitutes") '())
#$@(if one-shot? '("--one-shot") '())
#$@(if fallback? '("--fallback") '())
#$@extra-options)
--
2.41.0
L
L
Ludovic Courtès wrote on 4 May 2024 18:21
Re: [bug#70285] [PATCH 2/3] services: cuirass: Add new configuration options.
(name . Ahmad Draidi)(address . a.r.draidi@redscript.org)(address . 70285-done@debbugs.gnu.org)
87plu1tunp.fsf@gnu.org
Hello,

Ahmad Draidi <a.r.draidi@redscript.org> skribis:

Toggle quote (2 lines)
> Thanks for the review. I'll send in the updated patch in a bit.

Thanks, applied!

Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 70285@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 70285
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch