[bug#36404] [PATCH 0/3] Refactor out common behavior for system reconfiguration.

[zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze)]

[-- Attachment #1: Type: text/plain, Size: 3917 bytes --]

Hi, Chris and Ludovic!

Ludovic Courtès <ludo@gnu.org> writes:

> zerodaysfordays@sdf.lonestar.org (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.
> 
> > +;;; Copyright © 2019 Jakob L. Kreuze <zerodaysfordays@sdf.lonestar.org>
> 
> Could you preserve the copyright lines of (guix scripts system) that
> apply to these portions of code, roughly?

I've copied over all of the copyright lines in the original file.
Briefly looking through the log for 'guix/scripts/system.scm', it seems
that most of the committers have touched the code for system
reconfiguration at one point or another. Let me know if you'd like me to
comb through the logs more finely and update the copyright lines
accordingly.

> I think all the procedures in (guix scripts system reconfigure) could
> return a <scheme-file> rather than a gexp. Actually a <program-file>
> would even be cleaner than a <scheme-file>, as it could better handle
> transitions like you’re on a Guile 2.2 system reconfiguring towards a
> Guile 3 system.
> 
> Consequently you could rename ‘switch-to-system’ to
> ‘switch-system-program’, and so on.

That'd make writing tests for these procedures a little bit easier, too.

> I think it could simply take an <operating-system> record and derive
> the relevant bits from that.

You're right, and it's an especially easy change to make once I factor
in your later comment about not needing to invoke
'operating-system-derivation' directly.

> This comment may become irrelevant.

Good catch. At this point it's little more than a historical detail.

> Same here? For ‘guix system reconfigure’, we’d rather not lose
> messages written to stdout by ACTIVATION-SCRIPT.

That's a good point. I've modified 'install-bootloader-program' to
similarly return the installer script's output as a string (it was
previously being discarded.)

> I think you’d need to include the ‘call-with-service-upgrade-info’
> call in the service-upgrade program that (guix scripts system
> reconfigure) produces. It’s an important part of reconfiguration.
> 
> However, ‘call-with-service-upgrade-info’ relies on (guix graph),
> which pulls in (guix monads) and many modules that we don’t actually
> need.
> 
> It’s probably just an annoyance more than a real problem, but I think
> we should eventually change the (guix graph) API so that it no longer
> relies on the ‘%store-monad’, which in turn will make it a better fit
> in this context.

Services are serialized on the host side, so we can make use of
'call-with-service-upgrade-info' without drawing any of its dependencies
into the G-Expression. :)

It's nicer than my hacky 'target-services' expression, anyway.

Christopher Lemmer Webber <cwebber@dustycloud.org> writes:

> In some ways it looks like a portion of the previous patch and a
> portion of this patch are a "move and modify" of what are sort-of the
> same chunks of code. But it's a bit weird to me that the code is added
> in the previous commit and removed in this one? It might be clearer to
> the reader that this is what is happening if it's in the same commit.

Looking at the diff now, I definitely see what you're talking about.
Squashed!

Ludovic Courtès <ludo@gnu.org> writes:

> Christopher Lemmer Webber <cwebber@dustycloud.org> skribis:
>
>> Side note: I closed this issue when the initial set of patches were
>> merged.  It seems there's ongoing work... should we reopen it or make a
>> separate issue?  I'm unsure.
>
> We should probably open a new issue, indeed!  Jakob, do you want to
> continue the discussion in a separate issue?

Yes, please! I'll open a new ticket on guix-patches with my revisions.

Regards,
Jakob

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]