diff --git a/Shell/AST.cpp b/Shell/AST.cpp index b71b751d7d..658494982e 100644 --- a/Shell/AST.cpp +++ b/Shell/AST.cpp @@ -184,22 +184,9 @@ void And::dump(int level) const RefPtr And::run(RefPtr shell) { - auto left = m_left->run(shell); - ASSERT(left->is_job()); - - auto* job_value = static_cast(left.ptr()); - const auto job = job_value->job(); - if (!job) { - // Something has gone wrong, let's just pretend that the job failed. - return job_value; - } - - shell->block_on_job(job); - - if (job->exit_code() == 0) - return m_right->run(shell); - - return job_value; + auto commands = m_left->run(shell)->resolve_as_commands(shell); + commands.last().next_chain.append(NodeWithAction { *m_right, NodeWithAction::And }); + return create(move(commands)); } void And::highlight_in_editor(Line::Editor& editor, Shell& shell, HighlightMetadata metadata) @@ -354,16 +341,11 @@ RefPtr Background::run(RefPtr shell) // all but their last subnode before yielding to this, causing a command // like `foo && bar&` to effectively be `foo && (bar&)`. auto value = m_command->run(shell)->resolve_without_cast(shell); - if (value->is_job()) { - auto job = static_cast(value.ptr())->job(); - job->set_running_in_background(true); - job->set_should_announce_exit(true); - return value; - } + ASSERT(!value->is_job()); auto commands = value->resolve_as_commands(shell); - auto& last = commands.last(); - last.should_wait = false; + for (auto& command : commands) + command.should_wait = false; return create(move(commands)); } @@ -925,7 +907,7 @@ void Execute::for_each_entry(RefPtr shell, Function shell, Function last_job; + auto jobs = shell->run_commands(commands); - for (auto& job : shell->run_commands(commands)) { - shell->block_on_job(job); - last_job = move(job); - } - - callback(create(move(last_job))); - - return; + if (!jobs.is_empty()) + callback(create(&jobs.last())); } RefPtr Execute::run(RefPtr shell) @@ -1150,7 +1126,7 @@ RefPtr IfCond::run(RefPtr shell) if (cond_job->signaled()) return create({}); // Exit early. - if (cond_job->exit_code() == 0) { + if (shell->last_return_code == 0) { if (m_true_branch) return m_true_branch->run(shell); } else { @@ -1287,23 +1263,9 @@ void Or::dump(int level) const RefPtr Or::run(RefPtr shell) { - - auto left = m_left->run(shell); - ASSERT(left->is_job()); - - auto* job_value = static_cast(left.ptr()); - const auto job = job_value->job(); - if (!job) { - // Something has gone wrong, let's just pretend that the job failed. - return m_right->run(shell); - } - - shell->block_on_job(job); - - if (job->exit_code() == 0) - return job_value; - - return m_right->run(shell); + auto commands = m_left->run(shell)->resolve_as_commands(shell); + commands.last().next_chain.empend(*m_right, NodeWithAction::Or); + return create(move(commands)); } void Or::highlight_in_editor(Line::Editor& editor, Shell& shell, HighlightMetadata metadata) @@ -1549,6 +1511,7 @@ RefPtr Sequence::run(RefPtr shell) execute_node = create(m_right->position(), m_right); return execute_node->run(shell); } + auto left = m_left->run(shell)->resolve_as_commands(shell); // This could happen if a comment is next to a command. if (left.size() == 1) { @@ -1557,13 +1520,12 @@ RefPtr Sequence::run(RefPtr shell) return m_right->run(shell); } - auto right = m_right->run(shell)->resolve_as_commands(shell); + if (left.last().should_wait) + left.last().next_chain.append(NodeWithAction { *m_right, NodeWithAction::Sequence }); + else + left.append(m_right->run(shell)->resolve_as_commands(shell)); - Vector commands; - commands.append(left); - commands.append(right); - - return create(move(commands)); + return create(move(left)); } void Sequence::highlight_in_editor(Line::Editor& editor, Shell& shell, HighlightMetadata metadata) diff --git a/Shell/AST.h b/Shell/AST.h index b4154a1990..ce8d7358b4 100644 --- a/Shell/AST.h +++ b/Shell/AST.h @@ -28,6 +28,7 @@ #include "Forward.h" #include "Job.h" +#include #include #include #include @@ -174,13 +175,30 @@ public: pid_t pgid { -1 }; }; +struct NodeWithAction { + mutable NonnullRefPtr node; + enum Action { + And, + Or, + Sequence, + } action; + + NodeWithAction(Node& node, Action action) + : node(node) + , action(action) + { + } +}; + struct Command { Vector argv; NonnullRefPtrVector redirections; - mutable RefPtr pipeline; bool should_wait { true }; bool is_pipe_source { false }; bool should_notify_if_in_background { true }; + + mutable RefPtr pipeline; + Vector next_chain; }; struct HitTestResult { @@ -215,7 +233,7 @@ public: } CommandValue(Vector argv) - : m_command({ move(argv), {}, {}, true, false, true }) + : m_command({ move(argv), {}, true, false, true, nullptr, {} }) { } @@ -424,7 +442,6 @@ private: virtual void highlight_in_editor(Line::Editor&, Shell&, HighlightMetadata = {}) override; virtual HitTestResult hit_test_position(size_t) override; virtual String class_name() const override { return "And"; } - virtual bool would_execute() const override { return true; } RefPtr m_left; RefPtr m_right; @@ -458,7 +475,6 @@ private: virtual void highlight_in_editor(Line::Editor&, Shell&, HighlightMetadata = {}) override; virtual HitTestResult hit_test_position(size_t) override; virtual String class_name() const override { return "Background"; } - virtual bool would_execute() const override { return m_command->would_execute(); } RefPtr m_command; }; @@ -725,7 +741,6 @@ private: virtual void highlight_in_editor(Line::Editor&, Shell&, HighlightMetadata = {}) override; virtual HitTestResult hit_test_position(size_t) override; virtual String class_name() const override { return "Or"; } - virtual bool would_execute() const override { return true; } RefPtr m_left; RefPtr m_right; diff --git a/Shell/Forward.h b/Shell/Forward.h index 5cf82ce2f0..bbe92ea807 100644 --- a/Shell/Forward.h +++ b/Shell/Forward.h @@ -29,10 +29,11 @@ class Shell; namespace AST { +struct Command; class Node; class Value; class SyntaxError; class Pipeline; -class Rewiring; +struct Rewiring; } diff --git a/Shell/Job.cpp b/Shell/Job.cpp index 498fc850c1..8584695099 100644 --- a/Shell/Job.cpp +++ b/Shell/Job.cpp @@ -26,6 +26,7 @@ #include "Job.h" #include "AST.h" +#include "Shell.h" #include #include #include @@ -70,3 +71,42 @@ bool Job::print_status(PrintStatusMode mode) return true; } + +Job::Job(pid_t pid, unsigned pgid, String cmd, u64 job_id, AST::Command&& command) + : m_pgid(pgid) + , m_pid(pid) + , m_job_id(job_id) + , m_cmd(move(cmd)) +{ + m_command = make(move(command)); + + set_running_in_background(false); + m_command_timer.start(); +} + +void Job::set_has_exit(int exit_code) +{ + if (m_exited) + return; + m_exit_code = exit_code; + m_exited = true; + if (on_exit) + on_exit(*this); +} + +void Job::set_signalled(int sig) +{ + if (m_exited) + return; + m_exited = true; + m_exit_code = 126; + m_term_sig = sig; + if (on_exit) + on_exit(*this); +} + +void Job::unblock() const +{ + if (!m_exited && on_exit) + on_exit(*this); +} diff --git a/Shell/Job.h b/Shell/Job.h index 394feef7b6..9f89bf54b8 100644 --- a/Shell/Job.h +++ b/Shell/Job.h @@ -41,9 +41,11 @@ # undef JOB_TIME_INFO #endif +struct LocalFrame; + class Job : public RefCounted { public: - static NonnullRefPtr create(pid_t pid, pid_t pgid, String command, u64 job_id, AST::Pipeline* pipeline = nullptr) { return adopt(*new Job(pid, pgid, move(command), job_id, pipeline)); } + static NonnullRefPtr create(pid_t pid, pid_t pgid, String cmd, u64 job_id, AST::Command&& command) { return adopt(*new Job(pid, pgid, move(cmd), job_id, move(command))); } ~Job() { @@ -53,13 +55,15 @@ public: dbg() << "Command \"" << m_cmd << "\" finished in " << elapsed << " ms"; } #endif - if (m_pipeline) - m_pipeline = nullptr; } + Function)> on_exit; + pid_t pgid() const { return m_pgid; } pid_t pid() const { return m_pid; } const String& cmd() const { return m_cmd; } + const AST::Command& command() const { return *m_command; } + AST::Command* command_ptr() { return m_command; } u64 job_id() const { return m_job_id; } bool exited() const { return m_exited; } bool signaled() const { return m_term_sig != -1; } @@ -78,34 +82,12 @@ public: bool is_running_in_background() const { return m_running_in_background; } bool should_announce_exit() const { return m_should_announce_exit; } bool is_suspended() const { return m_is_suspended; } - void unblock() const - { - if (!m_exited && on_exit) - on_exit(*this); - } - Function)> on_exit; + void unblock() const; Core::ElapsedTimer& timer() { return m_command_timer; } - void set_has_exit(int exit_code) - { - if (m_exited) - return; - m_exit_code = exit_code; - m_exited = true; - if (on_exit) - on_exit(*this); - } - void set_signalled(int sig) - { - if (m_exited) - return; - m_exited = true; - m_exit_code = 126; - m_term_sig = sig; - if (on_exit) - on_exit(*this); - } + void set_has_exit(int exit_code); + void set_signalled(int sig); void set_is_suspended(bool value) const { m_is_suspended = value; } @@ -127,18 +109,7 @@ public: bool print_status(PrintStatusMode); private: - Job(pid_t pid, unsigned pgid, String cmd, u64 job_id, AST::Pipeline* pipeline) - : m_pgid(pgid) - , m_pid(pid) - , m_job_id(job_id) - , m_cmd(move(cmd)) - { - if (pipeline) - m_pipeline = *pipeline; - - set_running_in_background(false); - m_command_timer.start(); - } + Job(pid_t pid, unsigned pgid, String cmd, u64 job_id, AST::Command&& command); pid_t m_pgid { 0 }; pid_t m_pid { 0 }; @@ -153,5 +124,5 @@ private: mutable bool m_active { true }; mutable bool m_is_suspended { false }; bool m_should_be_disowned { false }; - RefPtr m_pipeline; + OwnPtr m_command; }; diff --git a/Shell/Parser.cpp b/Shell/Parser.cpp index 4d4b3276ae..650a78cd8d 100644 --- a/Shell/Parser.cpp +++ b/Shell/Parser.cpp @@ -283,7 +283,7 @@ RefPtr Parser::parse_or_logical_sequence() if (!right_and_sequence) right_and_sequence = create("Expected an expression after '||'"); - return create(create(move(and_sequence)), create(move(right_and_sequence))); + return create(move(and_sequence), move(right_and_sequence)); } RefPtr Parser::parse_and_logical_sequence() @@ -305,7 +305,7 @@ RefPtr Parser::parse_and_logical_sequence() if (!right_and_sequence) right_and_sequence = create("Expected an expression after '&&'"); - return create(create(move(pipe_sequence)), create(move(right_and_sequence))); + return create(move(pipe_sequence), move(right_and_sequence)); } RefPtr Parser::parse_pipe_sequence() diff --git a/Shell/Shell.cpp b/Shell/Shell.cpp index 4e14d7bad5..77a1459445 100644 --- a/Shell/Shell.cpp +++ b/Shell/Shell.cpp @@ -421,15 +421,7 @@ int Shell::run_command(const StringView& cmd) tcgetattr(0, &termios); - auto result = command->run(*this); - if (result->is_job()) { - auto job_result = static_cast(result.ptr()); - auto job = job_result->job(); - if (!job) - last_return_code = 0; - else if (job->exited()) - last_return_code = job->exit_code(); - } + command->run(*this); return last_return_code; } @@ -500,8 +492,11 @@ RefPtr Shell::run_command(const AST::Command& command) } int retval = 0; - if (run_builtin(command, rewirings, retval)) + if (run_builtin(command, rewirings, retval)) { + for (auto& next_in_chain : command.next_chain) + run_tail(next_in_chain, retval); return nullptr; + } Vector argv; Vector copy_argv = command.argv; @@ -619,15 +614,19 @@ RefPtr Shell::run_command(const AST::Command& command) StringBuilder cmd; cmd.join(" ", command.argv); - auto job = Job::create(child, pgid, cmd.build(), find_last_job_id() + 1, command.pipeline); + auto job = Job::create(child, pgid, cmd.build(), find_last_job_id() + 1, AST::Command(command)); jobs.set((u64)child, job); - job->on_exit = [](auto job) { + job->on_exit = [this](auto job) { if (!job->exited()) return; if (job->is_running_in_background() && job->should_announce_exit()) fprintf(stderr, "Shell: Job %" PRIu64 "(%s) exited\n", job->job_id(), job->cmd().characters()); + + last_return_code = job->exit_code(); job->disown(); + + run_tail(job); }; fds.collect(); @@ -635,9 +634,42 @@ RefPtr Shell::run_command(const AST::Command& command) return *job; } +void Shell::run_tail(const AST::NodeWithAction& next_in_chain, int head_exit_code) +{ + switch (next_in_chain.action) { + case AST::NodeWithAction::And: + if (head_exit_code == 0) { + auto commands = next_in_chain.node->run(*this)->resolve_as_commands(*this); + run_commands(commands); + } + break; + case AST::NodeWithAction::Or: + if (head_exit_code != 0) { + auto commands = next_in_chain.node->run(*this)->resolve_as_commands(*this); + run_commands(commands); + } + break; + case AST::NodeWithAction::Sequence: + auto commands = next_in_chain.node->run(*this)->resolve_as_commands(*this); + run_commands(commands); + break; + } +} + +void Shell::run_tail(RefPtr job) +{ + if (auto cmd = job->command_ptr()) { + deferred_invoke([=, this](auto&) { + for (auto& next_in_chain : cmd->next_chain) { + run_tail(next_in_chain, job->exit_code()); + } + }); + } +} + NonnullRefPtrVector Shell::run_commands(Vector& commands) { - NonnullRefPtrVector jobs_to_wait_for; + NonnullRefPtrVector spawned_jobs; for (auto& command : commands) { #ifdef SH_DEBUG @@ -663,21 +695,19 @@ NonnullRefPtrVector Shell::run_commands(Vector& commands) if (!job) continue; + spawned_jobs.append(*job); if (command.should_wait) { block_on_job(job); - if (!job->is_suspended()) - jobs_to_wait_for.append(*job); } else { if (command.is_pipe_source) { job->set_running_in_background(true); - jobs_to_wait_for.append(*job); } else if (command.should_notify_if_in_background) { job->set_should_announce_exit(true); } } } - return jobs_to_wait_for; + return spawned_jobs; } bool Shell::run_file(const String& filename, bool explicitly_invoked) @@ -709,6 +739,9 @@ void Shell::block_on_job(RefPtr job) if (!job) return; + if (job->is_suspended()) + return; // We cannot wait for a suspended job. + ScopeGuard io_restorer { [&]() { if (job->exited() && !job->is_running_in_background()) { restore_ios(); diff --git a/Shell/Shell.h b/Shell/Shell.h index c842bbc28b..9376800bd2 100644 --- a/Shell/Shell.h +++ b/Shell/Shell.h @@ -195,6 +195,9 @@ private: const Job* m_current_job { nullptr }; LocalFrame* find_frame_containing_local_variable(const String& name); + void run_tail(RefPtr); + void run_tail(const AST::NodeWithAction&, int head_exit_code); + virtual void custom_event(Core::CustomEvent&) override; #define __ENUMERATE_SHELL_BUILTIN(builtin) \ diff --git a/Shell/Tests/if.sh b/Shell/Tests/if.sh index 342579da26..598a9471bc 100644 --- a/Shell/Tests/if.sh +++ b/Shell/Tests/if.sh @@ -1,13 +1,17 @@ #!/bin/sh +setopt --verbose + if test 1 -eq 1 { # Are comments ok? # Basic 'if' structure, empty block. if true { } else { + echo "if true runs false branch" exit 2 } if false { + echo "if false runs true branch" exit 2 } else { } diff --git a/Shell/Tests/valid.sh b/Shell/Tests/valid.sh index 6453d5f21b..c6a4918202 100644 --- a/Shell/Tests/valid.sh +++ b/Shell/Tests/valid.sh @@ -23,69 +23,69 @@ test "$(echo yes)" = yes || exit 2 alias fail="echo Failure: " # Redirections. -test -z "$(echo foo > /dev/null)" || fail direct path redirection -test -z "$(echo foo 2> /dev/null 1>&2)" || fail indirect redirection -test -n "$(echo foo 2> /dev/null)" || fail fds interfere with each other +test -z "$(echo foo > /dev/null)" || fail direct path redirection && exit 2 +test -z "$(echo foo 2> /dev/null 1>&2)" || fail indirect redirection && exit 2 +test -n "$(echo foo 2> /dev/null)" || fail fds interfere with each other && exit 2 # Argument unpack -test "$(echo (yes))" = yes || fail arguments inside bare lists -test "$(echo (no)() yes)" = yes || fail arguments inside juxtaposition: empty -test "$(echo (y)(es))" = yes || fail arguments inside juxtaposition: list -test "$(echo "y"es)" = yes || fail arguments inside juxtaposition: string +test "$(echo (yes))" = yes || fail arguments inside bare lists && exit 2 +test "$(echo (no)() yes)" = yes || fail arguments inside juxtaposition: empty && exit 2 +test "$(echo (y)(es))" = yes || fail arguments inside juxtaposition: list && exit 2 +test "$(echo "y"es)" = yes || fail arguments inside juxtaposition: string && exit 2 # String substitution foo=yes -test "$(echo $foo)" = yes || fail simple string var lookup -test "$(echo "$foo")" = yes || fail stringified string var lookup +test "$(echo $foo)" = yes || fail simple string var lookup && exit 2 +test "$(echo "$foo")" = yes || fail stringified string var lookup && exit 2 # List substitution foo=(yes) # Static lookup, as list -test "$(echo $foo)" = yes || fail simple list var lookup +test "$(echo $foo)" = yes || fail simple list var lookup && exit 2 # Static lookup, stringified -test "$(echo "$foo")" = yes || fail stringified list var lookup +test "$(echo "$foo")" = yes || fail stringified list var lookup && exit 2 # Dynamic lookup through static expression -test "$(echo $'foo')" = yes || fail dynamic lookup through static exp +test "$(echo $'foo')" = yes || fail dynamic lookup through static exp && exit 2 # Dynamic lookup through dynamic expression ref_to_foo=foo -test "$(echo $"$ref_to_foo")" = yes || fail dynamic lookup through dynamic exp +test "$(echo $"$ref_to_foo")" = yes || fail dynamic lookup through dynamic exp && exit 2 # More redirections echo test > /tmp/sh-test -test "$(cat /tmp/sh-test)" = test || fail simple path redirect +test "$(cat /tmp/sh-test)" = test || fail simple path redirect && exit 2 rm /tmp/sh-test # 'brace' expansions -test "$(echo x(yes no))" = "xyes xno" || fail simple juxtaposition expansion -test "$(echo (y n)(es o))" = "yes yo nes no" || fail list-list juxtaposition expansion -test "$(echo ()(foo bar baz))" = "" || fail empty expansion +test "$(echo x(yes no))" = "xyes xno" || fail simple juxtaposition expansion && exit 2 +test "$(echo (y n)(es o))" = "yes yo nes no" || fail list-list juxtaposition expansion && exit 2 +test "$(echo ()(foo bar baz))" = "" || fail empty expansion && exit 2 # Variables inside commands to_devnull=(>/dev/null) -test "$(echo hewwo $to_devnull)" = "" || fail variable containing simple command +test "$(echo hewwo $to_devnull)" = "" || fail variable containing simple command && exit 2 word_count=(() | wc -w) -test "$(echo well hello friends $word_count)" -eq 3 || fail variable containing pipeline +test "$(echo well hello friends $word_count)" -eq 3 || fail variable containing pipeline && exit 2 # Globs mkdir sh-test pushd sh-test touch (a b c)(d e f) - test "$(echo a*)" = "ad ae af" || fail '*' glob expansion - test "$(echo a?)" = "ad ae af" || fail '?' glob expansion + test "$(echo a*)" = "ad ae af" || fail '*' glob expansion && exit 2 + test "$(echo a?)" = "ad ae af" || fail '?' glob expansion && exit 2 glob_in_var='*' - test "$(echo $glob_in_var)" = '*' || fail substituted string acts as glob + test "$(echo $glob_in_var)" = '*' || fail substituted string acts as glob && exit 2 - test "$(echo (a*))" = "ad ae af" || fail globs in lists resolve wrong - test "$(echo x(a*))" = "xad xae xaf" || fail globs in lists do not resolve to lists - test "$(echo "foo"a*)" = "fooad fooae fooaf" || fail globs join to dquoted strings + test "$(echo (a*))" = "ad ae af" || fail globs in lists resolve wrong && exit 2 + test "$(echo x(a*))" = "xad xae xaf" || fail globs in lists do not resolve to lists && exit 2 + test "$(echo "foo"a*)" = "fooad fooae fooaf" || fail globs join to dquoted strings && exit 2 popd rm -fr sh-test # Setopt setopt --inline_exec_keep_empty_segments -test "$(echo -n "a\n\nb")" = "a b" || fail inline_exec_keep_empty_segments has no effect +test "$(echo -n "a\n\nb")" = "a b" || fail inline_exec_keep_empty_segments has no effect && exit 2 setopt --no_inline_exec_keep_empty_segments -test "$(echo -n "a\n\nb")" = "a b" || fail cannot unset inline_exec_keep_empty_segments +test "$(echo -n "a\n\nb")" = "a b" || fail cannot unset inline_exec_keep_empty_segments && exit 2