Hello! Ricardo Wurmus <ricardo.wur...@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 <rek...@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’.
