Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Ulrich Mueller]

> On Fri, 13 Nov 2015, Michał Górny wrote:

> Ok, I recalled the issue I was particularly concerned about.

>   PV="1_beta2"
>   MY_PV="${PV/_/~}"

> This triggers different behavior between bash<=4.2 and bash>=4.3,
> with the latter requiring:

>   PV="1_beta2"
>   MY_PV="${PV/_/\~}"

> (which in turns breaks older). Now, if someone put a conditional on
> BASH_VERSINFO that worked around this discrepancy, introducing
> BASH_COMPAT suddenly breaks it by forcing the old behavior when
> BASH_VERSINFO indicates the new behavior is expected.

> Not saying this is very likely to happen.

Highly unlikely, IMHO. There are several workarounds that are simpler
than making the code conditional on the bash version. For example:

   tilde="~"; MY_PV="${PV/_/${tilde}}"

or:

   MY_PV="${PV/_/$'\x7e'}"

> However, we are clearly violating the idea behind PMS by allowing it
> to happen retroactively.

Not sure. PMS is (and always was) quite clear about which bash version
belongs to which EAPI, so ebuilds have no business of circumventing
this by doing their own version checks.

> However, enabling BASH_COMPAT in EAPI 6 is a good thing. For
> example, because people in EAPI 6 no longer need to be concerned
> about this discrepancy.

Enabling it for EAPI 6 only will lead to the paradoxical situation
that bash will be compatible to 4.2 in EAPI 6, as it should be.
Whereas earlier EAPIs (which should use 3.2 really) will get the
behaviour of bash 4.3 or later instead.

Ulrich


pgpXfica1utMs.pgp
Description: PGP signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Ulrich Mueller]

> On Thu, 12 Nov 2015, Zac Medico wrote:

> On 11/12/2015 04:06 PM, Mike Frysinger wrote:
>> from ebuilds/eclasses that have already stopped using __:
>> __do_sed_fix ()
>> ___ECLASS_RECUR_MULTILIB=yes
>> ___ECLASS_RECUR_TOOLCHAIN_FUNCS=yes
>> __versionator_shopt_toggle ()
>> __versionator__test_version_compare ()
>> __versionator__test_version_is_at_least ()
>> 
>> grepping the tree, i see like two packages and one eclass still using __.
>> both of which are trivial to convert.

> Sure, but do we really want to confuse people who might be ignorant of
> this rule? Having functions disappear from the environment without
> warning is very likely to cause confusion...

Maybe repoman should make people aware of this rule then?

Ulrich


pgpzQwU1HBa1r.pgp
Description: PGP signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Zac Medico]

On 11/13/2015 12:12 AM, Ulrich Mueller wrote:
>> On Thu, 12 Nov 2015, Zac Medico wrote:
> 
>> On 11/12/2015 04:06 PM, Mike Frysinger wrote:
>>> from ebuilds/eclasses that have already stopped using __:
>>> __do_sed_fix ()
>>> ___ECLASS_RECUR_MULTILIB=yes
>>> ___ECLASS_RECUR_TOOLCHAIN_FUNCS=yes
>>> __versionator_shopt_toggle ()
>>> __versionator__test_version_compare ()
>>> __versionator__test_version_is_at_least ()
>>>
>>> grepping the tree, i see like two packages and one eclass still using __.
>>> both of which are trivial to convert.
> 
>> Sure, but do we really want to confuse people who might be ignorant of
>> this rule? Having functions disappear from the environment without
>> warning is very likely to cause confusion...
> 
> Maybe repoman should make people aware of this rule then?
> 
> Ulrich
> 

Yeah, that would be great. I'm a little more concerned about non-gentoo
people that don't necessarily run repoman, though. At my workplace, we
don't use repoman because it has too many checks that we don't care
about. We really need to make it more configurable for third-party users.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Michał Górny]

On Wed, 11 Nov 2015 12:52:05 +0100
Ulrich Mueller  wrote:

> > On Wed, 11 Nov 2015, Michał Górny wrote:  
> 
> > I'm not convinced we ought to do this for EAPI < 6. It is a breaking
> > change after all, and as such changes the behavior of EAPI < 6
> > ebuilds.  
> 
> Which according to PMS should assume bash version 3.2. Also, AFAICS
> the only feature where the compat settings could have any impact is
> this setting from compat42:
> 
>If set, bash does not process the replacement string in the pattern
>substitution word expansion using quote removal.
> 
> All other compat changes affect either only POSIX mode, or interactive
> mode, or string comparison (where the compat32 setting disables locale
> specific behaviour and uses strcmp instead, so effectively the compat
> setting should be saner).

