Merge "init: Stop combining actions" am: 8d644d2c96 am: a0b0c8a18e am: 779e922866
am: d949df5f57

Change-Id: I2b8b3e244089ea45a7b80fa7b88a8cd9d41e6896
diff --git a/init/README.md b/init/README.md
index 1837868..1eb42e0 100644
--- a/init/README.md
+++ b/init/README.md
@@ -16,9 +16,7 @@
 or options belong to the section most recently declared.  Commands
 or options before the first section are ignored.
 
-Actions and Services have unique names.  If a second Action is defined
-with the same name as an existing one, its commands are appended to
-the commands of the existing action.  If a second Service is defined
+Services have unique names.  If a second Service is defined
 with the same name as an existing one, it is ignored and an error
 message is logged.
 
@@ -31,13 +29,21 @@
 
 /init.rc is the primary .rc file and is loaded by the init executable
 at the beginning of its execution.  It is responsible for the initial
-set up of the system.  It imports /init.${ro.hardware}.rc which is the
-primary vendor supplied .rc file.
+set up of the system.
 
-During the mount\_all command, the init executable loads all of the
-files contained within the /{system,vendor,odm}/etc/init/ directories.
-These directories are intended for all Actions and Services used after
-file system mounting.
+Devices that mount /system, /vendor through the early mount mechanism
+load all of the files contained within the
+/{system,vendor,odm}/etc/init/ directories immediately after loading
+the primary /init.rc.  This is explained in more details in the
+Imports section of this file.
+
+Legacy devices without the early mount mechanism do the following:
+1. /init.rc imports /init.${ro.hardware}.rc which is the primary
+   vendor supplied .rc file.
+2. During the mount\_all command, the init executable loads all of the
+   files contained within the /{system,vendor,odm}/etc/init/ directories.
+   These directories are intended for all Actions and Services used after
+   file system mounting.
 
 One may specify paths in the mount\_all command line to have it import
 .rc files at the specified paths instead of the default ones listed above.
@@ -89,7 +95,7 @@
 Actions
 -------
 Actions are named sequences of commands.  Actions have a trigger which
-is used to determine when the action should occur.  When an event
+is used to determine when the action is executed.  When an event
 occurs which matches an action's trigger, that action is added to
 the tail of a to-be-executed queue (unless it is already on the
 queue).
@@ -106,6 +112,34 @@
        <command>
        <command>
 
+Actions are added to the queue and executed based on the order that
+the file that contains them was parsed (see the Imports section), then
+sequentially within an individual file.
+
+For example if a file contains:
+
+    on boot
+       setprop a 1
+       setprop b 2
+
+    on boot && property:true=true
+       setprop c 1
+       setprop d 2
+
+    on boot
+       setprop e 1
+       setprop f 2
+
+Then when the `boot` trigger occurs and assuming the property `true`
+equals `true`, then the order of the commands executed will be:
+
+    setprop a 1
+    setprop b 2
+    setprop c 1
+    setprop d 2
+    setprop e 1
+    setprop f 2
+
 
 Services
 --------
@@ -458,21 +492,54 @@
 
 Imports
 -------
-The import keyword is not a command, but rather its own section and is
-handled immediately after the .rc file that contains it has finished
-being parsed.  It takes the below form:
-
 `import <path>`
 > Parse an init config file, extending the current configuration.
   If _path_ is a directory, each file in the directory is parsed as
   a config file. It is not recursive, nested directories will
   not be parsed.
 
-There are only two times where the init executable imports .rc files:
+The import keyword is not a command, but rather its own section,
+meaning that it does not happen as part of an Action, but rather,
+imports are handled as a file is being parsed and follow the below logic.
 
