[bug#46031] services: cuirass: Add "simple-cuirass-services".

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]


Hello,

Here is a service that provides some syntactic sugar over the (complex)
Cuirass configuration.  It uses Guix Channels to declare Cuirass inputs.

In the future, it would be nice if Cuirass could operate directly on
Channels. For now, this service only act as a translation layer for
people willing to setup a simple Cuirass instance.

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-services-cuirass-Add-simple-cuirass-services.patch --]
[-- Type: text/x-diff, Size: 9677 bytes --]

From 14186c2d6ad7dd5a55f35b1364f34669c1e27a5e Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Fri, 22 Jan 2021 09:44:45 +0100
Subject: [PATCH] services: cuirass: Add "simple-cuirass-services".

* gnu/services/cuirass.scm (<build-manifest>,
<simple-cuirass-configuration>): New records.
(build-manifest, build-manifest?, simple-cuirass-configuration,
simple-cuirass-configuration?, simple-cuirass-services): New procedures.
(%default-cuirass-config): New variable.
* doc/guix.texi (Continuous Integration): Document it.

Signed-off-by: Mathieu Othacehe <othacehe@gnu.org>
---
 doc/guix.texi            | 102 ++++++++++++++++++++++++++++++++++++++
 gnu/services/cuirass.scm | 103 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 204 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 13d95b36d1..8982ad82f5 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -26010,6 +26010,108 @@ The Cuirass package to use.
 @end table
 @end deftp
 
