Subject: Re: [bug#41350] [PATCH v2 3/3] system: vm: Build vm-image using native qemu, for the Hurd.

From: Jan Nieuwenhuizen <[email-masked]>
Date: Sat, 23 May 2020 11:28:31 +0200

Mathieu Othacehe writes:

Hello Mathieu,

>> +                 #$(if (hurd-target?)
>> +                       #~#+(grub-root-search store-device font-file)
>> +                       (grub-root-search store-device font-file))
>> +                 #$(if (hurd-target?)
>> +                       #~#+(setup-gfxterm config font-file)
>> +                       (setup-gfxterm config font-file))
>
> I'm not certain that hurd-target? always return the expected answer
> here. Now that we have let-system, the safer way to check for the target
> system could be to write:
>
> #$(let-system (system target)
>               (hurd-target? system target))

Okay...

> Ludo, is this correct? Furthermore, if you make sure that
> "grub-root-search" and "setup-gfxterm" return #+, you could maybe drop
> this part?

...used #+ and dropped the IFs here, but used this insight below.

>> +
>> +                     (loader  #$(if (hurd-target?) #~#+loader loader))
>> +                     (linux   #$(if (hurd-target?)
>> +                                    #~(string-append
>> +                                       #+linux "/"
>> +                                       #+(system-linux-image-file-name))
>> +                                    #~(string-append
>> +                                       #$linux "/"
>> +                                       #$(system-linux-image-file-name))))
>
> Same concern as above about "hurd-target?".

...Ah, that makes sense...that's of course why Ludo already introduced

  (define-syntax-rule (check predicate)
    (let-system (system target)
      (predicate (or target system))))

here.  So, using "(check hurd-triplet?)", as Ludo was already doing in
some other places here.

>> +                  ;; (if #$(hurd-target?)
>> +                  ;;     '#+(append (list parted e2fsprogs dosfstools)
>> +                  ;;                    (map canonical-package
>> +                  ;;                         (list sed grep coreutils findutils gawk)))
>> +                  ;;     '#$(append (list parted e2fsprogs dosfstools)
>> +                  ;;                (map canonical-package
>> +                  ;;                     (list sed grep coreutils findutils gawk))))
>> +                  )
>
> Is this needed?

Oops -- that was unfinished business; that IF did not work yet; possibly
because of not going through LET-SYSTEM.

This helps a lot, I'm now again able to do both

    ./pre-inst-env guix system vm-image --target=i586-pc-gnu --no-grafts gnu/system/examples/bare-bones.tmpl
    ./pre-inst-env guix system vm-image gnu/system/examples/bare-bones.tmpl

again!  Phew... And that's also because I learned to include the full
list of exported symbols in the autoload of (gnu store database).

Mathieu, if it turns out that you are dropping qemu-image next week
altogether then great!, and I have no problem whatsoever dropping this
patch set as well.

For now, I have reset wip-hurd-vm and put your commits towards the
bottom of the wip-hurd-vm stack too.

Sending an updated v3 patch set just because I'm so pleased with this
result.

Greetings,
janneke

-- 
Jan Nieuwenhuizen <[email-masked]> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com