Hello to anyone reviewing this patch,

I probably should've held off on sending this reroll out. After taking
some more time to experiment with possible solutions, I was able to
figure most of this out. Comments would still be appreciated, but the
points I specifically asked for comments on no longer need special
treatment. Also, if you haven't already started reviewing this, v4 will
likely hit the mailing list tomorrow; everything's there, it just needs
to be cleaned up.

zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) writes:

> 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.

Well, except for this. I'll submit a separate patch series addressing
this.

> 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?

Comments, if anyone has them, would be appreciated, but I feel that I'm
in a good spot in terms of error handling now.

> 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.

It wasn't actually a bug, I was misinterpreting the intended behavior of
'guix system reconfigure'. :)

> 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.

Actually, a more generalized 'eval' (taking a G-Expression) was the
better way to go: it allowed me to simplify the interface to the
reconfiguration procedures even further. And, thanks to Ludovic's recent
patches with 'lower-gexp', I was able to collapse the Russian nesting
doll of 'primitive-load' calls.

> 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.

Again, separate patch series.

> 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'.

I'd still like comments on this, though.

Regards,
Jakob