From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52411) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fecM1-00023q-Iu for guix-patches@gnu.org; Sun, 15 Jul 2018 04:26:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fecLy-0003GI-DY for guix-patches@gnu.org; Sun, 15 Jul 2018 04:26:05 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:52579) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fecLy-0003GE-8u for guix-patches@gnu.org; Sun, 15 Jul 2018 04:26:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fecLy-00018A-2A for guix-patches@gnu.org; Sun, 15 Jul 2018 04:26:02 -0400 Subject: [bug#32121] [PATCH 5/5] Add support for multiple inputs. Resent-Message-ID: References: <20180710230247.16639-1-clement@lassieur.org> <20180710230247.16639-5-clement@lassieur.org> <87va9jjxgr.fsf@gnu.org> From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <87va9jjxgr.fsf@gnu.org> Date: Sun, 15 Jul 2018 10:25:32 +0200 Message-ID: <87efg4gb2b.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 32121@debbugs.gnu.org Ludovic Court=C3=A8s writes: > Cl=C3=A9ment Lassieur skribis: > >> (define* (main #:optional (args (command-line))) >> (match args >> - ((command load-path guix-package-path source specstr) >> - ;; Load FILE, a Scheme file that defines Hydra jobs. >> + ((command static-guix-package-path specstr checkoutsstr) >> + ;; Load PROC-FILE, a Scheme file that defines Hydra jobs. > > There=E2=80=99s no =E2=80=9Cproc-file=E2=80=9D; should it be =E2=80=9Cpro= c-source=E2=80=9D? It was #:PROC-PATH, but I'll bind it to FILE and use FILE in the comment, as it was initially. > Do I get it write that inputs do not necessarily contribute to > GUIX_PACKAGE_PATH? Yes! Only inputs in package-path-inputs contribute to GUIX_PACKAGE_PATH. > Some inputs may provide code (to be in %load-path) while not provide any > package definition (so nothing to add to GUIX_PACKAGE_PATH.) Indeed. And some inputs can contribute to both %load-path and GUIX_PACKAGE_PATH. It's flexible. >> ;; Since we have relative file name canonicalization by default,= better >> - ;; change to SOURCE to make sure things like 'include' with rela= tive >> - ;; file names work as expected. >> - (chdir source) >> + ;; change to PROC-SOURCE to make sure things like 'include' with >> + ;; relative file names work as expected. >> + (chdir proc-source) > > As a rule of thumb, identifiers for local variables should, IMO, almost > always be a single word or at most two words. Long names like > =E2=80=98static-guix-package-path=E2=80=99 in local scope tend to make co= de harder to > read; =E2=80=98proc-source=E2=80=99 here should probably be =E2=80=98sour= ce=E2=80=99 because we know > what it is we=E2=80=99re talking about. Okay! Well I'll just remove static-guix-package-path (you know, the --load-path argument to the cuirass command), because it's better to use inputs instead. And it'll simplify the code. I'll also rename my GET-something procedures. >> (save-module-excursion >> (lambda () >> (set-current-module %user-module) >> - (primitive-load (assq-ref spec #:file)))) >> + (primitive-load (assq-ref spec #:proc-path)))) > > Nitpick: in GNU =E2=80=9Cpath=E2=80=9D means =E2=80=9Csearch path=E2=80= =9D (a list of directories), so > here I think it should be =E2=80=9Cfile=E2=80=9D or =E2=80=9Cfile name=E2= =80=9D, not =E2=80=9Cpath=E2=80=9D. Ok I'll change it everywhere else too. >> @command{cuirass} acts as a daemon polling @acronym{VCS, version control >> -system} repositories for changes, and evaluating a derivation when >> -something has changed (@pxref{Derivations, Derivations,, guix, Guix}). >> -As a final step the derivation is realized and the result of that build >> -allows you to know if the job succeeded or not. >> +system} repositories (called @code{inputs}) for changes, and evaluating= a > > s/@code/@dfn/ > >> +derivation when an @code{input} has changed (@pxref{Derivations, Deriva= tions,, > > s/@code// > > @code is to refer to identifiers in the code, things like that. Got it :-) >> +There are three @code{inputs}: one tracking the Guix repository, one tr= acking > > s/@code// > >> +(define (compile-checkouts spec all-checkouts) >> + (let* ((checkouts (filter compile? all-checkouts)) >> + (thunks >> + (map >> + (lambda (checkout) >> + (lambda () >> + (log-message "compiling input '~a' of spec '~a' (commit = ~s)" >> + (assq-ref checkout #:name) >> + (assq-ref spec #:name) >> + (assq-ref checkout #:commit)) >> + (compile checkout))) >> + checkouts)) >> + (results (par-map %non-blocking thunks))) >> + (map (lambda (checkout) >> + (log-message "compiled input '~a' of spec '~a' (commit ~s)" >> + (assq-ref checkout #:name) >> + (assq-ref spec #:name) >> + (assq-ref checkout #:commit)) >> + checkout) >> + results))) > > Since the return value is unused, we could perhaps make it: > > (define (compile-checkouts spec checkouts) > (for-each (lambda (checkout) > (log-message =E2=80=A6) > (non-blocking (compile checkout))) > checkouts)) I use par-map because it's better to build them in parallel. Also, the return value is used to display a log message. > and move the =E2=80=98filter=E2=80=99 call to the call site (the job of > =E2=80=98compile-checkouts=E2=80=99, one might think, is to compile what = it=E2=80=99s given, not > to filter things.) Right! Thank you for the review, I'm learning a lot ;-). Cl=C3=A9ment