Re: Channel dependencies

[Ludovic Courtès]

Hello!

Ricardo Wurmus <address@hidden> 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 <address@hidden>
> 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’.