[osh/cmd_exec] Search the $PATH for executables, and cache locations.

demo/path-cache.sh shows this difference.

One more spec tests passes.  Still working on the others, which will
require the 'hash' builtin.

Addresses issue #44.  Related to #355.

core/process.py

@@ -22,7 +22,6 @@
 from core import ui
 from core.util import log
 from frontend import match
-from pylib import os_
 
 import posix_ as posix
 
@@ -432,21 +431,21 @@ def __init__(self, hijack_shebang, fd_state, search_path, errfmt, debug_f):
     self.errfmt = errfmt
     self.debug_f = debug_f
 
-  def Exec(self, arg_vec, environ):
+  def Exec(self, argv0_path, arg_vec, environ):
     """Execute a program and exit this process.
 
     Called by:
       ls /
       exec ls /
       ( ls / )
     """
-    self._Exec(arg_vec.strs, arg_vec.spids[0], environ)
+    self._Exec(argv0_path, arg_vec.strs, arg_vec.spids[0], environ, True)
     # NO RETURN
 
-  def _Exec(self, argv, argv0_spid, environ):
+  def _Exec(self, argv0_path, argv, argv0_spid, environ, should_retry):
     if self.hijack_shebang:
       try:
-        f = self.fd_state.Open(argv[0])
+        f = self.fd_state.Open(argv0_path)
       except OSError as e:
         pass
       else:
@@ -466,26 +465,19 @@ def _Exec(self, argv, argv0_spid, environ):
     # TODO: If there is an error, like the file isn't executable, then we should
     # exit, and the parent will reap it.  Should it capture stderr?
     try:
-      os_.execvpe(argv[0], argv, environ)
+      posix.execve(argv0_path, argv, environ)
     except OSError as e:
       # Run with /bin/sh when ENOEXEC error (no shebang).  All shells do this.
-      if e.errno == errno.ENOEXEC and argv[0] != '/bin/sh':  # no infinite loop
-        # TODO:
-        # - Use search_path in the NON-ERROR case ('hash' builtin).
-        # - Get rid of pylib/os_.py, which was copied from the Python standard
-        #   library.
-        abs_path = self.search_path.Lookup(argv[0])
-        if abs_path is None:  # Check if file was deleted in the meantime
-          e.errno = errno.ENOENT
-        else:
-          self._Exec(['/bin/sh', abs_path] + argv[1:], argv0_spid, environ)
-          # NO RETURN
+      if e.errno == errno.ENOEXEC and should_retry:
+        self._Exec('/bin/sh', ['/bin/sh', argv0_path] + argv[1:], argv0_spid,
+                   environ, False)
+        # NO RETURN
 
       # Would be nice: when the path is relative and ENOENT: print PWD and do
       # spelling correction?
 
       self.errfmt.Print(
-          "Can't execute %r: %s", argv[0], posix.strerror(e.errno),
+          "Can't execute %r: %s", argv0_path, posix.strerror(e.errno),
           span_id=argv0_spid)
 
       # POSIX mentions 126 and 127 for two specific errors.  The rest are
@@ -527,8 +519,9 @@ def __str__(self):
 class ExternalThunk(Thunk):
   """An external executable."""
 
-  def __init__(self, ext_prog, arg_vec, environ):
+  def __init__(self, ext_prog, argv0_path, arg_vec, environ):
     self.ext_prog = ext_prog
+    self.argv0_path = argv0_path
     self.arg_vec = arg_vec
     self.environ = environ
 
@@ -544,7 +537,7 @@ def Run(self):
     """
     An ExternalThunk is run in parent for the exec builtin.
     """
-    self.ext_prog.Exec(self.arg_vec, self.environ)
+    self.ext_prog.Exec(self.argv0_path, self.arg_vec, self.environ)
 
 
 class SubProgramThunk(Thunk):

core/process_test.py

@@ -45,7 +45,15 @@ def _CommandNode(code_str, arena):
 
 def _ExtProc(argv):
   arg_vec = arg_vector(argv, [0] * len(argv))
