[lit] Stop using PATH to lookup clang/lld/lldb unless requested

[jhenderson]

Summary

This patch stops lit from looking on the PATH for clang, lld and other users of use_llvm_tool (currently only the debuginfo-tests) unless the call explicitly requests to opt into using the PATH. When not opting in, tests will only look in the build directory.

See the mailing list thread starting from https://lists.llvm.org/pipermail/llvm-dev/2021-May/150421.html.

use_clang is used from:

• The main clang tests (we don't want to test the installed version here, only the built version).
• The debuginfo tests (this one is possibly debatable, but the conversation in D95339 suggests we should only use the build version in a monorepo setting; see also D96511).
• The clangd testing (although rGc29513f7e023f125c6d221db179dc40b79e5c074 suggests clang is not actually used).
• The lldb shell testing (this appears to deliberately allow using the installed version).

use_lld is used from:

• The main lld tests (we don't want to test the installed version here, only the built version).
• The lldb shell testing (this appears to deliberately allow using the installed version).

use_llvm_tool is used from:

• use_lld and use_clang
• The debuginfo_tests to lookup LLDB (see above reasoning for clang)
• The tests for lit itself (not in a way where the behaviour change is relevant)

[dblaikie]

@thopre - as an aside: It'd be helpful if you could include some text in the text box when marking something "approved" through phabricator. There's a bug/limitation that approvals without text don't produce email to the mailing list - so it looks like a patch hasn't been reviewed yet (or has been committed without review).

[thopre]

In D102630#2764705, @dblaikie wrote:

@thopre - as an aside: It'd be helpful if you could include some text in the text box when marking something "approved" through phabricator. There's a bug/limitation that approvals without text don't produce email to the mailing list - so it looks like a patch hasn't been reviewed yet (or has been committed without review).

Oh sorry, I was not aware of that limitation. Will be careful to include a message in the future

lldb/test/Shell/helper/toolchain.py

def use_support_substitutions(config):
     config.substitutions.append(('%clang_cl_host', '%clang_cl --target='+config.host_triple))
 
     additional_tool_dirs=[]
     if config.lldb_lit_tools_dir:
         additional_tool_dirs.append(config.lldb_lit_tools_dir)
 
     llvm_config.use_clang(additional_flags=['--target=specify-a-target-or-use-a-_host-substitution'],
                           additional_tool_dirs=additional_tool_dirs,
-                          required=True)
+                          required=True, use_installed=True)
 
 
     if sys.platform == 'win32':
         _use_msvc_substitutions(config)
 
     have_lld = llvm_config.use_lld(additional_tool_dirs=additional_tool_dirs,
-                                   required=False)
+                                   required=False, use_installed=True)
     if have_lld:
         config.available_features.add('lld')
 
 
     support_tools = ['yaml2obj', 'obj2yaml', 'llvm-dwp', 'llvm-pdbutil',
                      'llvm-mc', 'llvm-readobj', 'llvm-objdump',
                      'llvm-objcopy', 'lli']
     additional_tool_dirs += [config.lldb_tools_dir, config.llvm_tools_dir]

llvm/utils/lit/lit/llvm/config.py

def use_default_substitutions(self):
 
         self.config.substitutions.append(('%python', '"%s"' % (sys.executable)))
 
         self.add_tool_substitutions(
             tool_patterns, [self.config.llvm_tools_dir])
 
         self.add_err_msg_substitutions()
 
-    def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):
+    def use_llvm_tool(self, name, search_env=None, required=False, quiet=False,
+                      use_installed=False):
         """Find the executable program 'name', optionally using the specified
-        environment variable as an override before searching the
-        configuration's PATH."""
+        environment variable as an override before searching the build directory
+        and then optionally the configuration's PATH."""
         # If the override is specified in the environment, use it without
         # validation.
         tool = None
         if search_env:
             tool = self.config.environment.get(search_env)
 
         if not tool:
-            # Otherwise look in the path.
+            # Use the build directory version.
+            tool = lit.util.which(name, self.config.llvm_tools_dir)
+
+        if not tool and use_installed:
+            # Otherwise look in the path, if enabled.
             tool = lit.util.which(name, self.config.environment['PATH'])
 
         if required and not tool:
             message = "couldn't find '{}' program".format(name)
             if search_env:
                 message = message + \
                     ', try setting {} in your environment'.format(search_env)
             self.lit_config.fatal(message)
 
         if tool:
             tool = os.path.normpath(tool)
             if not self.lit_config.quiet and not quiet:
                 self.lit_config.note('using {}: {}'.format(name, tool))
         return tool
 
     def use_clang(self, additional_tool_dirs=[], additional_flags=[],
-                  required=True):
+                  required=True, use_installed=False):
         """Configure the test suite to be able to invoke clang.
 
         Sets up some environment variables important to clang, locates a
-        just-built or installed clang, and add a set of standard
+        just-built or optionally an installed clang, and add a set of standard
         substitutions useful to any test suite that makes use of clang.
 
         """
         # Clear some environment variables that might affect Clang.
         #
         # This first set of vars are read by Clang, but shouldn't affect tests
         # that aren't specifically looking for these features, or are required
         # simply to run the tests at all.
