From 0d0d9a9c56822919e9794658d31db57f8fc3e2bf Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Thu, 10 Dec 2015 13:54:59 +0200
Subject: Implement two-phase initialization of modules loaded from
 bootstrap.build

---
 build/b.cxx              | 19 ++++++++++------
 build/config/module      |  3 +++
 build/config/module.cxx  | 46 +++++++++++++++++++++++---------------
 build/context            |  1 +
 build/context.cxx        |  2 ++
 build/dist/module        |  3 +++
 build/dist/module.cxx    | 21 +++++++++++-------
 build/file.cxx           | 34 +++++++++++++++++++++++++----
 build/install/module     |  3 +++
 build/install/module.cxx | 19 ++++++++++------
 build/module             | 46 +++++++++++++++++++++++++++++---------
 build/module.cxx         | 57 ++++++++++++++++++++++++++++++++++++++++++++++--
 build/parser             |  9 ++++++--
 build/parser.cxx         | 28 ++++++++++++++----------
 build/test/module        |  3 +++
 build/test/module.cxx    | 21 +++++++++++-------
 16 files changed, 237 insertions(+), 78 deletions(-)

(limited to 'build')

diff --git a/build/b.cxx b/build/b.cxx
index 3eab437..2193ff5 100644
--- a/build/b.cxx
+++ b/build/b.cxx
@@ -99,13 +99,18 @@ main (int argc, char* argv[])
 
     // Register builtin modules.
     //
-    builtin_modules["config"] = &config::config_init;
-    builtin_modules["dist"] = &dist::dist_init;
-    builtin_modules["bin"] = &bin::bin_init;
-    builtin_modules["cxx"] = &cxx::cxx_init;
-    builtin_modules["cli"] = &cli::cli_init;
-    builtin_modules["test"] = &test::test_init;
-    builtin_modules["install"] = &install::install_init;
+    builtin_modules["config"]  = module_functions {&config::config_boot,
+                                                   &config::config_init};
+    builtin_modules["dist"]    = module_functions {&dist::dist_boot,
+                                                   &dist::dist_init};
+    builtin_modules["test"]    = module_functions {&test::test_boot,
+                                                   &test::test_init};
+    builtin_modules["install"] = module_functions {&install::install_boot,
+                                                   &install::install_init};
+
+    builtin_modules["bin"] = module_functions {nullptr, &bin::bin_init};
+    builtin_modules["cxx"] = module_functions {nullptr, &cxx::cxx_init};
+    builtin_modules["cli"] = module_functions {nullptr, &cli::cli_init};
 
     // Figure out work and home directories.
     //
