[gentoo-dev] The inherit-EXPORT_FUNCTIONS ordering problem

[gentoo-dev] The inherit-EXPORT_FUNCTIONS ordering problem

[Michał Górny]

Hi,

I've been informed of a slight inconsistency in package manager behavior
that affects combining EXPORT_FUNCTIONS with inherit (by ionic, thanks
for the report!).  Please consider the three following snippets:

xdg.eclass:

  EXPORT_FUNCTIONS src_prepare

ecm-1.eclass:

  inherit xdg
  EXPORT_FUNCTIONS src_prepare

ecm-2.eclass:

  EXPORT_FUNCTIONS src_prepare
  inherit xdg


Now, ecm-1.eclass produces consistent behavior across all PMs --
ecm-1_src_prepare takes precedence.  However, ecm-2.eclass is not
consistent:

- Portage will take ecm-2_src_prepare, i.e. applies precedence based
on inherit order and not actual call order

- PkgCore and Paludis will take xdg_src_prepare since its
EXPORT_FUNCTIONS are called later (since the inherit is done later).

Apparently, the Portage behavior was changed in 2009 [1].  PMS is not
very clear on what should happen.


Therefore:

1. I'd like to propose that we explicitly require all inherits to happen
before EXPORT_FUNCTIONS.  This will ensure consistent behavior across
all package managers.

2. I'd like to ask your opinion whether we should:

a. revert the Portage behavior to be consistent with PkgCore/Paludis

b. update PMS to identify the behavior as 'undefined', i.e. either
solution is correct.


WDYT?


[1] https://github.com/gentoo/portage/commit/06d4433e8b8be60d606733b9e23f57f8a5869d8f

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] The inherit-EXPORT_FUNCTIONS ordering problem

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 2213 bytes --]

>>>>> On Sat, 28 Aug 2021, Michał Górny wrote:

> I've been informed of a slight inconsistency in package manager behavior
> that affects combining EXPORT_FUNCTIONS with inherit (by ionic, thanks
> for the report!).  Please consider the three following snippets:

> xdg.eclass:

>   EXPORT_FUNCTIONS src_prepare

> ecm-1.eclass:

>   inherit xdg
>   EXPORT_FUNCTIONS src_prepare

> ecm-2.eclass:

>   EXPORT_FUNCTIONS src_prepare
>   inherit xdg


> Now, ecm-1.eclass produces consistent behavior across all PMs --
> ecm-1_src_prepare takes precedence.  However, ecm-2.eclass is not
> consistent:

> - Portage will take ecm-2_src_prepare, i.e. applies precedence based
> on inherit order and not actual call order

But it is the reverse of normal inherit order. That is, if an ebuild
does:

  inherit ecm-2 xdg

then xdg takes precedence. However, in your second example above, the
inherit order is the same (ecm-2 first, xdg second), but with Portage
the _earlier_ eclass takes precedence.

> - PkgCore and Paludis will take xdg_src_prepare since its
> EXPORT_FUNCTIONS are called later (since the inherit is done later).

Which is consistent with the behaviour when both eclasses are inherited
directly from the ebuild.

> Apparently, the Portage behavior was changed in 2009 [1].  PMS is not
> very clear on what should happen.

Can you provide any pointer to a discussion of that Portage change in
gentoo-dev?


> Therefore:

> 1. I'd like to propose that we explicitly require all inherits to happen
> before EXPORT_FUNCTIONS.  This will ensure consistent behavior across
> all package managers.

> 2. I'd like to ask your opinion whether we should:

> a. revert the Portage behavior to be consistent with PkgCore/Paludis

> b. update PMS to identify the behavior as 'undefined', i.e. either
> solution is correct.

> WDYT?

Not sure, but if we follow [2] then this qualifies as a Portage bug.
I'd really hate to retroactively change the spec to "undefined".

Ulrich


> [1] https://github.com/gentoo/portage/commit/06d4433e8b8be60d606733b9e23f57f8a5869d8f

[2] https://projects.gentoo.org/council/meeting-logs/20160313-summary.txt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

Re: [gentoo-dev] The inherit-EXPORT_FUNCTIONS ordering problem

[William Hubbs]

