Let the OS take care of the current working directory

[• espindola]

Summary

This make the internal shell a bit more like a regular shell. When 'cd' is executed, it just use chdir for example. This is more reliable as it is really hard to audit everything that might use the current working directory.

This works as is because we always chdir at the start of a test execution.

The next steps probably are

• Change the environment to be managed the same way.
• Implement export
• Implement environment variables expansion like $PATH
• Maybe use subprocess.run to have an fully isolated process for each test run. I think this is pretty much necessary if we are to implement subshells

[rnk]

chdir isn't thread safe, and lit still supports running the tests with threads. It looks like there are portability issues with requiring multiprocessing everywhere, so I think this change would require conditionally launching a child python process to implement the internal shell. That also kind of sucks, though, since python process startup isn't fast.

I also kind of liked emulating the shell environment. It seemed cleaner than manipulating global state. That's a minor concern, though.

I think you're right, if we want to implement subshells and ulimit, we need to create a process and manipulate its environment, cwd, etc. What we have stops just short of that, since I've basically assumed that we will never implement shell variable expansion or subshells. I'd be happy if lit could do more basic environment variable manipulation (export), but subshells felt like a bridge too far. I figured tests that needed this stuff could call bash directly and have a REQUIRES: bash line or something.

Do you think we can reasonably implement variable expansion? The last time I looked at it, it seemed too hard, and I worked around the problem by adding more lit substitutions. Maybe it would be OK if we just implemented the ${} expansion syntax?

llvm/utils/lit/lit/ShCommands.py

def __repr__(self):
         return self.pattern
 
     def __eq__(self, other):
         if not isinstance(other, Command):
             return False
 
         return (self.pattern == other.pattern)
 
-    def resolve(self, cwd):
+    def resolve(self):
         import glob
-        import os
-        if os.path.isabs(self.pattern):
-           abspath = self.pattern
-        else:
-            abspath = os.path.join(cwd, self.pattern)
-        results = glob.glob(abspath)
+        results = glob.glob(self.pattern)
         return [self.pattern] if len(results) == 0 else results
 
 class Pipeline:
     def __init__(self, commands, negate=False, pipe_err=False):
         self.commands = commands
         self.negate = negate
         self.pipe_err = pipe_err
 

llvm/utils/lit/lit/TestRunner.py

 
 class ShellEnvironment(object):
 
     """Mutable shell environment containing things like CWD and env vars.
 
     Environment variables are not implemented, but cwd tracking is.
     """
 
-    def __init__(self, cwd, env):
-        self.cwd = cwd
+    def __init__(self, env):
         self.env = dict(env)
 
 class TimeoutHelper(object):
     """
         Object used to helper manage enforcing a timeout in
         _executeShCmd(). It is passed through recursive calls
         to collect processes that have been executed so that when
         the timeout happens they can be killed.
def executeShCmd(cmd, shenv, results, timeout=0):
     finalExitCode = _executeShCmd(cmd, shenv, results, timeoutHelper)
     timeoutHelper.cancel()
     timeoutInfo = None
     if timeoutHelper.timeoutReached():
         timeoutInfo = 'Reached timeout of {} seconds'.format(timeout)
 
     return (finalExitCode, timeoutInfo)
 
-def expand_glob(arg, cwd):
+def expand_glob(arg):
     if isinstance(arg, GlobItem):
-        return arg.resolve(cwd)
+        return arg.resolve()
     return [arg]
 
-def expand_glob_expressions(args, cwd):
+def expand_glob_expressions(args):
     result = [args[0]]
     for arg in args[1:]:
-        result.extend(expand_glob(arg, cwd))
+        result.extend(expand_glob(arg))
     return result
 
 def quote_windows_command(seq):
     """
     Reimplement Python's private subprocess.list2cmdline for MSys compatibility
 
     Based on CPython implementation here:
       https://hg.python.org/cpython/file/849826a900d2/Lib/subprocess.py#l422
def _executeShCmd(cmd, shenv, results, timeoutHelper):
 
     # Handle shell builtins first.
     if cmd.commands[0].args[0] == 'cd':
         if len(cmd.commands) != 1:
             raise ValueError("'cd' cannot be part of a pipeline")
         if len(cmd.commands[0].args) != 2:
             raise ValueError("'cd' supports only one argument")
         newdir = cmd.commands[0].args[1]
-        # Update the cwd in the parent environment.
-        if os.path.isabs(newdir):
-            shenv.cwd = newdir
-        else:
-            shenv.cwd = os.path.realpath(os.path.join(shenv.cwd, newdir))
+        os.chdir(newdir)
         # The cd builtin always succeeds. If the directory does not exist, the
         # following Popen calls will fail instead.
         return 0
 
     procs = []
     input = subprocess.PIPE
     stderrTempFiles = []
     opened_files = []
     named_temp_files = []
     # To avoid deadlock, we use a single stderr stream for piped
     # output. This is null until we have seen some output using
     # stderr.
     for i,j in enumerate(cmd.commands):
         # Reference the global environment by default.
         cmd_shenv = shenv
         if j.args[0] == 'env':
             # Create a copy of the global environment and modify it for this one
             # command. There might be multiple envs in a pipeline:
             #   env FOO=1 llc < %s | env BAR=2 llvm-mc | FileCheck %s
