[debuginfo-tests] Fix environment variable used to specify LLDB

[jhenderson]

Summary

Currently, if the user specifies the environment variable 'CLANG', tests will attempt to use the value as a path to the clang executable. Previously, lldb could also be specified via the CLANG environment variable, but this was almost certainly a bug, because that meant both clang and lldb would have the same path. This patch changes the environment variable for lldb to 'LLDB'.

[thopre]

Should we add a test for that?

[jhenderson]

In D101982#2741492, @thopre wrote:

Should we add a test for that?

We could add a lit test for the additional pass-through variables, if you think there's value in it (I don't think the other variables in the list are tested however), but I don't think we can test the lit.cfg.py change. To test that, we'd need to have a lit tests within the debuginfo-tests directory which spawns lit itself, with certain variables specified, and show that the right tools are used. I don't think this is practical. I've manually confirmed that the expected tools are used when the variables are set.

[thopre]

In D101982#2741541, @jhenderson wrote:

In D101982#2741492, @thopre wrote:

Should we add a test for that?

We could add a lit test for the additional pass-through variables, if you think there's value in it (I don't think the other variables in the list are tested however), but I don't think we can test the lit.cfg.py change. To test that, we'd need to have a lit tests within the debuginfo-tests directory which spawns lit itself, with certain variables specified, and show that the right tools are used. I don't think this is practical. I've manually confirmed that the expected tools are used when the variables are set.

That's the overal feature I would like tested so more the lit.cfg.py side of things. If you think this is difficult to test for little value and since you have tested manually I'd say LGTM.

[jhenderson]

I'm going to hold off landing this for now until the discussion on the mailing list (https://lists.llvm.org/pipermail/llvm-dev/2021-May/150428.html) has reached a conclusion.

[jhenderson]

The mailing list discussion seems to suggest not adding these new variables. There are obviously no use-cases for the majority of them. LLDB is one exception, where it has been requested (see https://reviews.llvm.org/D95339#2638619). I'll update the patch accordingly.

[jhenderson]

Reduce scope of patch to just LLDB fix.

[jhenderson]

@thopre, I've reduce the scope of this patch to just the LLDB fix, as per the mailing list discussion. Please confirm that you are happy still!

[thopre]

Still LGTM

[teemperor]

LGTM. Please watch the Green Dragon bot for LLDB after landing (the bot runs the debuginfo-tests + LLDB tests). I noticed some time ago that part of the debuginfo-tests there were partly running the system LLDB (and it wasn't clear to me if that was actually on purpose). So this patch might bring up some previously hidden failures.

[jhenderson]

In D101982#2762953, @teemperor wrote:

LGTM. Please watch the Green Dragon bot for LLDB after landing (the bot runs the debuginfo-tests + LLDB tests). I noticed some time ago that part of the debuginfo-tests there were partly running the system LLDB (and it wasn't clear to me if that was actually on purpose). So this patch might bring up some previously hidden failures.

Thanks for the heads-up!

debuginfo-tests/lit.cfg.py

llvm_config.with_environment('PATHTOCLANGPP',
                                add_host_triple(llvm_config.use_llvm_tool('clang++')))
   llvm_config.with_environment('PATHTOCLANGCL',
                                add_host_triple(llvm_config.use_llvm_tool('clang-cl')))
 else:
   print('Host triple {} not supported. Skipping dexter tests in the '
         'debuginfo-tests project.'.format(config.host_triple))
 
 # Check which debuggers are available:
-built_lldb = llvm_config.use_llvm_tool('lldb', search_env='CLANG')
+built_lldb = llvm_config.use_llvm_tool('lldb', search_env='LLDB')
 lldb_path = None
 if built_lldb is not None:
     lldb_path = built_lldb
 elif lit.util.which('lldb') is not None:
     lldb_path = lit.util.which('lldb')
 
 if lldb_path is not None:
     config.available_features.add('lldb')

llvm/utils/lit/lit/TestingConfig.py

def fromdefaults(litConfig):
         # Set the environment based on the command line arguments.
         environment = {
             'PATH' : os.pathsep.join(litConfig.path +
                                      [os.environ.get('PATH','')]),
             'LLVM_DISABLE_CRASH_REPORT' : '1',
             }
 
         pass_vars = ['LIBRARY_PATH', 'LD_LIBRARY_PATH', 'SYSTEMROOT', 'TERM',
-                     'CLANG', 'LD_PRELOAD', 'ASAN_OPTIONS', 'UBSAN_OPTIONS',
-                     'LSAN_OPTIONS', 'ADB', 'ANDROID_SERIAL', 'SSH_AUTH_SOCK',
-                     'SANITIZER_IGNORE_CVE_2016_2143', 'TMPDIR', 'TMP', 'TEMP',
-                     'TEMPDIR', 'AVRLIT_BOARD', 'AVRLIT_PORT',
-                     'FILECHECK_OPTS', 'VCINSTALLDIR', 'VCToolsinstallDir',
-                     'VSINSTALLDIR', 'WindowsSdkDir', 'WindowsSDKLibVersion']
+                     'CLANG', 'LLDB', 'LD_PRELOAD', 'ASAN_OPTIONS',
+                     'UBSAN_OPTIONS', 'LSAN_OPTIONS', 'ADB', 'ANDROID_SERIAL',
+                     'SSH_AUTH_SOCK', 'SANITIZER_IGNORE_CVE_2016_2143',
+                     'TMPDIR', 'TMP', 'TEMP', 'TEMPDIR', 'AVRLIT_BOARD',
+                     'AVRLIT_PORT', 'FILECHECK_OPTS', 'VCINSTALLDIR',
+                     'VCToolsinstallDir', 'VSINSTALLDIR', 'WindowsSdkDir',
+                     'WindowsSDKLibVersion']
 
         if sys.platform == 'win32':
             pass_vars.append('INCLUDE')
             pass_vars.append('LIB')
             pass_vars.append('PATHEXT')
             pass_vars.append('USERPROFILE')
             environment['PYTHONBUFFERED'] = '1'