[bug#36555] [PATCH v4 1/3] guix system: Add 'reconfigure' module.

[Ludovic Courtès]

Hello Jakob!

address@hidden (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 with 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-parameters))
> +           (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
>  
> +            live-service

I like to avoid exposing constructors so that one cannot “forge” invalid
objects, but let’s see…

> +(define* (switch-to-system eval os #:optional profile)
> +  "Using EVAL, a monadic procedure taking a single G-Expression as an 
> argument,
> +create a new generation of PROFILE pointing to the directory of OS, switch 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 …))

here and in other places.

That’s probably better longer-term (for example when we switch to
Guile 3, 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 
> argument,
> +return the <live-service> 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 
> necessarily
> +                 ;; serialize arbitrary objects. This should be fine for now,
> +                 ;; since 'machine-current-services' is not exposed publicly,
> +                 ;; and the resultant <live-service> objects are only used 
> 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’ve done that (guix graph) demonadification we discussed before,
perhaps we can perform run ‘shepherd-service-upgrade’ entirely on the
“other side”, and at that point we won’t need to expose the
‘live-service’ 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’re
added back by one of the subsequent patches, right?

Thanks,
Ludo’.