<div dir="ltr"><div>Thanks, I think what you said makes sense, and looks like this might be a problem for a standalone LLD build too. I'm working on a fix for this now.</div><div><br></div><div>I didn't see any buildbot notification for this. I'm guessing we don't have any for standalone builds?<br></div><div><br></div><div>James<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 25 May 2021 at 23:26, Tom Stellard via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 5/18/21 2:45 AM, James Henderson via llvm-commits wrote:<br>
> <br>
> Author: James Henderson<br>
> Date: 2021-05-18T10:43:33+01:00<br>
> New Revision: a1e6565855784988aa6302d6672705baf2a84ff2<br>
> <br>
> URL: <a href="https://github.com/llvm/llvm-project/commit/a1e6565855784988aa6302d6672705baf2a84ff2" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/a1e6565855784988aa6302d6672705baf2a84ff2</a><br>
> DIFF: <a href="https://github.com/llvm/llvm-project/commit/a1e6565855784988aa6302d6672705baf2a84ff2.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/a1e6565855784988aa6302d6672705baf2a84ff2.diff</a><br>
> <br>
> LOG: [lit] Stop using PATH to lookup clang/lld/lldb unless requested<br>
> <br>
> This patch stops lit from looking on the PATH for clang, lld and other<br>
> users of use_llvm_tool (currently only the debuginfo-tests) unless the<br>
> call explicitly requests to opt into using the PATH. When not opting in,<br>
> tests will only look in the build directory.<br>
> <br>
> See the mailing list thread starting from<br>
> <a href="https://lists.llvm.org/pipermail/llvm-dev/2021-May/150421.html" rel="noreferrer" target="_blank">https://lists.llvm.org/pipermail/llvm-dev/2021-May/150421.html</a>.<br>
> <br>
> See the review for details of why decisions were made about when still<br>
> to use the PATH.<br>
> <br>
> Reviewed by: thopre<br>
> <br>
> Differential Revision: <a href="https://reviews.llvm.org/D102630" rel="noreferrer" target="_blank">https://reviews.llvm.org/D102630</a><br>
> <br>
> Added:<br>
> <br>
> <br>
> Modified:<br>
> lldb/test/Shell/helper/toolchain.py<br>
> llvm/utils/lit/lit/llvm/config.py<br>
> <br>
> Removed:<br>
> <br>
> <br>
> <br>
> ################################################################################<br>
> diff --git a/lldb/test/Shell/helper/toolchain.py b/lldb/test/Shell/helper/toolchain.py<br>
> index 6e564f31dbd37..6ccb529e89852 100644<br>
> --- a/lldb/test/Shell/helper/toolchain.py<br>
> +++ b/lldb/test/Shell/helper/toolchain.py<br>
> @@ -147,14 +147,14 @@ def use_support_substitutions(config):<br>
> <br>
> llvm_config.use_clang(additional_flags=['--target=specify-a-target-or-use-a-_host-substitution'],<br>
> additional_tool_dirs=additional_tool_dirs,<br>
> - required=True)<br>
> + required=True, use_installed=True)<br>
> <br>
> <br>
> if sys.platform == 'win32':<br>
> _use_msvc_substitutions(config)<br>
> <br>
> have_lld = llvm_config.use_lld(additional_tool_dirs=additional_tool_dirs,<br>
> - required=False)<br>
> + required=False, use_installed=True)<br>
> if have_lld:<br>
> config.available_features.add('lld')<br>
> <br>
> <br>
> diff --git a/llvm/utils/lit/lit/llvm/config.py b/llvm/utils/lit/lit/llvm/config.py<br>
> index 6ff3cba00f3e0..94824cd34773a 100644<br>
> --- a/llvm/utils/lit/lit/llvm/config.py<br>
> +++ b/llvm/utils/lit/lit/llvm/config.py<br>
> @@ -401,10 +401,11 @@ def use_default_substitutions(self):<br>
> <br>
> self.add_err_msg_substitutions()<br>
> <br>
> - def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):<br>
> + def use_llvm_tool(self, name, search_env=None, required=False, quiet=False,<br>
> + use_installed=False):<br>
> """Find the executable program 'name', optionally using the specified<br>
> - environment variable as an override before searching the<br>
> - configuration's PATH."""<br>
> + environment variable as an override before searching the build directory<br>
> + and then optionally the configuration's PATH."""<br>
> # If the override is specified in the environment, use it without<br>
> # validation.<br>
> tool = None<br>
> @@ -412,7 +413,11 @@ def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):<br>
> tool = self.config.environment.get(search_env)<br>
> <br>
> if not tool:<br>
> - # Otherwise look in the path.<br>
> + # Use the build directory version.<br>
> + tool = lit.util.which(name, self.config.llvm_tools_dir)<br>
> +<br>
> + if not tool and use_installed:<br>
> + # Otherwise look in the path, if enabled.<br>
> tool = lit.util.which(name, self.config.environment['PATH'])<br>
> <br>
<br>
This change beaks the stand-alone builds of clang, because the use_clang<br>
method below adds the clang build directory to PATH, and there is no<br>
other way to specify an alternative search path. Maybe the clang<br>
build directory should be passed as an argument to the use_llvm_tool()<br>
function instead?<br>
<br>
- Tom<br>
<br>
<br>
> if required and not tool:<br>
> @@ -429,11 +434,11 @@ def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):<br>
> return tool<br>
> <br>
> def use_clang(self, additional_tool_dirs=[], additional_flags=[],<br>
> - required=True):<br>
> + required=True, use_installed=False):<br>
> """Configure the test suite to be able to invoke clang.<br>
> <br>
> Sets up some environment variables important to clang, locates a<br>
> - just-built or installed clang, and add a set of standard<br>
> + just-built or optionally an installed clang, and add a set of standard<br>
> substitutions useful to any test suite that makes use of clang.<br>
> <br>
> """<br>
> @@ -497,7 +502,8 @@ def use_clang(self, additional_tool_dirs=[], additional_flags=[],<br>
> <br>
> # Discover the 'clang' and 'clangcc' to use.<br>
> self.config.clang = self.use_llvm_tool(<br>
> - 'clang', search_env='CLANG', required=required)<br>
> + 'clang', search_env='CLANG', required=required,<br>
> + use_installed=use_installed)<br>
> if self.config.clang:<br>
> self.config.available_features.add('clang')<br>
> builtin_include_dir = self.get_clang_builtin_include_dir(<br>
> @@ -569,11 +575,12 @@ def prefer(this, to):<br>
> (' %clang-cl ',<br>
> '''\"*** invalid substitution, use '%clang_cl'. ***\"'''))<br>
> <br>
> - def use_lld(self, additional_tool_dirs=[], required=True):<br>
> + def use_lld(self, additional_tool_dirs=[], required=True,<br>
> + use_installed=False):<br>
> """Configure the test suite to be able to invoke lld.<br>
> <br>
> Sets up some environment variables important to lld, locates a<br>
> - just-built or installed lld, and add a set of standard<br>
> + just-built or optionally an installed lld, and add a set of standard<br>
> substitutions useful to any test suite that makes use of lld.<br>
> <br>
> """<br>
> @@ -593,12 +600,16 @@ def use_lld(self, additional_tool_dirs=[], required=True):<br>
> <br>
> self.with_environment('LD_LIBRARY_PATH', paths, append_path=True)<br>
> <br>
> - # Discover the 'clang' and 'clangcc' to use.<br>
> + # Discover the LLD executables to use.<br>
> <br>
> - ld_lld = self.use_llvm_tool('ld.lld', required=required)<br>
> - lld_link = self.use_llvm_tool('lld-link', required=required)<br>
> - ld64_lld = self.use_llvm_tool('ld64.lld', required=required)<br>
> - wasm_ld = self.use_llvm_tool('wasm-ld', required=required)<br>
> + ld_lld = self.use_llvm_tool('ld.lld', required=required,<br>
> + use_installed=use_installed)<br>
> + lld_link = self.use_llvm_tool('lld-link', required=required,<br>
> + use_installed=use_installed)<br>
> + ld64_lld = self.use_llvm_tool('ld64.lld', required=required,<br>
> + use_installed=use_installed)<br>
> + wasm_ld = self.use_llvm_tool('wasm-ld', required=required,<br>
> + use_installed=use_installed)<br>
> <br>
> was_found = ld_lld and lld_link and ld64_lld and wasm_ld<br>
> tool_substitutions = []<br>
> <br>
> <br>
> <br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
> <br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>