Merge branch 'develop' of github.com:amoffat/sh into develop

sh.py

@@ -538,7 +538,7 @@ def canonicalize(path):
     return os.path.abspath(os.path.expanduser(path))
 
 
-def which(program, paths=None):
+def _which(program, paths=None):
     """ takes a program name or full path, plus an optional collection of search
     paths, and returns the full path of the requested executable.  if paths is
     specified, it is the entire list of search paths, and the PATH env is not
@@ -580,14 +580,14 @@ def is_exe(file_path):
 
 
 def resolve_command_path(program):
-    path = which(program)
+    path = _which(program)
     if not path:
         # our actual command might have a dash in it, but we can't call
         # that from python (we have to use underscores), so we'll check
         # if a dash version of our underscore command exists and use that
         # if it does
         if "_" in program:
-            path = which(program.replace("_", "-"))
+            path = _which(program.replace("_", "-"))
         if not path:
             return None
     return path
@@ -994,7 +994,7 @@ def __int__(self):
 
 
 def output_redirect_is_filename(out):
-    return isinstance(out, basestring)
+    return isinstance(out, basestring) or hasattr(out, '__fspath__')
 
 
 def get_prepend_stack():
@@ -1294,7 +1294,7 @@ class Command(object):
     )
 
     def __init__(self, path, search_paths=None):
-        found = which(path, search_paths)
+        found = _which(path, search_paths)
 
         self._path = encode_to_py3bytes_or_py2str("")
 
@@ -2416,6 +2416,7 @@ def is_alive(self):
                 return False, self.exit_code
             return True, self.exit_code
 
+        witnessed_end = False
         try:
             # WNOHANG is just that...we're calling waitpid without hanging...
             # essentially polling the process.  the return result is (0, 0) if
@@ -2424,7 +2425,7 @@ def is_alive(self):
             pid, exit_code = no_interrupt(os.waitpid, self.pid, os.WNOHANG)
             if pid == self.pid:
                 self.exit_code = handle_process_exit_code(exit_code)
-                self._process_just_ended()
+                witnessed_end = True
 
                 return False, self.exit_code
 
@@ -2435,6 +2436,8 @@ def is_alive(self):
             return True, self.exit_code
         finally:
             self._wait_lock.release()
+            if witnessed_end:
+                self._process_just_ended()
 
     def _process_just_ended(self):
         if self._timeout_timer:
@@ -2470,31 +2473,32 @@ def wait(self):
 
             else:
                 self.log.debug("exit code already set (%d), no need to wait", self.exit_code)
+        self._process_exit_cleanup(witnessed_end=witnessed_end)
+        return self.exit_code
 
-            self._quit_threads.set()
+    def _process_exit_cleanup(self, witnessed_end):
+        self._quit_threads.set()
 
-            # we may not have a thread for stdin, if the pipe has been connected
-            # via _piped="direct"
-            if self._input_thread:
-                self._input_thread.join()
+        # we may not have a thread for stdin, if the pipe has been connected
+        # via _piped="direct"
+        if self._input_thread:
+            self._input_thread.join()
 
-            # wait, then signal to our output thread that the child process is
-            # done, and we should have finished reading all the stdout/stderr
-            # data that we can by now
-            timer = threading.Timer(2.0, self._stop_output_event.set)
-            timer.start()
+        # wait, then signal to our output thread that the child process is
+        # done, and we should have finished reading all the stdout/stderr
+        # data that we can by now
+        timer = threading.Timer(2.0, self._stop_output_event.set)
+        timer.start()
 
-            # wait for our stdout and stderr streamreaders to finish reading and
-            # aggregating the process output
-            self._output_thread.join()
-            timer.cancel()
+        # wait for our stdout and stderr streamreaders to finish reading and
+        # aggregating the process output
+        self._output_thread.join()
+        timer.cancel()
 
-            self._background_thread.join()
-
-            if witnessed_end:
-                self._process_just_ended()
+        self._background_thread.join()
 
-            return self.exit_code
+        if witnessed_end:
+            self._process_just_ended()
 
 
 def input_thread(log, stdin, is_alive, quit_thread, close_before_term):
@@ -3296,16 +3300,22 @@ def __getitem__(self, k):
         if k.startswith("__") and k.endswith("__"):
             raise AttributeError
 
-        # is it a custom builtin?
-        builtin = getattr(self, "b_" + k, None)
-        if builtin:
-            return builtin
+        if k == 'cd':
+            # Don't resolve the system binary. It's useful in scripts to be
+            # able to switch directories in the current process. Can also be
+            # used as a context manager.
+            return Cd
 
         # is it a command?
         cmd = resolve_command(k, self.baked_args)
         if cmd:
             return cmd
 
+        # is it a custom builtin?
+        builtin = getattr(self, "b_" + k, None)
+        if builtin:
+            return builtin
+
         # how about an environment variable?
         # this check must come after testing if its a command, because on some
         # systems, there are an environment variables that can conflict with
@@ -3319,20 +3329,25 @@ def __getitem__(self, k):
         # nothing found, raise an exception
         raise CommandNotFound(k)
 
-    # methods that begin with "b_" are custom builtins and will override any
-    # program that exists in our path.  this is useful for things like
-    # common shell builtins that people are used to, but which aren't actually
-    # full-fledged system binaries
-    @staticmethod
-    def b_cd(path=None):
-        if path:
-            os.chdir(path)
-        else:
-            os.chdir(os.path.expanduser('~'))
-
+    # Methods that begin with "b_" are implementations of shell built-ins that
+    # people are used to, but which may not have an executable equivalent.
     @staticmethod
     def b_which(program, paths=None):
-        return which(program, paths)
+        return _which(program, paths)
+
+
+class Cd(object):
+    def __new__(cls, path=None):
+        res = super(Cd, cls).__new__(cls)
+        res.old_path = os.getcwd()
+        os.chdir(path or os.path.expanduser('~'))
+        return res
+
+    def __enter__(self):
+        pass
+
+    def __exit__(self, exc_type, exc_val, exc_tb):
+        os.chdir(self.old_path)
 
 
 class Contrib(ModuleType):  # pragma: no cover

test.py

@@ -136,6 +136,7 @@ def requires_progs(*progs):
 not_macos = skipUnless(not IS_MACOS, "Doesn't work on MacOS")
 requires_py3 = skipUnless(IS_PY3, "Test only works on Python 3")
 requires_py35 = skipUnless(IS_PY3 and MINOR_VER >= 5, "Test only works on Python 3.5 or higher")
+requires_py36 = skipUnless(IS_PY3 and MINOR_VER >= 6, "Test only works on Python 3.6 or higher")
 
 
 def requires_poller(poller):
@@ -243,7 +244,7 @@ def tearDown(self):
 
     def test_print_command(self):
         from sh import ls, which
-        actual_location = which("ls")
+        actual_location = str(which("ls")).strip()
         out = str(ls)
         self.assertEqual(out, actual_location)
 
@@ -578,12 +579,15 @@ def test_environment(self):
         self.assertEqual(out, "{'HERP': 'DERP'}")
 
     def test_which(self):
-        from sh import which, ls
+        # Test 'which' as built-in function
+        from sh import ls
+        which = sh._SelfWrapper__env.b_which
         self.assertEqual(which("fjoawjefojawe"), None)
         self.assertEqual(which("ls"), str(ls))
 
     def test_which_paths(self):
-        from sh import which
+        # Test 'which' as built-in function
+        which = sh._SelfWrapper__env.b_which
         py = create_tmp_test("""
 print("hi")
 """)
@@ -754,7 +758,7 @@ def do_import():
     def test_command_wrapper_equivalence(self):
         from sh import Command, ls, which
 
-        self.assertEqual(Command(which("ls")), ls)
+        self.assertEqual(Command(str(which("ls")).strip()), ls)
 
     def test_doesnt_execute_directories(self):
         save_path = os.environ['PATH']
@@ -959,8 +963,8 @@ def test_custom_long_prefix(self):
     def test_command_wrapper(self):
         from sh import Command, which
 
-        ls = Command(which("ls"))
-        wc = Command(which("wc"))
+        ls = Command(str(which("ls")).strip())
+        wc = Command(str(which("wc")).strip())
 
         c1 = int(wc(ls("-A1"), l=True))  # noqa: E741
         c2 = len(os.listdir("."))
@@ -1772,6 +1776,15 @@ def test_out_filename(self):
         outfile.seek(0)
         self.assertEqual(b"output\n", outfile.read())
 
+    @requires_py36
+    def test_out_pathlike(self):
+        from pathlib import Path
+        outfile = tempfile.NamedTemporaryFile()
+        py = create_tmp_test("print('output')")
+        python(py.name, _out=Path(outfile.name))
+        outfile.seek(0)
+        self.assertEqual(b"output\n", outfile.read())
+
     def test_bg_exit_code(self):
         py = create_tmp_test("""
 import time
