[PATCH] Resubmit "Improve Windows toolchain support for non-standard environments."

Zachary Turner zturner at google.com
Tue Oct 21 14:12:05 PDT 2014


>>! In D5892#5, @hans wrote:
> It seems the patch is based on top off the patch which got reverted. It would be better if it was relative to the current trunk.
> 
> I tried this on the Chromium build and it worked.

That's strange, it should be based off of current trunk.

================
Comment at: lib/Driver/WindowsToolChain.cpp:226
@@ +225,3 @@
+    path = vcinstalldir;
+    path = path.substr(0, path.find("\\VC"));
+    return true;
----------------
rnk wrote:
> This doesn't seem right, we need the path to VS/VC/bin or VS/VC/bin/amd64. I don't think we need to have this first case.
I kind of like checking VCINSTALLDIR first, because the only way that's set is if someone runs vcvars, which *also* sets PATH.  The only way checking VCINSTALLDIR first causes a problem is if someone runs vcvars, then further modifies the PATH to stick something else first.  If someone does that I feel like we could say "sorry, your'e on your own now".  PATH is kind of the garbage disposal of the environment, and everyone just dumps what they need into PATH.  So it's more likely to get polluted than VCINSTALLDIR, which is much more specific.

http://reviews.llvm.org/D5892






More information about the cfe-commits mailing list