Re: [PATCH] emake: explicitly set SHELL

--J/nvFe7cXXR+8zFV
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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 <[email protected]> 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=3D(
> > >> -	${MAKE:-make} ${MAKEOPTS} "$@" ${EXTRA_EMAKE}
> > >> +	${MAKE:-make} SHELL=3D"${BASH:-/bin/bash}" ${MAKEOPTS} "$@" ${EXTR=
A_EMAKE}
> > >> )
> > >>
> > >> if [[ ${PORTAGE_QUIET} !=3D 1 ]] ; then
> > >>
> > >=20
> > > I don't think I agree with this as it is. Why not just ${EPREFIX}/bin=
/sh to avoid using
> > > an ancient host sh?
> >=20
> > I was about to write the same (also using EPREFIX, but EBROOT seems wha=
t=20
> > you want, as you figured).
> >=20
> > But then I wondered if "make SHELL=3D$BROOT/bin/sh" wouldn't override=
=20
> > explicitly set SHELL values in Makefiles. Assume a package has
> >=20
> > SHELL =3D /bin/zsh
> >=20
> > in one of its Makefiles. Then emake would reset this to 'sh'. Which=20
> > appears like it could cause build issues.
> >=20
> > If this is the case, then I am not sure what we can do about it. It=20
> > appears fragile, if not impossible, to ask 'make' which value for SHELL=
=20
> > it would assume, so that emake could adjust the path. Another option=20
> > could be that affected packages define a variable in their ebuild, e.g.=
=20
> > EMAKE_SHELL=3D"zsh", which emake could extend with BROOT before passing=
=20
> > the resulting value as SHELL to make.
>=20
> 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.
>=20
> On an alternative note, there is CONFIG_SHELL (used for setting which she=
ll
> 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.
>=20
> (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=3D/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=3D/bin/zsh makes little sense
to me, SHELL=3D/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=3D$(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

--=20
Fabian Groffen
Gentoo on a different level

--J/nvFe7cXXR+8zFV
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEELUvHd/Gtp7LaU1vuzpXahU5EQpMFAmLiw3gACgkQzpXahU5E
QpOG7Af/RtDH2bZhWjOxvPkJ0y+Iaz1mAatK9uxVCDdAOr5yyBH2GLUQyr87QAzY
Y0IYmFMKLGc1O/1fe7QihoMMzCIajS+2iYDZtkA/I++Yld3hfFkvrWTlCafh49EB
AZigpuCwB2IbHW0drJyTugEzGtzU/sJXHHLjaQ0TOnGUyHJeYckmbtcBRB9UDPRt
jQlsVP2hh613UbpAHv/YPBACIiMFYSYnmdyBo6n+s0WAMUUqXJFDy7yktW/llRS0
WDmKM9fvXvWesqfPg4biVQdIy36GzCIGSlX9hdiIPHw565DcWKyL52OlmKnbw1a/
GQAbcMx29rqx6TC+TcW5paveNBIW/A==
=8iNa
-----END PGP SIGNATURE-----

--J/nvFe7cXXR+8zFV--