[test] Fix clang-test for FreeBSD and NetBSD

[lichray]

Summary

Lit tries to inject the shared library paths, but no action is taken when platform.system() is not recognized, results in an environment variable with an empty name, which is illegal.

The patch fixes this mechanism for FreeBSD and NetBSD, and gives an warning on other platforms, so that the latecomers don't have to spend time on debugging lit.

[krytarowski]

test/Unit/lit.cfg.py

39 ↗

(On Diff #119771)

Please include 'NetBSD' next to 'FreeBSD'.

[zturner]

Please don't throw an exception here. Instead, write this as:

shlibpath_var = None
if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
    shilbpath = 'LD_LIBRARY_PATH'
elif platform.system() == 'Darwin':
    shlibpath_var = 'DYLD_LIBRARY_PATH'
elif platform.system() == 'Windows':
    shlibpath_var = 'PATH'

if shlibpath_var:
    shlibpath = os.path.pathsep.join((config.shlibdir, config.llvm_libs_dir, config.environment.get(shlibpath_var,'')))
    config.environment[shlibpath_var] = shlibpath
else:
    lit_config.warning('Unable to determine shared library path variable for platform {}'.format(platform.system()))

[lichray]

In D39162#903179, @zturner wrote:

Please don't throw an exception here. Instead, write this as:

lit_config.warning('Unable to determine shared library path variable for platform {}'.format(platform.system()))

Why a warning rather than fatal?

[joerg]

I think we should special case Darwin and Windows and fall-back to LD_LIBRARY_PATH for the rest. Can't remember if there is a UNIX-like platform left where it doesn't work.

[lichray]

In D39162#903186, @joerg wrote:

I think we should special case Darwin and Windows and fall-back to LD_LIBRARY_PATH for the rest. Can't remember if there is a UNIX-like platform left where it doesn't work.

If those developers come to us, I would say. The script is for LLVM developers only, and to developers, a missed guess wastes more time than pointing out where it doesn't work.

[krytarowski]

In D39162#903186, @joerg wrote:

I think we should special case Darwin and Windows and fall-back to LD_LIBRARY_PATH for the rest. Can't remember if there is a UNIX-like platform left where it doesn't work.

SHLIB_PATH in HP/UX

[krytarowski]

In D39162#903190, @krytarowski wrote:

In D39162#903186, @joerg wrote:

I think we should special case Darwin and Windows and fall-back to LD_LIBRARY_PATH for the rest. Can't remember if there is a UNIX-like platform left where it doesn't work.

SHLIB_PATH in HP/UX

Hmm SHLIB_PATH has been marked as legacy and they probably switched to LD_LIBRARY_PATH.

AIX might still ship with LIBPATH.

I agree that adding defines for OS'es that are barely developed (especially closed-source) without LLVM developers might be accumulating unused code.

[lichray]

Changed to an warning given @zturner 's comments and experiments.

[lichray]

ping?

cfe/trunk/test/Unit/lit.cfg.py

 if 'TEMP' in os.environ:
     config.environment['TEMP'] = os.environ['TEMP']
 
 # Propagate path to symbolizer for ASan/MSan.
 for symbolizer in ['ASAN_SYMBOLIZER_PATH', 'MSAN_SYMBOLIZER_PATH']:
     if symbolizer in os.environ:
         config.environment[symbolizer] = os.environ[symbolizer]
 
-shlibpath_var = ''
-if platform.system() == 'Linux':
-    shlibpath_var = 'LD_LIBRARY_PATH'
+def find_shlibpath_var():
+    if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
+        yield 'LD_LIBRARY_PATH'
     elif platform.system() == 'Darwin':
-    shlibpath_var = 'DYLD_LIBRARY_PATH'
+        yield 'DYLD_LIBRARY_PATH'
     elif platform.system() == 'Windows':
-    shlibpath_var = 'PATH'
+        yield 'PATH'
 
+for shlibpath_var in find_shlibpath_var():
     # in stand-alone builds, shlibdir is clang's build tree
     # while llvm_libs_dir is installed LLVM (and possibly older clang)
-shlibpath = os.path.pathsep.join((config.shlibdir, config.llvm_libs_dir,
-                                 config.environment.get(shlibpath_var,'')))
-
+    shlibpath = os.path.pathsep.join(
+        (config.shlibdir,
+         config.llvm_libs_dir,
+         config.environment.get(shlibpath_var, '')))
     config.environment[shlibpath_var] = shlibpath
+    break
+else:
+    lit_config.warning("unable to inject shared library path on '{}'"
+                       .format(platform.system()))