+@cindex simple cuirass
+@subsubheading Simple Cuirass
+
+The Cuirass service configuration described above can be a little
+intimidating.  The @code{simple-cuirass-services} procedure offers a way
+to setup a continuous integration server more readily.
+
+It takes a @code{simple-cuirass-configuration} record as its first
+argument.
+
+@deftp {Data Type} simple-cuirass-configuration
+Data type representing the configuration of a simple Cuirass instance.
+
+@table @asis
+@item @code{build} (default: @code{'all})
+The packages to be built by Cuirass.  It defaults to @code{'all}, which
+means that all the discovered packages in the subsequent @code{channels}
+field are to be selected.
+
+It is also possible to set this field to a list of @code{build-manifest}
+records, so that only the packages that are part of the declared
+manifests are built.  This record is described below.
+
+@deftp {Data Type} build-manifest
+@table @asis
+@item @code{channel-name}
+The name of the channel where the manifest is located.
+
+@item @code{manifest}
+The manifest path inside the channel.
+
+@end table
+@end deftp
+
+@item @code{channels} (default: @code{%default-channels})
+The channels to be fetched by Cuirass, see @pxref{Channels}.
+
+@item @code{non-package-channels} (default: @code{'()})
+List the channel names that must not be searched for packages.  That is
+often the case for the channel containing the manifest.
+
+@item @code{systems} (default: @code{(list (%current-system))})
+Build every discovered package for each system in this list.  By default
+only the current system is selected.
+
+@end table
+@end deftp
+
+Here is an example of how to setup a Cuirass instance that builds all
+the packages declared by Guix and a user repository.  The package list
+is re-evaluated each time a commit is pushed in one of the declared
+channels.
+
+@lisp
+(simple-cuirass-services
+ (simple-cuirass-configuration
+  (build 'all)
+  (channels (cons (channel
+                   (name 'my-guix)
+                   (url "https://my-git-repo/guix.git"))
+                  %default-channels))))
+@end lisp
+
+In the same spirit, this builds all the packages that are part of the
+@code{'guix} or @code{'my-guix} channels and declared in the manifest
+located in the @code{'conf} channel.
+
+@lisp
+(simple-cuirass-services
+ (simple-cuirass-configuration
+  (build (list
+          (build-manifest
+           (channel-name 'conf)
+           (manifest "guix/manifest.scm"))))
+  (channels (cons* (channel
+                    (name 'my-guix)
+                    (url "https://my-git-repo/guix.git"))
+                   (channel
+                    (name 'conf)
+                    (url "https://my-git-repo/conf.git"))
+                   %default-channels))
+  (non-package-channels '(conf))))
+@end lisp
+
+Finally, @code{simple-cuirass-services} takes as a second optional
+argument a @code{cuirass-configuration} record.  It can be used to
+customize the configuration of the Cuirass instance.
+
+@lisp
+(simple-cuirass-services
+ (simple-cuirass-configuration
+  (build 'all)
+  (channels (cons (channel
+                   (name 'my-guix)
+                   (url "https://my-git-repo/guix.git"))
+                  %default-channels))
+  (non-package-channels '(conf)))
+ (cuirass-configuration
+  (inherit %default-cuirass-config)
+  (host "0.0.0.0"))) ;listen on all interfaces.
+@end lisp
+
 @node Power Management Services
 @subsection Power Management Services
 
diff --git a/gnu/services/cuirass.scm b/gnu/services/cuirass.scm
index f426b9a1a7..44b39cb78f 100644
--- a/gnu/services/cuirass.scm
+++ b/gnu/services/cuirass.scm
@@ -22,10 +22,13 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu services cuirass)
+  #:use-module (guix channels)
   #:use-module (guix gexp)
   #:use-module (guix records)
+  #:use-module (guix utils)
   #:use-module (gnu packages admin)
   #:use-module (gnu packages ci)
+  #:use-module (gnu packages databases)
   #:use-module (gnu packages version-control)
   #:use-module (gnu services)
   #:use-module (gnu services base)
@@ -33,6 +36,8 @@
   #:use-module (gnu services shepherd)
   #:use-module (gnu services admin)
   #:use-module (gnu system shadow)
+  #:use-module (srfi srfi-1)
+  #:use-module (ice-9 match)
   #:export (<cuirass-remote-server-configuration>
             cuirass-remote-server-configuration
             cuirass-remote-server-configuration?
@@ -45,7 +50,18 @@
             <cuirass-remote-worker-configuration>
             cuirass-remote-worker-configuration
             cuirass-remote-worker-configuration?
-            cuirass-remote-worker-service-type))
+            cuirass-remote-worker-service-type
+
+            <build-manifest>
+            build-manifest
+            build-manifest?
+
+            <simple-cuirass-configuration>
+            simple-cuirass-configuration
+            simple-cuirass-configuration?
+
+            %default-cuirass-config
+            simple-cuirass-services))
 
 ;;;; Commentary:
 ;;;
@@ -338,3 +354,88 @@ CONFIG."
                         cuirass-remote-worker-shepherd-service)))
    (description
     "Run the Cuirass remote build worker service.")))
+
+(define-record-type* <build-manifest>
+  build-manifest make-build-manifest
+  build-manifest?
+  (channel-name          build-manifest-channel-name) ;symbol
+  (manifest              build-manifest-manifest)) ;string
+
+(define-record-type* <simple-cuirass-configuration>
+  simple-cuirass-configuration make-simple-cuirass-configuration
+  simple-cuirass-configuration?
+  (build                 simple-cuirass-configuration-build
+                         (default 'all))  ;symbol or list of <build-manifest>
+  (channels              simple-cuirass-configuration-channels
+                         (default %default-channels))  ;list of <channel>
+  (non-package-channels  simple-cuirass-configuration-package-channels
+                         (default '())) ;list of channels name
+  (systems               simple-cuirass-configuration-systems
+                         (default (list (%current-system))))) ;list of strings
+
+(define %default-cuirass-config
+  (cuirass-configuration
+   (specifications #~())))
+
+(define* (simple-cuirass-services config
+                                  #:optional
+                                  (cuirass %default-cuirass-config))
+  (define (format-name name)
+    (if (string? name)
+        name
+        (symbol->string name)))
+
+  (define (format-manifests build-manifests)
+    (map (lambda (build-manifest)
+           (match-record build-manifest <build-manifest>
+             (channel-name manifest)
+             (cons (format-name channel-name) manifest)))
+         build-manifests))
+
+  (define (channel->input channel)
+    (let ((name   (channel-name channel))
+          (url    (channel-url channel))
+          (branch (channel-branch channel)))
+      `((#:name . ,(format-name name))
+        (#:url . ,url)
+        (#:load-path . ".")
+        (#:branch . ,branch)
+        (#:no-compile? #t))))
+
+  (define (package-path channels non-package-channels)
+    (filter-map (lambda (channel)
+                  (let ((name (channel-name channel)))
+                    (and (not (member name non-package-channels))
+                         (not (eq? name 'guix))
+                         (format-name name))))
+                channels))
+
+  (define (config->spec config)
+    (match-record config <simple-cuirass-configuration>
+      (build channels non-package-channels systems)
+      `((#:name . "simple-config")
+        (#:load-path-inputs . ("guix"))
+        (#:package-path-inputs . ,(package-path channels
+                                                non-package-channels))
+        (#:proc-input . "guix")
+        (#:proc-file . "build-aux/cuirass/gnu-system.scm")
+        (#:proc . cuirass-jobs)
+        (#:proc-args . ((systems . ,systems)
+                        ,@(if (eq? build 'all)
+                              '()
+                              `((subset . "manifests")
+                                (manifests . ,(format-manifests build))))))
+        (#:inputs  . ,(map channel->input channels))
+        (#:build-outputs . ())
+        (#:priority . 1))))
+
+  (list
+   (service cuirass-service-type
+            (cuirass-configuration
+             (inherit cuirass)
+             (specifications #~(list
+                                '#$(config->spec config)))))
+   (service postgresql-service-type
+            (postgresql-configuration
+             (postgresql postgresql-10)))
+   (service postgresql-role-service-type)))
-- 
2.29.2

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Jonathan Brielmaier]

On 22.01.21 09:50, Mathieu Othacehe wrote:
> Here is a service that provides some syntactic sugar over the (complex)
> Cuirass configuration.  It uses Guix Channels to declare Cuirass inputs.

On my server I stopped cuirass && cuirass-web. Removed any signs from
the config.scm and tried this barebone example from your branch:
https://gitlab.com/mothacehe/guix/-/tree/simple-cuirass

(use-modules (gnu)

              (guix channels))
[...]
(simple-cuirass-services

  (simple-cuirass-configuration

    (build 'all)

    (channels (cons (channel

                     (name 'my-guix)

                     (url "https://gitlab.com/jonsger/guix.git"))

                    %default-channels))))


It gave me this error: https://paste.opensuse.org/view/raw/110509

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Mathieu Othacehe]

Hello Jonathan,

> It gave me this error: https://paste.opensuse.org/view/raw/110509

Thanks for testing! Be careful, "simple-cuirass-services" returns a list
of services and has to be appended to the list of other services.

Could you paste a bit more of your configuration?

Thanks,

Mathieu

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Jonathan Brielmaier]

On 22.01.21 18:55, Mathieu Othacehe wrote:
>
> Hello Jonathan,
>
>> It gave me this error: https://paste.opensuse.org/view/raw/110509
>
> Thanks for testing! Be careful, "simple-cuirass-services" returns a list
> of services and has to be appended to the list of other services.

That is the problem, because of changes to guix-daemon. I have following
construct:

(services
   (cons*
     (service a-service-type)
     [...]
     (service z-service-type)
     (modify-services %base-services
       (guix-service-type config =>
         (guix-configuration
           (inherit config)
           (extra-options '("--cores=x")))))))

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Mathieu Othacehe]

Hello Jonathan,

> (services
>   (cons*
>     (service a-service-type)
>     [...]
>     (service z-service-type)
>     (modify-services %base-services
>       (guix-service-type config =>
>         (guix-configuration
>           (inherit config)
>           (extra-options '("--cores=x")))))))

That would impose to write something like:

--8<---------------cut here---------------start------------->8---
(services
 (append
  (cons*
    (service a-service-type)
    [...]
    (service z-service-type)
    (modify-services %base-services
      (guix-service-type config =>
        (guix-configuration
          (inherit config)
          (extra-options '("--cores=x"))))))
  (simple-cuirass-services
   (simple-cuirass-configuration
    (build 'all)
    (channels (cons (channel
                   (name 'my-guix)
                   (url "https://my-git-repo/guix.git"))
                  %default-channels))))))
--8<---------------cut here---------------end--------------->8---

which is admittedly not that elegant. The problem I'd like to hide that
"postgresql-service-type" and "postgresql-role-service-type" are
required by Cuirass. Any idea?

Thanks,

Mathieu

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Jonathan Brielmaier]

On 29.01.21 12:39, Mathieu Othacehe wrote:
>
> Hello Jonathan,
>
>> (services
>>    (cons*
>>      (service a-service-type)
>>      [...]
>>      (service z-service-type)
>>      (modify-services %base-services
>>        (guix-service-type config =>
>>          (guix-configuration
>>            (inherit config)
>>            (extra-options '("--cores=x")))))))
>
> That would impose to write something like:
>
> --8<---------------cut here---------------start------------->8---
> (services
>   (append
>    (cons*
>      (service a-service-type)
>      [...]
>      (service z-service-type)
>      (modify-services %base-services
>        (guix-service-type config =>
>          (guix-configuration
>            (inherit config)
>            (extra-options '("--cores=x"))))))
>    (simple-cuirass-services
>     (simple-cuirass-configuration
>      (build 'all)
>      (channels (cons (channel
>                     (name 'my-guix)
>                     (url "https://my-git-repo/guix.git"))
>                    %default-channels))))))
> --8<---------------cut here---------------end--------------->8---

Thanks Mathieu. I think we can merge and I'll try it then. If we found a
nicer solution we can still change it...

bug#46031: services: cuirass: Add "simple-cuirass-services".

[Mathieu Othacehe]

Hello,

> Thanks Mathieu. I think we can merge and I'll try it then. If we found a
> nicer solution we can still change it...

I added a system test and pushed, thanks for testing it.

Mathieu

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Jonathan Brielmaier]

On 19.02.21 20:16, Mathieu Othacehe wrote:
>
> Hello,
>
>> Thanks Mathieu. I think we can merge and I'll try it then. If we found a
>> nicer solution we can still change it...
>
> I added a system test and pushed, thanks for testing it.

Merci Mathieu.

Is it intended that I need to import `(guix channels)`? Shouldn't that
come with the cuirass service module?

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Ludovic Courtès]

Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

> Here is a service that provides some syntactic sugar over the (complex)
> Cuirass configuration.  It uses Guix Channels to declare Cuirass inputs.
>
> In the future, it would be nice if Cuirass could operate directly on
> Channels. For now, this service only act as a translation layer for
> people willing to setup a simple Cuirass instance.

I had overlooked these patches; that’s really nice!

> +@lisp
> +(simple-cuirass-services
> + (simple-cuirass-configuration
> +  (build (list
> +          (build-manifest
> +           (channel-name 'conf)
> +           (manifest "guix/manifest.scm"))))
> +  (channels (cons* (channel
> +                    (name 'my-guix)
> +                    (url "https://my-git-repo/guix.git"))
> +                   (channel
> +                    (name 'conf)
> +                    (url "https://my-git-repo/conf.git"))
> +                   %default-channels))
> +  (non-package-channels '(conf))))
> +@end lisp

I wonder if it would make sense to allow users to pass directly a
manifest, as in (pseudo syntax):

  (simple-cuirass-configuration
    (build (list (build-manifest … (local-file "my-manifest.scm"))))
    (channels …))

It’d be less expressive (you’d have to reconfigure when you change the
manifest), but perhaps easier to set up.  WDYT?

> +  (define (config->spec config)
> +    (match-record config <simple-cuirass-configuration>
> +      (build channels non-package-channels systems)
> +      `((#:name . "simple-config")
> +        (#:load-path-inputs . ("guix"))
> +        (#:package-path-inputs . ,(package-path channels
> +                                                non-package-channels))
> +        (#:proc-input . "guix")
> +        (#:proc-file . "build-aux/cuirass/gnu-system.scm")
> +        (#:proc . cuirass-jobs)
> +        (#:proc-args . ((systems . ,systems)
> +                        ,@(if (eq? build 'all)
> +                              '()
> +                              `((subset . "manifests")
> +                                (manifests . ,(format-manifests build))))))
> +        (#:inputs  . ,(map channel->input channels))
> +        (#:build-outputs . ())
> +        (#:priority . 1))))
> +
> +  (list
> +   (service cuirass-service-type
> +            (cuirass-configuration
> +             (inherit cuirass)
> +             (specifications #~(list
> +                                '#$(config->spec config)))))

What about exposing ‘simple-cuirass-configuration->specs’, and document
it such that one can do:

  (service cuirass-service-type
           (cuirass-configuration
             (specifications
              (simple-cuirass-configuration->specs config))))

or even:

  (service cuirass-service-type
           (compile-simple-cuirass-configuration config))

?

That way, the relationship between “simple” and “not simple” would be
clearer.

Thanks,
Ludo’.

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Mathieu Othacehe]

Hey Ludo!

> I had overlooked these patches; that’s really nice!

Thanks :)

> I wonder if it would make sense to allow users to pass directly a
> manifest, as in (pseudo syntax):
>
>   (simple-cuirass-configuration
>     (build (list (build-manifest … (local-file "my-manifest.scm"))))
>     (channels …))

It would be nice but Cuirass doesn't support specification update for
now. Passing a manifest this way would require to drop the database
content at each reconfiguration.

> What about exposing ‘simple-cuirass-configuration->specs’, and document
> it such that one can do:
>
>   (service cuirass-service-type
>            (cuirass-configuration
>              (specifications
>               (simple-cuirass-configuration->specs config))))

I agree it feels nicer, however with this service I'd like to hide the
dependencies to postgresql-service-type and
postgresql-role-service-type. That's why "simple-cuirass-services"
returns three services. Maybe you see another way?

Thanks,

Mathieu

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Mathieu Othacehe]

Hello Jonathan,

> Is it intended that I need to import `(guix channels)`? Shouldn't that
> come with the cuirass service module?

Yes you need it to declare the channels that are passed in the
"channels" field of "simple-cuirass-configuration". Maybe I should add a
comment about it in the documentation.

Thanks,

Mathieu

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Ludovic Courtès]

Hi!

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> I wonder if it would make sense to allow users to pass directly a
>> manifest, as in (pseudo syntax):
>>
>>   (simple-cuirass-configuration
>>     (build (list (build-manifest … (local-file "my-manifest.scm"))))
>>     (channels …))
>
> It would be nice but Cuirass doesn't support specification update for
> now. Passing a manifest this way would require to drop the database
> content at each reconfiguration.

Ah yes, that wouldn’t be convenient.

>> What about exposing ‘simple-cuirass-configuration->specs’, and document
>> it such that one can do:
>>
>>   (service cuirass-service-type
>>            (cuirass-configuration
>>              (specifications
>>               (simple-cuirass-configuration->specs config))))
>
> I agree it feels nicer, however with this service I'd like to hide the
> dependencies to postgresql-service-type and
> postgresql-role-service-type. That's why "simple-cuirass-services"
> returns three services. Maybe you see another way?

When a service extends a service type, an instance of that service type
is automatically added if it’s missing (provided that service type has a
default value).  This happens in ‘instantiate-missing-services’.

So, if postgresql-role-service-type and postgresql-service-type have a
default value, simple-cuirass-service could extend them both (possibly
with a dummy value) and it would just work.

HTH!

Ludo’.

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Mathieu Othacehe]

Hey,

> When a service extends a service type, an instance of that service type
> is automatically added if it’s missing (provided that service type has a
> default value).  This happens in ‘instantiate-missing-services’.

Oh! Didn't know about that one!

> So, if postgresql-role-service-type and postgresql-service-type have a
> default value, simple-cuirass-service could extend them both (possibly
> with a dummy value) and it would just work.

With 8163f74542300720f6ee5dc061b79ddf0c345bb8 and
108e2c6116f01c3b0a98498717d65a96c1857a51, I made sure that those
services are automatically instantiated when missing.

This means simple-cuirass-configuration->specs makes way more sense, as
you suggested. Took care of it with
bebcf97600b2fa65482ae8ee870800dafa34d3f8.

Now, as suggested by Jonathan on IRC yesterday, it would be nice to
be able to build only the packages of a given channel. For that, we
would need to be able to figure out which channel is providing a
package, with something like a package-channel field.

Do you think that would make sense?

Thanks,

Mathieu

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Ludovic Courtès]

Hello,

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> So, if postgresql-role-service-type and postgresql-service-type have a
>> default value, simple-cuirass-service could extend them both (possibly
>> with a dummy value) and it would just work.
>
> With 8163f74542300720f6ee5dc061b79ddf0c345bb8 and
> 108e2c6116f01c3b0a98498717d65a96c1857a51, I made sure that those
> services are automatically instantiated when missing.
>
> This means simple-cuirass-configuration->specs makes way more sense, as
> you suggested. Took care of it with
> bebcf97600b2fa65482ae8ee870800dafa34d3f8.

Yay!

> Now, as suggested by Jonathan on IRC yesterday, it would be nice to
> be able to build only the packages of a given channel. For that, we
> would need to be able to figure out which channel is providing a
> package, with something like a package-channel field.
>
> Do you think that would make sense?

One could filter packages from the manifest.

Currently there’s no definite way to know which channel a given package
comes from.  ‘package-provenance’ in (guix describe) approximates that.
Perhaps we should try something along these lines.

I guess the first step would be to provide an a ‘package-channel(s)’
procedure that does like ‘package-provenance’, but returns a list of
channels.

WDYT?

Ludo’.

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Mathieu Othacehe]

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]


Hey,

> I guess the first step would be to provide an a ‘package-channel(s)’
> procedure that does like ‘package-provenance’, but returns a list of
> channels.

Seems fine to me. Here's an implementation attached. If it works for
you, the next step will be to make the "cuirass-jobs" procedure of
"gnu-system.scm" operate on channels I guess.

WDYT?

Thanks,

Mathieu

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-describe-Add-package-channels.patch --]
[-- Type: text/x-diff, Size: 1780 bytes --]

From d44dcd5d153ba0a4627c205f24a0741384f3d301 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Tue, 23 Feb 2021 14:24:39 +0100
Subject: [PATCH] describe: Add package-channels.

* guix/describe.scm (package-channels): New procedure.
---
 guix/describe.scm | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/guix/describe.scm b/guix/describe.scm
index 03569b1db4..65e5772856 100644
--- a/guix/describe.scm
+++ b/guix/describe.scm
@@ -33,6 +33,7 @@
             package-path-entries
 
             package-provenance
+            package-channels
             manifest-entry-with-provenance
             manifest-entry-provenance))
 
@@ -178,6 +179,26 @@ property of manifest entries, or #f if it could not be determined."
                        `(,main
                          ,@(if extra (list extra) '()))))))))))
 
+(define (package-channels package)
+  "Return the list of channels providing PACKAGE or an empty list if it could
+not be determined."
+  (match (and=> (package-location package) location-file)
+    (#f #f)
+    (file
+     (let ((file (if (string-prefix? "/" file)
+                     file
+                     (search-path %load-path file))))
+       (and file
+            (string-prefix? (%store-prefix) file)
+
+            (filter-map
+             (lambda (entry)
+               (let ((item (manifest-entry-item entry)))
+                 (and (or (string-prefix? item file)
+                          (string=? "guix" (manifest-entry-name entry)))
+                      (manifest-entry-channel entry))))
+             (current-profile-entries)))))))
+
 (define (manifest-entry-with-provenance entry)
   "Return ENTRY with an additional 'provenance' property if it's not already
 there."
-- 
2.30.1

[bug#46031] services: cuirass: Add "simple-cuirass-services".

[Ludovic Courtès]

Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

>> I guess the first step would be to provide an a ‘package-channel(s)’
>> procedure that does like ‘package-provenance’, but returns a list of
>> channels.
>
> Seems fine to me. Here's an implementation attached. If it works for
> you, the next step will be to make the "cuirass-jobs" procedure of
> "gnu-system.scm" operate on channels I guess.

But by definition, “gnu-system.scm” is about the 'guix channel.
Intuitively I’d expect channel handling to happen in user code: either
in the user-provided manifest, or in some helper code in Cuirass.  WDYT?

> From d44dcd5d153ba0a4627c205f24a0741384f3d301 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <othacehe@gnu.org>
> Date: Tue, 23 Feb 2021 14:24:39 +0100
> Subject: [PATCH] describe: Add package-channels.
>
> * guix/describe.scm (package-channels): New procedure.

[...]

> +(define (package-channels package)
> +  "Return the list of channels providing PACKAGE or an empty list if it could
> +not be determined."
> +  (match (and=> (package-location package) location-file)
> +    (#f #f)

The #f return value doesn’t match the docstring.

> +    (file
> +     (let ((file (if (string-prefix? "/" file)
> +                     file
> +                     (search-path %load-path file))))
> +       (and file
> +            (string-prefix? (%store-prefix) file)
> +
> +            (filter-map
> +             (lambda (entry)
> +               (let ((item (manifest-entry-item entry)))
> +                 (and (or (string-prefix? item file)
> +                          (string=? "guix" (manifest-entry-name entry)))
> +                      (manifest-entry-channel entry))))
> +             (current-profile-entries)))))))

To avoid duplication, perhaps you should rewrite ‘package-provenance’ in
terms of ‘package-channels’?

Otherwise LGTM, thanks!

Thanks,
Ludo’.