all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Mark H Weaver <mhw@netris.org>
To: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Cc: Simon Tournier <zimon.toutoune@gmail.com>,
	Tobias Geerinckx-Rice <me@tobias.gr>,
	Guix Devel <guix-devel@gnu.org>
Subject: Re: The package/inherit trap
Date: Thu, 10 Aug 2023 16:40:57 -0400	[thread overview]
Message-ID: <878raid40b.fsf@netris.org> (raw)
In-Reply-To: <87sf8uewz7.fsf@gmail.com>

Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Mark H Weaver <mhw@netris.org> writes:
>
>> Simon Tournier <zimon.toutoune@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


  reply	other threads:[~2023-08-10 20:44 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-21 13:06 [bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky Sharlatan Hellseher
2023-02-21 13:07 ` [bug#61674] [PATCH 1/4] gnu: Add calcmysky Sharlatan Hellseher
2023-02-21 13:07   ` [bug#61674] [PATCH 2/4] gnu: qxlsx: Use Qt6 Sharlatan Hellseher
2023-02-21 13:07   ` [bug#61674] [PATCH 3/4] gnu: Add qxlsx-qt5 Sharlatan Hellseher
2023-02-21 13:07   ` [bug#61674] [PATCH 4/4] gnu: stellarium: Enable ShowMySky Sharlatan Hellseher
2023-02-26  0:37 ` [bug#61674] Sharlatan Hellseher
2023-02-26  0:44 ` [bug#61674] [PATCH 1/4] gnu: Add calcmysky Sharlatan Hellseher
2023-02-26  0:44   ` [bug#61674] [PATCH 2/4] gnu: qxlsx: Use Qt6 Sharlatan Hellseher
2023-02-26  0:44   ` [bug#61674] [PATCH 3/4] gnu: Add qxlsx-qt5 Sharlatan Hellseher
2023-02-27 21:15     ` [bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky Ludovic Courtès
2023-02-26  0:44   ` [bug#61674] [PATCH 4/4] " Sharlatan Hellseher
2023-02-27 21:15     ` [bug#61674] [PATCH 0/4] " Ludovic Courtès
2023-02-26  2:55   ` [bug#61674] [PATCH 1/4] gnu: Add calcmysky Leo Famulari
2023-02-27 21:14   ` [bug#61674] [PATCH 0/4] gnu: stellarium: Enable ShowMySky Ludovic Courtès
2023-02-28 10:35     ` Simon Tournier
2023-03-03 10:49       ` Ludovic Courtès
2023-03-03 11:04         ` Simon Tournier
2023-03-03 15:54           ` Maxim Cournoyer
2023-03-03 18:19             ` Simon Tournier
2023-03-04  3:32               ` Maxim Cournoyer
2023-03-04 11:11                 ` Sharlatan Hellseher
2023-03-03 19:21             ` The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.) Tobias Geerinckx-Rice
2023-03-04  3:44               ` The package/inherit trap Maxim Cournoyer
2023-03-07 11:46               ` The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.) Simon Tournier
2023-03-07 16:43                 ` The package/inherit trap Maxim Cournoyer
2023-03-07 17:46                   ` Simon Tournier
2023-03-07 22:11                     ` Tobias Geerinckx-Rice
2023-03-08  9:07                       ` Simon Tournier
2023-07-31 20:17                         ` John Kehayias
2023-08-06 23:10                         ` Mark H Weaver
2023-08-07 14:41                           ` Maxim Cournoyer
2023-08-10 20:40                             ` Mark H Weaver [this message]
2023-08-10 21:27                               ` Mark H Weaver
2023-09-02 18:46                               ` Maxim Cournoyer
2023-08-07  0:00               ` The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.) (
2023-08-07  6:29                 ` Attila Lendvai
2023-08-07 14:43                   ` The package/inherit trap Maxim Cournoyer
2023-02-28  0:12 ` [bug#61674] [PATCH v3 1/4] gnu: Add calcmysky Sharlatan Hellseher
2023-02-28  0:12   ` [bug#61674] [PATCH v3 2/4] gnu: qxlsx: Use Qt6 Sharlatan Hellseher
2023-02-28  0:12   ` [bug#61674] [PATCH v3 3/4] gnu: Add qxlsx-qt5 Sharlatan Hellseher
2023-02-28  0:12   ` [bug#61674] [PATCH v3 4/4] gnu: stellarium: Enable ShowMySky Sharlatan Hellseher
2023-03-07 10:39     ` bug#61674: [PATCH 0/4] " Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878raid40b.fsf@netris.org \
    --to=mhw@netris.org \
    --cc=guix-devel@gnu.org \
    --cc=maxim.cournoyer@gmail.com \
    --cc=me@tobias.gr \
    --cc=zimon.toutoune@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.