If the dup fails in _PushDup, then don't restore FDs later in Pop().

This fixes 'echo hi 6>&1', and also gold/de1.sh (an excerpt from
debootstrap).

Addresses issue #62.

Also:
- If there are no redirects, don't push empty frames on FdState.
- remove old comments

bin/oil.py

@@ -135,9 +135,7 @@ def InteractiveLoop(opts, ex, c_parser, w_parser, line_reader):
      if opts.print_status:
        print('STATUS', repr(status))

    # Reset prompt and clear memory.  TODO: If there are any function
    # definitions ANYWHERE in the node, you should not clear the underlying
    # memory.  We still need to execute those strings!
    # Reset prompt to PS1.
    line_reader.Reset()

    # Reset internal newline state.
@@ -254,9 +252,6 @@ def OshMain(argv, login_shell):
  exec_opts = state.ExecOpts(mem)
  builtin.SetExecOpts(exec_opts, opts.opt_changes)

  # TODO: How to get a handle to initialized builtins here?
  # tokens.py has it.  I think you just make a separate table, with
  # metaprogramming.
  fd_state = process.FdState()
  ex = cmd_exec.Executor(mem, fd_state, status_lines, funcs, completion,
                         comp_lookup, exec_opts, arena)
@@ -272,8 +267,6 @@ def OshMain(argv, login_shell):
      try:
        rc_node = rc_c_parser.ParseWholeFile()
        if not rc_node:
          # TODO: Error should return a token, and then the token should have a
          # arena index, and then look that up in the arena.
          err = rc_c_parser.Error()
          ui.PrintErrorStack(err, arena, sys.stderr)
          return 2  # parse error is code 2
@@ -331,9 +324,10 @@ def OshMain(argv, login_shell):
      completion.Init(pool, builtin.BUILTIN_DEF, mem, funcs, comp_lookup,
                      status_out, ev)

    # TODO: Could instantiate "printer" instead of showing ops
    InteractiveLoop(opts, ex, c_parser, w_parser, line_reader)
    status = 0  # TODO: set code
    # TODO: status should be last command.  Start bash, type "f() { return 33;
    # }; f"
    status = 0
  else:
    # Parse the whole thing up front
    #print('Parsing file')

core/cmd_exec.py

@@ -34,7 +34,6 @@
from core import word_eval
from core import ui
from core import util

from core import builtin
from core.id_kind import Id, RedirType, REDIR_TYPE, REDIR_DEFAULT_FD
from core import process
@@ -1073,19 +1072,21 @@ def _Execute(self, node, fork_external=True):

    check_errexit = True

    if redirects is not None:
      assert isinstance(redirects, list), redirects
    if redirects is None:  # evaluation error
      status = 1  

    elif redirects:
      if self.fd_state.Push(redirects, self.waiter):
        try:
          status, check_errexit = self._Dispatch(node, fork_external)
        finally:
          self.fd_state.Pop()
        check_status = False
        #log('_dispatch returned %d', status)
      else:  # Error applying redirects, e.g. bad file descriptor.
        status = 1
    else:  # Error evaluating redirects
      status = 1

    else:  # No redirects
      status, check_errexit = self._Dispatch(node, fork_external)

    self.mem.last_status = status

@@ -1241,7 +1242,7 @@ def RunProcessSub(self, node, op_id):
    else:
      raise AssertionError

    # Generalize?
    # TODO: Generalize process sub?
    #
    # - Make it work first, bare minimum.
    # - Then Make something like Pipeline()?
@@ -1275,10 +1276,11 @@ def RunFunc(self, func_node, argv):
    # f 2>&1

    def_redirects = self._EvalRedirects(func_node)
    if def_redirects is None:
    if def_redirects is None:  # error
      return None
    if not self.fd_state.Push(def_redirects, self.waiter):
      return 1  # error
    if def_redirects:
      if not self.fd_state.Push(def_redirects, self.waiter):
        return 1  # error

    self.mem.PushCall(func_node.name, argv[1:])

@@ -1294,7 +1296,8 @@ def RunFunc(self, func_node, argv):
        e_die('Unexpected %r (in function call)', e.token.val, token=e.token)
    finally:
      self.mem.PopCall()
      self.fd_state.Pop()
      if def_redirects:
        self.fd_state.Pop()

    return status

core/process.py

@@ -18,11 +18,11 @@

from core import runtime
from core import util
from core.util import log
from core.id_kind import Id, REDIR_DEFAULT_FD

redirect_e = runtime.redirect_e
e_die = util.e_die
log = util.log


