From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: Channel dependencies Date: Thu, 18 Oct 2018 22:24:32 +0200 Message-ID: <87tvljknpb.fsf@mdc-berlin.de> References: <877eimnxpq.fsf@mdc-berlin.de> <87pnwbim2q.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:45649) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gDEqi-0000Zb-R4 for guix-devel@gnu.org; Thu, 18 Oct 2018 16:24:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gDEqe-0004sO-9Y for guix-devel@gnu.org; Thu, 18 Oct 2018 16:24:52 -0400 In-Reply-To: <87pnwbim2q.fsf@gnu.org> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Ludovic =?utf-8?Q?Court=C3=A8s?= Cc: guix-devel@gnu.org --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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=E2=80=99d suggest declaring re= cord > type along with a =E2=80=98read-channel-metadata=E2=80=99 procedure that = takes care of > =E2=80=9Cparsing=E2=80=9D and metadata version handling. That way parsin= g code is in > just one place and the rest of the code can happily deal with > well-formed records. Yes, that=E2=80=99s a good idea. I added a record and a procedure =E2=80=9Cread-channel-metadata=E2=80=9D that produces values of t= his type given a channel instance (or #F). >> +(define (channel-instance-dependencies instance) >> + "Return the list of channels that are declared as dependencies for th= e given >> +channel INSTANCE." >> + (or (and=3D> (assoc-ref (channel-meta instance) 'dependencies) >> + (lambda (dependencies) >> + (map (lambda (item) >> + (let ((get (lambda* (key #:optional default) >> + (or (and=3D> (assoc-ref item key) ca= r) 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=E2=80=99d recommend =E2=80=98match=E2=80=99 for the outer sexp, and the= n something like the > =E2=80=98alist-let*=E2=80=99 macro from (gnu services herd) in places whe= re you=E2=80=99d like > to leave field ordering unspecified. I keep forgetting about alist-let* (from srfi-2, not herd), even though it=E2=80=99s so useful! =E2=80=9Cchannel-instance-dependencies=E2=80=9D no= w uses the record via =E2=80=9Cread-channel-metadata=E2=80=9D and u= ses match. > Then I think it would make sense to add the =E2=80=98dependencies=E2=80= =99 field to > directly (and keep internal.) > Each element of the =E2=80=98dependencies=E2=80=99 field would be another > . > > Actually =E2=80=98dependencies=E2=80=99 could be a promise that reads cha= nnel meta-data > and looks up the channel instances for the given dependencies. > Something like that. This sounds good, but I don=E2=80=99t know how to make it work well, because there=E2=80=99s a circular relationship here if we want to keep the abstrac= tions pretty. I can=E2=80=99t simply define the =E2=80=9Cdependencies=E2=80=9D f= ield of to have a default thunked procedure like this: (match (read-channel-metadata checkout) (#f '()) (($ _ dependencies) dependencies)) Because record fields cannot access other record fields such as =E2=80=9Ccheckout=E2=80=9D. This makes the code look rather silly as we=E2= =80=99re 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=E2=80=99d prefer to have a procedure =E2=80=9Cchannel-instance-dependencies=E2=80=9D that handles this for us, a= nd do without a =E2=80=9Cdependencies=E2=80=9D field on the record. What do you think? > Chris raises interesting issues. I think it=E2=80=99s OK to first come u= p with > an implementation that has some limitations but works with the simple > use cases we have in mind. I=E2=80=99ve fixed this according to what we=E2=80=99ve discussed: when mor= e 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 =E2=80=9Cappend-map= =E2=80=9D with a more complex =E2=80=9Cfold=E2=80=9D with a composite accumulator to avoid= mutation. Suggestions on how to simplify this are welcome! -- Ricardo --=-=-= Content-Type: text/x-patch; charset="utf-8" Content-Disposition: inline; filename="0001-guix-Add-support-for-channel-dependencies.patch" Content-Transfer-Encoding: quoted-printable >From e23225640e723988de215d110e377c93c8108245 Mon Sep 17 00:00:00 2001 From: Ricardo Wurmus Date: Sat, 13 Oct 2018 08:39:23 +0200 Subject: [PATCH] guix: Add support for channel dependencies. * guix/channels.scm (): 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 chann= el. =20 +@cindex dependencies, channels +@cindex meta-data, channels +@subsection Declaring Channel Dependencies + +Channel authors may decide to augment a package collection provided by oth= er +channels. They can declare their channel to be dependent on other channel= s 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 chann= els, +which will both be fetched automatically. The modules provided by the cha= nnel +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 dependen= cies +on channels that you don't control, and you should aim to keep the number = of +dependencies to a minimum. + @subsection Replicating Guix =20 @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 =C2=A9 2018 Ludovic Court=C3=A8s +;;; Copyright =C2=A9 2018 Ricardo Wurmus ;;; ;;; 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. =20 (define %default-channels ;; Default list of channels. @@ -92,6 +93,12 @@ (commit channel-instance-commit) (checkout channel-instance-checkout)) =20 +(define-record-type + (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))))) =20 -(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=3D> (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=3D> (assoc-ref item key) first) d= efault)))) + (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 g= iven +channel INSTANCE." + (match (read-channel-metadata instance) + (#f '()) + (($ version dependencies) + dependencies))) + +(define* (latest-channel-instances store channels #:optional (previous-cha= nnels '())) "Return a list of channel instances corresponding to the latest checkout= s 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 channe= l) - #:ref (channel-reference - channel)))) - (channel-instance channel commit checkout))) - channels)) +CHANNELS and the channels on which they depend. PREVIOUS-CHANNELS is a li= st +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 repo= sitory at '~a'...~%") + (channel-name channel) + (channel-url channel)) + (let-values (((checkout commit) + (latest-repository-commit store (c= hannel-url channel) + #:ref (c= hannel-reference + c= hannel)))) + (let ((instance (channel-instance channel comm= it checkout))) + (let-values (((new-instances new-channels) + (latest-channel-instances + store + (channel-instance-dependencie= s 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 b= y a + ;; more specific one. + (values (delete-duplicates instances + (lambda (a b) + (eq? (instance-name a) (instance-name = b)))) + channels))))) =20 (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 declare= d as + ;; dependencies. + (append channels %default-channel= s)))) + (build-channel-instance instance + (cons dependencies-derivation + (cons core dependencies= )))))))) instances))) =20 (define (whole-package-for-legacy name modules) --=20 2.19.0 --=-=-=--