Subject: bug#26339: [PATCH v2 01/12] system: Pass <bootloader-parameter> to grub.

From: [email-masked] (Ludovic Courtès)
Date: Tue, 09 May 2017 09:03:00 +0200

Hi,

Danny Milosavljevic <[email-masked]> skribis:

> On Mon, 08 May 2017 21:47:29 +0200
> [email-masked] (Ludovic Courtès) wrote:
>
>> This does not remove the circular dependency, it just expresses it
>> differently, so I don’t think it will help.  Using
>> 
>>   (module-ref (resolve-interface '(gnu system grub))
>>               'grub-configuration-file)
>> 
>> would have achieved the effect you wanted.
>
> Hmm.... I've tested the @@-version locally by removing all the .go files and then doing guix system reconfigure ... which took ages every time.  And it worked just fine, no errors, no warnings, nothing.  Did I test it wrong?

No apparently it worked on Hydra (but as I wrote before, the way
build-aux/compile-all.scm builds things doesn’t catch
macro-used-before-defined errors, so testing locally doesn’t help much).

I was concerned that @@ would effectively cause the same problem as
#:use-module, but apparently that’s fine.

>> However, that’s pretty much a band-aid, so we should look for a way to
>> improve this.
>
> Yes, but as soon as the other bootloader patches are merged the band-aid would be gone.
>
> We have multiple choices here, 
> (1) revert all ~12 commits, or
> (2) do the band-aid, leave the commits in and review and merge the remainder later, or
> (3) review and merge all the other bootloader patches now...

I think we’re fine with the band-aid for now.  ;-)

We’ll keep reviewing/merging the rest of the series, which apparently
provides a nicer bootloader abstraction.

> I don't have a strong preference.  If I knew how flaky even simple changes in these modules are I wouldn't have been in favor of refactoring them at all... (really, just having u-boot with Grub as payload - and grub-efi as part of regular grub - would have been much simpler than this, in retrospect)

There’s some flakiness, but I think the circular dependency in this case
was the consequence of a half-baked abstraction.  Fortunately the rest
of the patch series appears to address this, so we’re all good!

I think it remains easy to refactor GuixSD, especially now that we have
system tests.  Let’s not throw the baby out with the bath water.  ;-)

Thanks,
Ludo’.