Jookia <166...@gmail.com> skribis:
> This small refactor should simplify some duplicated effort across functions
> and
> allow smarter qemu-image to do smarter things based on the operating system
> configuration rather than having each function that uses qemu-image pass
> selective parameters whenever new information is needed.
Damn, it’s been more than a month, please accept my apologies!
Overall this sounds like a good idea. However…
> * gnu/system/vm.scm (qemu-image): Replace os-derivation, grub-configuration
> and
> inputs parameters with os-configuration, base-inputs and extra-inputs.
> (qemu-image): Based on base-inputs, generate grub.cfg and os-drv.
> (system-disk-image, system-qemu-image, system-qemu-image/shared-store):
> Pass in the operating system configuration and base-inputs to qemu-image
> instead of derivations.
One of the reasons ‘os-drv’ and ‘grub.cfg’ are passed around is that
recomputing them is relatively costly.
There’s a solution to that in 4c2ade20c65e94c41dc8c65db73dd128343a0ad5
(in ‘wip-build-systems-gexp’; it memoizes ‘operating-system-derivation’
and others), so we could almost consider it solved.
Nevertheless it’d be nice to make sure performance remains reasonable
even in the absence of the above commit.
> + (base-inputs (list 'grub.cfg 'system))
[...]
> + (mlet* %store-monad ((os-drv (operating-system-derivation
> os-configuration))
> + (grub.cfg (operating-system-grub.cfg
> os-configuration))
> + (inputs -> (append
> + (if (member 'grub.cfg base-inputs)
> + `(("grub.cfg" ,grub.cfg)) '())
> + (if (member 'system base-inputs)
> + `(("system" ,os-drv)) '())
Use of “magic” values like 'grub.cfg here is undesirable IMO, because it
introduces singularities in the API, and generally makes the interface
non-obvious.
So I think I’d leave it up to the caller to pass
#:inputs `(("grub.cfg" ,grub.cfg))
Same for "system".
All in all, I sympathize with the desire to avoid passing OS-DRV and
GRUB.CFG around, but I’m not convinced that this approach can improve
the situation.
WDYT?
Thank you!
Ludo’.