Ok, I recalled the issue I was particularly concerned about.

  PV="1_beta2"
  MY_PV="${PV/_/~}"

This triggers different behavior between bash<=4.2 and bash>=4.3, with
the latter requiring:

  PV="1_beta2"
  MY_PV="${PV/_/\~}"

(which in turns breaks older). Now, if someone put a conditional on
BASH_VERSINFO that worked around this discrepancy, introducing
BASH_COMPAT suddenly breaks it by forcing the old behavior when
BASH_VERSINFO indicates the new behavior is expected.

Not saying this is very likely to happen. However, we are clearly
violating the idea behind PMS by allowing it to happen retroactively.

However, enabling BASH_COMPAT in EAPI 6 is a good thing. For example,
because people in EAPI 6 no longer need to be concerned about this
discrepancy.

-- 
Best regards,
Michał Górny



pgpMHO9B6Z7Ne.pgp
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Mike Frysinger]

On 12 Nov 2015 16:58, Zac Medico wrote:
> On 11/12/2015 04:06 PM, Mike Frysinger wrote:
> > from ebuilds/eclasses that have already stopped using __:
> > __do_sed_fix ()
> > ___ECLASS_RECUR_MULTILIB=yes
> > ___ECLASS_RECUR_TOOLCHAIN_FUNCS=yes
> > __versionator_shopt_toggle ()
> > __versionator__test_version_compare ()
> > __versionator__test_version_is_at_least ()
> > 
> > grepping the tree, i see like two packages and one eclass still using __.
> > both of which are trivial to convert.
> 
> Sure, but do we really want to confuse people who might be ignorant of
> this rule? Having functions disappear from the environment without
> warning is very likely to cause confusion...

that already happens to a degree if you happen to use a name that portage
uses itself.  we can add a repoman check, and if we think it comes up enough,
have portage itself warn when it blows away things it didn't register.

> Also, there's the
> element of backward-compatibility for any __* functions in
> /var/db/pkg/*/*/environment.bz2 of users' installed systems that might
> be needed during pkg_prerm and pkg_postrm.

the ones i highlighted are not needed for those purposes.  you're right it
could be a problem, i think the likelihood is low considering how infrequently
these two are used, and how much we push people to use other phases instead.

> >> Also note that some internals have been intentionally preserved in
> >> environment.bz2. For example, __eapi6_src_install exposes the default
> >> src_install implementation, which someone might examine for debugging
> >> purposes.
> > 
> > is that actually useful ?  i can't see how it would be.
> 
> Shrug, probably not (unless there's a bug in a particular
> implementation, and someone wants to go back and check which
> implementation was used for a particular installed package).

if we want to tag that kind of metadata in a build, we should just explicitly
include something like PORTAGE_VERSION.
-mike


signature.asc
Description: Digital signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Mike Frysinger]

On 12 Nov 2015 21:07, Tim Harder wrote:
> On 2015-11-11 14:42, Zac Medico wrote:
> > Please unset all new internal function inside bin/save-ebuild-env.sh.
> > Note that it already uses this line to unset functions beginning with
> > ___eapi:
> >
> >unset -f $(compgen -A function ___eapi_)
> >
> > However, your __eapi functions will not be matched because they only
> > begin with 2 underscores.
> 
> Just to note another approach, pkgcore generates global and per-eapi
> function lists at install time (or uses the generation scripts when
> running from a checkout) so manually tracking lists of functions isn't
> required.
> 
> The main reason I set that up was because I sometimes forgot to add
> functions to a similar list that was previously used. :)
> 
> I'm not overly familiar with portage's bash code so I'm not sure if
> that's feasible with its current structure.

i was thinking of building lists too, but i'm not entirely sure how to
tease that out of portage.
-mike


signature.asc
Description: Digital signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Tim Harder]

On 2015-11-11 14:42, Zac Medico wrote:
> Please unset all new internal function inside bin/save-ebuild-env.sh.
> Note that it already uses this line to unset functions beginning with
> ___eapi:

>unset -f $(compgen -A function ___eapi_)

> However, your __eapi functions will not be matched because they only
> begin with 2 underscores.

