[lit] Report tool path from use_llvm_tool if found via env variable

[jhenderson]

Summary

Previously, if the search_env argument was specified, and the tool was found at that location, the path was not reported, unlike other situations when this function was called. Adding the reporting makes the function consistent.

[jhenderson]

llvm/utils/lit/tests/use-tool-search-env.py

8

I'll fix this before landing this patch.

[thopre]

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

414–416

Is the check for search_env needed?

[jhenderson]

Address review comment and attempt to fix pre-merge check.

There was something odd going on with the slash direction in the %{inputs} on the pre-merge bot: despite being a Windows machine, it was using / as the path separator in the substitution. This isn't what happens for me locally on my Windows machine. Either way, the new approach uses %p which should work.

[thopre]

LGTM, thanks!

[daltenty]

llvm/utils/lit/tests/use-tool-search-env.py

7

Looks like this test has issues if the path has a symlink in it, we may need to call realpath somewhere?

https://lab.llvm.org/staging/#/builders/126/builds/345/steps/5/logs/FAIL__lit___use-tool-search-env_py

(/home/powerllvm/powerllvm_env/aix-ppc64-ppc64le/clang-ppc64-aix-ppc64le/ is symlinked to /scratch/powerllvm/powerllvm_env/aix-ppc64-ppc64le/clang-ppc64-aix-ppc64le on the AIX builder)

[jhenderson]

llvm/utils/lit/tests/use-tool-search-env.py

7

Thanks. I've pushed a likely fix (see rGd2b2ad32b76989b68e7b525e7484e25b0f0cc4e6), but don't have an environment where I can easily verify it. I'll keep an eye on the build bot, but if my fix doesn't resolve the issue, I'll need someone else to take a look at it.

[daltenty]

llvm/utils/lit/tests/use-tool-search-env.py

7

Thanks for the quick fix! Seems to have resolved the issue.

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

def use_default_substitutions(self):
         self.add_err_msg_substitutions()
 
     def use_llvm_tool(self, name, search_env=None, required=False, quiet=False):
         """Find the executable program 'name', optionally using the specified
         environment variable as an override before searching 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 tool:
-                return tool
 
+        if not tool:
             # Otherwise look in the path.
             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)
 

llvm/utils/lit/tests/Inputs/use-tool-search-env/lit.cfg

+import lit.formats
+config.name = 'search-env'
+config.suffixes = ['.txt']
+config.test_format = lit.formats.ShTest()
+config.test_source_root = None
+config.test_exec_root = None
+import lit.llvm
+lit.llvm.initialize(lit_config, config)
+import os.path
+lit.llvm.llvm_config.with_environment('TOOL_LOCATION', os.path.dirname(__file__) + '/test.tool')
+lit.llvm.llvm_config.use_llvm_tool('test-tool', search_env='TOOL_LOCATION')

llvm/utils/lit/tests/Inputs/use-tool-search-env/true.txt

+# RUN: true

llvm/utils/lit/tests/use-tool-search-env.py

+## Show that lit reports the path to tools picked up via the use_llvm_tool
+## function when the tool is found via an environment variable.
+
+# RUN: %{lit} %{inputs}/use-tool-search-env 2>&1 | \
+# RUN:   FileCheck %s -DDIR=%p
+
+# CHECK: note: using test-tool: [[DIR]]{{[\\/]}}Inputs{{[\\/]}}use-tool-search-env{{[\\/]}}test.tool