Message ID | 87pnmc7nt1.fsf@gnu.org |
---|---|
State | Accepted |
Headers | show |
Series | [bug#36555,1/2] guix system: Add 'reconfigure' module. | expand |
Ludovic Courtès <ludo@gnu.org> writes: > Oh, I see. So in a way the problem is that ‘remote-eval’ doesn’t do > anything sensible with the output and error ports of that remote > evaluation. > > Ultimately we should probably fix (guix inferior) and (guix remote) so > that stdout and stderr are properly transmitted. Thinking about it now, that could make error reporting for 'guix deploy' less complicated. We'd be able to output the remote's stdout/stderr to the host's stdout/stderr and be done with it. > In the meantime, what about this patch? > > diff --git a/guix/remote.scm b/guix/remote.scm > index e503c76167..8ada5c0957 100644 > --- a/guix/remote.scm > +++ b/guix/remote.scm > @@ -76,8 +76,14 @@ result to the current output port using the (guix repl) protocol." > (with-imported-modules (source-module-closure '((guix repl))) > #~(begin > (use-modules (guix repl)) > - (send-repl-response '(primitive-load #$program) > + > + ;; We use CURRENT-OUTPUT-PORT for REPL messages, so redirect PROGRAM's > + ;; output to CURRENT-ERROR-PORT so that it does not interfere. > + (send-repl-response '(with-output-to-port (current-error-port) > + (lambda () > + (primitive-load #$program))) > (current-output-port)) > + > (force-output)))) > > (define* (remote-eval exp session LGTM, thanks! > ‘live-service-requirement’ gives you the graph of the currently loaded > services, but you also need the target service graph to determine what > to upgrade; that seems to be missing currently. Oh, good catch. Reusing 'shepherd-service-upgrade' is certainly the way to go, then. Regards, Jakob
zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) skribis: > Ludovic Courtès <ludo@gnu.org> writes: [...] >> In the meantime, what about this patch? >> >> diff --git a/guix/remote.scm b/guix/remote.scm >> index e503c76167..8ada5c0957 100644 >> --- a/guix/remote.scm >> +++ b/guix/remote.scm >> @@ -76,8 +76,14 @@ result to the current output port using the (guix repl) protocol." >> (with-imported-modules (source-module-closure '((guix repl))) >> #~(begin >> (use-modules (guix repl)) >> - (send-repl-response '(primitive-load #$program) >> + >> + ;; We use CURRENT-OUTPUT-PORT for REPL messages, so redirect PROGRAM's >> + ;; output to CURRENT-ERROR-PORT so that it does not interfere. >> + (send-repl-response '(with-output-to-port (current-error-port) >> + (lambda () >> + (primitive-load #$program))) >> (current-output-port)) >> + >> (force-output)))) >> >> (define* (remote-eval exp session > > LGTM, thanks! Cool, pushed as 6f8eb9f1d8bc8660349658602698db36965bba5d. >> ‘live-service-requirement’ gives you the graph of the currently loaded >> services, but you also need the target service graph to determine what >> to upgrade; that seems to be missing currently. > > Oh, good catch. Reusing 'shepherd-service-upgrade' is certainly the way > to go, then. I think so, which brings us back to the need to de-monadify (guix graph). :-) Ludo’.
Ludovic Courtès <ludo@gnu.org> writes: > I think so, which brings us back to the need to de-monadify (guix > graph). :-) Good news, I came up with a way of using 'shepherd-service-upgrade' on the host side. Stay tuned for v3 of this patch series ;) Though, I suppose cleaning up the dependencies of '(guix graph)' may be a good goal to have regardless. Regards, Jakob
Hi, all. Submitting this reroll to ask for some further feedback. Here's a summary of the more significant changes since v2: - All of the system tests for the reconfiguration procedures have been implemented. - 'upgrade-services-program' has been completely reimplemented; '(gnu machine ssh)' is now capable of (partially) serializing the <live-service> objects returned by 'current-services', so we can use 'shepherd-service-upgrade' to traverse the service dependency graph. - Procedures in '(guix scripts system reconfigure)' now use 'program-file' instead of 'gexp->script'. I hadn't realized the difference, but this makes invocations of 'remote-eval' a bit cleaner. - Thanks to Ludovic's patches to '(guix remote)', the reconfiguration procedures no longer need to capture output from the activation/installation scripts. - I've removed my awful hack of a solution for handling Shepherd errors in 'upgrade-services-program' in favor of handling exceptions on the host side. I have some questions about this. - 'upgrade-services-program' comes after 'install-bootloader-program' in 'guix deploy' and 'guix system reconfigure' now, as it's the procedure most likely to fail trivially. I still need to handle failed deployments in 'guix deploy'. I suspect that, for now, it would make sense to implement remote roll-backs and just roll-back the system on failure, at least until we've have some dialog about the proper way to do atomic deployments. My biggest concern at the moment is error handling reporting in the new 'guix system reconfigure'. I'd like to emulate what was done with the previous version, but I'm at somewhat of a loss for how I'd go about that, since the error reporting was mixed with the reconfiguration code. So I'd like to ask for some suggestions: is the best way to catch errors in '%store-monad' to do what 'with-shepherd-error-handling' does, and then 'leave' on failure? Ludovic suggested guarding against 'message-condition' and having the expression I send to 'remote-eval' return either ('error message) or ('success). Would it make sense to just do this in all of the reconfiguration procedures? Or is raising exceptions in the reconfiguration procedures and catching them in the scripts' code the way to go? There's also a slight bug in the new 'guix system reconfigure' that I'll need to figure out. At the moment, it installs a bootloader entry for all but the newest generation. Jakob L. Kreuze <zerodaysfordays@sdf.lonestar.org> writes: > Noted. That should be a relatively small change, so I'll see about > tackling that in my next revision for this series. Oh, how naïve I was four days ago. This reroll doesn't address this. Having the procedures "parameterized by an evaluation procedure" can be done in so many ways, and I think it would be best I put some serious thought into which of those ways would be the best. A 'local-eval' would clearly be much better than what I'm doing at the present in 'system.scm', but the solution I came up with today involved three layers of 'primitive-load', which I doubt is the way to go about it. I had the idea to parameterize on a procedure that takes a '<program-file>' rather than a G-Expression as I was making dinner tonight, which seems to me like a sound idea, but we'll see if it works tomorrow when I try to implement it. Also, it hit me today that the safety checks done in 'guix system reconfigure' -- 'check-mapped-devices', 'check-file-system-availability', and 'check-initrd-modules' -- should also be done in 'guix deploy'. It might make sense for me to submit that change as a separate patch series so the code review for this doesn't get too complicated, but since we're on the topic of unifying the code between 'guix deploy' and 'guix system reconfigure', should I perhaps reimplement those procedures as '<program-file>' objects like everything else in '(guix scripts system reconfigure)'? They aren't really effectful, but they concern system reconfiguration. And, on the same note, should I go ahead and refactor the rest of the reconfiguration code in 'system.scm' out into '(guix scripts system reconfigure)'? I mean, this will probably be a separate patch series for the same reason that the safety checks would be a separate patch series, and I'll likely do this _after_ I come up with a decent way to parameterize on an evaluation procedure, but I'd like to know if it's a good idea or not before going ahead and ripping apart 'system.scm'. Regards, and TYIA for reviewing this. Jakob Jakob L. Kreuze (3): guix system: Add 'reconfigure' module. guix system: Reimplement 'reconfigure'. tests: Add reconfigure system test. Makefile.am | 1 + gnu/local.mk | 1 + gnu/machine/ssh.scm | 266 ++++++++++----------------- gnu/services/herd.scm | 6 + gnu/tests/reconfigure.scm | 268 ++++++++++++++++++++++++++++ guix/scripts/system.scm | 152 +++++----------- guix/scripts/system/reconfigure.scm | 122 +++++++++++++ tests/services.scm | 4 - 8 files changed, 538 insertions(+), 282 deletions(-) create mode 100644 gnu/tests/reconfigure.scm create mode 100644 guix/scripts/system/reconfigure.scm
diff --git a/guix/remote.scm b/guix/remote.scm index e503c76167..8ada5c0957 100644 --- a/guix/remote.scm +++ b/guix/remote.scm @@ -76,8 +76,14 @@ result to the current output port using the (guix repl) protocol." (with-imported-modules (source-module-closure '((guix repl))) #~(begin (use-modules (guix repl)) - (send-repl-response '(primitive-load #$program) + + ;; We use CURRENT-OUTPUT-PORT for REPL messages, so redirect PROGRAM's + ;; output to CURRENT-ERROR-PORT so that it does not interfere. + (send-repl-response '(with-output-to-port (current-error-port) + (lambda () + (primitive-load #$program))) (current-output-port)) + (force-output)))) (define* (remote-eval exp session