[-- Attachment #1: Type: text/plain, Size: 1470 bytes --]

On Sat, Aug 28, 2021 at 06:35:06PM +0200, Michał Górny wrote:
> Hi,
> 
> I've been informed of a slight inconsistency in package manager behavior
> that affects combining EXPORT_FUNCTIONS with inherit (by ionic, thanks
> for the report!).  Please consider the three following snippets:
 
 ...

> 1. I'd like to propose that we explicitly require all inherits to happen
> before EXPORT_FUNCTIONS.  This will ensure consistent behavior across
> all package managers.
> 
> 2. I'd like to ask your opinion whether we should:
> 
> a. revert the Portage behavior to be consistent with PkgCore/Paludis
> 
> b. update PMS to identify the behavior as 'undefined', i.e. either
> solution is correct.

I would go with 1 and 2 b, but I also propose another option for the
next eapi which may be a bit controversial.

Starting with eapi 9, make export_functions a noop (you can't remove it
until all eclasses/ebuilds only support eapis that don't require it).

I understand that this is controversial, because it would require a lot
of work to convert ebuilds to eapi 9, but it would eliminate this
ambiguity/inconsistency in the future because it would require all
ebuilds to have phase functions unless they can use the default phases
the eapis provide.

Thoughts?

William

> 
> WDYT?
> 
> 
> [1] https://github.com/gentoo/portage/commit/06d4433e8b8be60d606733b9e23f57f8a5869d8f
> 
> -- 
> Best regards,
> Michał Górny
> 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [gentoo-dev] The inherit-EXPORT_FUNCTIONS ordering problem

[Michał Górny]

On Sun, 2021-08-29 at 12:23 -0500, William Hubbs wrote:
> On Sat, Aug 28, 2021 at 06:35:06PM +0200, Michał Górny wrote:
> > Hi,
> > 
> > I've been informed of a slight inconsistency in package manager behavior
> > that affects combining EXPORT_FUNCTIONS with inherit (by ionic, thanks
> > for the report!).  Please consider the three following snippets:
>  
>  ...
> 
> > 1. I'd like to propose that we explicitly require all inherits to happen
> > before EXPORT_FUNCTIONS.  This will ensure consistent behavior across
> > all package managers.
> > 
> > 2. I'd like to ask your opinion whether we should:
> > 
> > a. revert the Portage behavior to be consistent with PkgCore/Paludis
> > 
> > b. update PMS to identify the behavior as 'undefined', i.e. either
> > solution is correct.
> 
> I would go with 1 and 2 b, but I also propose another option for the
> next eapi which may be a bit controversial.
> 
> Starting with eapi 9, make export_functions a noop (you can't remove it
> until all eclasses/ebuilds only support eapis that don't require it).
> 
> I understand that this is controversial, because it would require a lot
> of work to convert ebuilds to eapi 9, but it would eliminate this
> ambiguity/inconsistency in the future because it would require all
> ebuilds to have phase functions unless they can use the default phases
> the eapis provide.
> 
> Thoughts?
> 

It would not only require work to convert ebuilds but it would also
require a lot of work when writing new ebuilds.  The developers would
have to explicitly remember to call the correct set of phases for each
inherited eclass, and this is a lot of work for the lot of otherwise
trivial ebuilds.

Not to mention the quite obvious risk of silent breakage when people
forget to call a phase function.  Just imagine that all python-single-r1
ebuilds would have to call python-single-r1_pkg_setup explicitly or they
would silently fall back to random Python version that's going to work
90% of the time, so people won't even notice that the ebuild is broken.

In the end, this would naturally lead to people hacking this around by
reinventing EXPORT_FUNCTIONS inside eclasses.  In the best case, we'd
see functions like 'cmake_export_phases' -- which maybe, just maybe,
would be better than implicit exports.

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] The inherit-EXPORT_FUNCTIONS ordering problem

[Ionen Wolkens]

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

On Sun, Aug 29, 2021 at 12:23:22PM -0500, William Hubbs wrote:
> On Sat, Aug 28, 2021 at 06:35:06PM +0200, Michał Górny wrote:
> > Hi,
> > 
> > I've been informed of a slight inconsistency in package manager behavior
> > that affects combining EXPORT_FUNCTIONS with inherit (by ionic, thanks
> > for the report!).  Please consider the three following snippets:
>  
>  ...
> 
> > 1. I'd like to propose that we explicitly require all inherits to happen
> > before EXPORT_FUNCTIONS.  This will ensure consistent behavior across
> > all package managers.
> > 
> > 2. I'd like to ask your opinion whether we should:
> > 
> > a. revert the Portage behavior to be consistent with PkgCore/Paludis
> > 
> > b. update PMS to identify the behavior as 'undefined', i.e. either
> > solution is correct.
> 
> I would go with 1 and 2 b, but I also propose another option for the
> next eapi which may be a bit controversial.
> 
> Starting with eapi 9, make export_functions a noop (you can't remove it
> until all eclasses/ebuilds only support eapis that don't require it).
> 
> I understand that this is controversial, because it would require a lot
> of work to convert ebuilds to eapi 9, but it would eliminate this
> ambiguity/inconsistency in the future because it would require all
> ebuilds to have phase functions unless they can use the default phases
> the eapis provide.
> 
> Thoughts?

If anything I'd personally prefer the rough opposite of this, in the
event multiple eclass export the same, have the package manager merge
them.

e.g. rather than have `inherit cmake xdg` run xdg_src_prepare over
cmake's, would export:

src_prepare() {
	cmake_src_prepare
	xdg_src_prepare
}

unless the ebuild redefines it (which I imagine would often be because
of optional support, e.g. use lua &&, or occasional incompatibilities)

Some ebuilds have special needs, but then there's all those generic
ebuilds that should stay nearly empty.
-- 
ionen

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [gentoo-dev] The inherit-EXPORT_FUNCTIONS ordering problem

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]



> On 29 Aug 2021, at 19:41, Ionen Wolkens <ionen@gentoo.org> wrote:
> 
> On Sun, Aug 29, 2021 at 12:23:22PM -0500, William Hubbs wrote:
>> On Sat, Aug 28, 2021 at 06:35:06PM +0200, Michał Górny wrote:
>>> Hi,
>>> 
>>> I've been informed of a slight inconsistency in package manager behavior
>>> that affects combining EXPORT_FUNCTIONS with inherit (by ionic, thanks
>>> for the report!).  Please consider the three following snippets:
>> 
>> ...
>> 
>>> 1. I'd like to propose that we explicitly require all inherits to happen
>>> before EXPORT_FUNCTIONS.  This will ensure consistent behavior across
>>> all package managers.
>>> 
>>> 2. I'd like to ask your opinion whether we should:
>>> 
>>> a. revert the Portage behavior to be consistent with PkgCore/Paludis
>>> 
>>> b. update PMS to identify the behavior as 'undefined', i.e. either
>>> solution is correct.
>> 
>> I would go with 1 and 2 b, but I also propose another option for the
>> next eapi which may be a bit controversial.
>> 
>> Starting with eapi 9, make export_functions a noop (you can't remove it
>> until all eclasses/ebuilds only support eapis that don't require it).
>> 
>> I understand that this is controversial, because it would require a lot
>> of work to convert ebuilds to eapi 9, but it would eliminate this
>> ambiguity/inconsistency in the future because it would require all
>> ebuilds to have phase functions unless they can use the default phases
>> the eapis provide.
>> 
>> Thoughts?
> 
> If anything I'd personally prefer the rough opposite of this, in the
> event multiple eclass export the same, have the package manager merge
> them.

This is my preference too, if we're to go this route.

It's similar to what was proposed in https://bugs.gentoo.org/516014, although
this topic tends to lead to lots of different ideas (sometimes solving different
problems!)

> 
> e.g. rather than have `inherit cmake xdg` run xdg_src_prepare over
> cmake's, would export:
> 
> src_prepare() {
> 	cmake_src_prepare
> 	xdg_src_prepare
> }
> 
> unless the ebuild redefines it (which I imagine would often be because
> of optional support, e.g. use lua &&, or occasional incompatibilities)
> 
> Some ebuilds have special needs, but then there's all those generic
> ebuilds that should stay nearly empty.

best,
sam


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]

Re: [gentoo-dev] The inherit-EXPORT_FUNCTIONS ordering problem

[Michał Górny]

On Sun, 2021-08-29 at 14:41 -0400, Ionen Wolkens wrote:
> On Sun, Aug 29, 2021 at 12:23:22PM -0500, William Hubbs wrote:
> > On Sat, Aug 28, 2021 at 06:35:06PM +0200, Michał Górny wrote:
> > > Hi,
> > > 
> > > I've been informed of a slight inconsistency in package manager behavior
> > > that affects combining EXPORT_FUNCTIONS with inherit (by ionic, thanks
> > > for the report!).  Please consider the three following snippets:
> >  
> >  ...
> > 
> > > 1. I'd like to propose that we explicitly require all inherits to happen
> > > before EXPORT_FUNCTIONS.  This will ensure consistent behavior across
> > > all package managers.
> > > 
> > > 2. I'd like to ask your opinion whether we should:
> > > 
> > > a. revert the Portage behavior to be consistent with PkgCore/Paludis
> > > 
> > > b. update PMS to identify the behavior as 'undefined', i.e. either
> > > solution is correct.
> > 
> > I would go with 1 and 2 b, but I also propose another option for the
> > next eapi which may be a bit controversial.
> > 
> > Starting with eapi 9, make export_functions a noop (you can't remove it
> > until all eclasses/ebuilds only support eapis that don't require it).
> > 
> > I understand that this is controversial, because it would require a lot
> > of work to convert ebuilds to eapi 9, but it would eliminate this
> > ambiguity/inconsistency in the future because it would require all
> > ebuilds to have phase functions unless they can use the default phases
> > the eapis provide.
> > 
> > Thoughts?
> 
> If anything I'd personally prefer the rough opposite of this, in the
> event multiple eclass export the same, have the package manager merge
> them.
> 
> e.g. rather than have `inherit cmake xdg` run xdg_src_prepare over
> cmake's, would export:
> 
> src_prepare() {
> 	cmake_src_prepare
> 	xdg_src_prepare
> }
> 
> unless the ebuild redefines it (which I imagine would often be because
> of optional support, e.g. use lua &&, or occasional incompatibilities)
> 
> Some ebuilds have special needs, but then there's all those generic
> ebuilds that should stay nearly empty.

What about transitive inherits?  Say, foo.eclass inherits bar.eclass. 
Does that imply that the ebuild calls foo_src_prepare
and bar_src_prepare, and the eclass has no way of overriding this?  What
if the eclass needs bar_src_prepare called explicitly before or after
its own src_prepare?

-- 
Best regards,
Michał Górny

Re: [gentoo-dev] The inherit-EXPORT_FUNCTIONS ordering problem

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 1343 bytes --]



> On 29 Aug 2021, at 19:44, Michał Górny <mgorny@gentoo.org> wrote:
> 
> On Sun, 2021-08-29 at 14:41 -0400, Ionen Wolkens wrote:
>>> [snip]
>> 
>> If anything I'd personally prefer the rough opposite of this, in the
>> event multiple eclass export the same, have the package manager merge
>> them.
>> 
>> e.g. rather than have `inherit cmake xdg` run xdg_src_prepare over
>> cmake's, would export:
>> 
>> src_prepare() {
>> 	cmake_src_prepare
>> 	xdg_src_prepare
>> }
>> 
>> unless the ebuild redefines it (which I imagine would often be because
>> of optional support, e.g. use lua &&, or occasional incompatibilities)
>> 
>> Some ebuilds have special needs, but then there's all those generic
>> ebuilds that should stay nearly empty.
> 
> What about transitive inherits?  Say, foo.eclass inherits bar.eclass.
> Does that imply that the ebuild calls foo_src_prepare
> and bar_src_prepare, and the eclass has no way of overriding this?  What
> if the eclass needs bar_src_prepare called explicitly before or after
> its own src_prepare?

My expectation would be that the ebuild only called depth=1 exported
functions and it would be the responsibility of each eclass to call its own
inherits. But this becomes a problem with repeated inherits given our
phases aren't usually idempotent.

best,
sam


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 618 bytes --]