-   1. When it imports /init.rc during initial boot
-   2. When it imports /{system,vendor,odm}/etc/init/ or .rc files at specified
-      paths during mount_all
+There are only three times where the init executable imports .rc files:
+
+   1. When it imports /init.rc or the script indicated by the property
+      `ro.boot.init_rc` during initial boot.
+   2. When it imports /{system,vendor,odm}/etc/init/ for early mount
+      devices immediately after importing /init.rc.
+   3. When it imports /{system,vendor,odm}/etc/init/ or .rc files at specified
+      paths during mount_all.
+
+The order that files are imported is a bit complex for legacy reasons
+and to keep backwards compatibility.  It is not strictly guaranteed.
+
+The only correct way to guarantee that a command has been run before a
+different command is to either 1) place it in an Action with an
+earlier executed trigger, or 2) place it in an Action with the same
+trigger within the same file at an earlier line.
+
+Nonetheless, the defacto order for early mount devices is:
+1. /init.rc is parsed then recursively each of its imports are
+   parsed.
+2. The contents of /system/etc/init/ are alphabetized and parsed
+   sequentially, with imports happening recursively after each file is
+   parsed.
+3. Step 2 is repeated for /vendor/etc/init then /odm/etc/init
+
+The below pseudocode may explain this more clearly:
+
+    fn Import(file)
+      Parse(file)
+      for (import : file.imports)
+        Import(import)
+
+    Import(/init.rc)
+    Directories = [/system/etc/init, /vendor/etc/init, /odm/etc/init]
+    for (directory : Directories)
+      files = <Alphabetical order of directory's contents>
+      for (file : files)
+        Import(file)
 
 
 Properties
diff --git a/init/action.cpp b/init/action.cpp
index 347edb0..c128968 100644
--- a/init/action.cpp
+++ b/init/action.cpp
@@ -26,10 +26,8 @@
 using android::base::Join;
 using android::base::StringPrintf;
 
-Command::Command(BuiltinFunction f, const std::vector<std::string>& args,
-                 const std::string& filename, int line)
-    : func_(f), args_(args), filename_(filename), line_(line) {
-}
+Command::Command(BuiltinFunction f, const std::vector<std::string>& args, int line)
+    : func_(f), args_(args), line_(line) {}
 
 int Command::InvokeFunc() const {
     std::vector<std::string> expanded_args;
@@ -49,21 +47,12 @@
     return Join(args_, ' ');
 }
 
-std::string Command::BuildSourceString() const {
-    if (!filename_.empty()) {
-        return StringPrintf(" (%s:%d)", filename_.c_str(), line_);
-    } else {
-        return std::string();
-    }
-}
-
-Action::Action(bool oneshot) : oneshot_(oneshot) {
-}
+Action::Action(bool oneshot, const std::string& filename, int line)
+    : oneshot_(oneshot), filename_(filename), line_(line) {}
 
 const KeywordMap<BuiltinFunction>* Action::function_map_ = nullptr;
 
