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