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

[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 <grobian@g.o> 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