From debbugs-submit-bounces@debbugs.gnu.org Sat Jul 20 10:29:59 2019 Received: (at 36555) by debbugs.gnu.org; 20 Jul 2019 14:29:59 +0000 Received: from localhost ([127.0.0.1]:57728 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hoqN4-0004vu-S3 for submit@debbugs.gnu.org; Sat, 20 Jul 2019 10:29:59 -0400 Received: from eggs.gnu.org ([209.51.188.92]:45791) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hoqN1-0004ve-9N for 36555@debbugs.gnu.org; Sat, 20 Jul 2019 10:29:58 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:51430) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hoqMv-0000tP-OK; Sat, 20 Jul 2019 10:29:49 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=42226 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1hoqMu-0002k0-Ay; Sat, 20 Jul 2019 10:29:48 -0400 From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) Subject: Re: [bug#36555] [PATCH v4 1/3] guix system: Add 'reconfigure' module. References: <87imsci9sj.fsf@sdf.lonestar.org> <87ef30i9fl.fsf@sdf.lonestar.org> <87y3129qsn.fsf@gnu.org> <87sgr9bziq.fsf@sdf.lonestar.org> <87pnmc7nt1.fsf@gnu.org> <8736j7nwcb.fsf@sdf.lonestar.org> <87muhfjm14.fsf@gnu.org> <87ftn63l7d.fsf@sdf.lonestar.org> <87v9w1zgon.fsf_-_@sdf.lonestar.org> <87y30v3qke.fsf@sdf.lonestar.org> <871rylrjt8.fsf_-_@sdf.lonestar.org> <87wogdq575.fsf_-_@sdf.lonestar.org> X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 2 Thermidor an 227 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0x090B11993D9AEBB5 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4 0CFB 090B 1199 3D9A EBB5 X-OS: x86_64-pc-linux-gnu Date: Sat, 20 Jul 2019 16:29:46 +0200 In-Reply-To: <87wogdq575.fsf_-_@sdf.lonestar.org> (Jakob L. Kreuze's message of "Fri, 19 Jul 2019 13:55:58 -0400") Message-ID: <874l3g7p9h.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 36555 Cc: 36555@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hello Jakob! zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) skribis: > * guix/scripts/system/reconfigure.scm: New file. > * Makefile.am (MODULES): Add it. > * guix/scripts/system.scm (bootloader-installer-script): Export variable. > * gnu/machine/ssh.scm (switch-to-system, upgrade-shepherd-services) > (install-bootloader): Delete variable. > * gnu/machine/ssh.scm (deploy-managed-host): Rewrite procedure. > * gnu/services/herd.scm (live-service): Export variable. > * gnu/services/herd.scm (live-service-canonical-name): New variable. > * tests/services.scm (live-service): Delete variable. It LGTM! I have some comments inline below, but nothing that should block this patch. > (define (deploy-managed-host machine) > "Internal implementation of 'deploy-machine' for MACHINE instances wit= h an > environment type of 'managed-host." > (maybe-raise-unsupported-configuration-error machine) > - (mbegin %store-monad > - (switch-to-system machine) > - (upgrade-shepherd-services machine) > - (install-bootloader machine))) > + (mlet %store-monad ((boot-parameters (machine-boot-parameters machine)= )) > + (let* ((os (machine-system machine)) > + (eval (cut machine-remote-eval machine <>)) > + (menu-entries (map boot-parameters->menu-entry boot-parameter= s)) > + (bootloader-configuration (operating-system-bootloader os)) > + (bootcfg (operating-system-bootcfg os menu-entries))) > + (mbegin %store-monad > + (switch-to-system eval os) > + (upgrade-shepherd-services eval os) > + (install-bootloader eval bootloader-configuration bootcfg))))) Really nice that it becomes this concise. > > ;;; > diff --git a/gnu/services/herd.scm b/gnu/services/herd.scm > index 0008746fe..2207b2d34 100644 > --- a/gnu/services/herd.scm > +++ b/gnu/services/herd.scm > @@ -40,10 +40,12 @@ > unknown-shepherd-error? > unknown-shepherd-error-sexp >=20=20 > + live-service I like to avoid exposing constructors so that one cannot =E2=80=9Cforge=E2= =80=9D invalid objects, but let=E2=80=99s see=E2=80=A6 > +(define* (switch-to-system eval os #:optional profile) > + "Using EVAL, a monadic procedure taking a single G-Expression as an ar= gument, > +create a new generation of PROFILE pointing to the directory of OS, swit= ch to > +it atomically, and run OS's activation script." > + (eval #~(primitive-load #$(switch-system-program os profile)))) I wonder it we should just use #~(begin (use-modules (guix build utils)) (invoke =E2=80=A6)) here and in other places. That=E2=80=99s probably better longer-term (for example when we switch to Guile=C2=A03, that could ease the transition since the right Guile would be used) but we can keep it this way and revisit it later. > +(define (running-services eval) > + "Using EVAL, a monadic procedure taking a single G-Expression as an ar= gument, > +return the objects that are currently running on MACHINE." > + (define remote-exp s/remote-exp/exp/ > + (with-imported-modules '((gnu services herd)) > + #~(begin > + (use-modules (gnu services herd)) > + (let ((services (current-services))) > + (and services > + ;; 'live-service-running' is ignored, as we can't neces= sarily > + ;; serialize arbitrary objects. This should be fine for= now, > + ;; since 'machine-current-services' is not exposed publ= icly, > + ;; and the resultant objects are only us= ed for > + ;; resolving service dependencies. > + (map (lambda (service) > + (list (live-service-provision service) > + (live-service-requirement service))) > + services)))))) > + (mlet %store-monad ((services (eval remote-exp))) > + (return (map (match-lambda > + ((provision requirement) > + (live-service provision requirement #f))) > + services)))) OK, that makes sense here. (Once we=E2=80=99ve done that (guix graph) demonadification we discussed be= fore, perhaps we can perform run =E2=80=98shepherd-service-upgrade=E2=80=99 entir= ely on the =E2=80=9Cother side=E2=80=9D, and at that point we won=E2=80=99t need to ex= pose the =E2=80=98live-service=E2=80=99 constructor.) > +;; (format (current-error-port) "error: ~a~%" (condition-message c)) > +;; (format #t "bootloader successfully installed on '~a'~%" > +;; #$device) A leftover? :-) These two statements disappeared in the process, but I think they=E2=80=99re added back by one of the subsequent patches, right? Thanks, Ludo=E2=80=99.