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

[stella.stamenova]

Summary

With vs2017 there were several changes to the installation paths for VS including the vcvarsall.bat script used for setting the environment. To correctly get the paths for the newer versions of VS, we should use vswhere instead. This is similar to how the annotated builder sets the environment as well

[gkistanova]

Stella,
Did you try the change with VS other than 2017?

[stella.stamenova]

This defaults to the original behavior when vswhere.exe doesn't exist (so older versions of Visual Studio).

[gkistanova]

I see that in the code. The question was if you actually tested it with something older than VS 2015.
Anyway, LGTM.

[stella.stamenova]

I did test the snippet locally with 2015 and 2017. Is there a way to simulate the way the bots will execute the code as well?

It did occur to me after your question that this won't work if someone has vs2017 installed but wants to use an older version, so I need to re-think the logic a little bit. I'll send an update soon.

[stella.stamenova]

I updated the patch, so that it will not auto detect the VS installation unless explicitly requested. I also propagated the flag to all the builders that currently work with VS, so that it is available immediately if anyone chooses to create a buildbot with a newer VS installation.

Is there a way to test this (beyond running the logic locally) to make sure that all the bots will work correctly?

Thanks!

[stella.stamenova]

@gkistanova do you have a concern with the current changes or any additional testing you want me to do before I check this in?

[gkistanova]

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.

zorg/buildbot/builders/Util.py

27 ↗

