Re: [gentoo-portage-dev] [PATCH] emake: explicitly set SHELL

2022-07-28 Thread Arfrever Frehtes Taifersar Arahesis
2022-07-26 07:03 UTCに、Florian Schmaus は書いた:
> But then I wondered if "make SHELL=$BROOT/bin/sh" wouldn't override
> explicitly set SHELL values in Makefiles. Assume a package has
>
> SHELL = /bin/zsh
>
> in one of its Makefiles. Then emake would reset this to 'sh'. Which
> appears like it could cause build issues.
>
> If this is the case, then I am not sure what we can do about it. It
> appears fragile, if not impossible, to ask 'make' which value for SHELL
> it would assume, so that emake could adjust the path. Another option
> could be that affected packages define a variable in their ebuild, e.g.
> EMAKE_SHELL="zsh", which emake could extend with BROOT before passing
> the resulting value as SHELL to make.

If there was such package, it could just override SHELL on emake invocation:

src_compile() {
emake SHELL="${BROOT}/bin/zsh"
# Or:
emake SHELL="$(type -P zsh)"
}



Re: [gentoo-portage-dev] [PATCH] emake: explicitly set SHELL

2022-07-28 Thread Fabian Groffen
On 26-07-2022 09:20:58 +0200, Fabian Groffen wrote:
> On 26-07-2022 09:03:18 +0200, Florian Schmaus wrote:
> > On 26.07.22 05:00, Sam James wrote:
> > >> On 25 Jul 2022, at 16:28, Fabian Groffen  wrote:
> > >>
> > >> bin/ebuild-helpers/emake: force SHELL to be set
> > >>
> [snip]
> > >>
> > >> diff --git a/bin/ebuild-helpers/emake b/bin/ebuild-helpers/emake
> > >> index 60718a2e4..21da85845 100755
> > >> --- a/bin/ebuild-helpers/emake
> > >> +++ b/bin/ebuild-helpers/emake
> > >> @@ -12,7 +12,7 @@
> > >> source "${PORTAGE_BIN_PATH}"/isolated-functions.sh || exit 1
> > >>
> > >> cmd=(
> > >> -${MAKE:-make} ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
> > >> +${MAKE:-make} SHELL="${BASH:-/bin/bash}" ${MAKEOPTS} "$@" 
> > >> ${EXTRA_EMAKE}
> > >> )
> > >>
> > >> if [[ ${PORTAGE_QUIET} != 1 ]] ; then
> > >>
> > > 
> > > I don't think I agree with this as it is. Why not just ${EPREFIX}/bin/sh 
> > > to avoid using
> > > an ancient host sh?
> > 
> > I was about to write the same (also using EPREFIX, but EBROOT seems what 
> > you want, as you figured).
> > 
> > But then I wondered if "make SHELL=$BROOT/bin/sh" wouldn't override 
> > explicitly set SHELL values in Makefiles. Assume a package has
> > 
> > SHELL = /bin/zsh
> > 
> > in one of its Makefiles. Then emake would reset this to 'sh'. Which 
> > appears like it could cause build issues.
> > 
> > If this is the case, then I am not sure what we can do about it. It 
> > appears fragile, if not impossible, to ask 'make' which value for SHELL 
> > it would assume, so that emake could adjust the path. Another option 
> > could be that affected packages define a variable in their ebuild, e.g. 
> > EMAKE_SHELL="zsh", which emake could extend with BROOT before passing 
> > the resulting value as SHELL to make.
> 
> So, I can also envision we drop this patch, and I see if I can patch
> make(1) to use $EPREFIX/bin/sh instead of /bin/sh by default.  Not sure,
> but this would retain the behaviour Portage is doing now for non-Prefix,
> and would get the behaviour we want in Prefix.
> 
> On an alternative note, there is CONFIG_SHELL (used for setting which shell
> to use with configure), which I think in many cases bleeds through to
> make, but should there be a MAKE_SHELL perhaps as well?  Then the
> default would be pretty much ok.
> 
> (We never ran into any problems forcing SHELL to bash in Prefix, but
> perhaps that's not a representative test for the whole of Gentoo.)

I've been looking around in make, and it seems we can set a default
shell there, would be pretty simple, I guess.  It doesn't solve,
however, a bigger problem, which is what make's source also mentions,
lots of Makefiles having SHELL=/bin/sh.  In other words, setting a
default in make makes no difference in preventing it from using /bin/sh
(which is the main aim here).

Therefore, I would like to consider the possibility to override SHELL
this way via emake, as it seems like the fix we need (for Prefix at
least) afterall.

Overriding should be safe, I think.  SHELL=/bin/zsh makes little sense
to me, SHELL=/bin/csh would be more of a thing, but is there any package
out there that needs it?  And if it does, wouldn't it be acceptable to
just handle that in that package?  It would require a dep on that shell
anyway (so you could consider it a kludgy sanity check as well).

I think using bash like the original patch did isn't quite nice, it
should use sh instead.  However, it cannot hardcode EPREFIX/bin/sh
without ensuring that binary exists, else we break bootstraps.

So I was thinking: how about something like SHELL=$(type -P sh)?  To be
completely safe here, it could become an if such that if there's no sh,
it falls back to ${BASH}.

Would this approach be acceptable?  Should it be perhaps conditional
based on whether an offset prefix is active?

Thanks,
Fabian

-- 
Fabian Groffen
Gentoo on a different level


signature.asc
Description: PGP signature


Re: [gentoo-portage-dev] [PATCH] emake: explicitly set SHELL

2022-07-26 Thread Fabian Groffen
On 26-07-2022 09:03:18 +0200, Florian Schmaus wrote:
> On 26.07.22 05:00, Sam James wrote:
> >> On 25 Jul 2022, at 16:28, Fabian Groffen  wrote:
> >>
> >> bin/ebuild-helpers/emake: force SHELL to be set
> >>
[snip]
> >>
> >> diff --git a/bin/ebuild-helpers/emake b/bin/ebuild-helpers/emake
> >> index 60718a2e4..21da85845 100755
> >> --- a/bin/ebuild-helpers/emake
> >> +++ b/bin/ebuild-helpers/emake
> >> @@ -12,7 +12,7 @@
> >> source "${PORTAGE_BIN_PATH}"/isolated-functions.sh || exit 1
> >>
> >> cmd=(
> >> -  ${MAKE:-make} ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
> >> +  ${MAKE:-make} SHELL="${BASH:-/bin/bash}" ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
> >> )
> >>
> >> if [[ ${PORTAGE_QUIET} != 1 ]] ; then
> >>
> > 
> > I don't think I agree with this as it is. Why not just ${EPREFIX}/bin/sh to 
> > avoid using
> > an ancient host sh?
> 
> I was about to write the same (also using EPREFIX, but EBROOT seems what 
> you want, as you figured).
> 
> But then I wondered if "make SHELL=$BROOT/bin/sh" wouldn't override 
> explicitly set SHELL values in Makefiles. Assume a package has
> 
> SHELL = /bin/zsh
> 
> in one of its Makefiles. Then emake would reset this to 'sh'. Which 
> appears like it could cause build issues.
> 
> If this is the case, then I am not sure what we can do about it. It 
> appears fragile, if not impossible, to ask 'make' which value for SHELL 
> it would assume, so that emake could adjust the path. Another option 
> could be that affected packages define a variable in their ebuild, e.g. 
> EMAKE_SHELL="zsh", which emake could extend with BROOT before passing 
> the resulting value as SHELL to make.

So, I can also envision we drop this patch, and I see if I can patch
make(1) to use $EPREFIX/bin/sh instead of /bin/sh by default.  Not sure,
but this would retain the behaviour Portage is doing now for non-Prefix,
and would get the behaviour we want in Prefix.

On an alternative note, there is CONFIG_SHELL (used for setting which shell
to use with configure), which I think in many cases bleeds through to
make, but should there be a MAKE_SHELL perhaps as well?  Then the
default would be pretty much ok.

(We never ran into any problems forcing SHELL to bash in Prefix, but
perhaps that's not a representative test for the whole of Gentoo.)

Thanks,
Fabian

-- 
Fabian Groffen
Gentoo on a different level


signature.asc
Description: PGP signature


Re: [gentoo-portage-dev] [PATCH] emake: explicitly set SHELL

2022-07-26 Thread Florian Schmaus

On 26.07.22 05:00, Sam James wrote:




On 25 Jul 2022, at 16:28, Fabian Groffen  wrote:

bin/ebuild-helpers/emake: force SHELL to be set

On Prefix systems /bin/sh can be anything, including very ancient.  So
ensure we're running with bash, since that's what Gentoo Linux is
expecting /bin/sh to be (by default, at least).

Provide a fallback for the (near impossible) case that we use a bash
that doesn't set BASH, or when we don't use bash at all.  This is not
expected, though, as we explicitly require bash throughout all Portage,
so we don't really care about using a non-Prefixed one, for this really
shouldn't happen.

Signed-off-by: Fabian Groffen 

diff --git a/bin/ebuild-helpers/emake b/bin/ebuild-helpers/emake
index 60718a2e4..21da85845 100755
--- a/bin/ebuild-helpers/emake
+++ b/bin/ebuild-helpers/emake
@@ -12,7 +12,7 @@
source "${PORTAGE_BIN_PATH}"/isolated-functions.sh || exit 1

cmd=(
-   ${MAKE:-make} ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
+   ${MAKE:-make} SHELL="${BASH:-/bin/bash}" ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
)

if [[ ${PORTAGE_QUIET} != 1 ]] ; then



I don't think I agree with this as it is. Why not just ${EPREFIX}/bin/sh to 
avoid using
an ancient host sh?


I was about to write the same (also using EPREFIX, but EBROOT seems what 
you want, as you figured).


But then I wondered if "make SHELL=$BROOT/bin/sh" wouldn't override 
explicitly set SHELL values in Makefiles. Assume a package has


SHELL = /bin/zsh

in one of its Makefiles. Then emake would reset this to 'sh'. Which 
appears like it could cause build issues.


If this is the case, then I am not sure what we can do about it. It 
appears fragile, if not impossible, to ask 'make' which value for SHELL 
it would assume, so that emake could adjust the path. Another option 
could be that affected packages define a variable in their ebuild, e.g. 
EMAKE_SHELL="zsh", which emake could extend with BROOT before passing 
the resulting value as SHELL to make.


- Flow



Re: [gentoo-portage-dev] [PATCH] emake: explicitly set SHELL

2022-07-25 Thread Sam James


> On 26 Jul 2022, at 04:00, Sam James  wrote:
> 
> 
> 
>> On 25 Jul 2022, at 16:28, Fabian Groffen  wrote:
>> 
>> bin/ebuild-helpers/emake: force SHELL to be set
>> 
>> On Prefix systems /bin/sh can be anything, including very ancient.  So
>> ensure we're running with bash, since that's what Gentoo Linux is
>> expecting /bin/sh to be (by default, at least).
>> 
>> Provide a fallback for the (near impossible) case that we use a bash
>> that doesn't set BASH, or when we don't use bash at all.  This is not
>> expected, though, as we explicitly require bash throughout all Portage,
>> so we don't really care about using a non-Prefixed one, for this really
>> shouldn't happen.
>> 
>> Signed-off-by: Fabian Groffen 
>> 
>> diff --git a/bin/ebuild-helpers/emake b/bin/ebuild-helpers/emake
>> index 60718a2e4..21da85845 100755
>> --- a/bin/ebuild-helpers/emake
>> +++ b/bin/ebuild-helpers/emake
>> @@ -12,7 +12,7 @@
>> source "${PORTAGE_BIN_PATH}"/isolated-functions.sh || exit 1
>> 
>> cmd=(
>> -${MAKE:-make} ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
>> +${MAKE:-make} SHELL="${BASH:-/bin/bash}" ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
>> )
>> 
>> if [[ ${PORTAGE_QUIET} != 1 ]] ; then
>> 
> 
> I don't think I agree with this as it is. Why not just ${EPREFIX}/bin/sh to 
> avoid using
> an ancient host sh?
> 

Sorry, ${BROOT}, I guess.

> I might use Bash for Portage but my /bin/sh is dash usually.
> 
>> --
>> Fabian Groffen
>> Gentoo on a different level



signature.asc
Description: Message signed with OpenPGP


Re: [gentoo-portage-dev] [PATCH] emake: explicitly set SHELL

2022-07-25 Thread Sam James


> On 25 Jul 2022, at 16:28, Fabian Groffen  wrote:
> 
> bin/ebuild-helpers/emake: force SHELL to be set
> 
> On Prefix systems /bin/sh can be anything, including very ancient.  So
> ensure we're running with bash, since that's what Gentoo Linux is
> expecting /bin/sh to be (by default, at least).
> 
> Provide a fallback for the (near impossible) case that we use a bash
> that doesn't set BASH, or when we don't use bash at all.  This is not
> expected, though, as we explicitly require bash throughout all Portage,
> so we don't really care about using a non-Prefixed one, for this really
> shouldn't happen.
> 
> Signed-off-by: Fabian Groffen 
> 
> diff --git a/bin/ebuild-helpers/emake b/bin/ebuild-helpers/emake
> index 60718a2e4..21da85845 100755
> --- a/bin/ebuild-helpers/emake
> +++ b/bin/ebuild-helpers/emake
> @@ -12,7 +12,7 @@
> source "${PORTAGE_BIN_PATH}"/isolated-functions.sh || exit 1
> 
> cmd=(
> - ${MAKE:-make} ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
> + ${MAKE:-make} SHELL="${BASH:-/bin/bash}" ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
> )
> 
> if [[ ${PORTAGE_QUIET} != 1 ]] ; then
> 

I don't think I agree with this as it is. Why not just ${EPREFIX}/bin/sh to 
avoid using
an ancient host sh?

I might use Bash for Portage but my /bin/sh is dash usually.

> --
> Fabian Groffen
> Gentoo on a different level



signature.asc
Description: Message signed with OpenPGP


Re: [gentoo-portage-dev] [PATCH] emake: explicitly set SHELL

2022-07-25 Thread Mike Gilbert
On Mon, Jul 25, 2022 at 11:28 AM Fabian Groffen  wrote:
>
> bin/ebuild-helpers/emake: force SHELL to be set
>
> On Prefix systems /bin/sh can be anything, including very ancient.  So
> ensure we're running with bash, since that's what Gentoo Linux is
> expecting /bin/sh to be (by default, at least).
>
> Provide a fallback for the (near impossible) case that we use a bash
> that doesn't set BASH, or when we don't use bash at all.  This is not
> expected, though, as we explicitly require bash throughout all Portage,
> so we don't really care about using a non-Prefixed one, for this really
> shouldn't happen.

I'm a little on the fence about this: in theory, Makefiles should use
POSIX-compatible shell commands unless the author explicitly chooses
to use bash.

I guess we can get away with this since ebuilds always require bash anyway.



[gentoo-portage-dev] [PATCH] emake: explicitly set SHELL

2022-07-25 Thread Fabian Groffen
bin/ebuild-helpers/emake: force SHELL to be set

On Prefix systems /bin/sh can be anything, including very ancient.  So
ensure we're running with bash, since that's what Gentoo Linux is
expecting /bin/sh to be (by default, at least).

Provide a fallback for the (near impossible) case that we use a bash
that doesn't set BASH, or when we don't use bash at all.  This is not
expected, though, as we explicitly require bash throughout all Portage,
so we don't really care about using a non-Prefixed one, for this really
shouldn't happen.

Signed-off-by: Fabian Groffen 

diff --git a/bin/ebuild-helpers/emake b/bin/ebuild-helpers/emake
index 60718a2e4..21da85845 100755
--- a/bin/ebuild-helpers/emake
+++ b/bin/ebuild-helpers/emake
@@ -12,7 +12,7 @@
 source "${PORTAGE_BIN_PATH}"/isolated-functions.sh || exit 1
 
 cmd=(
-   ${MAKE:-make} ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
+   ${MAKE:-make} SHELL="${BASH:-/bin/bash}" ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
 )
 
 if [[ ${PORTAGE_QUIET} != 1 ]] ; then

-- 
Fabian Groffen
Gentoo on a different level


signature.asc
Description: PGP signature