Hi,
Maxim Cournoyer <maxim.courno...@gmail.com> writes:
> Mark H Weaver <m...@netris.org> writes:
>
>> Simon Tournier <zimon.touto...@gmail.com> writes:
>>> and in the light of this discussion, 5f83dd03a2 seems incorrect, no?
>>
>> I agree that commit 5f83dd03a2 is incorrect. 'kodi/wayland' is quite
>> clearly a case where 'package/inherit' should be used.
>
> Would you be able to write a bit of doc explaining when both should be
> used? It's a common pitfalls among contributors, and I suspect few of
> us have an understanding good enough to write it down in a manner clear
> enough to be understood by new contributors.
I don't have time to write the doc, but I can offer a few words here.
The fundamental question that needs to be asked, to decide whether to
write (define DERIVED-PACKAGE (package/inherit BASE OVERRIDES ...)) or
(define DERIVED-PACKAGE (package (inherit BASE) OVERRIDES ...)) is this:
if BASE had a replacement, would it make sense to apply the same
OVERRIDES to BASE's replacement to automatically create a replacement
for DERIVED-PACKAGE?
Consider 'kodi/wayland' as an example:
__ (define-public kodi/wayland
____ (package
______ (inherit kodi)
______ (name "kodi-wayland")
______ (arguments
_______ (substitute-keyword-arguments (package-arguments kodi)
_________ ((#:configure-flags flags)
__________ `(cons "-DCORE_PLATFORM_NAME=wayland"
_________________ (delete "-DCORE_PLATFORM_NAME=x11" ,flags)))))
______ (inputs
_______ (modify-inputs (package-inputs kodi)
_________ (prepend libinput
__________________ libxkbcommon
__________________ waylandpp
__________________ wayland-protocols)))
______ (synopsis "Kodi with Wayland rendering backend")))
Suppose, at some future time, 'kodi' were given a replacement named
'kodi-with-fixes' to apply a security update.
The question to ask yourself is this: would it make sense for
'kodi/wayland' to automatically be given a replacement field defined
exactly as above except with (inherit kodi) changed to (inherit
kodi-with-fixes)?
If the answer is "yes", then use (package/inherit BASE OVERRIDES ...),
and make sure that BASE is simply a variable name.
If the answer is "no", use (package (inherit BASE) OVERRIDES ...), in
which case any replacement of BASE will simply be dropped, and the new
package will not have a replacement unless one is explicitly given in
OVERRIDES.
The rationale for 'package/inherit' is to ensure that, e.g., security
updates applied to 'kodi' will automatically be propagated to
'kodi/wayland', and similarly for other packages.
Unless I'm mistaken, the criterion given above is fully general, and
thus sufficient on its own. However, for the sake of reducing the
cognitive load on contributors, one could proceed to enumerate simpler
heuristics that can be used in common cases.
For example, if OVERRIDES includes a new 'source' field, the next
question to ask is: does the new 'source' field make an incremental
change to (package-source BASE) e.g. by simply adding another patch? If
so, then 'package/inherit' might do the right thing. However, if the
new 'source' field doesn't even look at (package-source BASE), then it's
certainly not going to work sensibly when applied to BASE's replacement.
Another common case is when the package you're defining *is* BASE's
replacement. In that case, you certainly don't want to use
'package/inherit'.
If you want to rename 'package/inherit', I'd suggest something like
'package/inherit-with-replacements'.
Hope this helps. Feel free to do as you wish without consulting me
further. I'm using a private branch that hasn't been merged with
upstream Guix since April 2021, so your decisions won't affect me
in any case.
Mark
