From b1b6d9f2b18928237a47c14436eee3b985223ed0 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Tue, 14 Jun 2022 06:19:25 +0200
Subject: Make all dependency values accessible in reflect clause for same
 position

---
 bpkg/package-skeleton.cxx | 71 +++++++++++++++++++++++++++++++++++++++++------
 bpkg/package-skeleton.hxx | 10 +++++++
 2 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx
index b2da2a8..6539077 100644
--- a/bpkg/package-skeleton.cxx
+++ b/bpkg/package-skeleton.cxx
@@ -76,7 +76,8 @@ namespace bpkg
         reflect_frag_ (move (v.reflect_frag_)),
         dependency_reflect_ (move (v.dependency_reflect_)),
         dependency_reflect_index_ (v.dependency_reflect_index_),
-        dependency_reflect_pending_ (v.dependency_reflect_pending_)
+        dependency_reflect_pending_ (v.dependency_reflect_pending_),
+        prefer_accept_ (v.prefer_accept_)
   {
     v.db_ = nullptr;
   }
@@ -109,6 +110,7 @@ namespace bpkg
       dependency_reflect_ = move (v.dependency_reflect_);
       dependency_reflect_index_ = v.dependency_reflect_index_;
       dependency_reflect_pending_ = v.dependency_reflect_pending_;
+      prefer_accept_ = v.prefer_accept_;
 
       v.db_ = nullptr;
     }
@@ -138,10 +140,17 @@ namespace bpkg
         reflect_frag_ (v.reflect_frag_),
         dependency_reflect_ (v.dependency_reflect_),
         dependency_reflect_index_ (v.dependency_reflect_index_),
-        dependency_reflect_pending_ (v.dependency_reflect_pending_)
+        dependency_reflect_pending_ (v.dependency_reflect_pending_),
+        prefer_accept_ (v.prefer_accept_)
   {
     // The idea here is to create an "unloaded" copy but with enough state
     // that it can be loaded if necessary.
+    //
+    // Note that there is a bit of a hole in this logic with regards to the
+    // prefer_accept_ semantics but it looks like we cannot plausible trigger
+    // it (which is fortified with an assert in evaluate_reflect(); note that
+    // doing it here would be overly strict since this may have a left-over
+    // prefer_accept_ position).
   }
 
   void package_skeleton::
@@ -164,6 +173,8 @@ namespace bpkg
     dependency_reflect_.clear ();
     dependency_reflect_index_ = 0;
     dependency_reflect_pending_ = 0;
+
+    prefer_accept_ = nullopt;
   }
 
   package_skeleton::
@@ -582,6 +593,14 @@ namespace bpkg
       using build2::info;
       using build2::endf;
 
+      // Drop the state from the previous evaluation of prefer/accept.
+      //
+      if (prefer_accept_)
+      {
+        ctx_ = nullptr;
+        prefer_accept_ = nullopt;
+      }
+
       scope& rs (load ());
 
       istringstream is ('(' + cond + ')');
@@ -693,6 +712,25 @@ namespace bpkg
       using build2::info;
       using build2::endf;
 
+      // Drop the state from the previous evaluation of prefer/accept if it's
+      // from the wrong position.
+      //
+      if (prefer_accept_)
+      {
+        if (*prefer_accept_ != indexes)
+        {
+          ctx_ = nullptr;
+          prefer_accept_ = nullopt;
+        }
+        else
+          // This could theoretically happen if we make a copy of the skeleton
+          // after evaluate_prefer_accept() and then attempt to continue with
+          // the call on the copy to evaluate_reflect() passing the same
+          // position. But it doesn't appear our usage should trigger this.
+          //
+          assert (ctx_ != nullptr);
+      }
+
       scope& rs (load ());
 
       istringstream is (refl);
@@ -973,6 +1011,14 @@ namespace bpkg
       using build2::info;
       using build2::endf;
 
+      // Drop the state from the previous evaluation of prefer/accept.
+      //
+      if (prefer_accept_)
+      {
+        ctx_ = nullptr;
+        prefer_accept_ = nullopt;
+      }
+
       // Drop any dependency reflect values from the previous evaluation of
       // this clause, if any.
       //
@@ -1215,13 +1261,14 @@ namespace bpkg
             }
           }
         }
-      }
 
-      // Drop the build system state since it needs reloading (for one, we
-      // could have dependency configuration, such as defaults and other
-      // dependent values, that further clauses should not see).
-      //
-      ctx_ = nullptr;
+        // Note: do not drop the build system state yet since it should be
+        // reused by the following reflect clause, if any.
+        //
+        prefer_accept_ = indexes;
+      }
+      else
+        ctx_ = nullptr;
 
       return r;
     }
@@ -1247,6 +1294,14 @@ namespace bpkg
       using build2::info;
       using build2::endf;
 
+      // Drop the state from the previous evaluation of prefer/accept.
+      //
+      if (prefer_accept_)
+      {
+        ctx_ = nullptr;
+        prefer_accept_ = nullopt;
+      }
+
       // Drop any dependency reflect values from the previous evaluation of
       // this clause, if any.
       //
diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx
index a0ac38a..aa9b2a9 100644
--- a/bpkg/package-skeleton.hxx
+++ b/bpkg/package-skeleton.hxx
@@ -298,6 +298,16 @@ namespace bpkg
     reflect_variable_values dependency_reflect_;
     size_t                  dependency_reflect_index_ = 0;
     size_t                  dependency_reflect_pending_ = 0;
+
+    // Position of the last successfully evaluated prefer/accept clauses.
+    //
+    // This information is used to make all (as opposed to only those set by
+    // the prefer clause) dependency configuration variables available to the
+    // reflect clause but only at the same position. This allows for some more
+    // advanced configuration techniques, such as, using a feature if enabled
+    // by someone else but not having any preferences ourselves.
+    //
+    optional<pair<size_t, size_t>> prefer_accept_;
   };
 }
 
-- 
cgit v1.1