Added 'cd -' built in to lit

[jmittert]

Summary

Pass '-' to cd return to the previous directory. I had encountered some (non
llvm) lit tests which use (cd %t && ... ) to run commands and this seemed
like a more portable way to do that.

[rnk]

Can you add a quick test to the lit test suite for this? Shouldn't be too hard, make some dirs, cd into and out, pwd, filecheck the output.

[jmittert]

Added a test for cd -

[rnk]

lgtm

[jmittert]

I don't have commit access, can you commit this for me please?

[joerg]

I'm quite against this. "cd -" is a non-portable extension to shell and should never be used in first place.

[jmittert]

Hmm, would pushd/popd be a more reasonable alternative then? I didn't want to have to maintain the extra state.

Edit: Actually, can you clarify what you mean by non-portable? This is being interpreted by lit.py so the underlying system/shell shouldn't matter. In addition, I though cd - is part of the posix standard. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cd.html

[joerg]

Personally, I would strongly prefer if the test cases are as explicit as possible. That makes debugging based on the llvm-lit output a lot easier. Ignoring anything else, this feature would make it a lot more tricky to figure out what exactly is going on.

[jmittert]

So then assuming I want to run a single command in a given directory, what's the recommended way of doing that if not pushd/cd -? (cd %t && ...) currently doesn't work on Windows which is what I was trying to work around.

[rnk]

In D54328#1294801, @joerg wrote:

I'm quite against this. "cd -" is a non-portable extension to shell and should never be used in first place.

Can you elaborate? In what way is cd - non-portable? Is it a bash-ism? If it is, why should lit care? Lit looks for "bash" by name on every platform, so far as I know.

[joerg]

As I said: because it makes it harder to rerun parts of the test interactively. Which is a good enough reason to forbid cd - as well as any directory stack operations, IMO.

[rnk]

In D54328#1297688, @joerg wrote:

As I said: because it makes it harder to rerun parts of the test interactively. Which is a good enough reason to forbid cd - as well as any directory stack operations, IMO.

Is your objection that you would prefer that tests remain written as (cd dir && dosomething), and not use cd -? If so, I think it would be better to comment on the reviews that migrate the tests from one pattern to another. cd - is a generally available bash feature that any test author could use anywhere. It's really up to the individuals owners writing and maintaining tests as to whether they care more about using subshells vs. having their test run on Windows.

My understanding is that implementing the full set of features offered by subshells is pretty out of scope for lit. To be faithful, we'd really need to start launching a shell process that updates its real CWD, environment, stdout, stdin, etc. The goal of the python lit shell is to be fast on Windows, and launching those extra processes works against that goal. I would be in favor of adding a limited amount of subshell handling, but just adding cd - seems like a reasonable shortcut to me.

[joerg]

I would actually rule out subshells as well. I see no reason why a test case should have to use them, i.e. compared to just change the working directory back afterwards. As I said, I want to be able to repeat a problematic command manually and interactively as easy as possible. Using non-portable shell features is a problem for that. Depending on state between commands is also annoying. Let's really try to keep it minimal and where necessary, adjust test case guidelines and test cases to simplify things.

[jmittert]

I can work around not having this.

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
+        self.prevCwd = cwd
         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, 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 newdir == '-':
+            shenv.prevCwd,shenv.cwd = shenv.cwd,shenv.prevCwd
+            return 0
+        shenv.prevCwd = shenv.cwd
         if os.path.isabs(newdir):
             shenv.cwd = newdir
         else:
             shenv.cwd = os.path.realpath(os.path.join(shenv.cwd, newdir))
         # The cd builtin always succeeds. If the directory does not exist, the
         # following Popen calls will fail instead.
         return 0
 

utils/lit/tests/Inputs/shtest-shell/cd.txt

+# Check cd and cd -
+#
+# RUN: mkdir -p %t/foo
+# RUN: mkdir -p %t/bar
+# RUN: mkdir -p %t/baz
+# RUN: cd %t/foo
+# RUN: pwd | FileCheck --check-prefix=FOO_PATH %s
+# RUN: cd %t/bar
+# RUN: pwd | FileCheck --check-prefix=BAR_PATH %s
+# RUN: cd -
+# RUN: pwd | FileCheck --check-prefix=FOO_PATH %s
+# RUN: cd -
+# RUN: pwd | FileCheck --check-prefix=BAR_PATH %s
+# RUN: cd %t/baz
+# RUN: pwd | FileCheck --check-prefix=BAZ_PATH %s
+# RUN: cd -
+# RUN: pwd | FileCheck --check-prefix=BAR_PATH %s
+
+# FOO_PATH: foo
+# BAR_PATH: bar
+# BAZ_PATH: baz

utils/lit/tests/shtest-shell.py

 # CHECK: FAIL: shtest-shell :: cat-error-1.txt
 # CHECK: *** TEST 'shtest-shell :: cat-error-1.txt' FAILED ***
 # CHECK: $ "cat" "temp1.txt"
 # CHECK: # command stderr:
 # CHECK: [Errno 2] No such file or directory: 'temp1.txt'
 # CHECK: error: command failed with exit status: 1
 # CHECK: ***
 
+# CHECK: PASS: shtest-shell :: cd.txt
+
 # CHECK: FAIL: shtest-shell :: colon-error.txt
 # CHECK: *** TEST 'shtest-shell :: colon-error.txt' FAILED ***
 # CHECK: $ ":"
 # CHECK: # command stderr:
 # CHECK: Unsupported: ':' cannot be part of a pipeline
 # CHECK: error: command failed with exit status: 127
 # CHECK: ***