diff --git a/build/config/module b/build/config/module
index 530fa7b..3e43c7b 100644
--- a/build/config/module
+++ b/build/config/module
@@ -14,6 +14,9 @@ namespace build
 {
   namespace config
   {
+    extern "C" void
+    config_boot (scope&, const location&, unique_ptr<module>&);
+
     extern "C" bool
     config_init (
     scope&, scope&, const location&, unique_ptr<module>&, bool, bool);
diff --git a/build/config/module.cxx b/build/config/module.cxx
index 77a786b..c9139ed 100644
--- a/build/config/module.cxx
+++ b/build/config/module.cxx
@@ -24,9 +24,35 @@ namespace build
     //
     static const path config_file ("build/config.build");
 
+    extern "C" void
+    config_boot (scope& root, const location&, unique_ptr<module>&)
+    {
+      tracer trace ("config::boot");
+
+      const dir_path& out_root (root.out_path ());
+      level5 ([&]{trace << "for " << out_root;});
+
+      // Register meta-operations.
+      //
+      root.meta_operations.insert (configure_id, configure);
+      root.meta_operations.insert (disfigure_id, disfigure);
+
+      // Load config.build if one exists.
+      //
+      // Note that we have to do this during bootstrap since the order in
+      // which the modules will be initialized is unspecified. So it is
+      // possible that some module which needs the configuration will get
+      // called first.
+      //
+      path f (out_root / config_file);
+
+      if (file_exists (f))
+        source (f, root, root);
+    }
+
     extern "C" bool
     config_init (scope& root,
-                 scope& base,
+                 scope&,
                  const location& l,
                  std::unique_ptr<module>&,
                  bool first,
@@ -34,22 +60,13 @@ namespace build
     {
       tracer trace ("config::init");
 
-      if (&root != &base)
-        fail (l) << "config module must be initialized in bootstrap.build";
-
       if (!first)
       {
         warn (l) << "multiple config module initializations";
         return true;
       }
 
-      const dir_path& out_root (root.out_path ());
-      level5 ([&]{trace << "for " << out_root;});
-
-      // Register meta-operations.
-      //
-      root.meta_operations.insert (configure_id, configure);
-      root.meta_operations.insert (disfigure_id, disfigure);
+      level5 ([&]{trace << "for " << root.out_path ();});
 
       // Register alias and fallback rule for the configure meta-operation.
       //
@@ -67,13 +84,6 @@ namespace build
         r.insert<alias> (configure_id, 0, "config.alias", alias_rule::instance);
       }
 
-      // Load config.build if one exists.
-      //
-      path f (out_root / config_file);
-
-      if (file_exists (f))
-        source (f, root, root);
-
       return true;
     }
   }
diff --git a/build/context b/build/context
index ffe88bc..b0ac6dd 100644
--- a/build/context
+++ b/build/context
@@ -23,6 +23,7 @@ namespace build
   extern dir_path work;
   extern dir_path home;
 
+  extern string_pool path_pool;
   extern string_pool extension_pool;
   extern string_pool project_name_pool;
 
diff --git a/build/context.cxx b/build/context.cxx
index f98ca96..a4815fb 100644
--- a/build/context.cxx
+++ b/build/context.cxx
@@ -22,6 +22,7 @@ namespace build
   dir_path work;
   dir_path home;
 
+  string_pool path_pool;
   string_pool extension_pool;
   string_pool project_name_pool;
 
@@ -34,6 +35,7 @@ namespace build
   void
   reset ()
   {
+    path_pool.clear ();
     extension_pool.clear ();
     project_name_pool.clear ();
 
diff --git a/build/dist/module b/build/dist/module
index 20082ec..f2e024a 100644
--- a/build/dist/module
+++ b/build/dist/module
@@ -14,6 +14,9 @@ namespace build
 {
   namespace dist
   {
+    extern "C" void
+    dist_boot (scope&, const location&, unique_ptr<module>&);
+
     extern "C" bool
     dist_init (
       scope&, scope&, const location&, unique_ptr<module>&, bool, bool);
diff --git a/build/dist/module.cxx b/build/dist/module.cxx
index f0a446b..f39ef18 100644
--- a/build/dist/module.cxx
+++ b/build/dist/module.cxx
@@ -22,9 +22,21 @@ namespace build
   {
     static rule rule_;
 
+    extern "C" void
+    dist_boot (scope& r, const location&, unique_ptr<module>&)
+    {
+      tracer trace ("dist::boot");
+
+      level5 ([&]{trace << "for " << r.out_path ();});
+
+      // Register meta-operation.
+      //
+      r.meta_operations.insert (dist_id, dist);
+    }
+
     extern "C" bool
     dist_init (scope& r,
-               scope& b,
+               scope&,
                const location& l,
                unique_ptr<module>&,
                bool first,
@@ -32,9 +44,6 @@ namespace build
     {
       tracer trace ("dist::init");
 
-      if (&r != &b)
-        fail (l) << "dist module must be initialized in bootstrap.build";
-
       if (!first)
       {
         warn (l) << "multiple dist module initializations";
@@ -66,10 +75,6 @@ namespace build
         v.find ("config.dist.archives", strings_type);
       }
 
-      // Register meta-operation.
-      //
-      r.meta_operations.insert (dist_id, dist);
-
       // Register our wildcard rule. Do it explicitly for the alias
       // to prevent something like insert<target>(dist_id, test_id)
       // taking precedence.
diff --git a/build/file.cxx b/build/file.cxx
index 9595d8f..59fe1b9 100644
--- a/build/file.cxx
+++ b/build/file.cxx
@@ -75,8 +75,8 @@ namespace build
     return dir_path ();
   }
 
-  void
-  source (const path& bf, scope& root, scope& base)
+  static void
+  source (const path& bf, scope& root, scope& base, bool boot)
   {
     tracer trace ("source");
 
@@ -90,7 +90,7 @@ namespace build
 
       level5 ([&]{trace << "sourcing " << bf;});
 
-      parser p;
+      parser p (boot);
       p.parse_buildfile (ifs, bf, root, base);
     }
     catch (const ifstream::failure&)
@@ -100,6 +100,12 @@ namespace build
   }
 
   void
+  source (const path& bf, scope& root, scope& base)
+  {
+    return source (bf, root, base, false);
+  }
+
+  void
   source_once (const path& bf, scope& root, scope& base, scope& once)
   {
     tracer trace ("source_once");
@@ -440,7 +446,11 @@ namespace build
       // process hard to reason about. But we may try to bootstrap the
       // same root scope multiple time.
       //
-      source_once (bf, root, root);
+      if (root.buildfiles.insert (bf).second)
+        source (bf, root, root, true);
+      else
+        level5 ([&]{trace << "skipping already sourced " << bf;});
+
       r = true;
     }
 
@@ -722,6 +732,22 @@ namespace build
     if (scope* rs = root.parent_scope ()->root_scope ())
       load_root_pre (*rs);
 
+    // Finish off loading bootstrapped modules.
+    //
+    for (auto& p: root.modules)
+    {
+      const string& n (p.first);
+      module_state& s (p.second);
+
+      if (s.boot)
+      {
+        load_module (false, n, root, root, s.loc);
+        assert (!s.boot);
+      }
+    }
+
+    // Load root.build.
+    //
     path bf (root.src_path () / path ("build/root.build"));
 
     if (file_exists (bf))
diff --git a/build/install/module b/build/install/module
index 78004ef..7a814c8 100644
--- a/build/install/module
+++ b/build/install/module
@@ -14,6 +14,9 @@ namespace build
 {
   namespace install
   {
+    extern "C" void
+    install_boot (scope&, const location&, unique_ptr<module>&);
+
     extern "C" bool
     install_init (
       scope&, scope&, const location&, unique_ptr<module>&, bool, bool);
diff --git a/build/install/module.cxx b/build/install/module.cxx
index f8f7d73..204bddd 100644
--- a/build/install/module.cxx
+++ b/build/install/module.cxx
@@ -99,6 +99,18 @@ namespace build
     static alias_rule alias_;
     static file_rule file_;
 
+    extern "C" void
+    install_boot (scope& r, const location&, unique_ptr<module>&)
+    {
+      tracer trace ("install::boot");
+
+      level5 ([&]{trace << "for " << r.out_path ();});
+
+      // Register the install operation.
+      //
+      r.operations.insert (install_id, install);
+    }
+
     extern "C" bool
     install_init (scope& r,
                   scope& b,
@@ -109,9 +121,6 @@ namespace build
     {
       tracer trace ("install::init");
 
-      if (&r != &b)
-        fail (l) << "install module must be initialized in bootstrap.build";
-
       if (!first)
       {
         warn (l) << "multiple install module initializations";
@@ -132,10 +141,6 @@ namespace build
         v.find ("install", dir_path_type);
       }
 
-      // Register the install operation.
-      //
-      r.operations.insert (install_id, install);
-
       // Register our alias and file installer rule.
       //
       b.rules.insert<alias> (perform_install_id, "install.alias", alias_);
diff --git a/build/module b/build/module
index 984a50f..061ef60 100644
--- a/build/module
+++ b/build/module
@@ -6,8 +6,11 @@
 #define BUILD_MODULE
 
 #include <map>
-#include <string>
-#include <memory> // unique_ptr
+
+#include <build/types>
+#include <build/utility>
+
+#include <build/diagnostics>
 
 namespace build
 {
@@ -21,6 +24,10 @@ namespace build
     ~module () = default;
   };
 
+  extern "C"
+  using module_boot_function =
+    void (scope& root, const location&, unique_ptr<module>&);
+
   // Return false if the module configuration (normally based on the default
   // values) was unsuccessful but this is not (yet) an error. One example
   // would be the optional use of a module. Or a module might remain
@@ -32,17 +39,30 @@ namespace build
     bool (scope& root,
           scope& base,
           const location&,
-          std::unique_ptr<module>&,
+          unique_ptr<module>&,
           bool first,     // First time for this project.
           bool optional); // Loaded with 'using?' (optional module).
 
-  using loaded_module_map =
-    std::map<std::string,
-             std::pair<module_init_function*, std::unique_ptr<module>>>;
 
-  // Load the specified module. Used by the parser but also by some
-  // modules to load prerequisite modules. Return true if the module
-  // was both successfully loaded and configured.
+  struct module_state
+  {
+    bool boot; // True if the module boot'ed but not yet init'ed.
+    module_init_function* init;
+    unique_ptr<build::module> module;
+    const location loc; // Boot location.
+  };
+
+  using loaded_module_map = std::map<string, module_state>;
+
+  // Load and boot the specified module.
+  //
+  void
+  boot_module (const string& name, scope& root, const location&);
+
+  // Load (if not already loaded) and initialize the specified module. Used
+  // by the parser but also by some modules to load prerequisite modules.
+  // Return true if the module was both successfully loaded and configured
+  // (false can only be returned if optional).
   //
   bool
   load_module (bool optional,
@@ -53,7 +73,13 @@ namespace build
 
   // Builtin modules.
   //
-  using available_module_map = std::map<std::string, module_init_function*>;
+  struct module_functions
+  {
+    module_boot_function* boot;
+    module_init_function* init;
+  };
+
+  using available_module_map = std::map<string, module_functions>;
   extern available_module_map builtin_modules;
 }
 
diff --git a/build/module.cxx b/build/module.cxx
index 5f1aeff..79a9bdb 100644
--- a/build/module.cxx
+++ b/build/module.cxx
@@ -16,6 +16,41 @@ namespace build
 {
   available_module_map builtin_modules;
 
+  void
+  boot_module (const string& name, scope& rs, const location& loc)
+  {
+    // First see if this modules has already been loaded for this project.
+    //
+    loaded_module_map& lm (rs.modules);
+    auto i (lm.find (name));
+
+    if (i != lm.end ())
+    {
+      module_state& s (i->second);
+
+      // The only valid situation here is if the module has already been
+      // bootstrapped.
+      //
+      assert (s.boot);
+      return;
+    }
+
+    // Otherwise search for this module.
+    //
+    auto j (builtin_modules.find (name));
+
+    if (j == builtin_modules.end ())
+      fail (loc) << "unknown module " << name;
+
+    const module_functions& mf (j->second);
+
+    if (mf.boot == nullptr)
+      fail (loc) << "module " << name << " shouldn't be loaded in bootstrap";
+
+    i = lm.emplace (name, module_state {true, mf.init, nullptr, loc}).first;
+    mf.boot (rs, loc, i->second.module);
+  }
+
   bool
   load_module (bool opt,
                const string& name,
@@ -41,11 +76,29 @@ namespace build
           fail (loc) << "unknown module " << name;
       }
       else
-        i = lm.emplace (name, make_pair (j->second, nullptr)).first;
+      {
+        const module_functions& mf (j->second);
+
+        if (mf.boot != nullptr)
+          fail (loc) << "module " << name << " should be loaded in bootstrap";
+
+        i = lm.emplace (
+          name, module_state {false, mf.init, nullptr, loc}).first;
+      }
+    }
+    else
+    {
+      module_state& s (i->second);
+
+      if (s.boot)
+      {
+        s.boot = false;
+        f = true; // This is a first call to init.
+      }
     }
 
     bool l (i != lm.end ());
-    bool c (l && i->second.first (rs, bs, loc, i->second.second, f, opt));
+    bool c (l && i->second.init (rs, bs, loc, i->second.module, f, opt));
 
     const variable& lv (var_pool.find (name + ".loaded",
                                        variable_visibility::project,
diff --git a/build/parser b/build/parser
index 08d587a..4630110 100644
--- a/build/parser
+++ b/build/parser
@@ -27,7 +27,10 @@ namespace build
     typedef build::names names_type;
     typedef build::variable variable_type;
 
-    parser (): fail (&path_) {}
+    // If boot is true, then we are parsing bootstrap.build and modules
+    // should only be bootstrapped.
+    //
+    parser (bool boot = false): fail (&path_), boot_ (boot) {}
 
     // Issue diagnostics and throw failed in case of an error.
     //
@@ -183,7 +186,9 @@ namespace build
     const fail_mark<failed> fail;
 
   protected:
-    const std::string* path_; // Path processed by diag_relative().
+    bool boot_;
+
+    const std::string* path_; // Path processed by diag_relative() and pooled.
     lexer* lexer_;
     target* target_; // Current target, if any.
     scope* scope_;   // Current base scope (out_base).
diff --git a/build/parser.cxx b/build/parser.cxx
index 8cfa665..7f8b570 100644
--- a/build/parser.cxx
+++ b/build/parser.cxx
@@ -42,10 +42,9 @@ namespace build
   {
     enter_buildfile (p);
 
-    string rw (diag_relative (p)); // Relative to work.
-    path_ = &rw;
+    path_ = &path_pool.find (diag_relative (p)); // Relative to work.
 
-    lexer l (is, rw);
+    lexer l (is, *path_);
     lexer_ = &l;
     target_ = nullptr;
     scope_ = &base;
@@ -67,7 +66,7 @@ namespace build
   token parser::
   parse_variable (lexer& l, scope& s, string name, type kind)
   {
-    path_ = &l.name ();
+    path_ = &l.name (); // Note: not pooled.
     lexer_ = &l;
     target_ = nullptr;
     scope_ = &s;
@@ -531,11 +530,10 @@ namespace build
 
         enter_buildfile (p);
 
-        string rw (diag_relative (p)); // Relative to work.
         const string* op (path_);
-        path_ = &rw;
+        path_ = &path_pool.find (diag_relative (p)); // Relative to work.
 
-        lexer l (ifs, rw);
+        lexer l (ifs, *path_);
         lexer* ol (lexer_);
         lexer_ = &l;
 
@@ -668,11 +666,10 @@ namespace build
 
         enter_buildfile (p);
 
-        string rw (diag_relative (p)); // Relative to work.
         const string* op (path_);
-        path_ = &rw;
+        path_ = &path_pool.find (diag_relative (p)); // Relative to work.
 
-        lexer l (ifs, rw);
+        lexer l (ifs, *path_);
         lexer* ol (lexer_);
         lexer_ = &l;
 
@@ -807,6 +804,9 @@ namespace build
 
     bool optional (t.value.back () == '?');
 
+    if (optional && boot_)
+      fail (t) << "optional module in bootstrap";
+
     // The rest should be a list of module names. Parse them as names
     // to get variable expansion, etc.
     //
@@ -856,7 +856,11 @@ namespace build
       else
       {
         assert (v.empty ()); // Module versioning not yet implemented.
-        load_module (optional, n, *root_, *scope_, l);
+
+        if (boot_)
+          boot_module (n, *root_, l);
+        else
+          load_module (optional, n, *root_, *scope_, l);
       }
     }
 
@@ -1838,7 +1842,7 @@ namespace build
   buildspec parser::
   parse_buildspec (istream& is, const std::string& name)
   {
-    path_ = &name;
+    path_ = &name; // Note: caller pools.
 
     lexer l (is, name, &paren_processor);
     lexer_ = &l;
diff --git a/build/test/module b/build/test/module
index 25bb2f2..83c6d60 100644
--- a/build/test/module
+++ b/build/test/module
@@ -12,6 +12,9 @@ namespace build
 {
   namespace test
   {
+    extern "C" void
+    test_boot (scope&, const location&, unique_ptr<module>&);
+
     extern "C" bool
     test_init (
       scope&, scope&, const location&, unique_ptr<module>&, bool, bool);
diff --git a/build/test/module.cxx b/build/test/module.cxx
index 19a31d2..9372103 100644
--- a/build/test/module.cxx
+++ b/build/test/module.cxx
@@ -21,9 +21,21 @@ namespace build
   {
     static rule rule_;
 
+    extern "C" void
+    test_boot (scope& root, const location&, unique_ptr<module>&)
+    {
+      tracer trace ("test::boot");
+
+      level5 ([&]{trace << "for " << root.out_path ();});
+
+      // Register the test operation.
+      //
+      root.operations.insert (test_id, test);
+    }
+
     extern "C" bool
     test_init (scope& root,
-               scope& base,
+               scope&,
                const location& l,
                unique_ptr<module>&,
                bool first,
@@ -31,9 +43,6 @@ namespace build
     {
       tracer trace ("test::init");
 
-      if (&root != &base)
-        fail (l) << "test module must be initialized in bootstrap.build";
-
       if (!first)
       {
         warn (l) << "multiple test module initializations";
@@ -56,10 +65,6 @@ namespace build
         v.find ("test.arguments", strings_type);
       }
 
-      // Register the test operation.
-      //
-      root.operations.insert (test_id, test);
-
       // Register rules.
       //
       {
-- 
cgit v1.1