On Tue, Mar 29, 2016 at 09:46:14PM +0200, Ludovic Courtès wrote:
> Damn, it’s been more than a month, please accept my apologies!
No problem!
> 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.
I thought it was only recomputed and used in once place now?
> > + (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.
How much more non-obvious than how it is now with having to pass certain inputs?
But yes, you're right. If I was doing a bigger refactor I would've added them as
flags to the function.
> 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?
Well, I'm working here on the assumption that callers shouldn't have to know
about the contents of their os-configuration, specifically in this situation
which bootloader is being used. This is part of a bigger effort to organize
things a bit better so different bootloaders could be switched out. Having to
pass these things violate this assumption.
> Thank you!
>
> Ludo’.
Thank you,
Jookia.