-            cmd_shenv = ShellEnvironment(shenv.cwd, shenv.env)
+            cmd_shenv = ShellEnvironment(shenv.env)
             arg_idx = 1
             for arg_idx, arg in enumerate(j.args[1:]):
                 # Partition the string into KEY=VALUE.
                 key, eq, val = arg.partition('=')
                 # Stop if there was no equals.
                 if eq == '':
                     break
                 cmd_shenv.env[key] = val
for i,j in enumerate(cmd.commands):
                 else:
                     result = subprocess.STDOUT
             elif r == (2,):
                 if index != 2:
                     raise InternalShellError(j,"Unsupported redirect on stdout")
                 result = subprocess.PIPE
             else:
                 if r[2] is None:
-                    redir_filename = None
-                    name = expand_glob(r[0], cmd_shenv.cwd)
+                    name = expand_glob(r[0])
                     if len(name) != 1:
                        raise InternalShellError(j,"Unsupported: glob in redirect expanded to multiple files")
                     name = name[0]
                     if kAvoidDevNull and name == '/dev/null':
                         r[2] = tempfile.TemporaryFile(mode=r[1])
                     elif kIsWindows and name == '/dev/tty':
                         # Simulate /dev/tty on Windows.
                         # "CON" is a special filename for the console.
                         r[2] = open("CON", r[1])
                     else:
-                        # Make sure relative paths are relative to the cwd.
-                        redir_filename = os.path.join(cmd_shenv.cwd, name)
-                        r[2] = open(redir_filename, r[1])
+                        r[2] = open(name, r[1])
                     # Workaround a Win32 and/or subprocess bug when appending.
                     #
                     # FIXME: Actually, this is probably an instance of PR6753.
                     if r[1] == 'a':
                         r[2].seek(0, 2)
-                    opened_files.append(tuple(r) + (redir_filename,))
+                    opened_files.append(tuple(r) + (name,))
                 result = r[2]
             final_redirects.append(result)
 
         stdin, stdout, stderr = final_redirects
 
         # If stderr wants to come from stdout, but stdout isn't a pipe, then put
         # stderr on a pipe and treat it as stdout.
         if (stderr == subprocess.STDOUT and stdout != subprocess.PIPE):
for i,j in enumerate(cmd.commands):
                 stderr = tempfile.TemporaryFile(mode='w+b')
                 stderrTempFiles.append((i, stderr))
 
         # Resolve the executable path ourselves.
         args = list(j.args)
         executable = None
         # For paths relative to cwd, use the cwd of the shell environment.
         if args[0].startswith('.'):
-            exe_in_cwd = os.path.join(cmd_shenv.cwd, args[0])
-            if os.path.isfile(exe_in_cwd):
-                executable = exe_in_cwd
+            excutable = args[0]
         if not executable:
             executable = lit.util.which(args[0], cmd_shenv.env['PATH'])
         if not executable:
             raise InternalShellError(j, '%r: command not found' % j.args[0])
 
         # Replace uses of /dev/null with temporary files.
         if kAvoidDevNull:
             for i,arg in enumerate(args):
                 if arg == "/dev/null":
                     f = tempfile.NamedTemporaryFile(delete=False)
                     f.close()
                     named_temp_files.append(f.name)
                     args[i] = f.name
 
         # Expand all glob expressions
-        args = expand_glob_expressions(args, cmd_shenv.cwd)
+        args = expand_glob_expressions(args)
 
         # On Windows, do our own command line quoting for better compatibility
         # with some core utility distributions.
         if kIsWindows:
             args = quote_windows_command(args)
 
         try:
-            procs.append(subprocess.Popen(args, cwd=cmd_shenv.cwd,
+            procs.append(subprocess.Popen(args,
                                           executable = executable,
                                           stdin = stdin,
                                           stdout = stdout,
                                           stderr = stderr,
                                           env = cmd_shenv.env,
                                           close_fds = kUseCloseFDs))
             # Let the helper know about this process
             timeoutHelper.addProcess(procs[-1])
for f in named_temp_files:
             pass
 
     if cmd.negate:
         exitCode = not exitCode
 
     return exitCode
 
 def executeScriptInternal(test, litConfig, tmpBase, commands, cwd):
+    os.chdir(cwd)
     cmds = []
     for ln in commands:
         try:
             cmds.append(ShUtil.ShParser(ln, litConfig.isWindows,
                                         test.config.pipefail).parse())
         except:
             return lit.Test.Result(Test.FAIL, "shell parser error on: %r" % ln)
 
     cmd = cmds[0]
     for c in cmds[1:]:
         cmd = ShUtil.Seq(cmd, '&&', c)
 
     results = []
     timeoutInfo = None
     try:
-        shenv = ShellEnvironment(cwd, test.config.environment)
+        shenv = ShellEnvironment(test.config.environment)
         exitCode, timeoutInfo = executeShCmd(cmd, shenv, results, timeout=litConfig.maxIndividualTestTime)
     except InternalShellError:
         e = sys.exc_info()[1]
         exitCode = 127
         results.append(
             ShellCommandResult(e.command, '', e.message, exitCode, False))
 
     out = err = ''