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

[Alexandre Janniaux]

Hi,

I think you missed the whole point of my argument. I might not have been
clear enough, so let me rephrase it with yours:

The point is not that __all__ special variables, which by your definition
is equal to the set of variables used by the underlying tools, need to be
escaped.

The focus is more on variables that are __already__ taken into account by
the underlying buildsystem, which are the HOSTVARS, as stated by the commit
message of Marvin.

> they are anyway included in HOSTVARS already for other build systems.

In the example variables that you gave, none of them is included in this.

About:

> The only case where you really need unexport is if you need a variable to be
> undefined. I can see the point for some job management variables. I have yet to
> see an explanation why any of the proposed variable here needs to be
> *specifically* undefined.

It is because, as stated by Marvin in the commit message:

> Meson always interprets these env variables for the build compiler,

which conflicts the semantic given by the contrib makefile for this variable.

About:

> You also get different build results with different locale settings in the
> environment. I've seen real life build system that even conditionally crashed
> depending on the locale.
>
> Again, if the argument is that potentially variables that affect the build are
> harmful, then all variables are harmful.

You're completely right on the fact that some variables can be harmful and
affect the build result. But that is not the point of this patch at all.

This patch addresses the semantic issue of those variables which should be
exported with control, the contrib system being a wrapper layer over all
each of other contribs entry's build system.

Maybe some work is needed to fix those potentially harmful variable, but
I fail to see how that's not out of the scope of this patch.

I hope this is clear enough to find a common ground on this.

Regards,
--
Alexandre Janniaux
Videolabs

On Thu, Oct 03, 2019 at 06:35:07PM +0300, Rémi Denis-Courmont wrote:
> Le torstaina 3. lokakuuta 2019, 16.00.00 EEST Alexandre Janniaux a écrit :
> > 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.
>
> By that logic, all variables that have an effect on the build system are
> special, and all should be unexported. There are plenty of other variables
> that have an effect, and yet this patch does _not_ unexport then: PATH,
> LD_PRELOAD, LD_LIBRARY_PATH, LANG, LC_*, IFS...
>
> And those with no effects could evidently be unexported too. So really, there
> is no special case here - with your logic, all variables should be unexported.
> That argument seems absurd.
>
> The only case where you really need unexport is if you need a variable to be
> undefined. I can see the point for some job management variables. I have yet to
> see an explanation why any of the proposed variable here needs to be
> *specifically* undefined.
>
> > 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.
>
> There are no needs to export variables to autotools. If/when --build & --host
> are insufficient, you can pass values as command line arguments, as opposed to
> environment variables. Ditto make(files).
>
> > 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.
>
> You also get different build results with different locale settings in the
> environment. I've seen real life build system that even conditionally crashed
> depending on the locale.
>
> Again, if the argument is that potentially variables that affect the build are
> harmful, then all variables are harmful.
>
>
> For this reason, I am against this patch until somebody proves that it is
> necessary and correct.
>
> --
> Rémi Denis-Courmont
> http://www.remlab.net/
>
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel