Add build context to package_skeleton

3 files changed, 216 insertions, 103 deletions

diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx
index ebcc044..a0fd419 100644
--- a/bpkg/package-skeleton.cxx
+++ b/bpkg/package-skeleton.cxx
@@ -5,6 +5,10 @@
 
 #include <libbutl/manifest-serializer.hxx>
 
+#include <libbuild2/types.hxx>
+#include <libbuild2/utility.hxx>
+#include <libbuild2/context.hxx>
+
 #include <bpkg/package.hxx>
 #include <bpkg/database.hxx>
 #include <bpkg/manifest-utility.hxx>
@@ -14,6 +18,66 @@ using namespace butl;
 
 namespace bpkg
 {
+  // These are defined in bpkg.cxx and initialized in main().
+  //
+  extern build2::scheduler      build2_sched;
+  extern build2::global_mutexes build2_mutexes;
+  extern build2::file_cache     build2_fcache;
+
+  package_skeleton::
+  ~package_skeleton ()
+  {
+  }
+
+  package_skeleton::
+  package_skeleton (package_skeleton&& v)
+      : db_ (v.db_), available_ (v.available_)
+  {
+    *this = move (v);
+  }
+
+  package_skeleton& package_skeleton::
+  operator= (package_skeleton&& v)
+  {
+    if (this != &v)
+    {
+      db_ = v.db_;
+      available_ = v.available_;
+      config_vars_ = move (v.config_vars_);
+      src_root_ = move (v.src_root_);
+      out_root_ = move (v.out_root_);
+      ctx_ = move (v.ctx_);
+      created_ = v.created_;
+      dirty_ = v.dirty_;
+      reflect_ = move (v.reflect_);
+    }
+
+    return *this;
+  }
+
+  package_skeleton::
+  package_skeleton (const package_skeleton& v)
+      : db_ (v.db_),
+        available_ (v.available_),
+        config_vars_ (v.config_vars_),
+        src_root_ (v.src_root_),
+        out_root_ (v.out_root_),
+        created_ (v.created_)
+  {
+    // The idea here is to create an unloaded copy but with enough state that
+    // it can be loaded if necessary.
+
+    if (v.ctx_ != nullptr)
+    {
+      // @@ extract reflection
+    }
+    else
+    {
+      // @@ TODO: copy already extracted?
+      reflect_ = v.reflect_;
+    }
+  }
+
   package_skeleton::
   package_skeleton (database& db,
                     const available_package& ap,
@@ -35,12 +99,10 @@ namespace bpkg
   void package_skeleton::
   load ()
   {
-    if (loaded_ && !dirty_)
+    if (ctx_ != nullptr && !dirty_)
       return;
 
-    const available_package& ap (available_);
-
-    // The overall plan is as follows:
+    // The overall plan is as follows: @@ TODO: revise
     //
     // 0. Create filesystem state if necessary (could have been created by
     //    another instance, e.g., during simulation).
@@ -52,112 +114,154 @@ namespace bpkg
 
     // Create the skeleton filesystem state, if it doesn't exist yet.
     //
-    // Note that we create the skeleton directories in the skeletons/
-    // subdirectory of the configuration temporary directory to make sure they
-    // never clash with other temporary subdirectories (git repositories, etc).
-    //
-    if (!src_root_)
+    if (!created_)
     {
-      auto i (temp_dir.find (db_.get ().config_orig));
-      assert (i != temp_dir.end ());
-
-      dir_path d (i->second);
-      d /= "skeletons";
-      d /= name ().string () + '-' + ap.version.string ();
-
-      src_root_ = d;
-      out_root_ = move (d);
-    }
+      const available_package& ap (available_);
 
-    if (!exists (*src_root_))
-    {
-      // Create the buildfiles.
-      //
-      // Note that it's probably doesn't matter which naming scheme to use for
-      // the buildfiles, unless in the future we allow specifying additional
-      // files.
+      // Note that we create the skeleton directories in the skeletons/
+      // subdirectory of the configuration temporary directory to make sure
+      // they never clash with other temporary subdirectories (git
+      // repositories, etc).
       //
+      if (!src_root_)
       {
-        path bf (*src_root_ / std_bootstrap_file);
+        auto i (temp_dir.find (db_.get ().config_orig));
+        assert (i != temp_dir.end ());
 
-        mk_p (bf.directory ());
+        dir_path d (i->second);
+        d /= "skeletons";
+        d /= name ().string () + '-' + ap.version.string ();
 
-        // Save the {bootstrap,root}.build files.
-        //
-        auto save = [] (const string& s, const path& f)
-        {
-          try
-          {
-            ofdstream os (f);
-            os << s;
-            os.close ();
-          }
-          catch (const io_error& e)
-          {
-            fail << "unable to write to " << f << ": " << e;
-          }
-        };
-
-        save (*ap.bootstrap_build, bf);
-
-        if (ap.root_build)
-          save (*ap.root_build, *src_root_ / std_root_file);
+        src_root_ = d;
+        out_root_ = move (d);
       }
 
-      // Create the manifest file containing the bare minimum of values
-      // which can potentially be required to load the build system state.
-      //
+      if (!exists (*src_root_))
       {
-        package_manifest m;
-        m.name = name ();
-        m.version = ap.version;
-
-        // Note that there is no guarantee that the potential build2
-        // constraint has already been verified. Thus, we also serialize the
-        // depends value, delegating the constraint verification to the
-        // version module. Also note that normally the toolchain build-time
-        // dependencies are specified first and, if that's the case, their
-        // constraints are already verified at this point and so build2 will
-        // not fail due to the constraint violation.
+        // Create the buildfiles.
         //
-        // Also note that the resulting file is not quite a valid package
-        // manifest, since it doesn't contain all the required values
-        // (summary, etc). It, however, is good enough for build2 which
-        // doesn't perform exhaustive manifest validation.
+        // Note that it's probably doesn't matter which naming scheme to use
+        // for the buildfiles, unless in the future we allow specifying
+        // additional files.
         //
-        m.dependencies.reserve (ap.dependencies.size ());
-        for (const dependency_alternatives_ex& das: ap.dependencies)
         {
-          // Skip the the special (inverse) test dependencies.
+          path bf (*src_root_ / std_bootstrap_file);
+
+          mk_p (bf.directory ());
+
+          // Save the {bootstrap,root}.build files.
           //
-          if (!das.type)
-            m.dependencies.push_back (das);
-        }
+          auto save = [] (const string& s, const path& f)
+          {
+            try
+            {
+              ofdstream os (f);
+              os << s;
+              os.close ();
+            }
+            catch (const io_error& e)
+            {
+              fail << "unable to write to " << f << ": " << e;
+            }
+          };
 
-        path mf (*src_root_ / manifest_file);
+          save (*ap.bootstrap_build, bf);
 
-        try
-        {
-          ofdstream os (mf);
-          manifest_serializer s (os, mf.string ());
-          m.serialize (s);
-          os.close ();
+          if (ap.root_build)
+            save (*ap.root_build, *src_root_ / std_root_file);
         }
-        catch (const manifest_serialization&)
+
+        // Create the manifest file containing the bare minimum of values
+        // which can potentially be required to load the build system state.
+        //
         {
-          // We shouldn't be creating a non-serializable manifest, since it's
-          // crafted from the parsed values.
+          package_manifest m;
+          m.name = name ();
+          m.version = ap.version;
+
+          // Note that there is no guarantee that the potential build2
+          // constraint has already been verified. Thus, we also serialize the
+          // depends value, delegating the constraint verification to the
+          // version module. Also note that normally the toolchain build-time
+          // dependencies are specified first and, if that's the case, their
+          // constraints are already verified at this point and so build2 will
+          // not fail due to the constraint violation.
           //
-          assert (false);
-        }
-        catch (const io_error& e)
-        {
-          fail << "unable to write to " << mf << ": " << e;
+          // Also note that the resulting file is not quite a valid package
+          // manifest, since it doesn't contain all the required values
+          // (summary, etc). It, however, is good enough for build2 which
+          // doesn't perform exhaustive manifest validation.
+          //
+          m.dependencies.reserve (ap.dependencies.size ());
+          for (const dependency_alternatives_ex& das: ap.dependencies)
+          {
+            // Skip the the special (inverse) test dependencies.
+            //
+            if (!das.type)
+              m.dependencies.push_back (das);
+          }
+
+          path mf (*src_root_ / manifest_file);
+
+          try
+          {
+            ofdstream os (mf);
+            manifest_serializer s (os, mf.string ());
+            m.serialize (s);
+            os.close ();
+          }
+          catch (const manifest_serialization&)
+          {
+            // We shouldn't be creating a non-serializable manifest, since
+            // it's crafted from the parsed values.
+            //
+            assert (false);
+          }
+          catch (const io_error& e)
+          {
+            fail << "unable to write to " << mf << ": " << e;
+          }
         }
       }
+
+      created_ = true;
     }
 
-    loaded_ = true;
-    dirty_ = false;
+    // Creating a new context is not exactly cheap (~1.2ms debug, 0.08ms
+    // release) so we could try to re-use it by cleaning all the scopes other
+    // than the global scope (and probably some other places, like var pool).
+    // But we will need to carefully audit everything to make sure we don't
+    // miss anything (like absolute scope variable overrides being lost). So
+    // maybe, one day.
+    //
+    if (dirty_)
+    {
+      ctx_.reset ();
+      dirty_ = false;
+    }
+
+    // Create build context.
+    //
+    // @@ BUILD2_VAR_OVR, environment/default options files? E.g., !config
+    //    to find module... Can't we make it reusable in build2?
+    //
+    // @@ Can we release context memory early, for example, when
+    //    collect_reflect() is called?
+    //
+
+    // We can reasonably assume reflect cannot have global or absolute scope
+    // variable overrides so we don't need to pass them to context.
+    //
+    using namespace build2;
+
+    ctx_.reset (
+      new context (build2_sched,
+                   build2_mutexes,
+                   build2_fcache,
+                   false /* match_only */,          // Shouldn't matter.
+                   false /* no_external_modules */,
+                   false /* dry_run */,             // Shouldn't matter.
+                   false /* keep_going */,          // Shouldnt' matter.
+                   config_vars_));
   }
 }
diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx
index 889a937..00be6a3 100644
--- a/bpkg/package-skeleton.hxx
+++ b/bpkg/package-skeleton.hxx
@@ -4,6 +4,8 @@
 #ifndef BPKG_PACKAGE_SKELETON_HXX
 #define BPKG_PACKAGE_SKELETON_HXX
 
+#include <libbuild2/forward.hxx> // build2::context
+
 #include <bpkg/types.hxx>
 #include <bpkg/utility.hxx>
 
@@ -79,7 +81,10 @@ namespace bpkg
       //
       reflect_.push_back (r);
 
-      dirty ();
+      // Mark the build system state as needing reloading since some computed
+      // values in root.build may depend on the new configuration values.
+      //
+      dirty_ = true;
     }
 
     // Return the accumulated reflect values.
