Subject: bug#36555: closed (Re: [bug#36555] [PATCH v6 3/3] tests: Add reconfigure system test.)

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 26 Jul 2019 17:01:03 +0000

Your bug report

#36555: [PATCH 0/2] Refactor out common behavior for system reconfiguration.

which was filed against the guix-patches package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
36555: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=36555
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [bug#36555] [PATCH v6 3/3] tests: Add reconfigure system test.
From: Ludovic Courtès <[email-masked]>
To: [email-masked] (Jakob L. Kreuze)
Cc: [email-masked]
Date: Fri, 26 Jul 2019 18:59:50 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi there!

I’ve applied the whole series with the change below.  \o/

Because of the monadic style, the ‘guard’ clause had no effect:

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,run-in-store (guard (c (#t 'caught)) (mbegin %store-monad (return 1)(return (raise (condition (&message (message "oh!")))))))
While executing meta-command:
Throw to key `srfi-34' with args `(#<condition &message [message: "oh!"] 1cab2c0>)'.
--8<---------------cut here---------------end--------------->8---

I thought about adding it in some other way, but it turns out not to be
needed at all because error conditions are guarded against in
‘guix-system’.  Hence the patch.

Thank you for the hard work on this series!

I’ll be away from keyboard roughly until August 17th.  Hopefully you can
get feedback from David or Chris, and maybe you can get others on board
as well.  :-)  If my opinion on changes to the core is needed, you can
always push to a separate branch in the meantime.  Anyway, I’m confident!

Ludo’.

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 115da665b4..9fc3a10e98 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -804,19 +804,16 @@ static checks."
             ((reconfigure)
              (newline)
              (format #t (G_ "activating system...~%"))
-             (guard (c ((message-condition? c)
-                        (leave (G_ "failed to reconfigure system:~%~a~%")
-                               (condition-message c))))
-               (mbegin %store-monad
-                 (switch-to-system local-eval os)
-                 (mwhen install-bootloader?
-                   (install-bootloader local-eval bootloader bootcfg
-                                       #:target (or target "/"))
-                   (return
-                    (info (G_ "bootloader successfully installed on '~a'~%")
-                          (bootloader-configuration-target bootloader))))
-                 (with-shepherd-error-handling
-                  (upgrade-shepherd-services local-eval os)))))
+             (mbegin %store-monad
+               (switch-to-system local-eval os)
+               (mwhen install-bootloader?
+                 (install-bootloader local-eval bootloader bootcfg
+                                     #:target (or target "/"))
+                 (return
+                  (info (G_ "bootloader successfully installed on '~a'~%")
+                        (bootloader-configuration-target bootloader))))
+               (with-shepherd-error-handling
+                  (upgrade-shepherd-services local-eval os))))
             ((init)
              (newline)
              (format #t (G_ "initializing operating system under '~a'...~%")

Subject: [PATCH 0/2] Refactor out common behavior for system reconfiguration.
From: [email-masked] (Jakob L. Kreuze)
To: [email-masked]
Date: Mon, 08 Jul 2019 15:52:12 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello, Guix!

This is the preliminary version of a patch series to turn the behavior
common between 'guix deploy' and 'guix system reconfigure' into a module
that both can use. I am submitting it as-is both for comments and for
tracking the refactoring effort.

Note that this is _not_ ready to be merged. There are several things
that I need to do before I would consider it ready for upstream Guix:

- This passes my old test suite for 'guix deploy', but I haven't dared
  to run the new 'guix system reconfigure'. I'll set up a new virtual
  machine so I don't put myself out of a working laptop.
- 'switch-system-program', 'upgrade-services-program', and
  'install-bootloader-program' omit some of the features that were
  present in the procedures they replace. For example,
  'install-bootloader' previously supported installing the bootloader
  configuration without actually running the installation script. This
  was fine for 'guix deploy', but I'll need to add it back in for
  'guix system reconfigure'.
- I plan to implement system tests for '(guix scripts system
  reconfigure)'. I suppose I can always submit them as a separate patch,
  but I'll likely finish them before we're through with code review, so
  it may make sense to include them with as part of this patch series,
  albeit as a distinct commit.
- I suspect that some of the effectful procedures in 'system.scm' could
  be refactored out in a similar fashion. Not that 'guix deploy' would
  necessarily be using them, but it would be more consistent to have
  them as 'program-file' objects, and those procedures could then also
  be tested.

I look forward to your comments.

Regards,
Jakob

Jakob L. Kreuze (2):
  guix system: Add 'reconfigure' module.
  guix system: Reimplement 'reconfigure'.

 Makefile.am                         |   1 +
 gnu/machine/ssh.scm                 | 235 ++++++++--------------------
 guix/scripts/system.scm             | 140 +++++------------
 guix/scripts/system/reconfigure.scm | 158 +++++++++++++++++++
 4 files changed, 255 insertions(+), 279 deletions(-)
 create mode 100644 guix/scripts/system/reconfigure.scm

--
2.22.0