Reset build state for each meta-operation

5 files changed, 203 insertions, 181 deletions

diff --git a/build2/b.cxx b/build2/b.cxx
index b80fe5b..1770d2d 100644
--- a/build2/b.cxx
+++ b/build2/b.cxx
@@ -65,7 +65,7 @@ main (int argc, char* argv[])
     // and buildspecs in any order (it is really handy to just add -v at the
     // end of the command line).
     //
-    strings vars;
+    strings cmd_vars;
     string args;
     try
     {
@@ -115,7 +115,7 @@ main (int argc, char* argv[])
 
           if (strchr (s, '=') != nullptr) // Covers =, +=, and =+.
           {
-            vars.push_back (s);
+            cmd_vars.push_back (s);
             continue;
           }
 
@@ -224,39 +224,6 @@ main (int argc, char* argv[])
       trace << "home dir: " << home;
     }
 
-    // Initialize the dependency state.
-    //
-    reset ();
-
-    // Parse the command line variables.
-    //
-    for (const string& v: vars)
-    {
-      istringstream is (v);
-      is.exceptions (istringstream::failbit | istringstream::badbit);
-      lexer l (is, path ("<cmdline>"));
-
-      // This should be a name followed by =, +=, or =+.
-      //
-      token t (l.next ());
-      token_type tt (l.next ().type);
-
-      if (t.type != token_type::name ||
-          (tt != token_type::assign &&
-           tt != token_type::prepend &&
-           tt != token_type::append))
-      {
-        fail << "expected variable assignment instead of '" << v << "'" <<
-          info << "use double '--' to treat this argument as buildspec";
-      }
-
-      parser p;
-      t = p.parse_variable (l, *global_scope, t.value, tt);
-
-      if (t.type != token_type::eos)
-        fail << "unexpected " << t << " in variable assignment '" << v << "'";
-    }
-
     // Parse the buildspec.
     //
     buildspec bspec;
@@ -278,24 +245,77 @@ main (int argc, char* argv[])
     if (bspec.empty ())
       bspec.push_back (metaopspec ()); // Default meta-operation.
 
-    for (metaopspec& ms: bspec)
+    // If not NULL, then lifted points to the operation that has been "lifted"
+    // to the meta-operaion (see the logic below for details). Skip is the
+    // position of the next operation. Dirty indicated whether we managed to
+    // execute anything before lifting an operation.
+    //
+    opspec* lifted (nullptr);
+    size_t skip (0);
+    bool dirty (true);
+
+    for (auto mit (bspec.begin ()); mit != bspec.end (); )
     {
-      if (ms.empty ())
-        ms.push_back (opspec ()); // Default operation.
+      vector_view<opspec> opspecs;
+      const string& mname (lifted == nullptr ? mit->name : lifted->name);
+
+      if (lifted == nullptr)
+      {
+        metaopspec& ms (*mit);
+
+        if (ms.empty ())
+          ms.push_back (opspec ()); // Default operation.
+
+        // Continue where we left off after lifting an operation.
+        //
+        opspecs.assign (ms.data () + skip, ms.size () - skip);
+
+        // Reset since unless we lift another operation, we move to the
+        // next meta-operation (see bottom of the loop).
+        //
+        skip = 0;
+
+        // This can happen if we have lifted the last operation in opspecs.
+        //
+        if (opspecs.empty ())
+        {
+          ++mit;
+          continue;
+        }
+      }
+      else
+        opspecs.assign (lifted, 1);
+
+      // Reset the build state for each meta-operation since there is no
+      // guarantee their assumptions (e.g., in the load callback) are
+      // compatible.
+      //
+      if (dirty)
+      {
+        reset (cmd_vars);
+        dirty = false;
+      }
 
       meta_operation_id mid (0); // Not yet translated.
       const meta_operation_info* mif (nullptr);
 
-      bool lifted (false); // See below.
-
-      for (opspec& os: ms)
+      for (auto oit (opspecs.begin ()); oit != opspecs.end (); ++oit)
       {
-        const path p ("<buildspec>");
-        const location l (&p, 1, 0); //@@ TODO
+        opspec& os (*oit);
+
+        // A lifted meta-operation will always have default operation.
+        //
+        const string& oname (lifted == nullptr ? os.name : string ());
+
+        if (lifted != nullptr)
+          lifted = nullptr; // Clear for the next iteration.
 
         if (os.empty ()) // Default target: dir{}.
           os.push_back (targetspec (name ("dir", string ())));
 
+        const path p ("<buildspec>");
+        const location l (&p, 1, 0); //@@ TODO
+
         operation_id oid (0); // Not yet translated.
         const operation_info* oif (nullptr);
 
@@ -312,21 +332,6 @@ main (int argc, char* argv[])
         action_targets tgs;
         tgs.reserve (os.size ());
 
-        // If the previous operation was lifted to meta-operation,
-        // end the meta-operation batch.
-        //
-        if (lifted)
-        {
-          if (mif->meta_operation_post != nullptr)
-            mif->meta_operation_post ();
-
-          l5 ([&]{trace << "end meta-operation batch " << mif->name << ", id "
-                        << static_cast<uint16_t> (mid);});
-
-          mid = 0;
-          lifted = false;
-        }
-
         for (targetspec& ts: os)
         {
           name& tn (ts.name);
@@ -585,50 +590,34 @@ main (int argc, char* argv[])
           // known.
           //
           {
-            const auto& mn (ms.name);
-            const auto& on (os.name);
-
             meta_operation_id m (0);
             operation_id o (0);
 
-            if (!on.empty ())
+            if (!oname.empty ())
             {
-              m = meta_operation_table.find (on);
+              m = meta_operation_table.find (oname);
 
               if (m != 0)
               {
-                if (!mn.empty ())
-                  fail (l) << "nested meta-operation " << mn
-                           << '(' << on << ')';
+                if (!mname.empty ())
+                  fail (l) << "nested meta-operation " << mname
+                           << '(' << oname << ')';
 
-                if (!lifted) // If this is the first target.
-                {
-                  // End the previous meta-operation batch if there was one
-                  // and start a new one.
-                  //
-                  if (mid != 0)
-                  {
-                    assert (oid == 0);
+                l5 ([&]{trace << "lifting operation " << oname
+                              << ", id " << uint16_t (m);});
 
-                    if (mif->meta_operation_post != nullptr)
-                      mif->meta_operation_post ();
-
-                    l5 ([&]{trace << "end meta-operation batch " << mif->name
-                                  << ", id " << static_cast<uint16_t> (mid);});
-                    mid = 0;
-                  }
-
-                  lifted = true; // Flag to also end it; see above.
-                }
+                lifted = &os;
+                skip = lifted - mit->data () + 1;
+                break; // Out of targetspec loop.
               }
               else
               {
-                o = operation_table.find (on);
+                o = operation_table.find (oname);
 
                 if (o == 0)
                 {
                   diag_record dr;
-                  dr << fail (l) << "unknown operation " << on;
+                  dr << fail (l) << "unknown operation " << oname;
 
                   // If we guessed src_root and didn't load anything during
                   // bootstrap, then this is probably a meta-operation that
@@ -642,14 +631,14 @@ main (int argc, char* argv[])
               }
             }
 
-            if (!mn.empty ())
+            if (!mname.empty ())
             {
-              m = meta_operation_table.find (mn);
+              m = meta_operation_table.find (mname);
 
               if (m == 0)
               {
                 diag_record dr;
-                dr << fail (l) << "unknown meta-operation " << mn;
+                dr << fail (l) << "unknown meta-operation " << mname;
 
                 // Same idea as for the operation case above.
                 //
@@ -684,6 +673,7 @@ main (int argc, char* argv[])
                 mif->meta_operation_pre ();
 
               current_mif = mif;
+              dirty = true;
             }
             //
             // Otherwise, check that all the targets in a meta-operation
@@ -834,78 +824,94 @@ main (int argc, char* argv[])
           }
         }
 
-        if (pre_oid != 0)
+        // We don't do anything if we lifted the first operation.
+        //
+        if (oid != 0)
         {
-          l5 ([&]{trace << "start pre-operation batch " << pre_oif->name
-                        << ", id " << static_cast<uint16_t> (pre_oid);});
+          if (pre_oid != 0)
+          {
+            l5 ([&]{trace << "start pre-operation batch " << pre_oif->name
+                          << ", id " << static_cast<uint16_t> (pre_oid);});
 
-          if (mif->operation_pre != nullptr)
-            mif->operation_pre (pre_oid); // Cannot be translated.
+            if (mif->operation_pre != nullptr)
+              mif->operation_pre (pre_oid); // Cannot be translated.
 
-          current_inner_oif = pre_oif;
-          current_outer_oif = oif;
-          current_mode = pre_oif->mode;
-          dependency_count = 0;
+            current_inner_oif = pre_oif;
+            current_outer_oif = oif;
+            current_mode = pre_oif->mode;
+            dependency_count = 0;
 
-          action a (mid, pre_oid, oid);
+            action a (mid, pre_oid, oid);
 
-          mif->match (a, tgs);
-          mif->execute (a, tgs, true); // Run quiet.
+            mif->match (a, tgs);
+            mif->execute (a, tgs, true); // Run quiet.
 
-          if (mif->operation_post != nullptr)
-            mif->operation_post (pre_oid);
+            if (mif->operation_post != nullptr)
+              mif->operation_post (pre_oid);
 
-          l5 ([&]{trace << "end pre-operation batch " << pre_oif->name
-                        << ", id " << static_cast<uint16_t> (pre_oid);});
-        }
+            l5 ([&]{trace << "end pre-operation batch " << pre_oif->name
+                          << ", id " << static_cast<uint16_t> (pre_oid);});
+          }
 
-        current_inner_oif = oif;
-        current_outer_oif = nullptr;
-        current_mode = oif->mode;
-        dependency_count = 0;
+          current_inner_oif = oif;
+          current_outer_oif = nullptr;
+          current_mode = oif->mode;
+          dependency_count = 0;
 
-        action a (mid, oid, 0);
+          action a (mid, oid, 0);
 
-        mif->match (a, tgs);
-        mif->execute (a, tgs, verb == 0);
+          mif->match (a, tgs);
+          mif->execute (a, tgs, verb == 0);
 
-        if (post_oid != 0)
-        {
-          l5 ([&]{trace << "start post-operation batch " << post_oif->name
-                        << ", id " << static_cast<uint16_t> (post_oid);});
+          if (post_oid != 0)
+          {
+            l5 ([&]{trace << "start post-operation batch " << post_oif->name
+                          << ", id " << static_cast<uint16_t> (post_oid);});
 
-          if (mif->operation_pre != nullptr)
-            mif->operation_pre (post_oid); // Cannot be translated.
+            if (mif->operation_pre != nullptr)
+              mif->operation_pre (post_oid); // Cannot be translated.
 
-          current_inner_oif = post_oif;
-          current_outer_oif = oif;
-          current_mode = post_oif->mode;
-          dependency_count = 0;
+            current_inner_oif = post_oif;
+            current_outer_oif = oif;
+            current_mode = post_oif->mode;
+            dependency_count = 0;
 
-          action a (mid, post_oid, oid);
+            action a (mid, post_oid, oid);
 
-          mif->match (a, tgs);
-          mif->execute (a, tgs, true); // Run quiet.
+            mif->match (a, tgs);
+            mif->execute (a, tgs, true); // Run quiet.
+
+            if (mif->operation_post != nullptr)
+              mif->operation_post (post_oid);
+
+            l5 ([&]{trace << "end post-operation batch " << post_oif->name
+                          << ", id " << static_cast<uint16_t> (post_oid);});
+          }
 
           if (mif->operation_post != nullptr)
-            mif->operation_post (post_oid);
+            mif->operation_post (oid);
 
-          l5 ([&]{trace << "end post-operation batch " << post_oif->name
-                        << ", id " << static_cast<uint16_t> (post_oid);});
+          l5 ([&]{trace << "end operation batch " << oif->name
+                        << ", id " << static_cast<uint16_t> (oid);});
         }
 
-        if (mif->operation_post != nullptr)
-          mif->operation_post (oid);
-
-        l5 ([&]{trace << "end operation batch " << oif->name
-                      << ", id " << static_cast<uint16_t> (oid);});
+        // If this operation has been lifted, break out.
+        //
+        if (lifted == &os)
+          break;
       }
 
-      if (mif->meta_operation_post != nullptr)
-        mif->meta_operation_post ();
+      if (mid != 0)
+      {
+        if (mif->meta_operation_post != nullptr)
+          mif->meta_operation_post ();
+
+        l5 ([&]{trace << "end meta-operation batch " << mif->name
+                      << ", id " << static_cast<uint16_t> (mid);});
+      }
 
-      l5 ([&]{trace << "end meta-operation batch " << mif->name
-                    << ", id " << static_cast<uint16_t> (mid);});
+      if (lifted == nullptr && skip == 0)
+        ++mit;
     }
   }
   catch (const failed&)
diff --git a/build2/config/operation.cxx b/build2/config/operation.cxx
index 245aed7..e75f8d0 100644
--- a/build2/config/operation.cxx
+++ b/build2/config/operation.cxx
@@ -427,18 +427,6 @@ namespace build2
       }
     }
 