Just to note another approach, pkgcore generates global and per-eapi
function lists at install time (or uses the generation scripts when
running from a checkout) so manually tracking lists of functions isn't
required.

The main reason I set that up was because I sometimes forgot to add
functions to a similar list that was previously used. :)

I'm not overly familiar with portage's bash code so I'm not sure if
that's feasible with its current structure.

Tim


signature.asc
Description: PGP signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Zac Medico]

On 11/12/2015 06:07 PM, Tim Harder wrote:
> On 2015-11-11 14:42, Zac Medico wrote:
>> Please unset all new internal function inside bin/save-ebuild-env.sh.
>> Note that it already uses this line to unset functions beginning with
>> ___eapi:
> 
>>unset -f $(compgen -A function ___eapi_)
> 
>> However, your __eapi functions will not be matched because they only
>> begin with 2 underscores.
> 
> Just to note another approach, pkgcore generates global and per-eapi
> function lists at install time (or uses the generation scripts when
> running from a checkout) so manually tracking lists of functions isn't
> required.
> 
> The main reason I set that up was because I sometimes forgot to add
> functions to a similar list that was previously used. :)
> 
> I'm not overly familiar with portage's bash code so I'm not sure if
> that's feasible with its current structure.
> 
> Tim
> 

That seems like a pretty reasonable approach, at least up until PMS
reserved the __ prefix for package managers. I'm pretty sure that Mike
is intend on exploiting this reserved prefix now. :)
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Tim Harder]

On 2015-11-12 21:25, Zac Medico wrote:
> > Just to note another approach, pkgcore generates global and per-eapi
> > function lists at install time (or uses the generation scripts when
> > running from a checkout) so manually tracking lists of functions isn't
> > required.

> That seems like a pretty reasonable approach, at least up until PMS
> reserved the __ prefix for package managers. I'm pretty sure that Mike
> is intend on exploiting this reserved prefix now. :)

I don't think it's always possible to use a prefix for functions that
shouldn't be saved to the env that are internally implemented in the PM
but externally accessible in ebuilds, e.g. insinto, docinto, inherit,
etc.

Tim


signature.asc
Description: PGP signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Zac Medico]

On 11/12/2015 06:33 PM, Tim Harder wrote:
> On 2015-11-12 21:25, Zac Medico wrote:
>>> Just to note another approach, pkgcore generates global and per-eapi
>>> function lists at install time (or uses the generation scripts when
>>> running from a checkout) so manually tracking lists of functions isn't
>>> required.
> 
>> That seems like a pretty reasonable approach, at least up until PMS
>> reserved the __ prefix for package managers. I'm pretty sure that Mike
>> is intend on exploiting this reserved prefix now. :)
> 
> I don't think it's always possible to use a prefix for functions that
> shouldn't be saved to the env that are internally implemented in the PM
> but externally accessible in ebuilds, e.g. insinto, docinto, inherit,
> etc.
> 
> Tim
> 

That's true, I guess we should look into generating the list automatically.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Mike Frysinger]

On 11 Nov 2015 10:32, Michał Górny wrote:
> I'm not convinced we ought to do this for EAPI < 6. It is a breaking
> change after all, and as such changes the behavior of EAPI < 6 ebuilds.

that is a false statement.  anything not working with bash-3.2 is already
broken according to the PMS.

> There are some ebuilds/eclasses that have bash version checks,
> and execute bash-4 code when bash-4 is available. As far as I
> understand, this will effectively prohibit bash-4 code even though
> BASH_VERSINFO will still indicate bash-4 is being used.

no, that's not what it does.  it changes behavior for code that itself
has changed behavior between versions.  it does not disable any newer
functionality.  it would have been nice to have a knob that would turn
off newer functionality as well, but upstream didn't seem keen on it.
-mike


signature.asc
Description: Digital signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Mike Frysinger]

