Re: Fix installer restart.

[Ludovic Courtès]

Hi Mathieu,

Mathieu Othacehe <address@hidden> skribis:

> Here are two patches that should fix the "Installer fails when
> restarted" issue.

Great!

> I also fixed the "installation failed dialog". There are now two
> options:
> * "Resume": let the user resume the installation from any step (from the
> installation menu)
> * "Retry": restart the installer process.

Nice.

> From f02ef650526973af55cf1629de1cc48e888c714f Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <address@hidden>
> Date: Mon, 10 Feb 2020 14:43:05 +0100
> Subject: [PATCH 1/2] installer: Remove the cow-store overlay after system
>  installation.
>
> * gnu/installer/final.scm (umount-cow-store): New procedure ...,
> (install-system): ... used here, to remove the store overlay so that the
> target device is not seen as busy during further umount calls.

Please add a “Fixes …” line in the log, for future reference.

> +    (dynamic-wind
> +      (lambda ()
> +        (start-service 'cow-store (list (%installer-target-dir))))
> +      (lambda ()
> +        (run-shell-command install-command #:locale locale))
> +      (lambda ()
> +        (stop-service 'cow-store)
> +        ;; Remove the store overlay created at cow-store service start.
> +        ;; Failing to do that will result in further umount calls to fail
> +        ;; because the target device is seen as busy. See:
> +        ;; 
> https://lists.gnu.org/archive/html/guix-devel/2018-12/msg00161.html.
> +        (umount-cow-store)
> +        #f))))

LGTM!

Did you make sure that:

  make check-system TESTS=installed-os

still passes?

> From 50884fca871cfceb020d63e556c957163fdd6064 Mon Sep 17 00:00:00 2001
> From: Mathieu Othacehe <address@hidden>
> Date: Mon, 10 Feb 2020 14:47:56 +0100
> Subject: [PATCH 2/2] installer: Fix installer restart dialog.
>
> * gnu/installer/newt/final.scm (run-install-failed-page): Propose between
> installer resume or restart. Do actually resume the installation by raising an
> &installer-step-abort condition if "Resume" button is pressed. Otherwise, keep
> going as the installer will be restarted by login.
> * gnu/installer.scm (installer-program): Remove the associated TODO comment.

[…]

> --- a/gnu/installer/newt/final.scm
> +++ b/gnu/installer/newt/final.scm
> @@ -73,12 +73,15 @@ press the button to reboot."))
>    'success)
>  
>  (define (run-install-failed-page)
> -  (choice-window
> -   (G_ "Installation failed")
> -   (G_ "Restart installer")
> -   (G_ "Retry system install")
> -   (G_ "The final system installation step failed.  You can retry the \
> -last step, or restart the installer.")))
> +  (case (choice-window
> +         (G_ "Installation failed")
> +         (G_ "Resume")
> +         (G_ "Restart the installer")
> +         (G_ "The final system installation step failed.  You can resume 
> from \
> +a specific step, or restart the installer."))
> +    ((1) (raise
> +          (condition
> +           (&installer-step-abort))))))

I’d recommend using ‘match’ rather than ‘case’, because ‘case’ silently
returns *unspecified* when it fails to match the value.

Apart from that, LGTM!

Thank you,
Ludo’.

PS: I’m still looking at the installer tests we discussed before FOSDEM,
    but life got in the way so progress is slow!