Big fat refactoring of how redirections work. In fish 1.x and 2.0.0, the redirections for a process were flattened into a big list associated with the job, so there was no way to tell which redirections applied to each process. Each process therefore got all the redirections associated with the job. See https://github.com/fish-shell/fish-shell/issues/877 for how this could manifest.

With this change, jobs only track their block-level redirections. Process level redirections are correctly associated with the process, and at exec time we stitch them together (block, pipe, and process redirects).

This fixes the weird issues where redirects bleed across pipelines (like #877), and also allows us to play with the order in which redirections are applied, since the final list is constructed right before it's needed.  This lets us put pipes after block level redirections but before process level redirections, so that a 2>&1-type redirection gets picked up after the pipe, i.e. it should fix https://github.com/fish-shell/fish-shell/issues/110

This is a significant change. The tests all pass. Cross your fingers.

1 files changed, 13 insertions, 8 deletions

diff --git a/parser.cpp b/parser.cpp
index af5e1107..c80d09ca 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -1168,9 +1168,9 @@ int parser_t::is_help(const wchar_t *s, int min_match) const
            (len >= (size_t)min_match && (wcsncmp(L"--help", s, len) == 0));
 }
 
-job_t *parser_t::job_create(void)
+job_t *parser_t::job_create()
 {
-    job_t *res = new job_t(acquire_job_id());
+    job_t *res = new job_t(acquire_job_id(), this->block_io);
     this->my_job_list.push_front(res);
 
     job_set_flag(res,
@@ -1256,6 +1256,9 @@ void parser_t::parse_job_argument_list(process_t *p,
     wcstring unmatched;
     int unmatched_pos=0;
 
+    /* The set of IO redirections that we construct for the process */
+    io_chain_t process_io_chain;
+
     /*
       Test if this is the 'count' command. We need to special case
       count in the shell, since it should display a help message on
@@ -1559,7 +1562,7 @@ void parser_t::parse_job_argument_list(process_t *p,
 
                 if (new_io.get() != NULL)
                 {
-                    j->append_io(new_io);
+                    process_io_chain.push_back(new_io);
                 }
 
             }
@@ -1613,7 +1616,9 @@ void parser_t::parse_job_argument_list(process_t *p,
         }
     }
 
-    return;
+    /* Store our IO chain. The existing chain should be empty. */
+    assert(p->io_chain().empty());
+    p->set_io_chain(process_io_chain);
 }
 
 /*
@@ -2256,7 +2261,7 @@ void parser_t::skipped_exec(job_t * j)
             {
                 if (!current_block->outer->skip)
                 {
-                    exec(*this, j);
+                    exec_job(*this, j);
                     return;
                 }
                 parser_t::pop_block();
@@ -2269,7 +2274,7 @@ void parser_t::skipped_exec(job_t * j)
                     const if_block_t *ib = static_cast<const if_block_t*>(current_block);
                     if (ib->if_expr_evaluated && ! ib->any_branch_taken)
                     {
-                        exec(*this, j);
+                        exec_job(*this, j);
                         return;
                     }
                 }
@@ -2278,7 +2283,7 @@ void parser_t::skipped_exec(job_t * j)
             {
                 if (current_block->type() == SWITCH)
                 {
-                    exec(*this, j);
+                    exec_job(*this, j);
                     return;
                 }
             }
@@ -2415,7 +2420,7 @@ void parser_t::eval_job(tokenizer_t *tok)
                     if (j->first_process->type==INTERNAL_BUILTIN && !j->first_process->next)
                         was_builtin = 1;
                     scoped_push<int> tokenizer_pos_push(&current_tokenizer_pos, job_begin_pos);
-                    exec(*this, j);
+                    exec_job(*this, j);
 
                     /* Only external commands require a new fishd barrier */
                     if (!was_builtin)