-  return Process(ExternalThunk(_EXT_PROG, arg_vec, {}), _JOB_STATE)
+  argv0_path = None
+  for path_entry in ['/bin', '/usr/bin']:
+    full_path = os.path.join(path_entry, argv[0])
+    if os.path.exists(full_path):
+      argv0_path = full_path
+      break
+  if not argv0_path:
+    argv0_path = argv[0]  # fallback that tests failure case
+  return Process(ExternalThunk(_EXT_PROG, argv0_path, arg_vec, {}), _JOB_STATE)
 
 
 class ProcessTest(unittest.TestCase):

osh/cmd_exec.py

@@ -259,9 +259,16 @@ def _Exec(self, arg_vec):
       return 0
 
     environ = self.mem.GetExported()
+    cmd = arg_vec.strs[1]
+    argv0_path = self.search_path.CachedLookup(cmd)
+    if argv0_path is None:
+      self.errfmt.Print('exec: %r not found', cmd,
+                        span_id=arg_vec.spids[1])
+      sys.exit(127)  # exec never returns
+
     # shift off 'exec'
     arg_vec2 = arg_vector(arg_vec.strs[1:], arg_vec.spids[1:])
-    self.ext_prog.Exec(arg_vec2, environ)  # NEVER RETURNS
+    self.ext_prog.Exec(argv0_path, arg_vec2, environ)  # NEVER RETURNS
 
   def _RunBuiltinAndRaise(self, builtin_id, arg_vec, fork_external):
     """
@@ -291,7 +298,7 @@ def _RunBuiltinAndRaise(self, builtin_id, arg_vec, fork_external):
       status = self._Source(arg_vec)
 
     elif builtin_id == builtin_e.COMMAND:
-      # TODO: How do we hadnle fork_external?  It doesn't fit the common
+      # TODO: How do we handle fork_external?  It doesn't fit the common
       # signature.
       b = builtin.Command(self, self.funcs, self.aliases, self.search_path)
       status = b(arg_vec, fork_external)
@@ -547,13 +554,19 @@ def RunSimpleCommand(self, arg_vec, fork_external, funcs=True):
 
     environ = self.mem.GetExported()  # Include temporary variables
 
+    # Resolve argv[0] BEFORE forking.
+    argv0_path = self.search_path.CachedLookup(argv[0])
+    if argv0_path is None:
+      self.errfmt.Print('%r not found', argv[0], span_id=span_id)
+      return 127
+
     if fork_external:
-      thunk = process.ExternalThunk(self.ext_prog, arg_vec, environ)
+      thunk = process.ExternalThunk(self.ext_prog, argv0_path, arg_vec, environ)
       p = process.Process(thunk, self.job_state)
       status = p.Run(self.waiter)
       return status
 
-    self.ext_prog.Exec(arg_vec, environ)  # NEVER RETURNS
+    self.ext_prog.Exec(argv0_path, arg_vec, environ)  # NEVER RETURNS
 
   def _RunPipeline(self, node):
     pi = process.Pipeline()

osh/state.py

@@ -44,6 +44,7 @@ class SearchPath(object):
 
   def __init__(self, mem):
     self.mem = mem
+    self.cache = {}
 
   def Lookup(self, name):
     """
@@ -70,6 +71,26 @@ def Lookup(self, name):
 
     return None
 
+  def CachedLookup(self, name):
+    if name in self.cache:
+      return self.cache[name]
+
+    full_path = self.Lookup(name)
+    if full_path is not None:
+      self.cache[name] = full_path
+    return full_path
+
+  def MaybeRemoveEntry(self, name):
+    """When the file system changes."""
+    try:
+      del self.cache[name]
+    except KeyError:
+      pass
+
+  def ClearCache(self):
+    """For hash -r."""
+    self.cache.clear()
+
 
 class _ErrExit(object):
   """Manages the errexit setting.

test/runtime-errors.sh

@@ -549,6 +549,11 @@ builtin_wait() {
   wait 1234578
 }
 
+builtin_exec() {
+  exec nonexistent-command 1 2 3
+  echo $?
+}
+
 #
 # Strict options (see spec/strict-options.sh)
 #
@@ -636,6 +641,7 @@ all() {
     builtin_bracket builtin_builtin builtin_source builtin_cd builtin_pushd \
     builtin_popd builtin_unset builtin_alias_unalias builtin_help \
     builtin_trap builtin_getopts builtin_wait \
+    builtin_exec \
     strict_word_eval_warnings strict_arith_warnings \
     strict_control_flow_warnings control_flow_subshell; do
     _run_test $t