[llvm] a1e6565 - [lit] Stop using PATH to lookup clang/lld/lldb unless requested
Tom Stellard via llvm-commits
llvm-commits at lists.llvm.org
Tue May 25 15:26:11 PDT 2021
On 5/18/21 2:45 AM, James Henderson via llvm-commits wrote:
>
> Author: James Henderson
> Date: 2021-05-18T10:43:33+01:00
> New Revision: a1e6565855784988aa6302d6672705baf2a84ff2
>
> URL: https://github.com/llvm/llvm-project/commit/a1e6565855784988aa6302d6672705baf2a84ff2
> DIFF: https://github.com/llvm/llvm-project/commit/a1e6565855784988aa6302d6672705baf2a84ff2.diff
>
> LOG: [lit] Stop using PATH to lookup clang/lld/lldb unless requested
>
> 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.
>
> See the review for details of why decisions were made about when still
> to use the PATH.
>
> Reviewed by: thopre
>
> Differential Revision: https://reviews.llvm.org/D102630
>
> Added:
>
>
> Modified:
> lldb/test/Shell/helper/toolchain.py
> llvm/utils/lit/lit/llvm/config.py
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/lldb/test/Shell/helper/toolchain.py b/lldb/test/Shell/helper/toolchain.py
> index 6e564f31dbd37..6ccb529e89852 100644
> --- a/lldb/test/Shell/helper/toolchain.py
> +++ b/lldb/test/Shell/helper/toolchain.py
> @@ -147,14 +147,14 @@ def use_support_substitutions(config):
>
> 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')
>
>
> diff --git a/llvm/utils/lit/lit/llvm/config.py b/llvm/utils/lit/lit/llvm/config.py
> index 6ff3cba00f3e0..94824cd34773a 100644
> --- a/llvm/utils/lit/lit/llvm/config.py
> +++ b/llvm/utils/lit/lit/llvm/config.py
> @@ -401,10 +401,11 @@ def use_default_substitutions(self):
>
> 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
> @@ -412,7 +413,11 @@ def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):
> 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'])
>
This change beaks the stand-alone builds of clang, because the use_clang
method below adds the clang build directory to PATH, and there is no
other way to specify an alternative search path. Maybe the clang
build directory should be passed as an argument to the use_llvm_tool()
function instead?
- Tom
> if required and not tool:
> @@ -429,11 +434,11 @@ def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):
> 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.
>
> """
> @@ -497,7 +502,8 @@ def use_clang(self, additional_tool_dirs=[], additional_flags=[],
>
> # 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(
> @@ -569,11 +575,12 @@ def prefer(this, to):
> (' %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.
>
> """
> @@ -593,12 +600,16 @@ def use_lld(self, additional_tool_dirs=[], required=True):
>
> 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 = []
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list