@@ -2258,8 +2271,6 @@ def test_pushd(self):
 
     def test_pushd_cd(self):
         """ test that pushd works like pushd/popd with built-in cd correctly """
-        import sh
-
         child = realpath(tempfile.mkdtemp())
         try:
             old_wd = os.getcwd()
@@ -2280,6 +2291,12 @@ def test_cd_homedir(self):
         self.assertNotEqual(orig, os.getcwd())
         self.assertEqual(my_dir, os.getcwd())
 
+    def test_cd_context_manager(self):
+        orig = os.getcwd()
+        with sh.cd(tempdir):
+            self.assertEqual(tempdir, os.getcwd())
+        self.assertEqual(orig, os.getcwd())
+
     def test_non_existant_cwd(self):
         from sh import ls
 
@@ -2330,6 +2347,29 @@ def __call__(self, p, success, exit_code):
         self.assertEqual(callback.exit_code, 0)
         self.assertTrue(callback.success)
 
+    # https://github.com/amoffat/sh/issues/564
+    def test_done_callback_no_deadlock(self):
+        import time
+
+        py = create_tmp_test("""
+from sh import sleep
+
+def done(cmd, success, exit_code):
+    print(cmd, success, exit_code)
+
+sleep('1', _done=done)
+""")
+
+        p = python(py.name, _bg=True, _timeout=2)
+
+        # do a little setup to prove that a command with a _done callback is run
+        # in the background
+        wait_start = time.time()
+        p.wait()
+        wait_elapsed = time.time() - wait_start
+
+        self.assertLess(abs(wait_elapsed - 1.0), 1.0)
+
     def test_fork_exc(self):
         from sh import ForkException