On 11 Nov 2015 07:33, Ulrich Mueller wrote:
> > On Tue, 10 Nov 2015, Mike Frysinger wrote:
> > +   # Set the compat level in case things change with newer ones.  We must 
> > not
> > +   # export this into the env otherwise we might break  other shell 
> > scripts we
> > +   # execute (e.g. ones in /usr/bin).
> > +   BASH_COMPAT="${maj}.${min}"
> > +
> > +   # The variable above is new to bash-4.3.  For older versions, we have 
> > to use
> > +   # a compat knob.  Further, the compat knob only exists with older 
> > versions
> > +   # (e.g. bash-4.3 has compat42 but not compat43).  This means we only 
> > need to
> > +   # turn the knob with older EAPIs, and only when running newer bash 
> > versions:
> > +   # there is no bash-3.3 (it went 3.2 to 4.0), and when requiring 
> > bash-4.2, the
> > +   # var works with bash-4.3+, and you don't need to set compat to 4.2 
> > when you
> > +   # are already running 4.2.
> > +   if __eapi_bash_3_2 && [[ ${BASH_VERSINFO[0]} -gt 3 ]] ; then
> > +   shopt -s compat32
> > +   fi
> 
> Wouldn't this profit from an additional test for  understood the upstream discussion correctly, they were thinking about
> dropping the compat* options in some future version?

my take away was that they weren't going to be adding new compat levels.
i don't think they were planning on dropping existing ones.
-mike


signature.asc
Description: Digital signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Michał Górny]

On Tue, 10 Nov 2015 23:39:29 -0500
Mike Frysinger  wrote:

