hscript: Allow multiple inheritance

This allows a HorizonScript to inherit from multiple files.  Files are parsed
in a system-defined, unspecified order, but all scripts of a given depth are
guaranteed to be parsed before the next depth.

10 files changed, 129 insertions, 42 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index fcfaf81..f10a5e5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -5,7 +5,7 @@ include(CheckIncludeFiles)
 
 project(Horizon
         LANGUAGES C CXX
-        VERSION 0.9.7)
+        VERSION 0.9.8)
 
 
 add_definitions(-DVERSTR="${PROJECT_VERSION}")
diff --git a/hscript/script.cc b/hscript/script.cc
index 59a799a..d86216d 100644
--- a/hscript/script.cc
+++ b/hscript/script.cc
@@ -203,8 +203,6 @@ Script *Script::load(const std::string &path, const ScriptOptions &opts) {
 }
 
 
-Script *Script::load(std::istream &sstream, const ScriptOptions &opts,
-                     const std::string &name) {
 #define PARSER_ERROR(err_str) \
     errors++;\
     output_error(pos, err_str, "");\
@@ -216,24 +214,16 @@ Script *Script::load(std::istream &sstream, const ScriptOptions &opts,
     warnings++;\
     output_warning(pos, warn_str, "");
 
-    using namespace Horizon::Keys;
 
-    Script *the_script = new Script;
-    the_script->opts = opts;
-
-    int lineno = 0;
+std::pair< bool, std::vector<std::string> >
+Script::readFromStream(std::istream *my_stream, Script *the_script,
+                       const ScriptOptions &opts, const std::string &curr_name,
+                       int &errors, int &warnings, bool inherit) {
     char nextline[SCRIPT_LINE_MAX];
+    int lineno = 0;
+    const int error_start = errors, warning_start = warnings;
     const std::string delim(" \t");
-    int errors = 0, warnings = 0;
-    std::string curr_name;
-    if(name == "/dev/stdin") {
-        curr_name = "<stdin>";
-    } else {
-        curr_name = fs::canonical(fs::path(name)).native();
-    }
-    std::set<std::string> seen = {curr_name};
-    bool inherit = false;
-    std::istream *my_stream = &sstream;
+    std::vector<std::string> inherits;
 
     while(my_stream->getline(nextline, sizeof(nextline))) {
         lineno++;
@@ -245,15 +235,14 @@ Script *Script::load(std::istream &sstream, const ScriptOptions &opts,
         const ScriptLocation pos(curr_name, lineno, inherit);
         const std::string line(nextline);
         std::string key;
-        std::string::size_type start, key_end, value_begin;
-        start = line.find_first_not_of(delim);
+        const std::string::size_type start{line.find_first_not_of(delim)},
+                                     key_end{line.find_first_of(delim, start)},
+                                     value_begin{line.find_first_not_of(delim, key_end)};
         if(start == std::string::npos) {
             /* This is a blank line; ignore it. */
             continue;
         }
 
-        key_end = line.find_first_of(delim, start);
-        value_begin = line.find_first_not_of(delim, key_end);
         key = line.substr(start, (key_end - start));
         if(key_end == std::string::npos || value_begin == std::string::npos) {
             /* Key without value */
@@ -273,21 +262,17 @@ Script *Script::load(std::istream &sstream, const ScriptOptions &opts,
                 next_name = fs::absolute(better_path).native();
             }
 
-            if(seen.find(next_name) != seen.end()) {
-                PARSER_ERROR("attempt to inherit from already inherited file")
-                break;
-            }
-            seen.insert(next_name);
-
             if(!fs::exists(next_name)) {
-                PARSER_ERROR("attempt to inherit from non-existent file")
-                break;
-            } else {
-                if(my_stream != &sstream) delete my_stream;
-                curr_name = next_name;
-                inherit = true;
-                my_stream = new std::ifstream(curr_name);
+                errors++;
+                output_error(pos, "attempt to inherit from non-existent file",
+                             next_name);
+                if(!opts.test(ScriptOptionFlags::KeepGoing)) {
+                    break;
+                }
+                continue;
             }
+
+            inherits.push_back(next_name);
             continue;
         }
 
@@ -329,6 +314,63 @@ Script *Script::load(std::istream &sstream, const ScriptOptions &opts,
         errors++;
     }
 
+    return std::make_pair((errors == error_start && warnings == warning_start),
+                          inherits);
+}
+
+
+Script *Script::load(std::istream &sstream, const ScriptOptions &opts,
+                     const std::string &name) {
+    using namespace Horizon::Keys;
+
+    Script *the_script = new Script;
+    the_script->opts = opts;
+
+    int errors = 0, warnings = 0;
+    std::string curr_name;
+    if(name == "/dev/stdin") {
+        curr_name = "<stdin>";
+    } else {
+        curr_name = fs::canonical(fs::path(name)).native();
+    }
+    std::set<std::string> seen = {curr_name};
+    std::set<std::string> this_depth = {curr_name};
+    std::set<std::string> next_depth;
+    bool inherit = false, keep_going = true;
+    std::istream *my_stream = &sstream;
+
+    while(keep_going) {
+        auto result = readFromStream(my_stream, the_script, opts, curr_name,
+                                     errors, warnings, inherit);
+        seen.insert(curr_name);
+        for(const auto &next_name: result.second) {
+            if (seen.find(next_name) != seen.end()) {
+                output_error(curr_name, "attempt to inherit from already "\
+                             "inherited file", next_name);
+                keep_going = false;
+                break;
+            }
+            next_depth.insert(next_name);
+        }
+
+        this_depth.erase(curr_name);
+        if(this_depth.empty()) {
+            /* We've processed all scripts at this depth; let's go deeper. */
+            this_depth = next_depth;
+            next_depth = {};
+        }
+
+        if(this_depth.empty()) {
+            /* We are finished. */
+            break;
+        }
+
+        if(my_stream != &sstream) delete my_stream;
+        curr_name = *this_depth.begin();
+        inherit = true;
+        my_stream = new std::ifstream(curr_name);
+    }
+
     /* Ensure all required keys are present. */
 #define MISSING_ERROR(key) \
     output_error(curr_name, "expected value for key '" + std::string(key) + "'",\
@@ -342,13 +384,13 @@ Script *Script::load(std::istream &sstream, const ScriptOptions &opts,
         if(!the_script->internal->hostname) {
             MISSING_ERROR("hostname")
         }
-        if(the_script->internal->packages.size() == 0) {
+        if(the_script->internal->packages.empty()) {
             MISSING_ERROR("pkginstall")
         }
         if(!the_script->internal->rootpw) {
             MISSING_ERROR("rootpw")
         }
-        if(the_script->internal->mounts.size() == 0 && !opts.test(ImageOnly)) {
+        if(the_script->internal->mounts.empty() && !opts.test(ImageOnly)) {
             MISSING_ERROR("mount")
         }
     }
diff --git a/hscript/script.hh b/hscript/script.hh
index 9befb41..a34668a 100644
--- a/hscript/script.hh
+++ b/hscript/script.hh
@@ -59,6 +59,10 @@ private:
     /*! Initialise the Script class. */
     Script();
     ScriptOptions opts;
+
+    static std::pair< bool, std::vector<std::string> >
+    readFromStream(std::istream *, Script *, const ScriptOptions &,
+                   const std::string &, int &, int &, bool);
 public:
     /*! Free resources associated with the Script. */
     ~Script();
diff --git a/tests/fixtures/0232-inherit-basic.installfile b/tests/fixtures/0232-inherit-basic.installfile
index 64a4c6a..2e65f1e 100644
--- a/tests/fixtures/0232-inherit-basic.installfile
+++ b/tests/fixtures/0232-inherit-basic.installfile
@@ -1,3 +1,3 @@
 arch ppc64
 pkginstall kde x11
-inherit 0222-complete.installfile
+inherit ../../fixtures/0222-complete.installfile
diff --git a/tests/fixtures/0236-inherit-relative.installfile b/tests/fixtures/0236-inherit-relative.installfile
index eb6fe27..1e2e9d1 100644
--- a/tests/fixtures/0236-inherit-relative.installfile
+++ b/tests/fixtures/0236-inherit-relative.installfile
@@ -1,2 +1,2 @@
 arch ppc64
-inherit ../fixtures/0222-complete.installfile
+inherit ../../fixtures/0222-complete.installfile
diff --git a/tests/fixtures/0237-inherit-incomplete.installfile b/tests/fixtures/0237-inherit-incomplete.installfile
index a4b1309..dd99b5d 100644
--- a/tests/fixtures/0237-inherit-incomplete.installfile
+++ b/tests/fixtures/0237-inherit-incomplete.installfile
@@ -1,2 +1,2 @@
 arch ppc64
-inherit 0006-no-network.installfile
+inherit ../../fixtures/0006-no-network.installfile
diff --git a/tests/fixtures/0238-inherit-override.installfile b/tests/fixtures/0238-inherit-override.installfile
index 161e8f8..14e8b06 100644
--- a/tests/fixtures/0238-inherit-override.installfile
+++ b/tests/fixtures/0238-inherit-override.installfile
@@ -1,3 +1,3 @@
 arch ppc64
-hostname overriden.hostname.local
-inherit 0222-complete.installfile
+hostname overridden.hostname.local
+inherit ../../fixtures/0222-complete.installfile
diff --git a/tests/fixtures/0264-inherit-test-loop.installfile b/tests/fixtures/0264-inherit-test-loop.installfile
new file mode 100644
index 0000000..510264a
--- /dev/null
+++ b/tests/fixtures/0264-inherit-test-loop.installfile
@@ -0,0 +1,5 @@
+# Because of the fact we need to use relative paths,
+# and because of the fact RSpec copies the first installfile to tmp/aruba,
+# and because the first loop has to reference the second and vis versa,
+# we need a 'stub' that kicks the loop off.
+inherit ../../fixtures/0234-inherit-loop1.installfile
diff --git a/tests/spec/simulator_spec.rb b/tests/spec/simulator_spec.rb
index 6c2d095..8ccf98e 100644
--- a/tests/spec/simulator_spec.rb
+++ b/tests/spec/simulator_spec.rb
@@ -448,4 +448,11 @@ mtu 9001
             expect(last_command_started.stdout).to include("cp /usr/share/user-manager/avatars/circles/Cat.png /target/var/lib/AccountsService/icons/awilfox")
         end
     end
+    context "simulating inheritance" do
+        it "overrides keys correctly when multiple values are specified" do
+            use_fixture '0238-inherit-override.installfile'
+            run_simulate
+            expect(last_command_started.stdout).to include("hostname overridden.hostname.local")
+        end
+    end
 end
diff --git a/tests/spec/validator_spec.rb b/tests/spec/validator_spec.rb
index 282d7ca..3a491a0 100644
--- a/tests/spec/validator_spec.rb
+++ b/tests/spec/validator_spec.rb
@@ -80,6 +80,35 @@ RSpec.describe 'HorizonScript validation', :type => :aruba do
             run_validate
             expect(last_command_started).to have_output(/error: .*length/)
         end
+        context "inheritance" do
+            it "supports basic single inheritance" do
+                use_fixture '0232-inherit-basic.installfile'
+                run_validate
+                expect(last_command_started).to have_output(PARSER_SUCCESS)
+                expect(last_command_started).to have_output(VALIDATOR_SUCCESS)
+            end
+            it "requires inherited files to exist" do
+                use_fixture '0233-inherit-missing.installfile'
+                run_validate
+                expect(last_command_started).to have_output(/error: .*non-existent/)
+            end
+            it "doesn't get stuck in a loop" do
+                use_fixture '0264-inherit-test-loop.installfile'
+                run_validate
+                expect(last_command_started).to have_output(/error: .*already inherited/)
+            end
+            it "supports relative paths" do
+                use_fixture '0236-inherit-relative.installfile'
+                run_validate
+                expect(last_command_started).to have_output(PARSER_SUCCESS)
+                expect(last_command_started).to have_output(VALIDATOR_SUCCESS)
+            end
+            it "still knows incomplete files are incomplete" do
+                use_fixture '0237-inherit-incomplete.installfile'
+                run_validate
+                expect(last_command_started).to have_output(/error: expected/)
+            end
+        end
         context "required keys" do
             # Runner.Validate.Required.
             # Runner.Validate.network.