-bool Action::AddCommand(const std::vector<std::string>& args,
-                        const std::string& filename, int line, std::string* err) {
+bool Action::AddCommand(const std::vector<std::string>& args, int line, std::string* err) {
     if (!function_map_) {
         *err = "no function map available";
         return false;
@@ -79,20 +68,12 @@
         return false;
     }
 
-    AddCommand(function, args, filename, line);
+    AddCommand(function, args, line);
     return true;
 }
 
-void Action::AddCommand(BuiltinFunction f,
-                        const std::vector<std::string>& args,
-                        const std::string& filename, int line) {
-    commands_.emplace_back(f, args, filename, line);
-}
-
-void Action::CombineAction(const Action& action) {
-    for (const auto& c : action.commands_) {
-        commands_.emplace_back(c);
-    }
+void Action::AddCommand(BuiltinFunction f, const std::vector<std::string>& args, int line) {
+    commands_.emplace_back(f, args, line);
 }
 
 std::size_t Action::NumCommands() const {
@@ -122,7 +103,7 @@
         android::base::GetMinimumLogSeverity() <= android::base::DEBUG) {
         std::string trigger_name = BuildTriggersString();
         std::string cmd_str = command.BuildCommandString();
-        std::string source = command.BuildSourceString();
+        std::string source = StringPrintf(" (%s:%d)", filename_.c_str(), command.line());
 
         LOG(INFO) << "Command '" << cmd_str << "' action=" << trigger_name << source
                   << " returned " << result << " took " << duration_ms << "ms.";
@@ -234,11 +215,6 @@
     return event_trigger_.empty() && CheckPropertyTriggers(name, value);
 }
 
-bool Action::TriggersEqual(const Action& other) const {
-    return property_triggers_ == other.property_triggers_ &&
-        event_trigger_ == other.event_trigger_;
-}
-
 std::string Action::BuildTriggersString() const {
     std::vector<std::string> triggers;
 
@@ -306,17 +282,7 @@
 }
 
 void ActionManager::AddAction(std::unique_ptr<Action> action) {
-    auto old_action_it =
-        std::find_if(actions_.begin(), actions_.end(),
-                     [&action] (std::unique_ptr<Action>& a) {
-                         return action->TriggersEqual(*a);
-                     });
-
-    if (old_action_it != actions_.end()) {
-        (*old_action_it)->CombineAction(*action);
-    } else {
-        actions_.emplace_back(std::move(action));
-    }
+    actions_.emplace_back(std::move(action));
 }
 
 void ActionManager::QueueEventTrigger(const std::string& trigger) {
@@ -332,16 +298,15 @@
     QueuePropertyTrigger("", "");
 }
 
-void ActionManager::QueueBuiltinAction(BuiltinFunction func,
-                                       const std::string& name) {
-    auto action = std::make_unique<Action>(true);
+void ActionManager::QueueBuiltinAction(BuiltinFunction func, const std::string& name) {
+    auto action = std::make_unique<Action>(true, "<Builtin Action>", 0);
     std::vector<std::string> name_vector{name};
 
     if (!action->InitSingleTrigger(name)) {
         return;
     }
 
-    action->AddCommand(func, name_vector);
+    action->AddCommand(func, name_vector, 0);
 
     trigger_queue_.push(std::make_unique<BuiltinTrigger>(action.get()));
     actions_.emplace_back(std::move(action));
@@ -366,7 +331,8 @@
 
     if (current_command_ == 0) {
         std::string trigger_name = action->BuildTriggersString();
-        LOG(INFO) << "processing action (" << trigger_name << ")";
+        LOG(INFO) << "processing action (" << trigger_name << ") from (" << action->filename()
+                  << ":" << action->line() << ")";
     }
 
     action->ExecuteOneCommand(current_command_);
@@ -397,15 +363,15 @@
     }
 }
 
-bool ActionParser::ParseSection(const std::vector<std::string>& args,
-                                std::string* err) {
+bool ActionParser::ParseSection(const std::vector<std::string>& args, const std::string& filename,
+                                int line, std::string* err) {
     std::vector<std::string> triggers(args.begin() + 1, args.end());
     if (triggers.size() < 1) {
         *err = "actions must have a trigger";
         return false;
     }
 
-    auto action = std::make_unique<Action>(false);
+    auto action = std::make_unique<Action>(false, filename, line);
     if (!action->InitTriggers(triggers, err)) {
         return false;
     }
@@ -414,10 +380,9 @@
     return true;
 }
 
-bool ActionParser::ParseLineSection(const std::vector<std::string>& args,
-                                    const std::string& filename, int line,
-                                    std::string* err) const {
-    return action_ ? action_->AddCommand(args, filename, line, err) : false;
+bool ActionParser::ParseLineSection(const std::vector<std::string>& args, int line,
+                                    std::string* err) {
+    return action_ ? action_->AddCommand(args, line, err) : false;
 }
 
 void ActionParser::EndSection() {
diff --git a/init/action.h b/init/action.h
index 0bae9f0..25e5a3e 100644
--- a/init/action.h
+++ b/init/action.h
@@ -27,31 +27,26 @@
 #include "keyword_map.h"
 
 class Command {
-public:
-    Command(BuiltinFunction f, const std::vector<std::string>& args,
-            const std::string& filename, int line);
+  public:
+    Command(BuiltinFunction f, const std::vector<std::string>& args, int line);
 
     int InvokeFunc() const;
     std::string BuildCommandString() const;
-    std::string BuildSourceString() const;
 
-private:
+    int line() const { return line_; }
+
+  private:
     BuiltinFunction func_;
     std::vector<std::string> args_;
-    std::string filename_;
     int line_;
 };
 
 class Action {
-public:
-    explicit Action(bool oneshot = false);
+  public:
+    explicit Action(bool oneshot, const std::string& filename, int line);
 
-    bool AddCommand(const std::vector<std::string>& args,
-                    const std::string& filename, int line, std::string* err);
-    void AddCommand(BuiltinFunction f,
-                    const std::vector<std::string>& args,
-                    const std::string& filename = "", int line = 0);
-    void CombineAction(const Action& action);
+    bool AddCommand(const std::vector<std::string>& args, int line, std::string* err);
+    void AddCommand(BuiltinFunction f, const std::vector<std::string>& args, int line);
     bool InitTriggers(const std::vector<std::string>& args, std::string* err);
     bool InitSingleTrigger(const std::string& trigger);
     std::size_t NumCommands() const;
@@ -60,11 +55,12 @@
     bool CheckEventTrigger(const std::string& trigger) const;
     bool CheckPropertyTrigger(const std::string& name,
                               const std::string& value) const;
-    bool TriggersEqual(const Action& other) const;
     std::string BuildTriggersString() const;
     void DumpState() const;
 
     bool oneshot() const { return oneshot_; }
+    const std::string& filename() const { return filename_; }
+    int line() const { return line_; }
     static void set_function_map(const KeywordMap<BuiltinFunction>* function_map) {
         function_map_ = function_map;
     }
@@ -80,6 +76,8 @@
     std::string event_trigger_;
     std::vector<Command> commands_;
     bool oneshot_;
+    std::string filename_;
+    int line_;
     static const KeywordMap<BuiltinFunction>* function_map_;
 };
 
@@ -115,18 +113,17 @@
 };
 
 class ActionParser : public SectionParser {
-public:
+  public:
     ActionParser() : action_(nullptr) {
     }
-    bool ParseSection(const std::vector<std::string>& args,
+    bool ParseSection(const std::vector<std::string>& args, const std::string& filename, int line,
                       std::string* err) override;
-    bool ParseLineSection(const std::vector<std::string>& args,
-                          const std::string& filename, int line,
-                          std::string* err) const override;
+    bool ParseLineSection(const std::vector<std::string>& args, int line, std::string* err) override;
     void EndSection() override;
     void EndFile(const std::string&) override {
     }
-private:
+
+  private:
     std::unique_ptr<Action> action_;
 };
 
diff --git a/init/import_parser.cpp b/init/import_parser.cpp
index 8a2bcc2..f66b2ba 100644
--- a/init/import_parser.cpp
+++ b/init/import_parser.cpp
@@ -20,8 +20,8 @@
 
 #include "util.h"
 
-bool ImportParser::ParseSection(const std::vector<std::string>& args,
-                                std::string* err) {
+bool ImportParser::ParseSection(const std::vector<std::string>& args, const std::string& filename,
+                                int line, std::string* err) {
     if (args.size() != 2) {
         *err = "single argument needed for import\n";
         return false;
diff --git a/init/import_parser.h b/init/import_parser.h
index 0e91025..e15d555 100644
--- a/init/import_parser.h
+++ b/init/import_parser.h
@@ -23,20 +23,20 @@
 #include <vector>
 
 class ImportParser : public SectionParser {
-public:
+  public:
     ImportParser()  {
     }
-    bool ParseSection(const std::vector<std::string>& args,
+    bool ParseSection(const std::vector<std::string>& args, const std::string& filename, int line,
                       std::string* err) override;
-    bool ParseLineSection(const std::vector<std::string>& args,
-                          const std::string& filename, int line,
-                          std::string* err) const override {
+    bool ParseLineSection(const std::vector<std::string>& args, int line,
+                          std::string* err) override {
         return true;
     }
     void EndSection() override {
     }
     void EndFile(const std::string& filename) override;
-private:
+
+  private:
     std::vector<std::string> imports_;
 };
 
diff --git a/init/init_parser.cpp b/init/init_parser.cpp
index 53e670b..b425497 100644
--- a/init/init_parser.cpp
+++ b/init/init_parser.cpp
@@ -71,14 +71,13 @@
                 }
                 section_parser = section_parsers_[args[0]].get();
                 std::string ret_err;
-                if (!section_parser->ParseSection(args, &ret_err)) {
+                if (!section_parser->ParseSection(args, state.filename, state.line, &ret_err)) {
                     parse_error(&state, "%s\n", ret_err.c_str());
                     section_parser = nullptr;
                 }
             } else if (section_parser) {
                 std::string ret_err;
-                if (!section_parser->ParseLineSection(args, state.filename,
-                                                      state.line, &ret_err)) {
+                if (!section_parser->ParseLineSection(args, state.line, &ret_err)) {
                     parse_error(&state, "%s\n", ret_err.c_str());
                 }
             }
diff --git a/init/init_parser.h b/init/init_parser.h
index 6935fdf..64f0cac 100644
--- a/init/init_parser.h
+++ b/init/init_parser.h
@@ -26,11 +26,10 @@
 public:
     virtual ~SectionParser() {
     }
-    virtual bool ParseSection(const std::vector<std::string>& args,
-                              std::string* err) = 0;
-    virtual bool ParseLineSection(const std::vector<std::string>& args,
-                                  const std::string& filename, int line,
-                                  std::string* err) const = 0;
+    virtual bool ParseSection(const std::vector<std::string>& args, const std::string& filename,
+                              int line, std::string* err) = 0;
+    virtual bool ParseLineSection(const std::vector<std::string>& args, int line,
+                                  std::string* err) = 0;
     virtual void EndSection() = 0;
     virtual void EndFile(const std::string& filename) = 0;
 };
diff --git a/init/service.cpp b/init/service.cpp
index 4adbbf0..caf5785 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -157,6 +157,7 @@
       gid_(0),
       namespace_flags_(0),
       seclabel_(""),
+      onrestart_(false, "<Service '" + name + "' onrestart>", 0),
       ioprio_class_(IoSchedClass_NONE),
       ioprio_pri_(0),
       priority_(0),
@@ -180,6 +181,7 @@
       capabilities_(capabilities),
       namespace_flags_(namespace_flags),
       seclabel_(seclabel),
+      onrestart_(false, "<Service '" + name + "' onrestart>", 0),
       ioprio_class_(IoSchedClass_NONE),
       ioprio_pri_(0),
       priority_(0),
@@ -438,7 +440,8 @@
 
 bool Service::ParseOnrestart(const std::vector<std::string>& args, std::string* err) {
     std::vector<std::string> str_args(args.begin() + 1, args.end());
-    onrestart_.AddCommand(str_args, "", 0, err);
+    int line = onrestart_.NumCommands() + 1;
+    onrestart_.AddCommand(str_args, line, err);
     return true;
 }
 
@@ -1092,8 +1095,8 @@
     }
 }
 
-bool ServiceParser::ParseSection(const std::vector<std::string>& args,
-                                 std::string* err) {
+bool ServiceParser::ParseSection(const std::vector<std::string>& args, const std::string& filename,
+                                 int line, std::string* err) {
     if (args.size() < 3) {
         *err = "services must have a name and a program";
         return false;
@@ -1110,9 +1113,8 @@
     return true;
 }
 
-bool ServiceParser::ParseLineSection(const std::vector<std::string>& args,
-                                     const std::string& filename, int line,
-                                     std::string* err) const {
+bool ServiceParser::ParseLineSection(const std::vector<std::string>& args, int line,
+                                     std::string* err) {
     return service_ ? service_->ParseLine(args, err) : false;
 }
 
diff --git a/init/service.h b/init/service.h
index 5e89b9f..6fe0258 100644
--- a/init/service.h
+++ b/init/service.h
@@ -212,18 +212,15 @@
 };
 
 class ServiceParser : public SectionParser {
-public:
-    ServiceParser() : service_(nullptr) {
-    }
-    bool ParseSection(const std::vector<std::string>& args,
+  public:
+    ServiceParser() : service_(nullptr) {}
+    bool ParseSection(const std::vector<std::string>& args, const std::string& filename, int line,
                       std::string* err) override;
-    bool ParseLineSection(const std::vector<std::string>& args,
-                          const std::string& filename, int line,
-                          std::string* err) const override;
+    bool ParseLineSection(const std::vector<std::string>& args, int line, std::string* err) override;
     void EndSection() override;
-    void EndFile(const std::string&) override {
-    }
-private:
+    void EndFile(const std::string&) override {}
+
+  private:
     bool IsValidName(const std::string& name) const;
 
     std::unique_ptr<Service> service_;