Re: The package/inherit trap
Hi Mark, Mark H Weaver writes: [...] > 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. Thank you, that is abundantly detailed and clear; I'll try adapting it in our manual. Cheers! -- Maxim
Re: The package/inherit trap
Hello again, I made a mistake in my previous message. Earlier, I wrote: > 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)? The above paragraph isn't quite right. It should be: > 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 all free references to 'kodi' within the > 'package' (or 'package/inherit') form changed to 'kodi-with-fixes'? Alternatively, for the sake of those who don't know what it means for a variable reference to be "free" within a given expression, here's another way to write it: > 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 the 'package' (or 'package/inherit') form > wrapped within (let ((kodi kodi-with-fixes)) ...). This point is crucial. Observe that in the 'kodi/wayland' definition, the OVERRIDES for the 'arguments' and 'inputs' fields are defined as incremental changes to (package-arguments kodi) and (package-inputs kodi), respectively. In order for (package/inherit BASE OVERRIDES ...) to work as intended, the incremental changes made in OVERRIDES should be based upon the variable BASE, because that is the only variable that will be rebound to (package-replacement BASE) when automatically producing the replacement for the derived package. Regards, Mark
Re: The package/inherit trap
Hi, Maxim Cournoyer writes: > Mark H Weaver writes: > >> Simon Tournier 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
Re: The package/inherit trap
Hi, Attila Lendvai writes: >> How about 'package/variant' or 'package/variant-of'? > > +1 for trying to capture the intention in the name, instead of the means of > implementation. I like 'package/variant'. It should be possible to automatically update our code base to use it, and deprecate the legacy name. -- Thanks, Maxim
Re: The package/inherit trap
Hello, Mark H Weaver writes: > Simon Tournier writes: > >> Hi Tobias, >> >> On Tue, 07 Mar 2023 at 22:11, Tobias Geerinckx-Rice wrote: >> >>> However, merely documenting something is not enough when we have the >>> chance to fix misleading naming, as we do here. It would be nice to >>> have, but orthogonal. >> >> I would not say it is orthogonal because renaming would not have been >> enough, at least for me, in order to get the difference with ’inherit’. >> >> For sure, it is a real trap. :-) For instance, >> >> $ git log --grep='package/inherit' --oneline | grep use >> 5f83dd03a2 gnu: kodi/wayland: Do not use package/inherit. >> 6ecf88a6a1 gnu: poppler-next: Don't use 'package/inherit'. >> 5a5b729d66 gnu: abseil-cpp: Don't use 'package/inherit'. >> dbcf2b61b1 gnu: Fix erroneous uses of 'package/inherit'. >> 2f97a666a5 gnu: python-urllib3: Don't use 'package/inherit' on >> replacement package. >> 4163b6d855 gnu: avahi: Don't use package/inherit. >> >> 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. -- Thanks, Maxim
Re: The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.)
> How about 'package/variant' or 'package/variant-of'? +1 for trying to capture the intention in the name, instead of the means of implementation. -- • attila lendvai • PGP: 963F 5D5F 45C7 DFCD 0A39 -- “Although teachers do care and do work very, very hard, the institution is psychopathic-it has no conscience. It rings a bell and the young man in the middle of writing a poem must close his notebook and move to a different cell where he must memorize that humans and monkeys derive from a common ancestor.” — John Taylor Gatto (1935–2018), 'Dumbing Us Down: The Hidden Curriculum of Compulsory Education' (1992)
Re: The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.)
Tobias Geerinckx-Rice writes: > But I'll gladly judge other bikesheds if they lead to a less misleading name. How about 'package/variant' or 'package/variant-of'? -- (
Re: The package/inherit trap
Simon Tournier writes: > Hi Tobias, > > On Tue, 07 Mar 2023 at 22:11, Tobias Geerinckx-Rice wrote: > >> However, merely documenting something is not enough when we have the >> chance to fix misleading naming, as we do here. It would be nice to >> have, but orthogonal. > > I would not say it is orthogonal because renaming would not have been > enough, at least for me, in order to get the difference with ’inherit’. > > For sure, it is a real trap. :-) For instance, > > $ git log --grep='package/inherit' --oneline | grep use > 5f83dd03a2 gnu: kodi/wayland: Do not use package/inherit. > 6ecf88a6a1 gnu: poppler-next: Don't use 'package/inherit'. > 5a5b729d66 gnu: abseil-cpp: Don't use 'package/inherit'. > dbcf2b61b1 gnu: Fix erroneous uses of 'package/inherit'. > 2f97a666a5 gnu: python-urllib3: Don't use 'package/inherit' on > replacement package. > 4163b6d855 gnu: avahi: Don't use package/inherit. > > 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. Mark
Re: The package/inherit trap
Hi all, On Wed, Mar 08, 2023 at 10:07 AM, Simon Tournier wrote: > Hi Tobias, > > On Tue, 07 Mar 2023 at 22:11, Tobias Geerinckx-Rice wrote: > >> However, merely documenting something is not enough when we have the >> chance to fix misleading naming, as we do here. It would be nice to >> have, but orthogonal. > > I would not say it is orthogonal because renaming would not have been > enough, at least for me, in order to get the difference with ’inherit’. > > For sure, it is a real trap. :-) For instance, > > $ git log --grep='package/inherit' --oneline | grep use > 5f83dd03a2 gnu: kodi/wayland: Do not use package/inherit. > 6ecf88a6a1 gnu: poppler-next: Don't use 'package/inherit'. > 5a5b729d66 gnu: abseil-cpp: Don't use 'package/inherit'. > dbcf2b61b1 gnu: Fix erroneous uses of 'package/inherit'. > 2f97a666a5 gnu: python-urllib3: Don't use 'package/inherit' on > replacement package. > 4163b6d855 gnu: avahi: Don't use package/inherit. > > and in the light of this discussion, 5f83dd03a2 seems incorrect, no? > > Well, I agree that naming is important and probably the most difficult > task. :-) > > Do we go for renaming + deprecated? Then do we apply the rename to all > occurrences in Guix? > > Cheers, > simon I know, reviving an old thread but I assume this wasn't resolved? Throw my vote for the more explicit renaming with better doc/examples, as one who never remembers the difference either. John
Re: The package/inherit trap
Hi Tobias, On Tue, 07 Mar 2023 at 22:11, Tobias Geerinckx-Rice wrote: > However, merely documenting something is not enough when we have the > chance to fix misleading naming, as we do here. It would be nice to > have, but orthogonal. I would not say it is orthogonal because renaming would not have been enough, at least for me, in order to get the difference with ’inherit’. For sure, it is a real trap. :-) For instance, $ git log --grep='package/inherit' --oneline | grep use 5f83dd03a2 gnu: kodi/wayland: Do not use package/inherit. 6ecf88a6a1 gnu: poppler-next: Don't use 'package/inherit'. 5a5b729d66 gnu: abseil-cpp: Don't use 'package/inherit'. dbcf2b61b1 gnu: Fix erroneous uses of 'package/inherit'. 2f97a666a5 gnu: python-urllib3: Don't use 'package/inherit' on replacement package. 4163b6d855 gnu: avahi: Don't use package/inherit. and in the light of this discussion, 5f83dd03a2 seems incorrect, no? Well, I agree that naming is important and probably the most difficult task. :-) Do we go for renaming + deprecated? Then do we apply the rename to all occurrences in Guix? Cheers, simon
Re: The package/inherit trap
Hi, On 7 March 2023 17:46:50 UTC, Simon Tournier wrote: >Well, from my point of view, we have a trap because the documentation is >not clear. :-) Both. However, merely documenting something is not enough when we have the chance to fix misleading naming, as we do here. It would be nice to have, but orthogonal. Kind regards, T G-R Sent on the go. Excuse or enjoy my brevity.
Re: The package/inherit trap
Hi Maxim, On Tue, 07 Mar 2023 at 11:43, Maxim Cournoyer wrote: >> @lisp >> (use-modules (gnu packages gdb)) ;for 'gdb' >> >> (define gdb-sans-guile >> (package >> (inherit gdb) >> (inputs (modify-inputs (package-inputs gdb) >> (delete "guile") >> @end lisp > > Do you mean inconsistent because based on what I wrote it should have > used "package/inherit gdb ..." instead of (package (inherit gdb) ...) ? Based on my understanding about what you wrote. > If so, I agree. It could be modified to use the former and an extra > explanation offered about why package/inherit is used here when it's to > be preferred to plain inheritance. Well, from my point of view, we have a trap because the documentation is not clear. :-) Well, I think it is not only by replacing in the example. I think the manual should provide 2 examples and makes a clear line when one needs to pick ’inherit’ or when one needs to pick ’package/inherit’. Somehow, we have a similar issue as we had before with “Snippet vs Phases“. It would help to have plain words for ’package/inherit’ use cases; assuming all the other use cases are covered by ’inherit’. ;-) Cheers, simon
Re: The package/inherit trap
Hi Simon,
Simon Tournier writes:
> Hi,
>
> On Fri, 03 Mar 2023 at 20:21, Tobias Geerinckx-Rice wrote:
>
>> Could we rename it to something like
>> ‘package+replacements/inherit’? To me, that captures the spirit,
>> without being overly longer.
>
> Well, I gave a look at the code and have seen the replacement. But I
> had not thought about the package transformation and the like.
>
> From my point of view, the best would to add a paragraph with index
> entries under “Defining-Package-Variants” section [1].
>
> However, in the light of Maxim’s explanations, the example from the
> manual appears to me inconsistent:
>
> You can just as well define variants with a different set of
> dependencies than the original package. For example, the default
> @code{gdb} package depends on @code{guile}, but since that is an
> optional dependency, you can define a variant that removes that
> dependency like so:
>
> @lisp
> (use-modules (gnu packages gdb)) ;for 'gdb'
>
> (define gdb-sans-guile
> (package
> (inherit gdb)
> (inputs (modify-inputs (package-inputs gdb)
> (delete "guile")
> @end lisp
Do you mean inconsistent because based on what I wrote it should have
used "package/inherit gdb ..." instead of (package (inherit gdb) ...) ?
If so, I agree. It could be modified to use the former and an extra
explanation offered about why package/inherit is used here when it's to
be preferred to plain inheritance.
--
Thanks,
Maxim
Re: The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.)
Hi,
On Fri, 03 Mar 2023 at 20:21, Tobias Geerinckx-Rice wrote:
> Could we rename it to something like
> ‘package+replacements/inherit’? To me, that captures the spirit,
> without being overly longer.
Well, I gave a look at the code and have seen the replacement. But I
had not thought about the package transformation and the like.
>From my point of view, the best would to add a paragraph with index
entries under “Defining-Package-Variants” section [1].
However, in the light of Maxim’s explanations, the example from the
manual appears to me inconsistent:
--8<---cut here---start->8---
You can just as well define variants with a different set of
dependencies than the original package. For example, the default
@code{gdb} package depends on @code{guile}, but since that is an
optional dependency, you can define a variant that removes that
dependency like so:
@lisp
(use-modules (gnu packages gdb)) ;for 'gdb'
(define gdb-sans-guile
(package
(inherit gdb)
(inputs (modify-inputs (package-inputs gdb)
(delete "guile")
@end lisp
--8<---cut here---end--->8---
Well, since the trap is not completely clear for me yet, I am not able
to propose a paragraph. IMHO, a paragraph here would help in mitigating
the trap. Whatever the name of ’package/inherit’. :-)
1:
Cheers,
simon
Re: The package/inherit trap
Hi, Tobias Geerinckx-Rice writes: > Hi! > > Maxim Cournoyer 写道： >> Simon Tournier writes: >>> It is not clear for me why you choose one over the other. From my >>> current understanding, I would be tempted to always use >>> 'package/inherit' and never plain 'inherit'. >> >> I also got confused by that in the past; > > Same. I think it's a rite of passage. A questionable one. > >> The way I process it internally now is this: >> >> If the inheritance is for *same-source/same-version* variants of a >> package, they should use package/inherit, as any security issues >> found >> in the parent package should also be applied to that package (since >> they >> use the same source). Otherwise, plain 'inherit' should be used >> (e.g. for newer version variants). > > That about jives with my intuition. > > Judging by the (IMO) universal confusion this causes, it is (IMO) > spectacularly poorly-named. A docstring doesn't fix that. > > Could we rename it to something like ‘package+replacements/inherit’? > To me, that captures the spirit, without being overly longer. That'd be better, I think. The more verbose name at least wouldn't let one think, 'oh, some questionable syntax sugar for the lazy', like I did in the past :-). -- Thanks, Maxim
The package/inherit trap (was: gnu: stellarium: Enable ShowMySky.)
Hi! Maxim Cournoyer 写道： Simon Tournier writes: It is not clear for me why you choose one over the other. From my current understanding, I would be tempted to always use 'package/inherit' and never plain 'inherit'. I also got confused by that in the past; Same. I think it's a rite of passage. A questionable one. The way I process it internally now is this: If the inheritance is for *same-source/same-version* variants of a package, they should use package/inherit, as any security issues found in the parent package should also be applied to that package (since they use the same source). Otherwise, plain 'inherit' should be used (e.g. for newer version variants). That about jives with my intuition. Judging by the (IMO) universal confusion this causes, it is (IMO) spectacularly poorly-named. A docstring doesn't fix that. Could we rename it to something like ‘package+replacements/inherit’? To me, that captures the spirit, without being overly longer. But I'll gladly judge other bikesheds if they lead to a less misleading name. Kind regards, T G-R signature.asc Description: PGP signature