-    static void
-    disfigure_meta_operation_post ()
-    {
-      tracer trace ("disfigure_meta_operation_post");
-
-      // Reset the dependency state since anything that could have been
-      // loaded earlier using a previous configuration is now invalid.
-      //
-      l6 ([&]{trace << "resetting dependency state";});
-      reset ();
-    }
-
     meta_operation_info disfigure {
       "disfigure",
       "disfigure",
@@ -451,7 +439,7 @@ namespace build2
       &disfigure_match,
       &disfigure_execute,
       nullptr, // operation post
-      &disfigure_meta_operation_post
+      nullptr, // meta-operation post
     };
   }
 }
diff --git a/build2/context b/build2/context
index b99ff73..661d9ff 100644
--- a/build2/context
+++ b/build2/context
@@ -41,11 +41,11 @@ namespace build2
   //
   extern uint64_t dependency_count;
 
-  // Reset the dependency state. In particular, this removes all the
-  // targets, scopes, and variable names.
+  // Reset the build state. In particular, this removes all the targets,
+  // scopes, and variables.
   //
   void
-  reset ();
+  reset (const strings& cmd_vars);
 
   // The dual interface wrapper for the {mk,rm}{file,dir}() functions
   // below that allows you to use it as a true/false return or a more
diff --git a/build2/context.cxx b/build2/context.cxx
index 8941f3e..96532c4 100644
--- a/build2/context.cxx
+++ b/build2/context.cxx
@@ -4,6 +4,8 @@
 
 #include <build2/context>
 
