[PATCH] D54963: [lit] Pass more environment variables through on Windows

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 30 07:59:16 PST 2018


Well, we were already passing INCLUDE through so they could already include
system headers . I could add a parameter to the function like
`allow_system_library_use` but I’m not sure if the value of that is high
enough to warrant it. I don’t have a strong opinion though
On Fri, Nov 30, 2018 at 7:28 AM Nico Weber via Phabricator <
reviews at reviews.llvm.org> wrote:

> thakis added a comment.
>
> In D54963#1310190 <https://reviews.llvm.org/D54963#1310190>, @zturner
> wrote:
>
> > I don't think that ever actually happened, or at least I never observed
> it.  The real behavior change is that we're now passing through "LIB",
> "VCINSTALLDIR", "VCToolsInstallDir", "VSINSTALLDIR", "WindowsSdkDir", and
> "WindowsSdkVersion", whereas before we were not propagating them into the
> child environment at all.  clang-cl reads these, so it could have an
> observable impact on clang-cl (but any observable impact should be
> beneficial, since before it would actually just fail if a test tried to
> include a system header or link against a system library)
>
>
> At least for clang tests, tests reading system libraries are usually a bug
> -- tests are supposed to be hermetic, cross-platform, etc. It sounds like
> now it's easier to accidentally use a system header?
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D54963/new/
>
> https://reviews.llvm.org/D54963
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181130/b77507a4/attachment.html>


More information about the llvm-commits mailing list