(On Diff #170137)

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.

[gkistanova]

Please revert r345309 and propose a smaller change.

[stella.stamenova]

zorg/buildbot/builders/Util.py

27 ↗

(On Diff #170137)

Ok, so this is not the right change then as vswhere needs to run on the OS where the build is going to happen - otherwise, we will not correctly detect the version of visual studio. Is there a way to specify that we want the command to run on the build slave?

[gkistanova]

Hi Stella,

Please see my notes inline.

Unless you want to do this by yourself, I can prepare this change. I think I have developed a good understanding of what you are after.

zorg/buildbot/builders/Util.py

26 ↗

(On Diff #170137)

This thing also would run on the build master, which does not look like what you want.

27 ↗

(On Diff #170137)

You do build a command that would run on the build machine. But the Python script you write to build that command would be running on the build master.

In other words, whatever you put in vcvars_command would go to a remote host to execute.

[stella.stamenova]

In D53158#1277851, @gkistanova wrote:

Hi Stella,

Please see my notes inline.

Unless you want to do this by yourself, I can prepare this change. I think I have developed a good understanding of what you are after.

Thanks, Galina. If you think it will be easier for you to make the change, that sounds great.

zorg/buildbot/builders/Util.py

26 ↗

(On Diff #170137)

Yes, that's where vswhere is called and it has to run on the machine that will run the build.

zorg/trunk/buildbot/osuosl/master/config/builders.py

return [
                                       "-DLLVM_USE_LINKER=gold"])},
         {'name': "lldb-x64-windows-ninja",
          'slavenames': ["win-py3-buildbot"],
          'builddir': "lldb-x64-windows-ninja",
          'category' : 'lldb',
          'factory': LLDBBuilder.getLLDBWindowsCMakeBuildFactory(
                     clean=True,
                     python_source_dir=r'"C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python36_64"',
+                    autodetectVS=True,
                     target_arch='x64',
                     test=True,
                     extra_cmake_args=['-DLLVM_ENABLE_ASSERTIONS=OFF'])}
        ]
 
 # LLD builders.
 def _get_lld_builders():
     return [

zorg/trunk/zorg/buildbot/builders/ClangBuilder.py

def getClangCMakeGCSBuildFactory(
             test=True,
             cmake='cmake',
             jobs=None,
 
             # VS tools environment variable if using MSVC. For example,
             # %VS120COMNTOOLS% selects the 2013 toolchain.
             vs=None,
             vs_target_arch='x86',
+            autodetectVS=False,
 
             # Multi-stage compilation
             useTwoStage=False,
             testStage1=True,
             stage1_config='Release',
             stage2_config='Release',
 
             # Test-suite
def getClangCMakeGCSBuildFactory(
             # Use a lower compression level to generate the build-cache package faster.
             # defuault is 6 according to xz documentation
             xz_compression_factor=6,
             use_pixz_compression=False,
 
             # Triggers
             trigger_after_stage1=[]):
     return _getClangCMakeBuildFactory(
-               clean=clean, test=test, cmake=cmake, jobs=jobs, vs=vs,
-               vs_target_arch=vs_target_arch, useTwoStage=useTwoStage,
+               clean=clean, test=test, cmake=cmake, jobs=jobs,
+               vs=vs, vs_target_arch=vs_target_arch, autodetectVS=autodetectVS,
+               useTwoStage=useTwoStage,
                testStage1=testStage1, stage1_config=stage1_config,
                stage2_config=stage2_config, runTestSuite=runTestSuite,
                nt_flags=nt_flags, testsuite_flags=testsuite_flags,
                submitURL=submitURL, testerName=testerName,
                env=env, extra_cmake_args=extra_cmake_args,
                checkout_clang_tools_extra=checkout_clang_tools_extra,
                checkout_compiler_rt=checkout_compiler_rt,
                checkout_lld=checkout_lld,
                checkout_libcxx=checkout_libcxx,
                stage1_upload_directory=stage1_upload_directory,
                xz_compression_factor=xz_compression_factor,
                use_pixz_compression=use_pixz_compression,
                trigger_after_stage1=trigger_after_stage1)
 
 def getClangCMakeBuildFactory(
             clean=True,
             test=True,
             cmake='cmake',
             jobs=None,
 
             # VS tools environment variable if using MSVC. For example,
             # %VS120COMNTOOLS% selects the 2013 toolchain.
             vs=None,
             vs_target_arch='x86',
+            autodetectVS=False,
 
             # Multi-stage compilation
             useTwoStage=False,
             testStage1=True,
             stage1_config='Release',
             stage2_config='Release',
 
             # Test-suite
def getClangCMakeBuildFactory(
 
             # Extra repositories
             checkout_clang_tools_extra=True,
             checkout_compiler_rt=True,
             checkout_lld=True,
             checkout_libcxx=False,
             checkout_test_suite=False):
     return _getClangCMakeBuildFactory(
-               clean=clean, test=test, cmake=cmake, jobs=jobs, vs=vs,
-               vs_target_arch=vs_target_arch, useTwoStage=useTwoStage,
+               clean=clean, test=test, cmake=cmake, jobs=jobs,
+               vs=vs, vs_target_arch=vs_target_arch, autodetectVS=autodetectVS,
+               useTwoStage=useTwoStage,
                testStage1=testStage1, stage1_config=stage1_config,
                stage2_config=stage2_config, runTestSuite=runTestSuite,
                nt_flags=nt_flags, testsuite_flags=testsuite_flags,
                submitURL=submitURL, testerName=testerName,
                env=env, extra_cmake_args=extra_cmake_args,
                checkout_clang_tools_extra=checkout_clang_tools_extra,
                checkout_lld=checkout_lld,
                checkout_compiler_rt=checkout_compiler_rt,
                checkout_libcxx=checkout_libcxx,
                checkout_test_suite=checkout_test_suite)
 
 def _getClangCMakeBuildFactory(
             clean=True,
             test=True,
             cmake='cmake',
             jobs=None,
 
             # VS tools environment variable if using MSVC. For example,
             # %VS120COMNTOOLS% selects the 2013 toolchain.
             vs=None,
             vs_target_arch='x86',
+            autodetectVS=False,
 
             # Multi-stage compilation
             useTwoStage=False,
             testStage1=True,
             stage1_config='Release',
             stage2_config='Release',
 
             # Test-suite
def _getClangCMakeBuildFactory(
 
     # Global configurations
     stage1_build = 'stage1'
     stage1_install = 'stage1.install'
     stage2_build = 'stage2'
     stage2_install = 'stage2.install'
 
     # Set up VS environment, if appropriate.
-    if vs:
+    if vs or autodetectVS:
         f.addStep(SetProperty(
-            command=builders_util.getVisualStudioEnvironment(vs, vs_target_arch),
+            command=builders_util.getVisualStudioEnvironment(vs, vs_target_arch, autodetectVS),
             extract_fn=builders_util.extractSlaveEnvironment))
         assert not env, "Can't have custom builder env vars with VS"
         env = Property('slave_env')
 
 
     ############# CLEANING
     f.addStep(ShellCommand(name='clean stage 1',
                            command=['rm','-rf',stage1_build],

zorg/trunk/zorg/buildbot/builders/LLDBBuilder.py

def getLLDBWindowsCMakeBuildFactory(
 
             # Source directory containing a built python
             python_source_dir=r'C:/Python35',
 
             # Default values for VS devenv and build configuration
             vs=r"""%VS140COMNTOOLS%""",
             config='Release',
             target_arch='x86',
+            autodetectVS=False,
 
             extra_cmake_args=None,
             test=False,
             install=False):
 
     ############# PREPARING
     f = buildbot.process.factory.BuildFactory()
 
     # Determine Slave Environment and Set MSVC environment.
     f.addStep(SetProperty(
-        command=getVisualStudioEnvironment(vs, target_arch),
+        command=getVisualStudioEnvironment(vs, target_arch, autodetectVS),
         extract_fn=extractSlaveEnvironment))
 
     f = getLLDBSource(f,'llvm')
 
     build_cmd=['ninja']
     install_cmd = ['ninja','install']
     test_cmd = ['ninja','check-lldb']
 

zorg/trunk/zorg/buildbot/builders/LLDBuilder.py

def getLLDBuildFactory(
 
     return f
 
 
 def getLLDWinBuildFactory(
            clean = True,
 
            # Default values for VS devenv and build configuration
-           vs = None,          # What to run to configure Visual Studio utils.
-           target_arch = None, # Native.
+           vs = None,            # What to run to configure Visual Studio utils.
+           autodetectVS = False, # Don't try to auto detect a VS installation.
+           target_arch = None,   # Native.
 
            extra_configure_args = None,
            env   = None):
 
     # Set defaults
     if vs is None:
         vs = r"""%VS140COMNTOOLS%"""   # Visual Studio 2015.
     if extra_configure_args is None:
def getLLDWinBuildFactory(
     f.addStep(RemoveDirectory(name='clean ' + f.llvm_objdir,
               dir=f.llvm_objdir,
               haltOnFailure=False,
               flunkOnFailure=False,
               doStepIf=cleanBuildRequested
               ))
 
     # If set up environment step is requested, do this now.
-    if vs:
+    if vs or autodetectVS:
         f.addStep(SetProperty(
-            command=getVisualStudioEnvironment(vs, target_arch),
+            command=getVisualStudioEnvironment(vs, target_arch, autodetectVS),
             extract_fn=extractSlaveEnvironment))
         assert not env, "Can't have custom builder env vars with VS"
         env = Property('slave_env')
 
     # Always build with ninja.
     cmake_options = ["-G", "Ninja"]
     # Reconsile configure args with the defaults we want.
     if not any(a.startswith('-DCMAKE_BUILD_TYPE=')   for a in extra_configure_args):

zorg/trunk/zorg/buildbot/builders/SanitizerBuilderWindows.py

 def getSanitizerWindowsBuildFactory(
             clean=False,
             cmake='cmake',
 
             # Default values for VS devenv and build configuration
             vs=r"""%VS120COMNTOOLS%""",
             config='Release',
             target_arch='x86',
+            autodetectVS=False,
 
             extra_cmake_args=[]):
 
     ############# PREPARING
     f = buildbot.process.factory.BuildFactory()
 
     # Kill any stale symbolizer processes for the last run. If there are any
     # stale processes, the build will fail during linking. This can happen to
     # any process, but it is most likely to happen to llvm-symbolizer if its
     # pipe isn't closed.
     taskkill_cmd = 'taskkill /f /im llvm-symbolizer.exe || exit /b 0'
     f.addStep(ShellCommand(name='taskkill',
                            description='kill stale processes',
                            command=['cmd', '/c', taskkill_cmd],
                            haltOnFailure=False))
 
     # Determine Slave Environment and Set MSVC environment.
     f.addStep(SetProperty(
-        command=getVisualStudioEnvironment(vs, target_arch),
+        command=getVisualStudioEnvironment(vs, target_arch, autodetectVS),
         extract_fn=extractSlaveEnvironment))
 
     f = getSource(f,'llvm')
 
     # Global configurations
     build_dir = 'build'
 
     ############# CLEANING

zorg/trunk/zorg/buildbot/builders/UnifiedTreeBuilder.py

def getCmakeWithNinjaWithMSVCBuildFactory(
            checks = None,
            install_dir = None,
            clean = False,
            extra_configure_args = None,
            # VS tools environment variable if using MSVC. For example,
            # %VS140COMNTOOLS% selects the 2015 toolchain.
            vs=None,
            target_arch=None,
+           autodetectVS=False,
            env = None,
            **kwargs):
 
     assert not env, "Can't have custom builder env vars with MSVC build"
 
     # Make a local copy of the configure args, as we are going to modify that.
     if extra_configure_args:
         cmake_args = extra_configure_args[:]
def getCmakeWithNinjaWithMSVCBuildFactory(
     f = getLLVMBuildFactoryAndSVNSteps(
             depends_on_projects=depends_on_projects,
             llvm_srcdir=llvm_srcdir,
             obj_dir=obj_dir,
             install_dir=install_dir,
             **kwargs) # Pass through all the extra arguments.
 
     f.addStep(SetProperty(
-        command=builders_util.getVisualStudioEnvironment(vs, target_arch),
+        command=builders_util.getVisualStudioEnvironment(vs, target_arch, autodetectVS),
         extract_fn=builders_util.extractSlaveEnvironment))
     env = Property('slave_env')
 
     # Some options are required for this build no matter what.
     CmakeCommand.applyRequiredOptions(cmake_args, [
         ('-G',                      'Ninja'),
         ])
 

zorg/trunk/zorg/buildbot/builders/Util.py

 import buildbot.status.results
+import os
 import re
+import subprocess
 
-def getVisualStudioEnvironment(vs=r"""%VS120COMNTOOLS%""", target_arch=None):
+from os.path import join as pjoin
+
+VSWHERE_PATH = "C:/Program Files (x86)/Microsoft Visual Studio/Installer/vswhere.exe"
+
+def getVisualStudioEnvironment(vs=r"""%VS120COMNTOOLS%""", target_arch=None, autodetectVS=False):
     # x86 builds should use the 64 bit -> x86 cross compilation toolchain to avoid
     # out of memory linker errors
     arch_arg = {'x86': 'amd64_x86', 'x64': 'amd64', 'amd64': 'amd64'}.get(target_arch, '%PROCESSOR_ARCHITECTURE%')
+
+    """Get the VC tools environment using vswhere.exe from VS 2017 if it exists and was requested.
+    Otherwise, use the vs argument to construct a path to the expected location of vcvarsall.bat
+
+    This code is following the guidelines from strategy 1 in this blog post:
+        https://blogs.msdn.microsoft.com/vcblog/2017/03/06/finding-the-visual-c-compiler-tools-in-visual-studio-2017/
+
+    It doesn't work when VS is not installed at the default location.
+    """
+    # Use vswhere.exe if it exists.
+    if autodetectVS and os.path.exists(VSWHERE_PATH):
+        cmd = [VSWHERE_PATH, "-latest", "-property", "installationPath"]
+        vs_path = subprocess.check_output(cmd).strip()
+        vcvars_command = pjoin(vs_path, 'VC', 'Auxiliary', 'Build', 'vcvarsall.bat')
+    else:
         vcvars_command = "\"" + "\\".join((vs, '..','..','VC', 'vcvarsall.bat')) + "\""
+
     vcvars_command = "%s %s && set" % (vcvars_command, arch_arg)
     return vcvars_command
 
 def extractSlaveEnvironment(exit_status, stdout, stderr):
     '''Helper function for SetPropertyCommand. Loads Slave Environment
     into a dictionary, and returns slave_env property for ShellCommands.'''
     if exit_status:
         return {}