def use_clang(self, additional_tool_dirs=[], additional_flags=[],
         pext = getattr(self.config, 'llvm_plugin_ext', None)
         if shl:
             self.config.substitutions.append(('%llvmshlibdir', shl))
         if pext:
             self.config.substitutions.append(('%pluginext', pext))
 
         # Discover the 'clang' and 'clangcc' to use.
         self.config.clang = self.use_llvm_tool(
-            'clang', search_env='CLANG', required=required)
+            'clang', search_env='CLANG', required=required,
+            use_installed=use_installed)
         if self.config.clang:
           self.config.available_features.add('clang')
           builtin_include_dir = self.get_clang_builtin_include_dir(
               self.config.clang)
           tool_substitutions = [
               ToolSubst('%clang', command=self.config.clang,
                         extra_args=additional_flags),
               ToolSubst('%clang_analyze_cc1', command='%clang_cc1',
def use_clang(self, additional_tool_dirs=[], additional_flags=[],
              '''\"*** invalid substitution, use '%clang_cc1'. ***\"'''))
         self.config.substitutions.append(
             (' %clang-cpp ',
              '''\"*** invalid substitution, use '%clang_cpp'. ***\"'''))
         self.config.substitutions.append(
             (' %clang-cl ',
              '''\"*** invalid substitution, use '%clang_cl'. ***\"'''))
 
-    def use_lld(self, additional_tool_dirs=[], required=True):
+    def use_lld(self, additional_tool_dirs=[], required=True,
+                use_installed=False):
         """Configure the test suite to be able to invoke lld.
 
         Sets up some environment variables important to lld, locates a
-        just-built or installed lld, and add a set of standard
+        just-built or optionally an installed lld, and add a set of standard
         substitutions useful to any test suite that makes use of lld.
 
         """
 
         # Tweak the PATH to include the tools dir and the scripts dir.
         exe_dir_props = [self.config.name.lower() + '_tools_dir',
                          'lld_tools_dir', 'llvm_tools_dir']
         paths = [getattr(self.config, pp) for pp in exe_dir_props
                  if getattr(self.config, pp, None)]
         paths = additional_tool_dirs + paths
         self.with_environment('PATH', paths, append_path=True)
 
         lib_dir_props = [self.config.name.lower() + '_libs_dir',
                          'lld_libs_dir', 'llvm_libs_dir']
         paths = [getattr(self.config, pp) for pp in lib_dir_props
                  if getattr(self.config, pp, None)]
 
         self.with_environment('LD_LIBRARY_PATH', paths, append_path=True)
 
-        # Discover the 'clang' and 'clangcc' to use.
+        # Discover the LLD executables to use.
 
-        ld_lld = self.use_llvm_tool('ld.lld', required=required)
-        lld_link = self.use_llvm_tool('lld-link', required=required)
-        ld64_lld = self.use_llvm_tool('ld64.lld', required=required)
-        wasm_ld = self.use_llvm_tool('wasm-ld', required=required)
+        ld_lld = self.use_llvm_tool('ld.lld', required=required,
+                                    use_installed=use_installed)
+        lld_link = self.use_llvm_tool('lld-link', required=required,
+                                      use_installed=use_installed)
+        ld64_lld = self.use_llvm_tool('ld64.lld', required=required,
+                                      use_installed=use_installed)
+        wasm_ld = self.use_llvm_tool('wasm-ld', required=required,
+                                     use_installed=use_installed)
 
         was_found = ld_lld and lld_link and ld64_lld and wasm_ld
         tool_substitutions = []
         if ld_lld:
             tool_substitutions.append(ToolSubst(r'ld\.lld', command=ld_lld))
         if lld_link:
             tool_substitutions.append(ToolSubst('lld-link', command=lld_link))
         if ld64_lld:
             tool_substitutions.append(ToolSubst(r'ld64\.lld', command=ld64_lld))
         if wasm_ld:
             tool_substitutions.append(ToolSubst('wasm-ld', command=wasm_ld))
         self.add_tool_substitutions(tool_substitutions)
         return was_found