Fix operation-specific variables logic for inner/outer operations

6 files changed, 52 insertions, 23 deletions

diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx
index d0b24e0..a052481 100644
--- a/libbuild2/context.cxx
+++ b/libbuild2/context.cxx
@@ -706,18 +706,28 @@ namespace build2
     current_mode = inner_oif.mode;
     current_diag_noise = diag_noise;
 
-    if (oif.var_name != nullptr)
+    auto find_ovar = [this] (const char* n)
     {
-      current_ovar = var_pool.find (oif.var_name);
+      const variable* v (var_pool.find (n));
 
       // The operation variable should have prerequisite or target visibility.
       //
-      assert (current_ovar != nullptr &&
-              (current_ovar->visibility == variable_visibility::prereq ||
-               current_ovar->visibility == variable_visibility::target));
-    }
-    else
-      current_ovar = nullptr;
+      assert (v != nullptr &&
+              (v->visibility == variable_visibility::prereq ||
+               v->visibility == variable_visibility::target));
+
+      return v;
+    };
+
+    current_inner_ovar =
+      inner_oif.var_name != nullptr
+      ? find_ovar (inner_oif.var_name)
+      : nullptr;
+
+    current_outer_ovar =
+      outer_oif != nullptr && outer_oif->var_name != nullptr
+      ? find_ovar (outer_oif->var_name)
+      : nullptr;
 
     // Reset counters (serial execution).
     //
diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx
index 20098dc..1483eb3 100644
--- a/libbuild2/context.hxx
+++ b/libbuild2/context.hxx
@@ -280,10 +280,14 @@ namespace build2
     string current_oname;
 
     const meta_operation_info* current_mif;
+
     const operation_info* current_inner_oif;
     const operation_info* current_outer_oif;
 
-    const variable* current_ovar; // Current (outer) operation variable.
+    // Current operation-specific variables.
+    //
+    const variable* current_inner_ovar;
+    const variable* current_outer_ovar;
 
     action
     current_action () const
diff --git a/libbuild2/install/operation.cxx b/libbuild2/install/operation.cxx
index 6ae2819..8c59ac1 100644
--- a/libbuild2/install/operation.cxx
+++ b/libbuild2/install/operation.cxx
@@ -81,7 +81,7 @@ namespace build2
       update_id, // Note: not update_for_install_id.
       install_id,
       op_update.name,
-      nullptr, // Outer operation variable is always used.
+      op_update.var_name,
       op_update.name_do,
       op_update.name_doing,
       op_update.name_did,
diff --git a/libbuild2/install/rule.cxx b/libbuild2/install/rule.cxx
index db9c64a..3ed61f3 100644
--- a/libbuild2/install/rule.cxx
+++ b/libbuild2/install/rule.cxx
@@ -37,6 +37,17 @@ namespace build2
       return r.simple () && r.string () == "false" ? nullptr : &r;
     }
 
+    // Note that the below rules are called for both install and
+    // update-for-install.
+    //
+    static inline const variable&
+    var_install (context& ctx)
+    {
+      return ctx.current_outer_ovar != nullptr
+        ? *ctx.current_outer_ovar
+        : *ctx.current_inner_ovar;
+    }
+
     // alias_rule
     //
     const alias_rule alias_rule::instance;
@@ -127,7 +138,7 @@ namespace build2
         //
         // Note: not the same as lookup_install() above.
         //
-        auto l ((*pt)[ctx.current_ovar]); // "install"
+        auto l ((*pt)[var_install (ctx)]);
         if (l && cast<path> (l).string () == "false")
         {
           l5 ([&]{trace << "ignoring " << *pt << " (not installable)";});
@@ -249,7 +260,7 @@ namespace build2
           //
           // Note: not the same as lookup_install() above.
           //
-          auto l ((*mt)[ctx.current_ovar]); // "install"
+          auto l ((*mt)[var_install (ctx)]);
           if (l && cast<path> (l).string () == "false")
           {
             l5 ([&]{trace << "ignoring " << *mt << " (not installable)";});
@@ -300,7 +311,7 @@ namespace build2
         // through and also diagnose any invalid values.
         //
         if (p.vars.empty () ||
-            cast_empty<path> (p.vars[t.ctx.current_ovar]).string () != "true")
+            cast_empty<path> (p.vars[var_install (t.ctx)]).string () != "true")
           return nullptr;
       }
 
@@ -389,7 +400,7 @@ namespace build2
         //
         // Note: not the same as lookup_install() above.
         //
-        auto l ((*pt)[ctx.current_ovar]); // "install"
+        auto l ((*pt)[var_install (ctx)]);
         if (l && cast<path> (l).string () == "false")
         {
           l5 ([&]{trace << "ignoring " << *pt << " (not installable)";});
@@ -1043,7 +1054,7 @@ namespace build2
       //
       if (!tp.empty ())
       {
-        install_target (t, cast<path> (t[ctx.current_ovar]), 1); // "install"
+        install_target (t, cast<path> (t[var_install (ctx)]), 1);
         r |= target_state::changed;
       }
 
@@ -1312,9 +1323,7 @@ namespace build2
       target_state r (target_state::unchanged);
 
       if (!tp.empty ())
-        r |= uninstall_target (t,
-                               cast<path> (t[ctx.current_ovar]), // "install"
-                               1);
+        r |= uninstall_target (t, cast<path> (t[var_install (ctx)]), 1);
 
       // Then installable ad hoc group members, if any. To be anally precise,
       // we would have to do it in reverse, but that's not easy (it's a
diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx
index f829cca..7ab0abd 100644
--- a/libbuild2/target.cxx
+++ b/libbuild2/target.cxx
@@ -564,17 +564,23 @@ namespace build2
              << "'" << *v << "' specified for prerequisite " << p;
     }
 
-    // Handle operation-specific override.
+    // Handle operation-specific override (see var_include documentation
+    // for details).
     //
     lookup l;
     optional<bool> r1; // Absent means something other than true|false.
 
     names storage;
     names_view ns;
+    const variable* current_ovar (nullptr);
 
-    if (r != include_type::excluded && ctx.current_ovar != nullptr)
+    if (r != include_type::excluded)
     {
-      if ((l = p.vars[*ctx.current_ovar]))
+      current_ovar = a.outer ()
+        ? ctx.current_outer_ovar
+        : ctx.current_inner_ovar;
+
+      if (current_ovar != nullptr && (l = p.vars[*current_ovar]))
       {
         // Maybe we should optimize this for the common cases (bool, path,
         // name)? But then again we don't expect many such overrides. Plus
@@ -619,7 +625,7 @@ namespace build2
         // Note: we have to delay this until the meta-operation callback above
         // had a chance to override it.
         //
-        fail << "unrecognized " << *ctx.current_ovar << " variable value "
+        fail << "unrecognized " << *current_ovar << " variable value "
              << "'" << ns << "' specified for prerequisite " << p;
       }
     }
diff --git a/libbuild2/test/operation.cxx b/libbuild2/test/operation.cxx
index ff841a6..b7ec357 100644
--- a/libbuild2/test/operation.cxx
+++ b/libbuild2/test/operation.cxx
@@ -83,7 +83,7 @@ namespace build2
       update_id, // Note: not update_for_test_id.
       test_id,
       op_update.name,
-      nullptr, // Outer operation variable is always used.
+      op_update.var_name,
       op_update.name_do,
       op_update.name_doing,
       op_update.name_did,