Channel dependencies

Channel dependencies

[Ricardo Wurmus]

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

Hi,

the attached patch allows channel authors to declare other channels as
dependencies of their own channel.  In addition to explicitly requested
channels, “guix pull” will now also download and build channels that
have been declared as dependencies in the file ‘.guix-channel’ in the
repository root.

An example of a simple .guix-channel file is this:

--8<---------------cut here---------------start------------->8---
(channel
 (version 0)
 (dependencies
  (channel
   (name guix-bimsb)
   (url "https://github.com/BIMSBbioinfo/guix-bimsb"))))
--8<---------------cut here---------------end--------------->8---

What do you think?

--
Ricardo


[-- Attachment #2: 0001-guix-Add-support-for-channel-dependencies.patch --]
[-- Type: text/x-patch, Size: 8051 bytes --]

From 1783c17582906df970c7e68e89d761619a35caeb Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Sat, 13 Oct 2018 08:39:23 +0200
Subject: [PATCH] guix: Add support for channel dependencies.

* guix/channels.scm (%channel-meta-file): New variable.
(channel-meta, channel-instance-dependencies): New procedures.
(latest-channel-instances): Include channel dependencies.
(channel-instance-derivations): Build derivation for additional channels and
add it as dependency to the channel instance derivation.
* doc/guix.texi (Channels): Add subsection "Declaring Channel Dependencies".
---
 doc/guix.texi     | 33 ++++++++++++++++++++
 guix/channels.scm | 79 ++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 97 insertions(+), 15 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 5ae80917a..a4d5477f6 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -3020,6 +3020,39 @@ the new and upgraded packages that are listed, some like @code{my-gimp} and
 @code{my-emacs-with-cool-features} might come from
 @code{my-personal-packages}, while others come from the Guix default channel.
 
+@cindex dependencies, channels
+@cindex meta-data, channels
+@subsection Declaring Channel Dependencies
+
+Channel authors may decide to augment a package collection provided by other
+channels.  They can declare their channel to be dependent on other channels in
+a meta-data file @file{.guix-channel}, which is to be placed in the root of
+the channel repository.
+
+The meta-data file should contain a simple S-expression like this:
+
+@lisp
+(channel
+ (version 0)
+ (dependencies
+  (channel
+   (name 'some-collection)
+   (url "https://example.org/first-collection.git"))
+  (channel
+   (name 'some-other-collection)
+   (url "https://example.org/second-collection.git")
+   (branch "testing"))))
+@end lisp
+
+In the above example this channel is declared to depend on two other channels,
+which will both be fetched automatically.  The modules provided by the channel
+will be compiled in an environment where the modules of all these declared
+channels are available.
+
+For the sake of reliability and maintainability, you should avoid dependencies
+on channels that you don't control, and you should aim to keep the number of
+dependencies to a minimum.
+
 @subsection Replicating Guix
 
 @cindex pinning, channels
diff --git a/guix/channels.scm b/guix/channels.scm
index 82389eb58..fbe1b62bb 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2018 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -72,7 +73,6 @@
   (commit    channel-commit (default #f))
   (location  channel-location
              (default (current-source-location)) (innate)))
-;; TODO: Add a way to express dependencies among channels.
 
 (define %default-channels
   ;; Default list of channels.
@@ -81,6 +81,10 @@
          (branch "master")
          (url "https://git.savannah.gnu.org/git/guix.git"))))
 
+(define %channel-meta-file
+  ;; The file containing information about the channel.
+  ".guix-channel")
+
 (define (guix-channel? channel)
   "Return true if CHANNEL is the 'guix' channel."
   (eq? 'guix (channel-name channel)))
@@ -99,20 +103,52 @@
     (#f      `(branch . ,(channel-branch channel)))
     (commit  `(commit . ,(channel-commit channel)))))
 
+(define (channel-meta instance)
+  "Return an S-expression read from the channel INSTANCE's description file,
+or return #F if the channel instance does not include the file."
+  (let* ((source (channel-instance-checkout instance))
+         (meta-file (string-append source "/" %channel-meta-file)))
+    (and (file-exists? meta-file)
+         (call-with-input-file meta-file read))))
+
+(define (channel-instance-dependencies instance)
+  "Return the list of channels that are declared as dependencies for the given
+channel INSTANCE."
+  (or (and=> (assoc-ref (channel-meta instance) 'dependencies)
+             (lambda (dependencies)
+               (map (lambda (item)
+                      (let ((get (lambda* (key #:optional default)
+                                   (or (and=> (assoc-ref item key) car) default))))
+                        (let ((name (get 'name))
+                              (url (get 'url))
+                              (branch (get 'branch "master"))
+                              (commit (get 'commit)))
+                          (and name url branch
+                               (channel
+                                (name name)
+                                (branch branch)
+                                (url url)
+                                (commit commit))))))
+                    dependencies)))
+      '()))
+
 (define (latest-channel-instances store channels)
   "Return a list of channel instances corresponding to the latest checkouts of
-CHANNELS."
-  (map (lambda (channel)
-         (format (current-error-port)
-                 (G_ "Updating channel '~a' from Git repository at '~a'...~%")
-                 (channel-name channel)
-                 (channel-url channel))
-         (let-values (((checkout commit)
-                       (latest-repository-commit store (channel-url channel)
-                                                 #:ref (channel-reference
-                                                        channel))))
-           (channel-instance channel commit checkout)))
-       channels))
+CHANNELS and the channels on which they depend."
+  (append-map (lambda (channel)
+                (format (current-error-port)
+                        (G_ "Updating channel '~a' from Git repository at '~a'...~%")
+                        (channel-name channel)
+                        (channel-url channel))
+                (let-values (((checkout commit)
+                              (latest-repository-commit store (channel-url channel)
+                                                        #:ref (channel-reference
+                                                               channel))))
+                  (let ((instance (channel-instance channel commit checkout)))
+                    (cons instance (latest-channel-instances
+                                    store
+                                    (channel-instance-dependencies instance))))))
+              channels))
 
 (define %self-build-file
   ;; The file containing code to build Guix.  This serves the same purpose as
@@ -223,8 +259,21 @@ INSTANCES."
           (lambda (instance)
             (if (eq? instance core-instance)
                 (return core)
-                (build-channel-instance instance
-                                        (cons core dependencies))))
+                (match (channel-instance-dependencies instance)
+                  (()
+                   (build-channel-instance instance
+                                           (cons core dependencies)))
+                  (channels
+                   (mlet %store-monad ((dependencies-derivation
+                                        (latest-channel-derivation
+                                         ;; %default-channels is used here to
+                                         ;; ensure that the core channel is
+                                         ;; available for channels declared as
+                                         ;; dependencies.
+                                         (append channels %default-channels))))
+                     (build-channel-instance instance
+                                             (cons dependencies-derivation
+                                                   (cons core dependencies))))))))
           instances)))
 
 (define (whole-package-for-legacy name modules)
-- 
2.19.0

Re: Channel dependencies

[Pierre Neidhardt]

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

Sounds very useful!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Channel dependencies

[Chris Marusich]

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

Hi Ricardo,

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:

> the attached patch allows channel authors to declare other channels as
> dependencies of their own channel.
>
> [...]
>
> What do you think?

It's very cool!

> +(define (channel-instance-dependencies instance)
> +  "Return the list of channels that are declared as dependencies for the given
> +channel INSTANCE."
> +  (or (and=> (assoc-ref (channel-meta instance) 'dependencies)
> +             (lambda (dependencies)
> +               (map (lambda (item)
> +                      (let ((get (lambda* (key #:optional default)
> +                                   (or (and=> (assoc-ref item key) car) default))))

Nitpick: to improve readability, is it possible to use pattern matching
here instead of using procedures like "car"?

>  (define (latest-channel-instances store channels)
>    "Return a list of channel instances corresponding to the latest checkouts of
> -CHANNELS."
> -  (map (lambda (channel)
> -         (format (current-error-port)
> -                 (G_ "Updating channel '~a' from Git repository at '~a'...~%")
> -                 (channel-name channel)
> -                 (channel-url channel))
> -         (let-values (((checkout commit)
> -                       (latest-repository-commit store (channel-url channel)
> -                                                 #:ref (channel-reference
> -                                                        channel))))
> -           (channel-instance channel commit checkout)))
> -       channels))
> +CHANNELS and the channels on which they depend."
> +  (append-map (lambda (channel)
> +                (format (current-error-port)
> +                        (G_ "Updating channel '~a' from Git repository at '~a'...~%")
> +                        (channel-name channel)
> +                        (channel-url channel))
> +                (let-values (((checkout commit)
> +                              (latest-repository-commit store (channel-url channel)
> +                                                        #:ref (channel-reference
> +                                                               channel))))
> +                  (let ((instance (channel-instance channel commit checkout)))
> +                    (cons instance (latest-channel-instances
> +                                    store
> +                                    (channel-instance-dependencies instance))))))
> +              channels))

What happens if the dependency list contains duplicate channels?  This
might happen if two unrelated channels mutually depend upon a third
channel.

What happens if the dependency list contains two channels that differ
only in their branch (or commit), and are the same in every other way?
This might happen if two unrelated channels mutually depend upon two
different versions of the same third channel.

>  (define %self-build-file ;; The file containing code to build Guix.
> This serves the same purpose as @@ -223,8 +259,21 @@ INSTANCES."
> (lambda (instance) (if (eq? instance core-instance) (return core)
> -                (build-channel-instance instance
> -                                        (cons core dependencies))))
> +                (match (channel-instance-dependencies instance)
> +                  (()
> +                   (build-channel-instance instance
> +                                           (cons core dependencies)))
> +                  (channels
> +                   (mlet %store-monad ((dependencies-derivation
> +                                        (latest-channel-derivation
> +                                         ;; %default-channels is used here to
> +                                         ;; ensure that the core channel is
> +                                         ;; available for channels declared as
> +                                         ;; dependencies.
> +                                         (append channels %default-channels))))
> +                     (build-channel-instance instance
> +                                             (cons dependencies-derivation
> +                                                   (cons core dependencies))))))))

I think we should clarify in the manual the fact that channel authors do
not need to specify the core "guix" channel in their dependencies, since
it is an implicit dependency of all non-core channels.

Thank you for working on this!  I'm catching up on developments over the
last few months, and it's really exciting to see channels become a
reality.

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: Channel dependencies

[Ricardo Wurmus]

Hi Chris,

>>  (define (latest-channel-instances store channels)
>>    "Return a list of channel instances corresponding to the latest checkouts of
>> -CHANNELS."
>> -  (map (lambda (channel)
>> -         (format (current-error-port)
>> -                 (G_ "Updating channel '~a' from Git repository at '~a'...~%")
>> -                 (channel-name channel)
>> -                 (channel-url channel))
>> -         (let-values (((checkout commit)
>> -                       (latest-repository-commit store (channel-url channel)
>> -                                                 #:ref (channel-reference
>> -                                                        channel))))
>> -           (channel-instance channel commit checkout)))
>> -       channels))
>> +CHANNELS and the channels on which they depend."
>> +  (append-map (lambda (channel)
>> +                (format (current-error-port)
>> +                        (G_ "Updating channel '~a' from Git repository at '~a'...~%")
>> +                        (channel-name channel)
>> +                        (channel-url channel))
>> +                (let-values (((checkout commit)
>> +                              (latest-repository-commit store (channel-url channel)
>> +                                                        #:ref (channel-reference
>> +                                                               channel))))
>> +                  (let ((instance (channel-instance channel commit checkout)))
>> +                    (cons instance (latest-channel-instances
>> +                                    store
>> +                                    (channel-instance-dependencies instance))))))
>> +              channels))
>
> What happens if the dependency list contains duplicate channels?  This
> might happen if two unrelated channels mutually depend upon a third
> channel.
>
> What happens if the dependency list contains two channels that differ
> only in their branch (or commit), and are the same in every other way?
> This might happen if two unrelated channels mutually depend upon two
> different versions of the same third channel.

I was going to write an email about this just now.  This is indeed a
problem with this naive implementation.

For reproducibility a user may want to pin all channels to a certain
commit, but channels that are dependencies are exempt from that
mechanism as they are added to the list of channels to be instantiated
during the run of “guix pull”.

It should be possible to pin *all* channels, even if they are
dependencies of other channels.  One way is to treat all channels and
their dependencies as a unique set where duplicates are replaced with
the most specific description (e.g. those specifying a commit).  This
way users could fully specify dependencies, which would then be used
instead of the channel’s declared dependency.

This needs some more thought.

--
Ricardo

Re: Channel dependencies

[Ludovic Courtès]

Hello,

Ricardo Wurmus <rekado@elephly.net> skribis:

>>>  (define (latest-channel-instances store channels)
>>>    "Return a list of channel instances corresponding to the latest checkouts of
>>> -CHANNELS."
>>> -  (map (lambda (channel)
>>> -         (format (current-error-port)
>>> -                 (G_ "Updating channel '~a' from Git repository at '~a'...~%")
>>> -                 (channel-name channel)
>>> -                 (channel-url channel))
>>> -         (let-values (((checkout commit)
>>> -                       (latest-repository-commit store (channel-url channel)
>>> -                                                 #:ref (channel-reference
>>> -                                                        channel))))
>>> -           (channel-instance channel commit checkout)))
>>> -       channels))
>>> +CHANNELS and the channels on which they depend."
>>> +  (append-map (lambda (channel)
>>> +                (format (current-error-port)
>>> +                        (G_ "Updating channel '~a' from Git repository at '~a'...~%")
>>> +                        (channel-name channel)
>>> +                        (channel-url channel))
>>> +                (let-values (((checkout commit)
>>> +                              (latest-repository-commit store (channel-url channel)
>>> +                                                        #:ref (channel-reference
>>> +                                                               channel))))
>>> +                  (let ((instance (channel-instance channel commit checkout)))
>>> +                    (cons instance (latest-channel-instances
>>> +                                    store
>>> +                                    (channel-instance-dependencies instance))))))
>>> +              channels))
>>
>> What happens if the dependency list contains duplicate channels?  This
>> might happen if two unrelated channels mutually depend upon a third
>> channel.
>>
>> What happens if the dependency list contains two channels that differ
>> only in their branch (or commit), and are the same in every other way?
>> This might happen if two unrelated channels mutually depend upon two
>> different versions of the same third channel.

We should error out in this case because at run time, there can be only
one Guile module with a given name.  Thus, conflicting dependencies
cannot be honored.  (Not like Node.js, for better or worse.  ;-))

> I was going to write an email about this just now.  This is indeed a
> problem with this naive implementation.
>
> For reproducibility a user may want to pin all channels to a certain
> commit, but channels that are dependencies are exempt from that
> mechanism as they are added to the list of channels to be instantiated
> during the run of “guix pull”.
>
> It should be possible to pin *all* channels, even if they are
> dependencies of other channels.  One way is to treat all channels and
> their dependencies as a unique set where duplicates are replaced with
> the most specific description (e.g. those specifying a commit).  This
> way users could fully specify dependencies, which would then be used
> instead of the channel’s declared dependency.

Of course it’s easier to pin all channels when they are directly
expressed in ~/.config/guix/channels.scm.

Say, I need channel A, which depends on B.

What we could do is that if B is already in ~/.config/guix/channels.scm,
then we take this particular B; if it’s not there, we take the latest
version.

In terms of the dependency code, it means that dependencies found in
.guix-channels file are added if and only if they are not already
present in the user-provided channel list.

Does that make sense?

Thanks!

Ludo’.

Re: Channel dependencies

[Ricardo Wurmus]

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

> Of course it’s easier to pin all channels when they are directly
> expressed in ~/.config/guix/channels.scm.
>
> Say, I need channel A, which depends on B.
>
> What we could do is that if B is already in ~/.config/guix/channels.scm,
> then we take this particular B; if it’s not there, we take the latest
> version.

Yes, this sounds good to me.  I’ll implement this soon and hope that we
can merge it before Friday when I’ll present this to cluster users at
the MDC :)

--
Ricardo

Re: Channel dependencies

[Ludovic Courtès]

Hello!

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:

> the attached patch allows channel authors to declare other channels as
> dependencies of their own channel.  In addition to explicitly requested
> channels, “guix pull” will now also download and build channels that
> have been declared as dependencies in the file ‘.guix-channel’ in the
> repository root.
>
> An example of a simple .guix-channel file is this:
>
> (channel
>  (version 0)
>  (dependencies
>   (channel
>    (name guix-bimsb)
>    (url "https://github.com/BIMSBbioinfo/guix-bimsb"))))
>
> What do you think?

I think that’s great.  :-)

> From 1783c17582906df970c7e68e89d761619a35caeb Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Sat, 13 Oct 2018 08:39:23 +0200
> Subject: [PATCH] guix: Add support for channel dependencies.
>
> * guix/channels.scm (%channel-meta-file): New variable.
> (channel-meta, channel-instance-dependencies): New procedures.
> (latest-channel-instances): Include channel dependencies.
> (channel-instance-derivations): Build derivation for additional channels and
> add it as dependency to the channel instance derivation.
> * doc/guix.texi (Channels): Add subsection "Declaring Channel Dependencies".

[...]

> +(define (channel-meta instance)
> +  "Return an S-expression read from the channel INSTANCE's description file,
> +or return #F if the channel instance does not include the file."
> +  (let* ((source (channel-instance-checkout instance))
> +         (meta-file (string-append source "/" %channel-meta-file)))
> +    (and (file-exists? meta-file)
> +         (call-with-input-file meta-file read))))

As a general pattern, I’d suggest declaring <channel-metadata> record
type along with a ‘read-channel-metadata’ procedure that takes care of
“parsing” and metadata version handling.  That way parsing code is in
just one place and the rest of the code can happily deal with
well-formed records.

> +(define (channel-instance-dependencies instance)
> +  "Return the list of channels that are declared as dependencies for the given
> +channel INSTANCE."
> +  (or (and=> (assoc-ref (channel-meta instance) 'dependencies)
> +             (lambda (dependencies)
> +               (map (lambda (item)
> +                      (let ((get (lambda* (key #:optional default)
> +                                   (or (and=> (assoc-ref item key) car) default))))
> +                        (let ((name (get 'name))
> +                              (url (get 'url))
> +                              (branch (get 'branch "master"))
> +                              (commit (get 'commit)))
> +                          (and name url branch
> +                               (channel
> +                                (name name)
> +                                (branch branch)
> +                                (url url)
> +                                (commit commit))))))
> +                    dependencies)))
> +      '()))

I’d recommend ‘match’ for the outer sexp, and then something like the
‘alist-let*’ macro from (gnu services herd) in places where you’d like
to leave field ordering unspecified.

Then I think it would make sense to add the ‘dependencies’ field to
<channel-instance> directly (and keep <channel-metadata> internal.)
Each element of the ‘dependencies’ field would be another
<channel-instance>.

Actually ‘dependencies’ could be a promise that reads channel meta-data
and looks up the channel instances for the given dependencies.
Something like that.

Thoughts?

Chris raises interesting issues.  I think it’s OK to first come up with
an implementation that has some limitations but works with the simple
use cases we have in mind.

Thanks for working on it!

Ludo’.

Re: Channel dependencies

[Ludovic Courtès]

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Of course it’s easier to pin all channels when they are directly
>> expressed in ~/.config/guix/channels.scm.
>>
>> Say, I need channel A, which depends on B.
>>
>> What we could do is that if B is already in ~/.config/guix/channels.scm,
>> then we take this particular B; if it’s not there, we take the latest
>> version.
>
> Yes, this sounds good to me.  I’ll implement this soon and hope that we
> can merge it before Friday when I’ll present this to cluster users at
> the MDC :)

Sounds good!  :-)

I’ll be paying attention but feel free to ping me if needed!

Ludo’.

Re: Channel dependencies

[Ricardo Wurmus]

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


Hi Ludo,

thanks for your helpful comments!

>> +(define (channel-meta instance)
>> +  "Return an S-expression read from the channel INSTANCE's description file,
>> +or return #F if the channel instance does not include the file."
>> +  (let* ((source (channel-instance-checkout instance))
>> +         (meta-file (string-append source "/" %channel-meta-file)))
>> +    (and (file-exists? meta-file)
>> +         (call-with-input-file meta-file read))))
>
> As a general pattern, I’d suggest declaring <channel-metadata> record
> type along with a ‘read-channel-metadata’ procedure that takes care of
> “parsing” and metadata version handling.  That way parsing code is in
> just one place and the rest of the code can happily deal with
> well-formed records.

Yes, that’s a good idea.  I added a record <channel-metadata> and a
procedure “read-channel-metadata” that produces values of this type
given a channel instance (or #F).

>> +(define (channel-instance-dependencies instance)
>> +  "Return the list of channels that are declared as dependencies for the given
>> +channel INSTANCE."
>> +  (or (and=> (assoc-ref (channel-meta instance) 'dependencies)
>> +             (lambda (dependencies)
>> +               (map (lambda (item)
>> +                      (let ((get (lambda* (key #:optional default)
>> +                                   (or (and=> (assoc-ref item key) car) default))))
>> +                        (let ((name (get 'name))
>> +                              (url (get 'url))
>> +                              (branch (get 'branch "master"))
>> +                              (commit (get 'commit)))
>> +                          (and name url branch
>> +                               (channel
>> +                                (name name)
>> +                                (branch branch)
>> +                                (url url)
>> +                                (commit commit))))))
>> +                    dependencies)))
>> +      '()))
>
> I’d recommend ‘match’ for the outer sexp, and then something like the
> ‘alist-let*’ macro from (gnu services herd) in places where you’d like
> to leave field ordering unspecified.

I keep forgetting about alist-let* (from srfi-2, not herd), even though
it’s so useful!  “channel-instance-dependencies” now uses the
<channel-metadata> record via “read-channel-metadata” and uses match.

> Then I think it would make sense to add the ‘dependencies’ field to
> <channel-instance> directly (and keep <channel-metadata> internal.)
> Each element of the ‘dependencies’ field would be another
> <channel-instance>.
>
> Actually ‘dependencies’ could be a promise that reads channel meta-data
> and looks up the channel instances for the given dependencies.
> Something like that.

This sounds good, but I don’t know how to make it work well, because
there’s a circular relationship here if we want to keep the abstractions
pretty.  I can’t simply define the “dependencies” field of
<channel-instance> to have a default thunked procedure like this:

   (match (read-channel-metadata checkout)
     (#f '())
     (($ <channel-metadata> _ dependencies)
      dependencies))

Because record fields cannot access other record fields such as
“checkout”.  This makes the code look rather silly as we’re creating an
instance with an explicit dependencies value only to read it from that
same record in the next expression.

In light of these complications I’d prefer to have a procedure
“channel-instance-dependencies” that handles this for us, and do without
a “dependencies” field on the <channel-instance> record.

What do you think?

> Chris raises interesting issues.  I think it’s OK to first come up with
> an implementation that has some limitations but works with the simple
> use cases we have in mind.

I’ve fixed this according to what we’ve discussed: when more than one of
the user-provided or channel-required channels have the same name we
ignore the more recent specification unless it is more specific
(i.e. the new channel specification mentions a commit while the former
did not).

This is a little verbose because I replaced the simple “append-map” with
a more complex “fold” with a composite accumulator to avoid mutation.

Suggestions on how to simplify this are welcome!

--
Ricardo


[-- Attachment #2: 0001-guix-Add-support-for-channel-dependencies.patch --]
[-- Type: text/x-patch, Size: 10644 bytes --]

From e23225640e723988de215d110e377c93c8108245 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Sat, 13 Oct 2018 08:39:23 +0200
Subject: [PATCH] guix: Add support for channel dependencies.

* guix/channels.scm (<channel-metadata>): New record.
(read-channel-metadata, channel-instance-dependencies): New procedures.
(latest-channel-instances): Include channel dependencies; add optional
argument PREVIOUS-CHANNELS.
(channel-instance-derivations): Build derivation for additional channels and
add it as dependency to the channel instance derivation.
* doc/guix.texi (Channels): Add subsection "Declaring Channel Dependencies".
---
 doc/guix.texi     |  33 +++++++++++++
 guix/channels.scm | 122 ++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 139 insertions(+), 16 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f4f19949f..7291a88ba 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -3020,6 +3020,39 @@ the new and upgraded packages that are listed, some like @code{my-gimp} and
 @code{my-emacs-with-cool-features} might come from
 @code{my-personal-packages}, while others come from the Guix default channel.
 
+@cindex dependencies, channels
+@cindex meta-data, channels
+@subsection Declaring Channel Dependencies
+
+Channel authors may decide to augment a package collection provided by other
+channels.  They can declare their channel to be dependent on other channels in
+a meta-data file @file{.guix-channel}, which is to be placed in the root of
+the channel repository.
+
+The meta-data file should contain a simple S-expression like this:
+
+@lisp
+(channel
+ (version 0)
+ (dependencies
+  (channel
+   (name 'some-collection)
+   (url "https://example.org/first-collection.git"))
+  (channel
+   (name 'some-other-collection)
+   (url "https://example.org/second-collection.git")
+   (branch "testing"))))
+@end lisp
+
+In the above example this channel is declared to depend on two other channels,
+which will both be fetched automatically.  The modules provided by the channel
+will be compiled in an environment where the modules of all these declared
+channels are available.
+
+For the sake of reliability and maintainability, you should avoid dependencies
+on channels that you don't control, and you should aim to keep the number of
+dependencies to a minimum.
+
 @subsection Replicating Guix
 
 @cindex pinning, channels
diff --git a/guix/channels.scm b/guix/channels.scm
index 82389eb58..6393179a4 100644
--- a/guix/channels.scm
+++ b/guix/channels.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2018 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -27,6 +28,7 @@
   #:use-module (guix store)
   #:use-module (guix i18n)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-2)
   #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-11)
   #:autoload   (guix self) (whole-package)
@@ -72,7 +74,6 @@
   (commit    channel-commit (default #f))
   (location  channel-location
              (default (current-source-location)) (innate)))
-;; TODO: Add a way to express dependencies among channels.
 
 (define %default-channels
   ;; Default list of channels.
@@ -92,6 +93,12 @@
   (commit    channel-instance-commit)
   (checkout  channel-instance-checkout))
 
+(define-record-type <channel-metadata>
+  (channel-metadata version dependencies)
+  channel-metadata?
+  (version       channel-metadata-version)
+  (dependencies  channel-metadata-dependencies))
+
 (define (channel-reference channel)
   "Return the \"reference\" for CHANNEL, an sexp suitable for
 'latest-repository-commit'."
@@ -99,20 +106,90 @@
     (#f      `(branch . ,(channel-branch channel)))
     (commit  `(commit . ,(channel-commit channel)))))
 
-(define (latest-channel-instances store channels)
+(define (read-channel-metadata instance)
+  "Return a channel-metadata record read from the channel INSTANCE's
+description file, or return #F if the channel instance does not include the
+file."
+  (let* ((source (channel-instance-checkout instance))
+         (meta-file (string-append source "/.guix-channel")))
+    (and (file-exists? meta-file)
+         (and-let* ((raw (call-with-input-file meta-file read))
+                    (version (and=> (assoc-ref raw 'version) first))
+                    (dependencies (or (assoc-ref raw 'dependencies) '())))
+           (channel-metadata
+            version
+            (map (lambda (item)
+                   (let ((get (lambda* (key #:optional default)
+                                (or (and=> (assoc-ref item key) first) default))))
+                     (and-let* ((name (get 'name))
+                                (url (get 'url))
+                                (branch (get 'branch "master")))
+                       (channel
+                        (name name)
+                        (branch branch)
+                        (url url)
+                        (commit (get 'commit))))))
+                 dependencies))))))
+
+(define (channel-instance-dependencies instance)
+  "Return the list of channels that are declared as dependencies for the given
+channel INSTANCE."
+  (match (read-channel-metadata instance)
+    (#f '())
+    (($ <channel-metadata> version dependencies)
+     dependencies)))
+
+(define* (latest-channel-instances store channels #:optional (previous-channels '()))
   "Return a list of channel instances corresponding to the latest checkouts of
-CHANNELS."
-  (map (lambda (channel)
-         (format (current-error-port)
-                 (G_ "Updating channel '~a' from Git repository at '~a'...~%")
-                 (channel-name channel)
-                 (channel-url channel))
-         (let-values (((checkout commit)
-                       (latest-repository-commit store (channel-url channel)
-                                                 #:ref (channel-reference
-                                                        channel))))
-           (channel-instance channel commit checkout)))
-       channels))
+CHANNELS and the channels on which they depend.  PREVIOUS-CHANNELS is a list
+of previously processed channels."
+  ;; Only process channels that are unique, or that are more specific than a
+  ;; previous channel specification.
+  (define (ignore? channel others)
+    (member channel others
+            (lambda (a b)
+              (and (eq? (channel-name a) (channel-name b))
+                   (or (channel-commit b)
+                       (not (or (channel-commit a)
+                                (channel-commit b))))))))
+  ;; Accumulate a list of instances.  A list of processed channels is also
+  ;; accumulated to decide on duplicate channel specifications.
+  (match (fold (lambda (channel acc)
+                 (match acc
+                   ((#:channels previous-channels #:instances instances)
+                    (if (ignore? channel previous-channels)
+                        acc
+                        (begin
+                          (format (current-error-port)
+                                  (G_ "Updating channel '~a' from Git repository at '~a'...~%")
+                                  (channel-name channel)
+                                  (channel-url channel))
+                          (let-values (((checkout commit)
+                                        (latest-repository-commit store (channel-url channel)
+                                                                  #:ref (channel-reference
+                                                                         channel))))
+                            (let ((instance (channel-instance channel commit checkout)))
+                              (let-values (((new-instances new-channels)
+                                            (latest-channel-instances
+                                             store
+                                             (channel-instance-dependencies instance)
+                                             previous-channels)))
+                                `(#:channels
+                                  ,(append (cons channel new-channels)
+                                           previous-channels)
+                                  #:instances
+                                  ,(append (cons instance new-instances)
+                                           instances))))))))))
+               `(#:channels ,previous-channels #:instances ())
+               channels)
+    ((#:channels channels #:instances instances)
+     (let ((instance-name (compose channel-name channel-instance-channel)))
+       ;; Remove all earlier channel specifications if they are followed by a
+       ;; more specific one.
+       (values (delete-duplicates instances
+                                  (lambda (a b)
+                                    (eq? (instance-name a) (instance-name b))))
+               channels)))))
 
 (define %self-build-file
   ;; The file containing code to build Guix.  This serves the same purpose as
@@ -223,8 +300,21 @@ INSTANCES."
           (lambda (instance)
             (if (eq? instance core-instance)
                 (return core)
-                (build-channel-instance instance
-                                        (cons core dependencies))))
+                (match (channel-instance-dependencies instance)
+                  (()
+                   (build-channel-instance instance
+                                           (cons core dependencies)))
+                  (channels
+                   (mlet %store-monad ((dependencies-derivation
+                                        (latest-channel-derivation
+                                         ;; %default-channels is used here to
+                                         ;; ensure that the core channel is
+                                         ;; available for channels declared as
+                                         ;; dependencies.
+                                         (append channels %default-channels))))
+                     (build-channel-instance instance
+                                             (cons dependencies-derivation
+                                                   (cons core dependencies))))))))
           instances)))
 
 (define (whole-package-for-legacy name modules)
-- 
2.19.0

Re: Channel dependencies

[Chris Marusich]

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

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:

> [...]
>
>> Chris raises interesting issues.  I think it’s OK to first come up with
>> an implementation that has some limitations but works with the simple
>> use cases we have in mind.
>
> I’ve fixed this according to what we’ve discussed: when more than one of
> the user-provided or channel-required channels have the same name we
> ignore the more recent specification unless it is more specific
> (i.e. the new channel specification mentions a commit while the former
> did not).

As long as the "channel resolution mechanism" is deterministic, it's
probably OK.  But if you have two channels A which depends on C, and B
which depends on C', where C' is a different version of C, then we can
arrive in a situation where the author of A tests and provides support
for their package definitions in the absence of channel B, and the
author of B tests and provides support for their package definitions in
the absence of channel A, and a user who wants to use packages from both
A and B is stuck because one of the channel owners (the one whose
dependency we didn't pick) doesn't want to support that use case.

It sounds to me like we are taking all the channels are merging them
into a single profile, and then we access it using a single Guix
inferior.  That's why we feel like we have to chose a specific version
of C to use with both A and B.  In this way, "installing" multiple
channels into this one profile is similar to the propagation of multiple
packages into a single profile.

Consider package propagation.  If I have a piece of software X which
depends on library Z and another piece of software Y which depends on
Z', where Z' is a different version of Z, then we have a similar
problem.  If I install packages X and Y into my profile and the
libraries Z and Z' are propagated, it will cause conflicts, and we will
need to choose a single version of the library.  When we do that, there
is no guarantee that X or Y will function correctly, since the person
who developed X didn't test using Z', and the person who developed Y
didn't test using Z.

Instead, if we install install X and Y without propagating Z and Z', we
have a solution: X and Y can coexist in the same profile.  X refers to Z
in the store (via its rpath or equivalent), and Y refers to Z' in
another location in the store.  When the user runs X, it runs using the
library Z.  When the user runs Y, it runs using library Z'.  The user
has not "voided their warranty", so to speak, by using a version of the
library that the developer doesn't want to support.  If there's a
problem, the user can go to the developer for support more easily.

I think your proposed solution is basically "channel propagation".
Don't get me wrong: It's great that we can choose a specific version of
channel C deterministically!  This means users can share their
channels.scm file and reproduce the exact channel configuration easily.
However, it might be even better if we could figure out how to avoid
"propagating" the channels and introducing conflicts.  Maybe there is a
way to run one Guix inferior per channel, so that one inferior can be
responsible for packages from channel A (using C), and another inferior
can be responsible for packages from channel B (using C')?

By the way, even if we come up with a solution like this, I think it's
safe to say that the core Guix channel must be the same version always.
I can't currently see how it might make sense for two third-party
channels to depend on different versions of the Guix channel, since the
Guix channel provides not only package definitions but also the Guix
code.

I hope that makes sense.  Either way, your work is already an
improvement.  Thank you for it!

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: Channel dependencies

[Ludovic Courtès]

Hello!

Chris Marusich <cmmarusich@gmail.com> skribis:

> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:
>
>> [...]
>>
>>> Chris raises interesting issues.  I think it’s OK to first come up with
>>> an implementation that has some limitations but works with the simple
>>> use cases we have in mind.
>>
>> I’ve fixed this according to what we’ve discussed: when more than one of
>> the user-provided or channel-required channels have the same name we
>> ignore the more recent specification unless it is more specific
>> (i.e. the new channel specification mentions a commit while the former
>> did not).
>
> As long as the "channel resolution mechanism" is deterministic, it's
> probably OK.  But if you have two channels A which depends on C, and B
> which depends on C', where C' is a different version of C, then we can
> arrive in a situation where the author of A tests and provides support
> for their package definitions in the absence of channel B, and the
> author of B tests and provides support for their package definitions in
> the absence of channel A, and a user who wants to use packages from both
> A and B is stuck because one of the channel owners (the one whose
> dependency we didn't pick) doesn't want to support that use case.

Good point.  I agree that it’s similar to the question of propagated
inputs, which we deal with by reporting an error when a collision
arises.

So, similarly, I think the safe way would be to report an error when
channel requirements conflict.

We must define what it means for two <channel>s to conflict:

  • if a channel’s ‘commit’ is #f, then any channel with the same name
    but a different ‘uri’ and/or a different ‘branch’ and/or a non-#f
    commit conflicts;

  • if a channel’s ‘commit’ is not #f, then any channel with the same
    name and otherwise different fields conflicts.

WDYT?

If we have inspiration later, we can liberalize this, for instance by
using several inferiors.  It would be quite a bit of extra work, and
it’s not immediately clear to me how that could work.  I believe what
Ricardo proposes already covers many use cases anyway.

Thanks,
Ludo’.

Re: Channel dependencies

[Ludovic Courtès]

Hello!

Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:

>> I’d recommend ‘match’ for the outer sexp, and then something like the
>> ‘alist-let*’ macro from (gnu services herd) in places where you’d like
>> to leave field ordering unspecified.
>
> I keep forgetting about alist-let* (from srfi-2, not herd), even though
> it’s so useful!  “channel-instance-dependencies” now uses the
> <channel-metadata> record via “read-channel-metadata” and uses match.

Heck I didn’t even know about SRFI-2.  :-)

>> Then I think it would make sense to add the ‘dependencies’ field to
>> <channel-instance> directly (and keep <channel-metadata> internal.)
>> Each element of the ‘dependencies’ field would be another
>> <channel-instance>.
>>
>> Actually ‘dependencies’ could be a promise that reads channel meta-data
>> and looks up the channel instances for the given dependencies.
>> Something like that.
>
> This sounds good, but I don’t know how to make it work well, because
> there’s a circular relationship here if we want to keep the abstractions
> pretty.  I can’t simply define the “dependencies” field of
> <channel-instance> to have a default thunked procedure like this:
>
>    (match (read-channel-metadata checkout)
>      (#f '())
>      (($ <channel-metadata> _ dependencies)
>       dependencies))
>
> Because record fields cannot access other record fields such as
> “checkout”.  This makes the code look rather silly as we’re creating an
> instance with an explicit dependencies value only to read it from that
> same record in the next expression.
>
> In light of these complications I’d prefer to have a procedure
> “channel-instance-dependencies” that handles this for us, and do without
> a “dependencies” field on the <channel-instance> record.
>
> What do you think?

Sure, that makes sense to me (sometimes I throw ideas that look great on
paper but simply don’t fly in practice!).

> From e23225640e723988de215d110e377c93c8108245 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Sat, 13 Oct 2018 08:39:23 +0200
> Subject: [PATCH] guix: Add support for channel dependencies.
>
> * guix/channels.scm (<channel-metadata>): New record.
> (read-channel-metadata, channel-instance-dependencies): New procedures.
> (latest-channel-instances): Include channel dependencies; add optional
> argument PREVIOUS-CHANNELS.
> (channel-instance-derivations): Build derivation for additional channels and
> add it as dependency to the channel instance derivation.
> * doc/guix.texi (Channels): Add subsection "Declaring Channel Dependencies".

[...]

> +  ;; Accumulate a list of instances.  A list of processed channels is also
> +  ;; accumulated to decide on duplicate channel specifications.
> +  (match (fold (lambda (channel acc)
> +                 (match acc
> +                   ((#:channels previous-channels #:instances instances)
> +                    (if (ignore? channel previous-channels)
> +                        acc
> +                        (begin
> +                          (format (current-error-port)
> +                                  (G_ "Updating channel '~a' from Git repository at '~a'...~%")
> +                                  (channel-name channel)
> +                                  (channel-url channel))
> +                          (let-values (((checkout commit)
> +                                        (latest-repository-commit store (channel-url channel)
> +                                                                  #:ref (channel-reference
> +                                                                         channel))))
> +                            (let ((instance (channel-instance channel commit checkout)))
> +                              (let-values (((new-instances new-channels)
> +                                            (latest-channel-instances
> +                                             store
> +                                             (channel-instance-dependencies instance)
> +                                             previous-channels)))
> +                                `(#:channels
> +                                  ,(append (cons channel new-channels)
> +                                           previous-channels)
> +                                  #:instances
> +                                  ,(append (cons instance new-instances)
> +                                           instances))))))))))
> +               `(#:channels ,previous-channels #:instances ())
> +               channels)

This seems to be assuming that CHANNELS is topologically-sorted, is that
right?

Otherwise LGTM.

There’s the conflict error reporting mentioned in another message that I
think we could implement, but that can come later.

Also we should consider adding unit tests for at least some of this; I
plaid guilty for not having provided a test strategy from the start.

Thank you, and apologies for not replying on time for your presentation!

Ludo’.

Re: Channel dependencies

[Chris Marusich]

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

ludo@gnu.org (Ludovic Courtès) writes:

> Good point.  I agree that it’s similar to the question of propagated
> inputs, which we deal with by reporting an error when a collision
> arises.
>
> So, similarly, I think the safe way would be to report an error when
> channel requirements conflict.

With profiles, two packages conflict if and only if a file exists at the
same relative path in both packages' outputs.  In other words, both
packages provide "the same" file.

With channels, we build a profile containing the union of the channels.
So, normal profile conflicts will definitely occur in the profile if two
channels provide the same module (e.g., $out/my/awesome/packages.scm),
right?  Because we're putting the two channels into a single profile, I
think the normal conflict resolution mechanism will force us to use only
one module in the resulting profile.  And I think that if two channels
provide the same module, it's definitely a conflict of some kind.

Furthermore, if two channels both provide "the same" package definition
for a tool called my-awesome-program in two different modules, Guile
scheme code can still specify precisely which my-awesome-program should
be used.  In that sense, there is no conflict.  However, if both package
definition use the same name and version, then a command like "guix
package -i my-awesome-program" (and the APIs that convert package
specifications to packages) will have to choose one specific package,
right?  In that sense, there is a conflict.

It seems to me that these kinds of conflicts are what we want to avoid.
Regardless of what the channel is named, regardless of what URI it comes
from, regardless of what commit it came from, if it provides the same
modules or the same packages as another channel, there's a "channel
conflict".

However, detecting that sort of conflict seems pretty complicated.  I
don't have a good idea for how to detect it.  It seems like you would
have to traverse the entire set of modules and packages that a channel
provides, and cross-check it with other channels to make sure there are
no conflicts.  We might have to use multiple inferiors to do that, like
you mentioned.

Also like you said, we can try to implement some heuristics to reject
situations in which a "channel conflict" is likely.  Would it be hard to
change the channel mechanism so that it fails if there are any (normal)
conflicts while generating the profile that contains all the channels?
If we could prevent those (normal) conflicts while generating the
profile, it would prevent a certain class of channel conflicts: namely,
it would be impossible for two channels to provide the same guile
modules.

> We must define what it means for two <channel>s to conflict:
>
>   • if a channel’s ‘commit’ is #f, then any channel with the same name
>     but a different ‘uri’ and/or a different ‘branch’ and/or a non-#f
>     commit conflicts;
>
>   • if a channel’s ‘commit’ is not #f, then any channel with the same
>     name and otherwise different fields conflicts.

This seems like a reasonable heuristic.  What will we do when two
channels differ only in their name?  What about when two channels only
have identical fields?  Maybe in those cases we should just pick one,
ignore the other, and log a warning, since their content will be the
same.

> If we have inspiration later, we can liberalize this, for instance by
> using several inferiors.  It would be quite a bit of extra work, and
> it’s not immediately clear to me how that could work.  I believe what
> Ricardo proposes already covers many use cases anyway.

You're probably right.  I'm just trying to think about how we might
apply the functional model to this problem, rather than implementing
heuristics.  But maybe heuristics are good enough!

-- 
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: Channel dependencies

[Ludovic Courtès]

Hello!

Chris Marusich <cmmarusich@gmail.com> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Good point.  I agree that it’s similar to the question of propagated
>> inputs, which we deal with by reporting an error when a collision
>> arises.
>>
>> So, similarly, I think the safe way would be to report an error when
>> channel requirements conflict.
>
> With profiles, two packages conflict if and only if a file exists at the
> same relative path in both packages' outputs.

What you describe here are “soft collisions”, which the profile builder
reports as warnings (which are invisible with today’s ‘guix package’.)

I was referring to profile collisions where two packages with the same
name end up in the same profile (the ‘&profile-collision-error’
exception.)

This exception would also be raised if ‘guix pull’ ended up adding the
same channels more than once in ~/.config/guix/current.

> Also like you said, we can try to implement some heuristics to reject
> situations in which a "channel conflict" is likely.  Would it be hard to
> change the channel mechanism so that it fails if there are any (normal)
> conflicts while generating the profile that contains all the channels?
> If we could prevent those (normal) conflicts while generating the
> profile, it would prevent a certain class of channel conflicts: namely,
> it would be impossible for two channels to provide the same guile
> modules.

‘union-build’ has a #:resolve-collision parameter.  We could set it when
building ~/.config/guix/current so that an error is raised when the same
file is provided more than once.

(It’s a simple change we can make independently of what Ricardo is
proposing.)

WDYT?

>> We must define what it means for two <channel>s to conflict:
>>
>>   • if a channel’s ‘commit’ is #f, then any channel with the same name
>>     but a different ‘uri’ and/or a different ‘branch’ and/or a non-#f
>>     commit conflicts;
>>
>>   • if a channel’s ‘commit’ is not #f, then any channel with the same
>>     name and otherwise different fields conflicts.
>
> This seems like a reasonable heuristic.  What will we do when two
> channels differ only in their name?  What about when two channels only
> have identical fields?  Maybe in those cases we should just pick one,
> ignore the other, and log a warning, since their content will be the
> same.

Yes, they would effectively be ‘equal?’.

>> If we have inspiration later, we can liberalize this, for instance by
>> using several inferiors.  It would be quite a bit of extra work, and
>> it’s not immediately clear to me how that could work.  I believe what
>> Ricardo proposes already covers many use cases anyway.
>
> You're probably right.  I'm just trying to think about how we might
> apply the functional model to this problem, rather than implementing
> heuristics.  But maybe heuristics are good enough!

Sure, and that’s good!

Thanks,
Ludo’.