From 4c3e451a852b537c04f5b73af23639902117b94f Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Fri, 22 Jul 2016 09:23:55 +0200
Subject: Change default var override from 'projects and subprojects' to
 amalgamation

The 'projects and subprojects' semantics resulted in some counter-intuitive
behavior. For example, in a project with tests/ as a subproject if one builds
one of the tests directly with a non-global override (say C++ compiler), then
the main project would be built without the overrides. I this light,
overriding in the whole amalgamation seems like the right thing to do. The
old behavior can still be obtained with scope qualification, for example:

b ./:foo=bar
---
 build2/b.cxx                     | 36 +++++++++++++++++++++++++++++++++---
 build2/context.cxx               |  3 +++
 tests/variable/override/p/loader |  1 +
 tests/variable/override/test.sh  | 10 ++++++++++
 4 files changed, 47 insertions(+), 3 deletions(-)
 create mode 100644 tests/variable/override/p/loader

diff --git a/build2/b.cxx b/build2/b.cxx
index 3cd8793..d16c0c6 100644
--- a/build2/b.cxx
+++ b/build2/b.cxx
@@ -790,14 +790,44 @@ main (int argc, char* argv[])
           // second sitution so if it is already set, then it can only be the
           // first case.
           //
-          bool first (true);
+          // This is further complicated by the project vs amalgamation logic
+          // (we may have already done the amalgamation but not the project).
+          //
+          bool first_a (true);
           for (const variable_override& o: var_ovs)
           {
+            if (o.ovr.visibility != variable_visibility::normal)
+              continue;
+
+            auto p (rs.weak_scope ()->vars.insert (o.ovr));
+
+            if (!p.second)
+            {
+              if (first_a)
+                break;
+
+              fail << "multiple amalgamation overrides of variable "
+                   << o.var.name;
+            }
+
+            value& v (p.first);
+            v = o.val;
+            first_a = false;
+          }
+
+          bool first_p (true);
+          for (const variable_override& o: var_ovs)
+          {
+            // Ours is either project (%foo) or scope (/foo).
+            //
+            if (o.ovr.visibility == variable_visibility::normal)
+              continue;
+
             auto p (rs.vars.insert (o.ovr));
 
             if (!p.second)
             {
-              if (first)
+              if (first_p)
                 break;
 
               fail << "multiple project overrides of variable " << o.var.name;
@@ -805,7 +835,7 @@ main (int argc, char* argv[])
 
             value& v (p.first);
             v = o.val;
-            first = false;
+            first_p = false;
           }
 
           ts.root_scope = &rs;
diff --git a/build2/context.cxx b/build2/context.cxx
index 60e55bd..e08fb52 100644
--- a/build2/context.cxx
+++ b/build2/context.cxx
@@ -183,6 +183,9 @@ namespace build2
       if (r.first.type != nullptr)
         fail << "typed override of variable " << n;
 
+      // Global and scope overrides we can enter directly. Project ones will
+      // be entered by the caller for for each amalgamation/project.
+      //
       if (c == '!' || !dir.empty ())
       {
         scope& s (c == '!' ? gs : scopes.insert (dir, false)->second);
diff --git a/tests/variable/override/p/loader b/tests/variable/override/p/loader
new file mode 100644
index 0000000..f298dcc
--- /dev/null
+++ b/tests/variable/override/p/loader
@@ -0,0 +1 @@
+include ../buildfile
diff --git a/tests/variable/override/test.sh b/tests/variable/override/test.sh
index e960929..a8b08b2 100755
--- a/tests/variable/override/test.sh
+++ b/tests/variable/override/test.sh
@@ -103,6 +103,16 @@ p/d   : X
 p/d/t : X
 EOF
 
+test v=X --buildfile loader ./p/ <<EOF
+/     :
+.     : X
+d     : X
+d/t   : X
+p     : X
+p/d   : X
+p/d/t : X
+EOF
+
 test /v=X <<EOF
 /     :
 .     : X
-- 
cgit v1.1