> To try and provide better stability across bash versions,
> set the language compat level based on the current EAPI.
> ---
>  bin/eapi.sh   |  8 
>  bin/ebuild.sh | 39 +++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/bin/eapi.sh b/bin/eapi.sh
> index 528e6f2..b236344 100644
> --- a/bin/eapi.sh
> +++ b/bin/eapi.sh
> @@ -191,3 +191,11 @@ ___eapi_enables_failglob_in_global_scope() {
>  ___eapi_enables_globstar() {
>   [[ ${1-${EAPI-0}} =~ ^(4-python|5-progress)$ ]]
>  }
> +
> +__eapi_bash_3_2() {
> + [[ ${1-${EAPI-0}} =~ 
> ^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
> +}
> +
> +__eapi_bash_4_2() {
> + [[ ${1-${EAPI-0}} =~ ^(6)$ ]]
> +}
> diff --git a/bin/ebuild.sh b/bin/ebuild.sh
> index 75a9d24..2d09fb8 100755
> --- a/bin/ebuild.sh
> +++ b/bin/ebuild.sh
> @@ -6,8 +6,47 @@
>  # Make sure it's before everything so we don't mess aliases that follow.
>  unalias -a
>  
> +# Make sure this isn't exported to scripts we execute.
> +unset BASH_COMPAT
> +
>  source "${PORTAGE_BIN_PATH}/isolated-functions.sh" || exit 1
>  
> +# Set up the bash version compatibility level.
> +__check_bash_version() {
> + # Figure out which min version of bash we require.
> + local maj min
> + if __eapi_bash_3_2 ; then
> + maj=3 min=2
> + elif __eapi_bash_4_2 ; then
> + maj=4 min=2
> + else
> + return
> + fi
> +
> + # Make sure the active bash is sane.
> + if [[ ${BASH_VERSINFO[0]} -lt ${maj} ]] ||
> +[[ ${BASH_VERSINFO[0]} -eq ${maj} && ${BASH_VERSINFO[1]} -lt ${min} 
> ]] ; then
> + die ">=bash-${maj}.${min} is required"
> + fi
> +
> + # Set the compat level in case things change with newer ones.  We must 
> not
> + # export this into the env otherwise we might break  other shell 
> scripts we
> + # execute (e.g. ones in /usr/bin).
> + BASH_COMPAT="${maj}.${min}"
> +
> + # The variable above is new to bash-4.3.  For older versions, we have 
> to use
> + # a compat knob.  Further, the compat knob only exists with older 
> versions
> + # (e.g. bash-4.3 has compat42 but not compat43).  This means we only 
> need to
> + # turn the knob with older EAPIs, and only when running newer bash 
> versions:
> + # there is no bash-3.3 (it went 3.2 to 4.0), and when requiring 
> bash-4.2, the
> + # var works with bash-4.3+, and you don't need to set compat to 4.2 
> when you
> + # are already running 4.2.
> + if __eapi_bash_3_2 && [[ ${BASH_VERSINFO[0]} -gt 3 ]] ; then
> + shopt -s compat32
> + fi
> +}
> +__check_bash_version
> +
>  if [[ $EBUILD_PHASE != depend ]] ; then
>   source "${PORTAGE_BIN_PATH}/phase-functions.sh" || die
>   source "${PORTAGE_BIN_PATH}/save-ebuild-env.sh" || die

I'm not convinced we ought to do this for EAPI < 6. It is a breaking
change after all, and as such changes the behavior of EAPI < 6 ebuilds.

There are some ebuilds/eclasses that have bash version checks,
and execute bash-4 code when bash-4 is available. As far as I
understand, this will effectively prohibit bash-4 code even though
BASH_VERSINFO will still indicate bash-4 is being used.

-- 
Best regards,
Michał Górny



pgpsuVssU_YOb.pgp
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Ulrich Mueller]

> On Wed, 11 Nov 2015, Michał Górny wrote:

> I'm not convinced we ought to do this for EAPI < 6. It is a breaking
> change after all, and as such changes the behavior of EAPI < 6
> ebuilds.

Which according to PMS should assume bash version 3.2. Also, AFAICS
the only feature where the compat settings could have any impact is
this setting from compat42:

   If set, bash does not process the replacement string in the pattern
   substitution word expansion using quote removal.

All other compat changes affect either only POSIX mode, or interactive
mode, or string comparison (where the compat32 setting disables locale
specific behaviour and uses strcmp instead, so effectively the compat
setting should be saner).

> There are some ebuilds/eclasses that have bash version checks,
> and execute bash-4 code when bash-4 is available. As far as I
> understand, this will effectively prohibit bash-4 code even though
> BASH_VERSINFO will still indicate bash-4 is being used. 

The compat settings don't disable any new features. They only restore
previous behaviour where there was an incompatible change.

Ulrich


pgpUvQguzzIMa.pgp
Description: PGP signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Zac Medico]

On 11/11/2015 01:11 PM, Mike Frysinger wrote:
> On 11 Nov 2015 13:04, Zac Medico wrote:
>> On 11/11/2015 12:55 PM, Mike Frysinger wrote:
>>> On 11 Nov 2015 11:42, Zac Medico wrote:
 On 11/10/2015 08:39 PM, Mike Frysinger wrote:
> +# Set up the bash version compatibility level.
> +__check_bash_version() {

 Please unset all new internal function inside bin/save-ebuild-env.sh.
 Note that it already uses this line to unset functions beginning with
 ___eapi:

unset -f $(compgen -A function ___eapi_)
>>>
>>> why don't we create a new namespace for portage funcs ?  something like 
>>> __e* ?
>>
>> That works for me. According to PMS, we're free to do anything we want
>> as long as it begins with at least 2 underscores.
> 
> interesting.  why don't we just unmap all things that begin with 2 underscores
> then ?  or maybe 3 ?
> -mike
> 

Well, that's sort of a "greedy" approach when you consider that you
might wipe out a function defined in an eclass or ebuild. Check this to
see what might be filtered:

  bzgrep ^__ /var/db/pkg/*/*/environment.bz2

A nice compromise is to choose a namespace like __portage or something
like that.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Zac Medico]

On 11/11/2015 10:33 PM, Zac Medico wrote:
> On 11/11/2015 01:11 PM, Mike Frysinger wrote:
>> On 11 Nov 2015 13:04, Zac Medico wrote:
>>> On 11/11/2015 12:55 PM, Mike Frysinger wrote:
 On 11 Nov 2015 11:42, Zac Medico wrote:
> On 11/10/2015 08:39 PM, Mike Frysinger wrote:
>> +# Set up the bash version compatibility level.
>> +__check_bash_version() {
>
> Please unset all new internal function inside bin/save-ebuild-env.sh.
> Note that it already uses this line to unset functions beginning with
> ___eapi:
>
>unset -f $(compgen -A function ___eapi_)

 why don't we create a new namespace for portage funcs ?  something like 
 __e* ?
>>>
>>> That works for me. According to PMS, we're free to do anything we want
>>> as long as it begins with at least 2 underscores.
>>
>> interesting.  why don't we just unmap all things that begin with 2 
>> underscores
>> then ?  or maybe 3 ?
>> -mike
>>
> 
> Well, that's sort of a "greedy" approach when you consider that you
> might wipe out a function defined in an eclass or ebuild. Check this to
> see what might be filtered:
> 
>   bzgrep ^__ /var/db/pkg/*/*/environment.bz2
> 
> A nice compromise is to choose a namespace like __portage or something
> like that.
> 

Also note that some internals have been intentionally preserved in
environment.bz2. For example, __eapi6_src_install exposes the default
src_install implementation, which someone might examine for debugging
purposes.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Zac Medico]

On 11/10/2015 08:39 PM, Mike Frysinger wrote:
> To try and provide better stability across bash versions,
> set the language compat level based on the current EAPI.
> ---
>  bin/eapi.sh   |  8 
>  bin/ebuild.sh | 39 +++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/bin/eapi.sh b/bin/eapi.sh
> index 528e6f2..b236344 100644
> --- a/bin/eapi.sh
> +++ b/bin/eapi.sh
> @@ -191,3 +191,11 @@ ___eapi_enables_failglob_in_global_scope() {
>  ___eapi_enables_globstar() {
>   [[ ${1-${EAPI-0}} =~ ^(4-python|5-progress)$ ]]
>  }
> +
> +__eapi_bash_3_2() {
> + [[ ${1-${EAPI-0}} =~ 
> ^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
> +}
> +
> +__eapi_bash_4_2() {
> + [[ ${1-${EAPI-0}} =~ ^(6)$ ]]
> +}
> diff --git a/bin/ebuild.sh b/bin/ebuild.sh
> index 75a9d24..2d09fb8 100755
> --- a/bin/ebuild.sh
> +++ b/bin/ebuild.sh
> @@ -6,8 +6,47 @@
>  # Make sure it's before everything so we don't mess aliases that follow.
>  unalias -a
>  
> +# Make sure this isn't exported to scripts we execute.
> +unset BASH_COMPAT
> +
>  source "${PORTAGE_BIN_PATH}/isolated-functions.sh" || exit 1
>  
> +# Set up the bash version compatibility level.
> +__check_bash_version() {

Please unset all new internal function inside bin/save-ebuild-env.sh.
Note that it already uses this line to unset functions beginning with
___eapi:

   unset -f $(compgen -A function ___eapi_)

However, your __eapi functions will not be matched because they only
begin with 2 underscores.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Mike Frysinger]

On 11 Nov 2015 13:04, Zac Medico wrote:
> On 11/11/2015 12:55 PM, Mike Frysinger wrote:
> > On 11 Nov 2015 11:42, Zac Medico wrote:
> >> On 11/10/2015 08:39 PM, Mike Frysinger wrote:
> >>> +# Set up the bash version compatibility level.
> >>> +__check_bash_version() {
> >>
> >> Please unset all new internal function inside bin/save-ebuild-env.sh.
> >> Note that it already uses this line to unset functions beginning with
> >> ___eapi:
> >>
> >>unset -f $(compgen -A function ___eapi_)
> > 
> > why don't we create a new namespace for portage funcs ?  something like 
> > __e* ?
> 
> That works for me. According to PMS, we're free to do anything we want
> as long as it begins with at least 2 underscores.

interesting.  why don't we just unmap all things that begin with 2 underscores
then ?  or maybe 3 ?
-mike


signature.asc
Description: Digital signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Mike Frysinger]

On 11 Nov 2015 11:42, Zac Medico wrote:
> On 11/10/2015 08:39 PM, Mike Frysinger wrote:
> > +# Set up the bash version compatibility level.
> > +__check_bash_version() {
> 
> Please unset all new internal function inside bin/save-ebuild-env.sh.
> Note that it already uses this line to unset functions beginning with
> ___eapi:
> 
>unset -f $(compgen -A function ___eapi_)

why don't we create a new namespace for portage funcs ?  something like __e* ?

> However, your __eapi functions will not be matched because they only
> begin with 2 underscores.

that wasn't intentional.  i'll change it to 3.
-mike


signature.asc
Description: Digital signature

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Zac Medico]

On 11/11/2015 12:55 PM, Mike Frysinger wrote:
> On 11 Nov 2015 11:42, Zac Medico wrote:
>> On 11/10/2015 08:39 PM, Mike Frysinger wrote:
>>> +# Set up the bash version compatibility level.
>>> +__check_bash_version() {
>>
>> Please unset all new internal function inside bin/save-ebuild-env.sh.
>> Note that it already uses this line to unset functions beginning with
>> ___eapi:
>>
>>unset -f $(compgen -A function ___eapi_)
> 
> why don't we create a new namespace for portage funcs ?  something like __e* ?

That works for me. According to PMS, we're free to do anything we want
as long as it begins with at least 2 underscores.
-- 
Thanks,
Zac

Re: [gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Ulrich Mueller]

> On Tue, 10 Nov 2015, Mike Frysinger wrote:

> + # Set the compat level in case things change with newer ones.  We must 
> not
> + # export this into the env otherwise we might break  other shell 
> scripts we
> + # execute (e.g. ones in /usr/bin).
> + BASH_COMPAT="${maj}.${min}"
> +
> + # The variable above is new to bash-4.3.  For older versions, we have 
> to use
> + # a compat knob.  Further, the compat knob only exists with older 
> versions
> + # (e.g. bash-4.3 has compat42 but not compat43).  This means we only 
> need to
> + # turn the knob with older EAPIs, and only when running newer bash 
> versions:
> + # there is no bash-3.3 (it went 3.2 to 4.0), and when requiring 
> bash-4.2, the
> + # var works with bash-4.3+, and you don't need to set compat to 4.2 
> when you
> + # are already running 4.2.
> + if __eapi_bash_3_2 && [[ ${BASH_VERSINFO[0]} -gt 3 ]] ; then
> + shopt -s compat32
> + fi

Wouldn't this profit from an additional test for 

[gentoo-portage-dev] [PATCH] ebuild: set up bash compat levels

[Mike Frysinger]

To try and provide better stability across bash versions,
set the language compat level based on the current EAPI.
---
 bin/eapi.sh   |  8 
 bin/ebuild.sh | 39 +++
 2 files changed, 47 insertions(+)

diff --git a/bin/eapi.sh b/bin/eapi.sh
index 528e6f2..b236344 100644
--- a/bin/eapi.sh
+++ b/bin/eapi.sh
@@ -191,3 +191,11 @@ ___eapi_enables_failglob_in_global_scope() {
 ___eapi_enables_globstar() {
[[ ${1-${EAPI-0}} =~ ^(4-python|5-progress)$ ]]
 }
+
+__eapi_bash_3_2() {
+   [[ ${1-${EAPI-0}} =~ 
^(0|1|2|3|4|4-python|4-slot-abi|5|5-hdepend|5-progress)$ ]]
+}
+
+__eapi_bash_4_2() {
+   [[ ${1-${EAPI-0}} =~ ^(6)$ ]]
+}
diff --git a/bin/ebuild.sh b/bin/ebuild.sh
index 75a9d24..2d09fb8 100755
--- a/bin/ebuild.sh
+++ b/bin/ebuild.sh
@@ -6,8 +6,47 @@
 # Make sure it's before everything so we don't mess aliases that follow.
 unalias -a
 
+# Make sure this isn't exported to scripts we execute.
+unset BASH_COMPAT
+
 source "${PORTAGE_BIN_PATH}/isolated-functions.sh" || exit 1
 
+# Set up the bash version compatibility level.
+__check_bash_version() {
+   # Figure out which min version of bash we require.
+   local maj min
+   if __eapi_bash_3_2 ; then
+   maj=3 min=2
+   elif __eapi_bash_4_2 ; then
+   maj=4 min=2
+   else
+   return
+   fi
+
+   # Make sure the active bash is sane.
+   if [[ ${BASH_VERSINFO[0]} -lt ${maj} ]] ||
+  [[ ${BASH_VERSINFO[0]} -eq ${maj} && ${BASH_VERSINFO[1]} -lt ${min} 
]] ; then
+   die ">=bash-${maj}.${min} is required"
+   fi
+
+   # Set the compat level in case things change with newer ones.  We must 
not
+   # export this into the env otherwise we might break  other shell 
scripts we
+   # execute (e.g. ones in /usr/bin).
+   BASH_COMPAT="${maj}.${min}"
+
+   # The variable above is new to bash-4.3.  For older versions, we have 
to use
+   # a compat knob.  Further, the compat knob only exists with older 
versions
+   # (e.g. bash-4.3 has compat42 but not compat43).  This means we only 
need to
+   # turn the knob with older EAPIs, and only when running newer bash 
versions:
+   # there is no bash-3.3 (it went 3.2 to 4.0), and when requiring 
bash-4.2, the
+   # var works with bash-4.3+, and you don't need to set compat to 4.2 
when you
+   # are already running 4.2.
+   if __eapi_bash_3_2 && [[ ${BASH_VERSINFO[0]} -gt 3 ]] ; then
+   shopt -s compat32
+   fi
+}
+__check_bash_version
+
 if [[ $EBUILD_PHASE != depend ]] ; then
source "${PORTAGE_BIN_PATH}/phase-functions.sh" || die
source "${PORTAGE_BIN_PATH}/save-ebuild-env.sh" || die
-- 
2.6.2