@@ -93,6 +98,18 @@ namespace bpkg
     const package_name&
     name () const {return available_.get ().id.name;}
 
+    // Implementation details.
+    //
+    // We have to define these because context is forward-declared. Also, copy
+    // constructor has some special logic.
+    //
+    ~package_skeleton ();
+    package_skeleton (package_skeleton&&);
+    package_skeleton& operator= (package_skeleton&&);
+
+    package_skeleton (const package_skeleton&);
+    package_skeleton& operator= (package_skeleton&) = delete;
+
   private:
     // Create the skeleton if necessary and (re)load the build system state.
     //
@@ -101,18 +118,9 @@ namespace bpkg
     void
     load ();
 
-    // Mark the build system state as needing reloading.
-    //
-    // Call this function after evaluating the reflect clause (since some
-    // computed values in root.build may depend on the new value).
-    //
-    void
-    dirty ()
-    {
-      dirty_ = true;
-    }
-
   private:
+    // NOTE: remember to update move/copy constructors!
+    //
     reference_wrapper<database> db_;
     reference_wrapper<const available_package> available_;
     strings config_vars_;
@@ -120,7 +128,8 @@ namespace bpkg
     optional<dir_path> src_root_;
     optional<dir_path> out_root_;
 
-    bool loaded_ = false;
+    unique_ptr<build2::context> ctx_;
+    bool created_ = false;
     bool dirty_ = false;
 
     strings reflect_;
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index f53c14c..f4432b6 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -4160,7 +4160,7 @@ namespace bpkg
       evaluate_dependency (db,
                            sp,
                            nullopt /* desired */,
-                           false /*desired_sys */,
+                           false /* desired_sys */,
                            db,
                            sp,
                            !*upgrade /* patch */,