[PATCH] D53158: [buildbot, windows] Update the way the visual studio environment is set

[Galina via Phabricator via llvm-commits]

gkistanova reopened this revision.
gkistanova added a comment.
This revision is now accepted and ready to land.

The latest changes look too invasive for the purpose. And it seems confusing to have two parameters, both supposedly controlling the Visual Studio selection, especially when one could be silently ignored in some cases.

Could you use the first argument as a selector instead please? 
Something like `command=builders_util.getVisualStudioEnvironment(vs="autodetect", target_arch)` would be good.

In this case no need to change any build factory and everything would keep working exactly as it was before your change.



================
Comment at: zorg/buildbot/builders/Util.py:27
+        vs_path = subprocess.check_output(cmd).strip()
+        vcvars_command = pjoin(vs_path, 'VC', 'Auxiliary', 'Build', 'vcvarsall.bat')
+    else:
----------------
Please do not use OS specific path handling, as this gets evaluated on the build master. OS build master is running under, and the OS where the build happens are usually different.


Repository:
  rL LLVM

https://reviews.llvm.org/D53158