class _FdFrame:
@@ -71,14 +71,19 @@ def _PushDup(self, fd1, fd2):
    """
    Save fd2 and dup fd1 onto fd2.
    """
    #log('---- _PushDup %s %s\n', fd1, fd2)
    #log('---- _PushDup %s %s', fd1, fd2)
    need_restore = True
    try:
      #log('DUPFD %s %s', fd2, self.next_fd)
      fcntl.fcntl(fd2, fcntl.F_DUPFD, self.next_fd)
    except IOError as e:
      # Example program that causes this error: exec 4>&1.  Descriptor 4 isn't
      # open.
      # This seems to be ignored in dash too in savefd()?
      if e.errno != errno.EBADF:
      if e.errno == errno.EBADF:
        #log('ERROR %s', e)
        need_restore = False
      else:
        raise
    else:
      os.close(fd2)
@@ -96,10 +101,8 @@ def _PushDup(self, fd1, fd2):
      # Undo it
      return False

    # Oh this is wrong?
    #os.close(fd1)

    self.cur_frame.saved.append((self.next_fd, fd2))
    if need_restore:
      self.cur_frame.saved.append((self.next_fd, fd2))
    self.next_fd += 1
    return True

@@ -110,10 +113,6 @@ def _PushWait(self, proc, waiter):
    self.cur_frame.need_wait.append((proc, waiter))

  def _ApplyRedirect(self, r, waiter):
    # NOTE: We only use self for self.waiter.

    # TODO: r.fd needs to be opened?  if it's not stdin or stdout
    # https://stackoverflow.com/questions/3425021/specifying-file-descriptor-number
    ok = True

    if r.tag == redirect_e.PathRedirect:
@@ -145,7 +144,8 @@ def _ApplyRedirect(self, r, waiter):
      # saved position (10), which closes it.
      #self._PushClose(r.fd)

    elif r.tag == redirect_e.DescRedirect:
    elif r.tag == redirect_e.DescRedirect:  # e.g. echo hi 1>&2

      if r.op_id == Id.Redir_GreatAnd:  # 1>&2
        if not self._PushDup(r.target_fd, r.fd):
          ok = False
@@ -164,14 +164,14 @@ def _ApplyRedirect(self, r, waiter):
      if not self._PushDup(read_fd, r.fd):  # stdin is now the pipe
        ok = False

      # We can't close liek we do in the filename case above?  The writer can
      # We can't close like we do in the filename case above?  The writer can
      # get a "broken pipe".
      self._PushClose(read_fd)

      thunk = _HereDocWriterThunk(write_fd, r.body)

      # TODO: Use PIPE_SIZE to save a process in the case of small here docs,
      # which are the common case.
      # which are the common case.  (dash does this.)
      start_process = True
      #start_process = False

@@ -197,7 +197,7 @@ def _ApplyRedirect(self, r, waiter):
    return ok

  def Push(self, redirects, waiter):
    #log('> PushFrame')
    #log('> fd_state.Push %s', redirects)
    new_frame = _FdFrame()
    self.stack.append(new_frame)
    self.cur_frame = new_frame
@@ -221,6 +221,8 @@ def Pop(self):
        os.dup2(saved, orig)
      except OSError as e:
        log('dup2(%d, %d) error: %s', saved, orig, e)
        #log('fd state:')
        #os.system('ls -l /proc/%s/fd' % os.getpid())
        raise
      os.close(saved)
      #log('dup2 %s %s', saved, orig)

gold/de1.sh

@@ -0,0 +1,24 @@
#!/bin/bash
#
# Snippet from deboostrap.

set -e

download_debs() {
  echo download_debs
}

MIRRORS='a b'

f() {
  for m in $MIRRORS; do
    echo "m $m"
    local pkgdest="foo"
    if [ ! -e "$pkgdest" ]; then continue; fi
    pkgs_to_get="$(download_debs "$m" "$pkgdest" $pkgs_to_get 5>&1 1>&6)"
    if [ -z "$pkgs_to_get" ]; then break; fi
  done 6>&1
  echo done
}

f

test/spec.sh

@@ -356,7 +356,7 @@ here-doc() {
  # - On Debian, the whole process hangs.
  # Is this due to Python 3.2 vs 3.4?  Either way osh doesn't implement the
  # functionality, so it's probably best to just implement it.
  sh-spec spec/here-doc.test.sh --osh-failures-allowed 2 --range 0-30 \
  sh-spec spec/here-doc.test.sh --osh-failures-allowed 1 --range 0-30 \
    ${REF_SHELLS[@]} $OSH "$@"
}