Respect PYTHONPATH

[greened]

Summary

If a user has PYTHONPATH set in the environment, append new entries to it rather than blindly setting PYTHONPATH to a fixed string. This allows tests to, for example, find psutil if it is in PYTHONPATH. Without this change, lit will detect psutil but then various tests will fail because PYTHONPATH has been overwritten and psutil cannot be found.

[greened]

Ping?

[delcypher]

@greened This seems reasonable but I'm wondering under what scenario psutil is installed in a non-standard location which requires the use of PYTHONPATH. The usual way to handle installing a python package that you don't want system wide is to use virtualenv and then run pip install psutil in that environment.

[delcypher]

utils/lit/tests/lit.cfg

39

You should add a comment above the if 'PYTHONPATH' explaining why this is being here because it looks like the Required because some tests import the lit module comment is describing the entire block of code, whereas it actually only describes the very first statement (pythonpath_list = [lit_path]).

[greened]

virtualenv isn't available everywhere. Some sites prefer that users point to a specific Python installation via PYTHONPATH.

[greened]

Moved comment as directed.

[greened]

Added comment about including the user's PYTHONPATH.

[greened]

Ping. We're trying to clean up the last few test failures we're seeing.

[greened]

Ping?

[greened]

Ping?

[delcypher]

@greened Sorry looks like I missed your earlier pings. LGTM

Can you commit this, or would you like me to do it on your behalf?

[greened]

Committed in r350536.

utils/lit/tests/Inputs/shtest-timeout/lit.cfg

 config.test_exec_root = config.test_source_root
 config.target_triple = '(unused)'
 src_root = os.path.join(config.test_source_root, '..')
-config.environment['PYTHONPATH'] = src_root
+
+pythonpath_list = [src_root]
+if 'PYTHONPATH' in os.environ:
+    pythonpath_list.append(os.environ['PYTHONPATH'])
+if 'PYTHONPATH' in config.environment:
+    pythonpath_list.append(config.environment['PYTHONPATH'])
+config.environment['PYTHONPATH'] = os.pathsep.join(pythonpath_list)
+
 config.substitutions.append(('%{python}', sys.executable))

utils/lit/tests/lit.cfg

 else:
   lit_path = src_root
 
-config.environment['PYTHONPATH'] = lit_path # Required because some tests import the lit module
+pythonpath_list = [lit_path] # Required because some tests import the lit module
+
+# Ensure the user's PYTHONPATH is included.
+if 'PYTHONPATH' in os.environ:
+    pythonpath_list.append(os.environ['PYTHONPATH'])
+if 'PYTHONPATH' in config.environment:
+    pythonpath_list.append(config.environment['PYTHONPATH'])
+config.environment['PYTHONPATH'] = os.pathsep.join(pythonpath_list)
+
 config.substitutions.append(('%{src_root}', src_root))
 config.substitutions.append(('%{inputs}', os.path.join(
             src_root, 'tests', 'Inputs')))