[vlc-devel] [PATCH] contrib: unexport env variables when cross-compiling

[Alexandre Janniaux]

Hi,

IMHO, there __is__ a special case around some variables anyway, because
you can find them in the toolchain file as well, and should only use
them from there as they are expected to be correctly set and parsed, even
from user input.

For instance, what we do with HOSTVARS is exactly because there is no
toolchain file for autotools (or others, or it doesn't behave the same) and
that we want to export precise variations of these inputs.

Other variables can be set, we don't care because we don't have to know,
but these are special variables that are already taken into account by
the build system. Taking them into account twice, potentially with different
values can only be harmful.

In the general way, when mixing multiple tools, and especially buildsystems,
I tend to prefer a more explicit way (HOSTVARS) than implicit one (exports or
implicit defaultly-exported variables). Doing this in the general way might
break some contribs, but it would mean that they were highly subject of
breakage for different reasons too. Here it's even specialized to different
cases from the former.

For those reason, I'm in favor of this patch until technical reasons prove
that it's wrong.

Regards,
--
Alexandre Janniaux
Videolabs

On Thu, Oct 03, 2019 at 02:52:08PM +0300, Rémi Denis-Courmont wrote:
> It's a gross hack because it's making a special case of some variables.
>
> Your argument is that the variables can be set... sure that's precise why they don't need to be unexported.
>
> Le 2 octobre 2019 11:55:56 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
> >I don't understand why this is a hack.
> >Variables are already set in HOSTVARS.
> >
> >For me, it's always better to keep the minimal number of exported
> >variables. It's way easier to debug in that case.
> >
> >On Tue, Oct 1, 2019, at 13:36, Steve Lhomme wrote:
> >> On 2019-09-30 17:50, Marvin Scholz wrote:
> >> > I am proposing this patch again, I know that I did a while ago and
> >> > some people had some concerns about this.
> >> > But without this patch cross compilation of contribs effectively
> >> > fails unless the build system can run the cross compiled binaries,
> >> > as the cross compilation flags are incorrectly used for the build
> >> > compiler too. Even though we do not build native tools in the
> >> > affected contribs, meson does check if the compiler works and
> >> > of course considers this not to be the case when the build machine
> >> > compiler can not create binaries that run on the build machine.
> >> >
> >> > Without this patch effectively cross compilation of contribs for
> >iOS
> >> > and probably Android is broken.
> >>
> >> It also fixes my build with clang where I have "-gcodeview" in the
> >> CFLAGS but the native gcc doesn't understand it.
> >>
> >> > Regarding the concern that it might break some contribs that do not
> >> > correctly use the HOSTVARS to prepends the env variables to the
> >> > command, I did not found any that would be affected by this.
> >> >
> >> > I compiled the contribs with this patch applied, for iOS and
> >> > Windows without errors. This patch does not affect non-cross
> >> > compilation at all.
> >> >
> >> > On 30 Sep 2019, at 17:43, Marvin Scholz wrote:
> >> >
> >> >> Meson always interprets these env variables for the build
> >compiler,
> >> >> even when cross-compiling, therefore we need to unset them,
> >> >> they are anyway included in HOSTVARS already for other build
> >systems.
> >> >> ---
> >> >>  contrib/src/main.mak | 13 +++++++++++++
> >> >>  1 file changed, 13 insertions(+)
> >> >>
> >> >> diff --git a/contrib/src/main.mak b/contrib/src/main.mak
> >> >> index 7eef7b85b6..b07b9a0855 100644
> >> >> --- a/contrib/src/main.mak
> >> >> +++ b/contrib/src/main.mak
> >> >> @@ -314,6 +314,19 @@ MESON_HOST_FLAGS := \
> >> >>      LDFLAGS="$(LDFLAGS)"
> >> >>  ifdef HAVE_CROSS_COMPILE
> >> >>  HOSTVARS_MESON := PATH="$(PREFIX)/bin:$(PATH)"
> >> >> +# For cross-compilation with meson, we need to unset compiler and
> >flags
> >> >> +# env variables as meson will always use them for the BUILD
> >machine
> >> >> compiler!
> >> >> +unexport CC
> >> >> +unexport CXX
> >> >> +unexport LD
> >> >> +unexport AR
> >> >> +unexport CCAS
> >> >> +unexport RANLIB
> >> >> +unexport STRIP
> >> >> +unexport CPPFLAGS
> >> >> +unexport CFLAGS
> >> >> +unexport CXXFLAGS
> >> >> +unexport LDFLAGS
> >> >>  else
> >> >>  HOSTVARS_MESON := $(HOSTTOOLS) $(MESON_HOST_FLAGS)
> >> >>  endif
> >> >> --
> >> >> 2.20.1 (Apple Git-117)
> >> > _______________________________________________
> >> > vlc-devel mailing list
> >> > To unsubscribe or modify your subscription options:
> >> > https://mailman.videolan.org/listinfo/vlc-devel
> >> _______________________________________________
> >> vlc-devel mailing list
> >> To unsubscribe or modify your subscription options:
> >> https://mailman.videolan.org/listinfo/vlc-devel
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
>
> --
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel