[PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

Michał Górny-5
The value of get_libdir depends on the profile, and so it is not useful
for dependency calculations. Furthermore, it seems that Portage does
not handle defining it in global scope well due to EAPI checking magic.
Ban it completely where it is defined as EAPI function to let developers
catch their mistakes early rather than see them as 'command not found'
errors during dependency calculation / cache updates.

Bug: https://bugs.gentoo.org/629010
---
 bin/ebuild.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/bin/ebuild.sh b/bin/ebuild.sh
index a400ef72e..f1ac3f278 100755
--- a/bin/ebuild.sh
+++ b/bin/ebuild.sh
@@ -66,6 +66,7 @@ else
  use useq usev use_with use_enable"
  ___eapi_has_usex && funcs+=" usex"
  ___eapi_has_in_iuse && funcs+=" in_iuse"
+ ___eapi_has_get_libdir && funcs+=" get_libdir"
  # These functions die because calls to them during the "depend" phase
  # are considered to be severe QA violations.
  funcs+=" best_version has_version portageq"
--
2.14.1


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

Zac Medico-2
On 08/30/2017 02:06 AM, Michał Górny wrote:

> The value of get_libdir depends on the profile, and so it is not useful
> for dependency calculations. Furthermore, it seems that Portage does
> not handle defining it in global scope well due to EAPI checking magic.
> Ban it completely where it is defined as EAPI function to let developers
> catch their mistakes early rather than see them as 'command not found'
> errors during dependency calculation / cache updates.
>
> Bug: https://bugs.gentoo.org/629010
> ---
>  bin/ebuild.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/bin/ebuild.sh b/bin/ebuild.sh
> index a400ef72e..f1ac3f278 100755
> --- a/bin/ebuild.sh
> +++ b/bin/ebuild.sh
> @@ -66,6 +66,7 @@ else
>   use useq usev use_with use_enable"
>   ___eapi_has_usex && funcs+=" usex"
>   ___eapi_has_in_iuse && funcs+=" in_iuse"
> + ___eapi_has_get_libdir && funcs+=" get_libdir"
>   # These functions die because calls to them during the "depend" phase
>   # are considered to be severe QA violations.
>   funcs+=" best_version has_version portageq"
>

It's possible that there are working ebuilds that call get_libdir in
global scope. Have we done an analysis of the ebuilds in the gentoo
repository? Obviously, it would be safer to call eqawarn.
--
Thanks,
Zac

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

Ulrich Mueller-2
>>>>> On Wed, 30 Aug 2017, Zac Medico wrote:

> It's possible that there are working ebuilds that call get_libdir in
> global scope.

How could that be possible when get_libdir() is defined in
phase-helpers.sh?

> Have we done an analysis of the ebuilds in the gentoo repository?
> Obviously, it would be safer to call eqawarn.

Ulrich

attachment0 (501 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

Zac Medico-2
On 08/30/2017 10:52 AM, Ulrich Mueller wrote:
>>>>>> On Wed, 30 Aug 2017, Zac Medico wrote:
>
>> It's possible that there are working ebuilds that call get_libdir in
>> global scope.
>
> How could that be possible when get_libdir() is defined in
> phase-helpers.sh?

During an "normal" ebuild phase, ebuild.sh will source phase-helpers.sh
before it sources the ebuild:

if [[ $EBUILD_PHASE != depend ]] ; then
        source "${PORTAGE_BIN_PATH}/phase-functions.sh" || die
        source "${PORTAGE_BIN_PATH}/save-ebuild-env.sh" || die
        source "${PORTAGE_BIN_PATH}/phase-helpers.sh" || die
        source "${PORTAGE_BIN_PATH}/bashrc-functions.sh" || die
else
--
Thanks,
Zac

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

Michał Górny-5
In reply to this post by Zac Medico-2
W dniu śro, 30.08.2017 o godzinie 10∶48 -0700, użytkownik Zac Medico
napisał:

> On 08/30/2017 02:06 AM, Michał Górny wrote:
> > The value of get_libdir depends on the profile, and so it is not useful
> > for dependency calculations. Furthermore, it seems that Portage does
> > not handle defining it in global scope well due to EAPI checking magic.
> > Ban it completely where it is defined as EAPI function to let developers
> > catch their mistakes early rather than see them as 'command not found'
> > errors during dependency calculation / cache updates.
> >
> > Bug: https://bugs.gentoo.org/629010
> > ---
> >  bin/ebuild.sh | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/bin/ebuild.sh b/bin/ebuild.sh
> > index a400ef72e..f1ac3f278 100755
> > --- a/bin/ebuild.sh
> > +++ b/bin/ebuild.sh
> > @@ -66,6 +66,7 @@ else
> >   use useq usev use_with use_enable"
> >   ___eapi_has_usex && funcs+=" usex"
> >   ___eapi_has_in_iuse && funcs+=" in_iuse"
> > + ___eapi_has_get_libdir && funcs+=" get_libdir"
> >   # These functions die because calls to them during the "depend" phase
> >   # are considered to be severe QA violations.
> >   funcs+=" best_version has_version portageq"
> >
>
> It's possible that there are working ebuilds that call get_libdir in
> global scope. Have we done an analysis of the ebuilds in the gentoo
> repository? Obviously, it would be safer to call eqawarn.

If there were any (more), we'd have caught them during cache regen,
wouldn't we? When I accidentally left it when bumping to EAPI 6, I've
got a bug report almost immediately.

--
Best regards,
Michał Górny


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

Zac Medico-2
On 08/30/2017 01:31 PM, Michał Górny wrote:

> W dniu śro, 30.08.2017 o godzinie 10∶48 -0700, użytkownik Zac Medico
> napisał:
>> On 08/30/2017 02:06 AM, Michał Górny wrote:
>>> The value of get_libdir depends on the profile, and so it is not useful
>>> for dependency calculations. Furthermore, it seems that Portage does
>>> not handle defining it in global scope well due to EAPI checking magic.
>>> Ban it completely where it is defined as EAPI function to let developers
>>> catch their mistakes early rather than see them as 'command not found'
>>> errors during dependency calculation / cache updates.
>>>
>>> Bug: https://bugs.gentoo.org/629010
>>> ---
>>>  bin/ebuild.sh | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/bin/ebuild.sh b/bin/ebuild.sh
>>> index a400ef72e..f1ac3f278 100755
>>> --- a/bin/ebuild.sh
>>> +++ b/bin/ebuild.sh
>>> @@ -66,6 +66,7 @@ else
>>>   use useq usev use_with use_enable"
>>>   ___eapi_has_usex && funcs+=" usex"
>>>   ___eapi_has_in_iuse && funcs+=" in_iuse"
>>> + ___eapi_has_get_libdir && funcs+=" get_libdir"
>>>   # These functions die because calls to them during the "depend" phase
>>>   # are considered to be severe QA violations.
>>>   funcs+=" best_version has_version portageq"
>>>
>>
>> It's possible that there are working ebuilds that call get_libdir in
>> global scope. Have we done an analysis of the ebuilds in the gentoo
>> repository? Obviously, it would be safer to call eqawarn.
>
> If there were any (more), we'd have caught them during cache regen,
> wouldn't we? When I accidentally left it when bumping to EAPI 6, I've
> got a bug report almost immediately.

We'll only catch it during cache regen if we delete all of the previous
cache, forcing all of the ebuilds to be sourced again. If all ebuilds in
the gentoo tree are compliant, the I think that's good enough for us to
die here.
--
Thanks,
Zac

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

Michał Górny-5
W dniu śro, 30.08.2017 o godzinie 13∶35 -0700, użytkownik Zac Medico
napisał:

> On 08/30/2017 01:31 PM, Michał Górny wrote:
> > W dniu śro, 30.08.2017 o godzinie 10∶48 -0700, użytkownik Zac Medico
> > napisał:
> > > On 08/30/2017 02:06 AM, Michał Górny wrote:
> > > > The value of get_libdir depends on the profile, and so it is not useful
> > > > for dependency calculations. Furthermore, it seems that Portage does
> > > > not handle defining it in global scope well due to EAPI checking magic.
> > > > Ban it completely where it is defined as EAPI function to let developers
> > > > catch their mistakes early rather than see them as 'command not found'
> > > > errors during dependency calculation / cache updates.
> > > >
> > > > Bug: https://bugs.gentoo.org/629010
> > > > ---
> > > >  bin/ebuild.sh | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/bin/ebuild.sh b/bin/ebuild.sh
> > > > index a400ef72e..f1ac3f278 100755
> > > > --- a/bin/ebuild.sh
> > > > +++ b/bin/ebuild.sh
> > > > @@ -66,6 +66,7 @@ else
> > > >   use useq usev use_with use_enable"
> > > >   ___eapi_has_usex && funcs+=" usex"
> > > >   ___eapi_has_in_iuse && funcs+=" in_iuse"
> > > > + ___eapi_has_get_libdir && funcs+=" get_libdir"
> > > >   # These functions die because calls to them during the "depend" phase
> > > >   # are considered to be severe QA violations.
> > > >   funcs+=" best_version has_version portageq"
> > > >
> > >
> > > It's possible that there are working ebuilds that call get_libdir in
> > > global scope. Have we done an analysis of the ebuilds in the gentoo
> > > repository? Obviously, it would be safer to call eqawarn.
> >
> > If there were any (more), we'd have caught them during cache regen,
> > wouldn't we? When I accidentally left it when bumping to EAPI 6, I've
> > got a bug report almost immediately.
>
> We'll only catch it during cache regen if we delete all of the previous
> cache, forcing all of the ebuilds to be sourced again. If all ebuilds in
> the gentoo tree are compliant, the I think that's good enough for us to
> die here.

I'm pretty sure all of them are. However, if someone has resources to
spare, I'd appreciate running a full regen with the patch to confirm.

--
Best regards,
Michał Górny


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

Zac Medico-2
On 08/30/2017 01:45 PM, Michał Górny wrote:

> W dniu śro, 30.08.2017 o godzinie 13∶35 -0700, użytkownik Zac Medico
> napisał:
>> On 08/30/2017 01:31 PM, Michał Górny wrote:
>>> W dniu śro, 30.08.2017 o godzinie 10∶48 -0700, użytkownik Zac Medico
>>> napisał:
>>>> On 08/30/2017 02:06 AM, Michał Górny wrote:
>>>>> The value of get_libdir depends on the profile, and so it is not useful
>>>>> for dependency calculations. Furthermore, it seems that Portage does
>>>>> not handle defining it in global scope well due to EAPI checking magic.
>>>>> Ban it completely where it is defined as EAPI function to let developers
>>>>> catch their mistakes early rather than see them as 'command not found'
>>>>> errors during dependency calculation / cache updates.
>>>>>
>>>>> Bug: https://bugs.gentoo.org/629010
>>>>> ---
>>>>>  bin/ebuild.sh | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/bin/ebuild.sh b/bin/ebuild.sh
>>>>> index a400ef72e..f1ac3f278 100755
>>>>> --- a/bin/ebuild.sh
>>>>> +++ b/bin/ebuild.sh
>>>>> @@ -66,6 +66,7 @@ else
>>>>>   use useq usev use_with use_enable"
>>>>>   ___eapi_has_usex && funcs+=" usex"
>>>>>   ___eapi_has_in_iuse && funcs+=" in_iuse"
>>>>> + ___eapi_has_get_libdir && funcs+=" get_libdir"
>>>>>   # These functions die because calls to them during the "depend" phase
>>>>>   # are considered to be severe QA violations.
>>>>>   funcs+=" best_version has_version portageq"
>>>>>
>>>>
>>>> It's possible that there are working ebuilds that call get_libdir in
>>>> global scope. Have we done an analysis of the ebuilds in the gentoo
>>>> repository? Obviously, it would be safer to call eqawarn.
>>>
>>> If there were any (more), we'd have caught them during cache regen,
>>> wouldn't we? When I accidentally left it when bumping to EAPI 6, I've
>>> got a bug report almost immediately.
>>
>> We'll only catch it during cache regen if we delete all of the previous
>> cache, forcing all of the ebuilds to be sourced again. If all ebuilds in
>> the gentoo tree are compliant, the I think that's good enough for us to
>> die here.
>
> I'm pretty sure all of them are. However, if someone has resources to
> spare, I'd appreciate running a full regen with the patch to confirm.
Confirmed. Please merge the patch.
--
Thanks,
Zac


signature.asc (231 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ebuild.sh: Explicitly ban get_libdir in global scope

Michał Górny-5
W dniu śro, 30.08.2017 o godzinie 23∶36 -0700, użytkownik Zac Medico
napisał:

> On 08/30/2017 01:45 PM, Michał Górny wrote:
> > W dniu śro, 30.08.2017 o godzinie 13∶35 -0700, użytkownik Zac Medico
> > napisał:
> > > On 08/30/2017 01:31 PM, Michał Górny wrote:
> > > > W dniu śro, 30.08.2017 o godzinie 10∶48 -0700, użytkownik Zac Medico
> > > > napisał:
> > > > > On 08/30/2017 02:06 AM, Michał Górny wrote:
> > > > > > The value of get_libdir depends on the profile, and so it is not useful
> > > > > > for dependency calculations. Furthermore, it seems that Portage does
> > > > > > not handle defining it in global scope well due to EAPI checking magic.
> > > > > > Ban it completely where it is defined as EAPI function to let developers
> > > > > > catch their mistakes early rather than see them as 'command not found'
> > > > > > errors during dependency calculation / cache updates.
> > > > > >
> > > > > > Bug: https://bugs.gentoo.org/629010
> > > > > > ---
> > > > > >  bin/ebuild.sh | 1 +
> > > > > >  1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/bin/ebuild.sh b/bin/ebuild.sh
> > > > > > index a400ef72e..f1ac3f278 100755
> > > > > > --- a/bin/ebuild.sh
> > > > > > +++ b/bin/ebuild.sh
> > > > > > @@ -66,6 +66,7 @@ else
> > > > > >   use useq usev use_with use_enable"
> > > > > >   ___eapi_has_usex && funcs+=" usex"
> > > > > >   ___eapi_has_in_iuse && funcs+=" in_iuse"
> > > > > > + ___eapi_has_get_libdir && funcs+=" get_libdir"
> > > > > >   # These functions die because calls to them during the "depend" phase
> > > > > >   # are considered to be severe QA violations.
> > > > > >   funcs+=" best_version has_version portageq"
> > > > > >
> > > > >
> > > > > It's possible that there are working ebuilds that call get_libdir in
> > > > > global scope. Have we done an analysis of the ebuilds in the gentoo
> > > > > repository? Obviously, it would be safer to call eqawarn.
> > > >
> > > > If there were any (more), we'd have caught them during cache regen,
> > > > wouldn't we? When I accidentally left it when bumping to EAPI 6, I've
> > > > got a bug report almost immediately.
> > >
> > > We'll only catch it during cache regen if we delete all of the previous
> > > cache, forcing all of the ebuilds to be sourced again. If all ebuilds in
> > > the gentoo tree are compliant, the I think that's good enough for us to
> > > die here.
> >
> > I'm pretty sure all of them are. However, if someone has resources to
> > spare, I'd appreciate running a full regen with the patch to confirm.
>
> Confirmed. Please merge the patch.

Thanks. Merged now.

--
Best regards,
Michał Górny