+#include <sstream>
+
 #include <butl/triplet>
 
 #include <build2/rule>
@@ -12,6 +14,12 @@
 #include <build2/version>
 #include <build2/diagnostics>
 
+// For command line variable parsing.
+//
+#include <build2/token>
+#include <build2/lexer>
+#include <build2/parser>
+
 using namespace std;
 using namespace butl;
 
@@ -31,19 +39,21 @@ namespace build2
   uint64_t dependency_count;
 
   void
-  reset ()
+  reset (const strings& cmd_vars)
   {
     tracer trace ("reset");
 
-    extension_pool.clear ();
-    project_name_pool.clear ();
+    l6 ([&]{trace << "resetting build state";});
 
     targets.clear ();
     scopes.clear ();
     var_pool.clear ();
 
-    // Reset meta/operation tables. Note that the order should match
-    // the id constants in <build2/operation>.
+    extension_pool.clear ();
+    project_name_pool.clear ();
+
+    // Reset meta/operation tables. Note that the order should match the id
+    // constants in <build2/operation>.
     //
     meta_operation_table.clear ();
     meta_operation_table.insert ("perform");
@@ -58,6 +68,45 @@ namespace build2
     operation_table.insert ("test");
     operation_table.insert ("install");
 
+    // Create global scope. For Win32 this is not a "real" root path.
+    // On POSIX, however, this is a real path. See the comment in
+    // <build2/path-map> for details.
+    //
+    scope& gs (*scopes.insert (dir_path ("/"), nullptr, true, false)->second);
+    global_scope = &gs;
+
+    // Parse and enter the command line variables. We do it before entering
+    // any other variables so that all the variables that are overriden are
+    // marked as such first. Then, as we enter variables, we can verify that
+    // the override is alowed.
+    //
+    for (const string& v: cmd_vars)
+    {
+      istringstream is (v);
+      is.exceptions (istringstream::failbit | istringstream::badbit);
+      lexer l (is, path ("<cmdline>"));
+
+      // This should be a name followed by =, +=, or =+.
+      //
+      token t (l.next ());
+      token_type tt (l.next ().type);
+
+      if (t.type != token_type::name ||
+          (tt != token_type::assign &&
+           tt != token_type::prepend &&
+           tt != token_type::append))
+      {
+        fail << "expected variable assignment instead of '" << v << "'" <<
+          info << "use double '--' to treat this argument as buildspec";
+      }
+
+      parser p;
+      t = p.parse_variable (l, gs, t.value, tt);
+
+      if (t.type != token_type::eos)
+        fail << "unexpected " << t << " in variable assignment '" << v << "'";
+    }
+
     // Enter builtin variables.
     //
     {
@@ -78,15 +127,6 @@ namespace build2
       v.find<string> ("extension");
     }
 
-    // Create global scope. For Win32 this is not a "real" root path.
-    // On POSIX, however, this is a real path. See the comment in
-    // <build2/path-map> for details.
-    //
-    global_scope = scopes.insert (
-      dir_path ("/"), nullptr, true, false)->second;
-
-    scope& gs (*global_scope);
-
     gs.assign<dir_path> ("build.work") = work;
     gs.assign<dir_path> ("build.home") = home;
 
diff --git a/build2/dist/operation.cxx b/build2/dist/operation.cxx
index 11ee313..86917ed 100644
--- a/build2/dist/operation.cxx
+++ b/build2/dist/operation.cxx
@@ -19,18 +19,6 @@ namespace build2
 {
   namespace dist
   {
-    static void
-    dist_meta_operation_pre ()
-    {
-      // Reset the dependency state so that we don't end up with stray
-      // files from previous batches.
-      //
-      // @@ This is called too late, after we have bootstrapped the
-      //    project.
-      //
-      //reset ();
-    }
-
     static operation_id
     dist_operation_pre (operation_id o)
     {
@@ -441,7 +429,7 @@ namespace build2
       "distribute",
       "distributing",
       "has nothing to distribute", // We cannot "be distributed".
-      &dist_meta_operation_pre,
+      nullptr, // meta-operation pre
       &dist_operation_pre,
       &load,   // normal load
       &search, // normal search