Initial up_negotiate_configuration() implementation

6 files changed, 899 insertions, 147 deletions

diff --git a/bpkg/package-configuration.cxx b/bpkg/package-configuration.cxx
new file mode 100644
index 0000000..fb742e8
--- /dev/null
+++ b/bpkg/package-configuration.cxx
@@ -0,0 +1,128 @@
+// file      : bpkg/package-configuration.cxx -*- C++ -*-
+// license   : MIT; see accompanying LICENSE file
+
+#include <bpkg/package-configuration.hxx>
+
+#include <bpkg/package-skeleton.hxx>
+
+namespace bpkg
+{
+  using build2::config::variable_origin;
+
+  bool
+  up_negotiate_configuration (
+    package_configurations& cfgs,
+    package_skeleton& dept,
+    pair<size_t, size_t> pos,
+    const small_vector<reference_wrapper<package_skeleton>, 1>& depcs)
+  {
+    dependent_config_variable_values old_cfgs;
+
+    // Step 1: save a snapshot of the old configuration while unsetting values
+    // that have this dependent as the originator and reloading the defaults.
+    //
+    // While at it, also collect the configurations to pass to dependent's
+    // evaluate_*() calls.
+    //
+    package_skeleton::dependency_configurations depc_cfgs;
+    depc_cfgs.reserve (depcs.size ());
+
+    for (package_skeleton& depc: depcs)
+    {
+      package_configuration& cfg (cfgs[depc.key]);
+
+      for (config_variable_value& v: cfg)
+      {
+        if (v.origin == variable_origin::buildfile)
+        {
+          if (*v.dependent == dept.key)
+          {
+            old_cfgs.push_back (
+              dependent_config_variable_value {
+                v.name, move (v.value), move (*v.dependent)});
+
+            v.origin = variable_origin::undefined;
+            v.dependent = nullopt;
+          }
+          else
+            old_cfgs.push_back (
+              dependent_config_variable_value {v.name, v.value, *v.dependent});
+        }
+      }
+
+      depc.reload_defaults (cfg);
+
+      depc_cfgs.push_back (cfg);
+    }
+
+    // Step 2: execute the prefer/accept or requires clauses.
+    //
+    pos.first--; pos.second--; // Convert to 0-base.
+
+    const dependency_alternative& da (
+      dept.available.get ().dependencies[pos.first][pos.second]);
+
+    if (!(da.require
+          ? dept.evaluate_require (depc_cfgs, *da.require, pos.first)
+          : dept.evaluate_prefer_accept (depc_cfgs,
+                                         *da.prefer, *da.accept, pos.first)))
+    {
+      fail << "unable to negotiate acceptable configuration"; // @@ TODO
+    }
+
+    // Check if anything changed by comparing to entries in old_cfgs.
+    //
+    {
+      optional<size_t> n (0); // Number of unchanged.
+
+      for (package_skeleton& depc: depcs)
+      {
+        package_configuration& cfg (cfgs[depc.key]);
+
+        for (config_variable_value& v: cfg)
+        {
+          if (v.origin == variable_origin::buildfile)
+          {
+            auto i (find_if (old_cfgs.begin (), old_cfgs.end (),
+                             [&v] (const dependent_config_variable_value& o)
+                             {
+                               return v.name == o.name;
+                             }));
+
+            if (i == old_cfgs.end () ||
+                i->value != v.value  ||
+                i->dependent != *v.dependent)
+            {
+              n = nullopt;
+              break;
+            }
+
+            ++*n;
+          }
+        }
+
+        if (!n)
+          break;
+      }
+
+      // If we haven't seen any changed and we've seen the same number, then
+      // nothing has changed.
+      //
+      if (n && *n == old_cfgs.size ())
+        return false;
+    }
+
+
+    // @@ TODO: look for cycles in change history.
+    // @@ TODO: save in change history.
+    //
+    /*
+    dependent_config_variable_values new_cfgs; // @@ TODO.
+
+    old_cfgs.sort ();
+    new_cfgs.sort ();
+    */
+
+    return true;
+  }
+}
diff --git a/bpkg/package-configuration.hxx b/bpkg/package-configuration.hxx
new file mode 100644
index 0000000..55a43a5
--- /dev/null
+++ b/bpkg/package-configuration.hxx
@@ -0,0 +1,119 @@
+// file      : bpkg/package-configuration.hxx -*- C++ -*-
+// license   : MIT; see accompanying LICENSE file
+
+#ifndef BPKG_PACKAGE_CONFIGURATION_HXX
+#define BPKG_PACKAGE_CONFIGURATION_HXX
+
+#include <map>
+
+#include <libbuild2/types.hxx>        // build2::names
+#include <libbuild2/config/types.hxx> // build2::config::variable_origin
+
+#include <bpkg/types.hxx>
+#include <bpkg/utility.hxx>
+
+#include <bpkg/package.hxx>
+
+using namespace std;
+
+namespace bpkg
+{
+  class package_skeleton;
+
+  struct config_variable_value
+  {
+    string name;
+
+    // The variable_origin values have the following meaning:
+    //
+    // default    -- default value from the config directive
+    // buildfile  -- dependent configuration (config_source::dependent)
+    // override   -- user configuration      (config_source::user)
+    // undefined  -- none of the above
+    //
+    build2::config::variable_origin origin;
+
+    // If origin is not undefined, then this is the reversed variable value
+    // with absent signifying NULL.
+    //
+    optional<build2::names> value;
+
+    // Variable type name with absent signifying untyped.
+    //
+    optional<string> type;
+
+    // If origin is buildfile, then this is the "originating dependent" which
+    // first set this variable to this value.
+    //
+    optional<package_key> dependent;
+
+    // Value version (used internally by package_skeleton).
+    //
+    size_t version;
+  };
+
+  class package_configuration: public vector<config_variable_value>
+  {
+  public:
+    config_variable_value*
+    find (const string& name)
+    {
+      auto i (find_if (begin (), end (),
+                       [&name] (const config_variable_value& v)
+                       {
+                         return v.name == name;
+                       }));
+      return i != end () ? &*i : nullptr;
+    }
+
+    const config_variable_value*
+    find (const string& name) const
+    {
+      auto i (find_if (begin (), end (),
+                       [&name] (const config_variable_value& v)
+                       {
+                         return v.name == name;
+                       }));
+      return i != end () ? &*i : nullptr;
+    }
+  };
+
+
+  // @@ Maybe redo as small_vector?
+  //
+  using package_configurations = map<package_key, package_configuration>;
+
+
+  // A subset of config_variable_value for variable values set by the
+  // dependents (origin is buildfile). Used to track change history.
+  //
+  struct dependent_config_variable_value
+  {
+    string                  name;
+    optional<build2::names> value;
+    package_key             dependent;
+  };
+
+  class dependent_config_variable_values:
+    public small_vector<dependent_config_variable_value, 1>
+  {
+  public:
+    /*
+      @@
+    void
+    sort ();
+    */
+  };
+
+  // Up-negotiate the configuration for the specified dependencies of the
+  // specified dependent. Return true if the configuration has changed.
+  //
+  bool
+  up_negotiate_configuration (
+    package_configurations&,
+    package_skeleton& dependent,
+    pair<size_t, size_t> position,
+    const small_vector<reference_wrapper<package_skeleton>, 1>& dependencies);
+}
+
+#endif // BPKG_PACKAGE_CONFIGURATION_HXX
diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx
index c81700e..0d07981 100644
--- a/bpkg/package-skeleton.cxx
+++ b/bpkg/package-skeleton.cxx
@@ -54,8 +54,26 @@ namespace bpkg
 
   package_skeleton::
   package_skeleton (package_skeleton&& v)
+      : key (move (v.key)),
+        available (v.available),
+        co_ (v.co_),
+        db_ (v.db_),
+        config_vars_ (move (v.config_vars_)),
+        disfigure_ (v.disfigure_),
+        config_srcs_ (v.config_srcs_),
+        src_root_ (move (v.src_root_)),
+        out_root_ (move (v.out_root_)),
+        created_ (v.created_),
+        verified_ (v.verified_),
+        ctx_ (move (v.ctx_)),
+        rs_ (v.rs_),
+        cmd_vars_ (move (v.cmd_vars_)),
+        cmd_vars_cache_ (v.cmd_vars_cache_),
+        dependent_vars_ (move (v.dependent_vars_)),
+        reflect_vars_ (move (v.reflect_vars_)),
+        reflect_frag_ (move (v.reflect_frag_))
   {
-    *this = move (v);
+    v.db_ = nullptr;
   }
 
   package_skeleton& package_skeleton::
@@ -63,9 +81,10 @@ namespace bpkg
   {
     if (this != &v)
     {
+      key = move (v.key);
+      available = v.available;
       co_ = v.co_;
       db_ = v.db_;
-      available_ = v.available_;
       config_vars_ = move (v.config_vars_);
       disfigure_ = v.disfigure_;
       config_srcs_ = v.config_srcs_;
@@ -82,7 +101,6 @@ namespace bpkg
       reflect_frag_ = move (v.reflect_frag_);
 
       v.db_ = nullptr;
-      v.available_ = nullptr;
     }
 
     return *this;
@@ -90,9 +108,10 @@ namespace bpkg
 
   package_skeleton::
   package_skeleton (const package_skeleton& v)
-      : co_ (v.co_),
+      : key (v.key),
+        available (v.available),
+        co_ (v.co_),
         db_ (v.db_),
-        available_ (v.available_),
         config_vars_ (v.config_vars_),
         disfigure_ (v.disfigure_),
         config_srcs_ (v.config_srcs_),
@@ -119,16 +138,17 @@ namespace bpkg
                     const vector<config_variable>* css,
                     optional<dir_path> src_root,
                     optional<dir_path> out_root)
-      : co_ (&co),
+      : key (db, ap.id.name),
+        available (ap),
+        co_ (&co),
         db_ (&db),
-        available_ (&ap),
         config_vars_ (move (cvs)),
         disfigure_ (df),
         config_srcs_ (df ? nullptr : css)
   {
     // Should not be created for stubs.
     //
-    assert (available_->bootstrap_build);
+    assert (ap.bootstrap_build);
 
     // We are only interested in old user configuration variables.
     //
@@ -153,10 +173,135 @@ namespace bpkg
       assert (!out_root);
   }
 
+  // Serialize a variable assignment for a buildfile fragment.
+  //
+  static void
+  serialize_buildfile (string& r,
+                       const string& var, const build2::value& val,
+                       build2::names& storage)
+  {
+    using namespace build2;
+
+    r += var;
+    r += " = ";
+
+    if (val.null)
+      r += "[null]";
+    else
+    {
+      storage.clear ();
+      names_view nv (reverse (val, storage));
+
+      if (!nv.empty ())
+      {
+        ostringstream os;
+        to_stream (os, nv, quote_mode::normal, '@');
+        r += os.str ();
+      }
+    }
+
+    r += '\n';
+  }
+
+  // Serialize a variable assignment for a command line override.
+  //
+  static string
+  serialize_cmdline (const string& var, const build2::value& val,
+                     build2::names& storage)
+  {
+    using namespace build2;
+
+    string r (var + '=');
+
+    if (val.null)
+      r += "[null]";
+    else
+    {
+      storage.clear ();
+      names_view nv (reverse (val, storage));
+
+      if (!nv.empty ())
+      {
+        // Note: we need to use command-line (effective) quoting.
+        //
+        ostringstream os;
+        to_stream (os, nv, quote_mode::effective, '@');
+        r += os.str ();
+      }
+    }
+
+    return r;
+  }
+
+  static string
+  serialize_cmdline (const string& var, const optional<build2::names>& val)
+  {
+    using namespace build2;
+
+    string r (var + '=');
+
+    if (!val)
+      r += "[null]";
+    else
+    {
+      if (!val->empty ())
+      {
+        // Note: we need to use command-line (effective) quoting.
+        //
+        ostringstream os;
+        to_stream (os, *val, quote_mode::effective, '@');
+        r += os.str ();
+      }
+    }
+
+    return r;
+  }
+
+  // Reverse value to names.
+  //
+  static optional<build2::names>
+  reverse_value (const build2::value& val)
+  {
+    using namespace build2;
+
+    if (val.null)
+      return nullopt;
+
+    names storage;
+    names_view nv (reverse (val, storage));
+
+    return (nv.data () == storage.data ()
+            ? move (storage)
+            : names (nv.begin (), nv.end ()));
+  }
+
+  // Return the dependent (origin==buildfile) configuration variables as
+  // command line overrides.
+  //
+  static strings
+  dependent_cmd_vars (const package_configuration& cfg)
+  {
+    using build2::config::variable_origin;
+
+    strings r;
+
+    for (const config_variable_value& v: cfg)
+    {
+      if (v.origin == variable_origin::buildfile)
+        r.push_back (serialize_cmdline (v.name, v.value));
+    }
+
+    return r;
+  }
+
   void package_skeleton::
-  load_defaults (const strings& dependent_vars)
+  reload_defaults (package_configuration& cfg)
   {
-    assert (ctx_ == nullptr); // Should only be called before evaluate_*().
+    // Should only be called before dependent_config()/evaluate_*().
+    //
+    assert (dependent_vars_.empty () &&
+            reflect_vars_.empty () &&
+            ctx_ == nullptr);
 
     if (config_srcs_ != nullptr)
       load_old_config ();
@@ -165,34 +310,59 @@ namespace bpkg
     {
       using namespace build2;
 
+      // This is what needs to happen to the variables of different origins in
+      // the passed configuration:
+      //
+      // default              -- reloaded
+      // buildfile/dependent  -- made command line override
+      // override/user        -- should match what's in config_vars_
+      // undefined            -- reloaded
+      //
+      // Note also that on the first call we will have no configuration. And
+      // so to keep things simple, we merge variable of the buildfile origin
+      // into cmd_vars and then rebuild things from scratch. Note, however,
+      // that below we need to sort our these merged overrides into user and
+      // dependent, so we keep the old configuration for reference.
+      //
+      // Note also that dependent values do not clash with user overrides by
+      // construction (in evaluate_{prefer_accept,require}()): we do not add
+      // as dependent variables that have the override origin.
+      //
+      package_configuration old (move (cfg));
+
       scope& rs (
-        *bootstrap (*this, merge_cmd_vars (dependent_vars))->second.front ());
+        *bootstrap (
+          *this, merge_cmd_vars (dependent_cmd_vars (cfg)))->second.front ());
 
       // Load project's root.build.
       //
       load_root (rs);
 
       // Note that a configuration variable may not have a default value so we
-      // cannot just iterate over all the config.<name>** values set on root
-      // scope. Our options seem to be either iterating over the variable pool
-      // or forcing the config module with config.config.module=true and then
-      // using its saved variables map. Since the amout of stuff we load is
-      // quite limited, there shouldn't be too many variables in the pool. So
-      // let's go with the simpler approach for now.
+      // cannot just iterate over all the config.<name>** values set on the
+      // root scope. Our options seem to be either iterating over the variable
+      // pool or forcing the config module with config.config.module=true and
+      // then using its saved variables map. Since the amout of stuff we load
+      // is quite limited, there shouldn't be too many variables in the pool.
+      // So let's go with the simpler approach for now.
       //
       // Though the saved variables map approach would have been more accurate
       // since that's the variables that were introduced with the config
       // directive. Potentially the user could just have a buildfile
       // config.<name>** variable but it feels like that should be harmless
       // (we will return it but nobody will presumably use that information).
+      // Also, if/when we start tracking the configuration variable
+      // dependencies (i.e., which default value depend on which config
+      // variable), then the saved variables map seem like the natural place
+      // to keep this information.
       //
       string p ("config." + name ().variable ());
       size_t n (p.size ());
 
       for (const variable& var: rs.ctx.var_pool)
       {
-        if (var.name.compare (0, n, p) != 0 || (var.name[n + 1] != '.' &&
-                                                var.name[n + 1] != '\0'))
+        if (var.name.compare (0, n, p) != 0 || (var.name[n] != '.' &&
+                                                var.name[n] != '\0'))
           continue;
 
         using config::variable_origin;
@@ -201,24 +371,42 @@ namespace bpkg
 
         switch (ol.first)
         {
-        case variable_origin::override_: break; // Value in config_vars.
-        case variable_origin::undefined: break; // No default value.
         case variable_origin::default_:
+        case variable_origin::override_:
+        case variable_origin::undefined:
           {
-            // @@ TODO: save in some form?
+            config_variable_value v {var.name, ol.first, {}, {}, {}, 0};
+
+            // Override could mean user override from config_vars_ or the
+            // dependent override that we have merged above.
             //
-            // How will we enter them (along with value.extra=1) in the
-            // dependent's context. Probably programmatically. Perhaps we
-            // should just save them as untyped names? Then enter and
-            // typify. Seems reasonable. Not clear how/if we can convey
-            // overriden. Maybe we can allow the prefer/require to override
-            // them but then ignore their values?
+            if (v.origin == variable_origin::override_)
+            {
+              if (config_variable_value* ov = old.find (v.name))
+              {
+                assert (ov->origin == variable_origin::buildfile);
+
+                v.origin = variable_origin::buildfile;
+                v.dependent = move (ov->dependent);
+              }
+            }
+
+            // Save value.
             //
+            if (v.origin != variable_origin::undefined)
+              v.value = reverse_value (*ol.second);
+
+            // Save type.
+            //
+            if (var.type != nullptr)
+              v.type = var.type->name;
+
+            cfg.push_back (move (v));
             break;
           }
         case variable_origin::buildfile:
           {
-            // How can this happen if we disfigured all of them?
+            // Feel like this shouldn't happen since we have disfigured them.
             //
             assert (false);
             break;
@@ -236,9 +424,13 @@ namespace bpkg
   }
 
   pair<bool, string> package_skeleton::
-  verify_sensible (const strings& dependent_vars)
+  verify_sensible (const package_configuration& cfg)
   {
-    assert (ctx_ == nullptr); // Should only be called before evaluate_*().
+    // Should only be called before dependent_config()/evaluate_*().
+    //
+    assert (dependent_vars_.empty () &&
+            reflect_vars_.empty () &&
+            ctx_ == nullptr);
 
     if (config_srcs_ != nullptr)
       load_old_config ();
@@ -269,7 +461,8 @@ namespace bpkg
       }
 
       scope& rs (
-        *bootstrap (*this, merge_cmd_vars (dependent_vars))->second.front ());
+        *bootstrap (
+          *this, merge_cmd_vars (dependent_cmd_vars (cfg)))->second.front ());
 
       // Load project's root.build while redirecting the diagnostics stream.
       //
@@ -299,11 +492,11 @@ namespace bpkg
   }
 
   void package_skeleton::
-  dependent_config (strings&& vars)
+  dependent_config (const package_configuration& cfg)
   {
     assert (dependent_vars_.empty ()); // Must be called at most once.
 
-    dependent_vars_ = move (vars);
+    dependent_vars_ = dependent_cmd_vars (cfg);
   }
 
   // Print the location of a depends value in the specified manifest file.
@@ -405,66 +598,6 @@ namespace bpkg
     }
   }
 
-  // Serialize a variable assignment for a buildfile fragment.
-  //
-  static void
-  serialize_buildfile (string& r,
-                       const string& var, const build2::value& val,
-                       build2::names& storage)
-  {
-    using namespace build2;
-
-    r += var;
-    r += " = ";
-
-    if (val.null)
-      r += "[null]";
-    else
-    {
-      storage.clear ();
-      names_view nv (reverse (val, storage));
-
-      if (!nv.empty ())
-      {
-        ostringstream os;
-        to_stream (os, nv, quote_mode::normal, '@');
-        r += os.str ();
-      }
-    }
-
-    r += '\n';
-  }
-
-  // Serialize a variable assignment for a command line override.
-  //
-  static string
-  serialize_cmdline (const string& var, const build2::value& val,
-                     build2::names& storage)
-  {
-    using namespace build2;
-
-    string r (var + '=');
-
-    if (val.null)
-      r += "[null]";
-    else
-    {
-      storage.clear ();
-      names_view nv (reverse (val, storage));
-
-      if (!nv.empty ())
-      {
-        // Note: we need to use command-line (effective) quoting.
-        //
-        ostringstream os;
-        to_stream (os, nv, quote_mode::effective, '@');
-        r += os.str ();
-      }
-    }
-
-    return r;
-  }
-
   void package_skeleton::
   evaluate_reflect (const string& refl, size_t depends_index)
   {
@@ -476,7 +609,9 @@ namespace bpkg
     // confusion.
     //
     // @@ They could also clash with dependent configuration. Probably should
-    //    be handled in the same way (it's just another type of "user").
+    //    be handled in the same way (it's just another type of "user"). Yes,
+    //    since dependent_vars_ are entered as cmd line overrides, this is
+    //    how they are treated (but may need to adjust the diagnostics).
     //
     // It seems like the most straightforward way to achieve the desired
     // semantics with the mechanisms that we have (in other words, without
@@ -497,9 +632,9 @@ namespace bpkg
     //
     // We may also have configuration values from the previous reflect clause
     // which we want to "factor in" before evaluating the next enable or
-    // reflect clauses (so that they can use the previously reflect values or
-    // values that are derived from them in root.build). It seems like we have
-    // two options here: either enter them as true overrides similar to
+    // reflect clauses (so that they can use the previously reflected values
+    // or values that are derived from them in root.build). It seems like we
+    // have two options here: either enter them as true overrides similar to
     // config_vars_ or just evaluate them similar to loading config.build
     // (which, BTW, we might have, in case of an external package). The big
     // problem with the former approach is that it will then prevent any
@@ -512,7 +647,7 @@ namespace bpkg
     //
     // BTW, a plan B would be to restrict reflect to just config vars in which
     // case we could merge them with true overrides. Though how exactly would
-    // we do this merging is unclear.
+    // we do this merging is unclear. Hm, but they are config vars...
     //
     try
     {
@@ -554,7 +689,7 @@ namespace bpkg
       // Note: a lot of this code is inspired by the config module.
       //
 
-      // Collect all the config.<name>.* variables on the first pass and
+      // Collect all the set config.<name>.* variables on the first pass and
       // filter out unchanged on the second.
       //
       auto& vp (rs.var_pool ());
@@ -566,6 +701,8 @@ namespace bpkg
         size_t       ver;
       };
 
+      // @@ Maybe redo as small_vector?
+      //
       map<const variable*, value_data> vars;
 
       auto process = [&rs, &ns, &vars] (bool collect)
@@ -779,6 +916,195 @@ namespace bpkg
     }
   }
 
+  bool package_skeleton::
+  evaluate_prefer_accept (const dependency_configurations& cfgs,
+                          const string& prefer,
+                          const string& accept,
+                          size_t depends_index)
+  {
+    try
+    {
+      using namespace build2;
+      using config::variable_origin;
+      using build2::fail;
+      using build2::endf;
+
+      // This is what needs to happen to the variables of different origins in
+      // the passed dependency configurations:
+      //
+      // default              -- set as default (value.extra=1)
+      // buildfile/dependent  -- set as buildfile
+      // override/user        -- set as override (so cannot be overriden)
+      // undefined            -- ignored
+      //
+      // Additionally, for all origins we need to typify the variables.
+      //
+      // All of this is done by load().
+      //
+      scope& rs (load (cfgs));
+
+      // Evaluate the prefer clause.
+      //
+      {
+        istringstream is (prefer);
+        is.exceptions (istringstream::failbit | istringstream::badbit);
+
+        path_name in ("<depends-prefer-clause>");
+        uint64_t il (1);
+
+        auto df = build2::make_diag_frame (
+          [&prefer, &rs, depends_index] (const diag_record& dr)
+          {
+            dr << info << "prefer fragment:\n"
+               << prefer;
+
+            // For external packages we have the manifest so print the
+            // location of the depends value in questions.
+            //
+            if (!rs.out_eq_src ())
+              depends_location (dr,
+                                rs.src_path () / manifest_file,
+                                depends_index);
+          });
+
+        lexer l (is, in, il /* start line */);
+        parser p (rs.ctx);
+        p.parse_buildfile (l, &rs, rs);
+      }
+
+      // Evaluate the accept clause.
+      //
+      bool r;
+      {
+        istringstream is ('(' + accept + ')');
+        is.exceptions (istringstream::failbit | istringstream::badbit);
+
+        path_name in ("<depends-accept-clause>");
+        uint64_t il (1);
+
+        auto df = build2::make_diag_frame (
+          [&accept, &rs, depends_index] (const diag_record& dr)
+          {
+            dr << info << "accept condition: (" << accept << ")";
+
+            // For external packages we have the manifest so print the location
+            // of the depends value in questions.
+            //
+            if (!rs.out_eq_src ())
+              depends_location (dr,
+                                rs.src_path () / manifest_file,
+                                depends_index);
+          });
+
+        lexer l (is, in, il /* start line */);
+        parser p (rs.ctx);
+        value v (p.parse_eval (l, rs, rs, parser::pattern_mode::expand));
+
+        try
+        {
+          // Should evaluate to 'true' or 'false'.
+          //
+          r = build2::convert<bool> (move (v));
+        }
+        catch (const invalid_argument& e)
+        {
+          fail (build2::location (in, il)) << e << endf;
+        }
+      }
+
+      // If acceptable, update the configuration with the new values, if any.
+      //
+      // @@ TODO: we also need to save the subset of values that were "set"
+      //    (for some definition of "set") by this dependent to be reflected
+      //    to further clauses.
+      //
+      if (r)
+      {
+        for (package_configuration& cfg: cfgs)
+        {
+          for (config_variable_value& v: cfg)
+          {
+            pair<variable_origin, lookup> ol (config::origin (rs, v.name));
+
+            // An override cannot become non-override. And a non-override
+            // cannot become an override.
+            //
+            assert ((v.origin == variable_origin::override_) ==
+                    (ol.first == variable_origin::override_));
+
+            switch (ol.first)
+            {
+            case variable_origin::buildfile:
+              {
+                // Possible transitions:
+                //
+                // default/undefine -> buildfile -- override dependency default
+                // buildfile        -> buildfile -- override other dependent
+                //
+                optional<names> val (reverse_value (*ol.second));
+
+                if (v.origin == variable_origin::buildfile)
+                {
+                  // If unchanged, then we keep the old originating dependent
+                  // (even if the value was technically "overwritten" by this
+                  // dependent).
+                  //
+                  if (v.value == val)
+                    break;
+                }
+                else
+                  v.origin = variable_origin::buildfile;
+
+                v.value = move (val);
+                v.dependent = key; // We are the originating dependent.
+                break;
+              }
+            case variable_origin::default_:
+            case variable_origin::undefined:
+              {
+                // An undefined can only come from undefined. Likewise,
+                // default can only come from default.
+                //
+                assert (ol.first == v.origin);
+                break;
+              }
+            case variable_origin::override_:
+              {
+                // @@ We may still need to see if there is original set
+                //    by this dependent and if so, reflect the overridden
+                //    value.
+                break;
+              }
+            }
+          }
+        }
+      }
+
+      // 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;
+
+      return r;
+    }
+    catch (const build2::failed&)
+    {
+      throw failed (); // Assume the diagnostics has already been issued.
+    }
+  }
+
+  bool package_skeleton::
+  evaluate_require (const dependency_configurations&,
+                    const string&, size_t /*depends_index*/)
+  {
+    // How will we square the implied accept logic with user overrides?
+    // Feels like the only way is to not enter them as overrides and then
+    // see of any of them override manually? Nasty.
+
+    return false; // @@ TODO
+  }
+
   pair<strings, vector<config_variable>> package_skeleton::
   collect_config () &&
   {
@@ -911,13 +1237,14 @@ namespace bpkg
 
     ctx_ = nullptr; // Free.
     db_ = nullptr;
-    available_ = nullptr;
 
     return make_pair (move (vars), move (srcs));
   }
 
   const strings& package_skeleton::
-  merge_cmd_vars (const strings& dependent_vars, bool cache)
+  merge_cmd_vars (const strings& dependent_vars,
+                  const strings& dependency_vars,
+                  bool cache)
   {
     // Merge variable overrides (note that the order is important). See also a
     // custom/optimized version in load_old_config().
@@ -926,11 +1253,13 @@ namespace bpkg
     {
       const strings& vs1 (build2_cmd_vars);
       const strings& vs2 (config_vars_);
-      const strings& vs3 (dependent_vars); // Should not override.
+      const strings& vs3 (dependent_vars);  // Should not override.
+      const strings& vs4 (dependency_vars); // Should not override.
 
       // Try to reuse both vector and string buffers.
       //
-      cmd_vars_.resize (1 + vs1.size () + vs2.size () + vs3.size ());
+      cmd_vars_.resize (
+        1 + vs1.size () + vs2.size () + vs3.size () + vs4.size ());
 
       size_t i (0);
       {
@@ -957,6 +1286,7 @@ namespace bpkg
       for (const string& v: vs1) cmd_vars_[i++] = v;
       for (const string& v: vs2) cmd_vars_[i++] = v;
       for (const string& v: vs3) cmd_vars_[i++] = v;
+      for (const string& v: vs4) cmd_vars_[i++] = v;
 
       cmd_vars_cache_ = cache;
     }
@@ -1071,10 +1401,17 @@ namespace bpkg
   }
 
   build2::scope& package_skeleton::
-  load ()
+  load (const dependency_configurations& cfgs)
   {
     if (ctx_ != nullptr)
-      return *rs_;
+    {
+      // We have to reload if there is any dependency configuration.
+      //
+      if (cfgs.empty ())
+        return *rs_;
+
+      ctx_ = nullptr;
+    }
 
     if (config_srcs_ != nullptr)
       load_old_config ();
@@ -1082,12 +1419,35 @@ namespace bpkg
     try
     {
       using namespace build2;
+      using build2::config::variable_origin;
+
+      // If we have any dependency configurations, then here we need to add
+      // dependency configuration variables with the override origin to the
+      // command line overrides (see evaluate_prefer_accept() for details).
+      //
+      strings dependency_vars;
+      for (package_configuration& cfg: cfgs)
+      {
+        for (config_variable_value& v: cfg)
+        {
+          if (v.origin == variable_origin::override_)
+          {
+            dependency_vars.push_back (serialize_cmdline (v.name, v.value));
+            v.version = 0; // No value, only override.
+          }
+        }
+      }
 
-      // We can reuse already merged cmd_vars if this has already been done
-      // (they don't change during evaluate_*() calls).
+      // If there aren't any, then we can reuse already merged cmd_vars (they
+      // don't change during evaluate_*() calls except for the dependency
+      // overrides case).
       //
-      auto rsi (
-        bootstrap (*this, merge_cmd_vars (dependent_vars_, true /* cache */)));
+      const strings& cmd_vars (
+        merge_cmd_vars (dependent_vars_,
+                        dependency_vars,
+                        dependency_vars.empty () /* cache */));
+
+      auto rsi (bootstrap (*this, cmd_vars));
       scope& rs (*rsi->second.front ());
 
       // Load project's root.build as well as potentially accumulated reflect
@@ -1101,20 +1461,78 @@ namespace bpkg
       // configuration and then load it with config.config.load and this
       // approach should work for that case too.
       //
+      // This is also where we set dependency configuration variables with the
+      // default and buildfile origins and typify all dependency variables
+      // (see evaluate_prefer_accept() for details).
+      //
       function<void (parser&)> pre;
 
-      if (!reflect_frag_.empty ())
+      struct data
       {
-        pre = [this, &rs] (parser& p)
+        scope& rs;
+        const dependency_configurations& cfgs;
+      } d {rs, cfgs};
+
+      if (!reflect_frag_.empty () || !cfgs.empty ())
+      {
+        pre = [this, &d] (parser& p)
         {
-          istringstream is (reflect_frag_);
-          is.exceptions (istringstream::failbit | istringstream::badbit);
+          scope& rs (d.rs);
 
-          // Note that the fragment is just a bunch of variable assignments
-          // and thus unlikely to cause any errors.
-          //
-          path_name in ("<accumulated-reflect-fragment>");
-          p.parse_buildfile (is, in, &rs, rs);
+          if (!reflect_frag_.empty ())
+          {
+            istringstream is (reflect_frag_);
+            is.exceptions (istringstream::failbit | istringstream::badbit);
+
+            // Note that the fragment is just a bunch of variable assignments
+            // and thus unlikely to cause any errors.
+            //
+            path_name in ("<accumulated-reflect-fragment>");
+            p.parse_buildfile (is, in, &rs, rs);
+          }
+
+          for (package_configuration& cfg: d.cfgs)
+          {
+            for (config_variable_value& v: cfg)
+            {
+              const value_type* vt (nullptr);
+              if (v.type)
+              {
+                vt = parser::find_value_type (&rs, *v.type);
+                assert (vt != nullptr);
+              }
+
+              const variable& var (rs.var_pool ().insert (v.name, vt));
+
+              switch (v.origin)
+              {
+              case variable_origin::default_:
+              case variable_origin::buildfile:
+                {
+                  auto& val (
+                    static_cast<variable_map::value_data&> (
+                      rs.assign (var)));
+
+                  if (v.value)
+                    val.assign (names (*v.value), &var);
+                  else
+                    val = nullptr;
+
+                  if (v.origin == variable_origin::default_)
+                    val.extra = 1;
+
+                  v.version = val.version;
+                  break;
+                }
+              case variable_origin::undefined:
+                {
+                  v.version = 0;
+                  break;
+                }
+              case variable_origin::override_: break; // Already handled.
+              }
+            }
+          }
         };
       }
 
@@ -1138,7 +1556,7 @@ namespace bpkg
   static build2::scope_map::iterator
   bootstrap (package_skeleton& skl, const strings& cmd_vars)
   {
-    assert (skl.ctx_ == nullptr);
+    assert (skl.db_ != nullptr && skl.ctx_ == nullptr);
 
     // The overall plan is as follows:
     //
@@ -1158,7 +1576,7 @@ namespace bpkg
     //
     if (!skl.created_)
     {
-      const available_package& ap (*skl.available_);
+      const available_package& ap (skl.available);
 
       // Note that we create the skeleton directories in the skeletons/
       // subdirectory of the configuration temporary directory to make sure
diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx
index 1b7e019..1c4ae32 100644
--- a/bpkg/package-skeleton.hxx
+++ b/bpkg/package-skeleton.hxx
@@ -4,12 +4,13 @@
 #ifndef BPKG_PACKAGE_SKELETON_HXX
 #define BPKG_PACKAGE_SKELETON_HXX
 
-#include <libbuild2/forward.hxx> // build2::context
+#include <libbuild2/forward.hxx>
 
 #include <bpkg/types.hxx>
 #include <bpkg/utility.hxx>
 
 #include <bpkg/package.hxx>
+#include <bpkg/package-configuration.hxx>
 #include <bpkg/common-options.hxx>
 
 namespace bpkg
@@ -62,13 +63,17 @@ namespace bpkg
                       optional<dir_path> src_root,
                       optional<dir_path> out_root);
 
+
+    package_key key;
+    reference_wrapper<const available_package> available;
+
     const package_name&
-    name () const {return available_->id.name;}
+    name () const {return key.name;} // @@ TMP: get rid (use key.name).
 
     // The following functions should be called in the following sequence
     // (* -- zero or more, ? -- zero or one):
     //
-    // * load_defaults()
+    // * reload_defaults()
     // * verify_sensible()
     // ? dependent_config()
     // * evaluate_enable() | evaluate_reflect()
@@ -78,31 +83,26 @@ namespace bpkg
     // sequence rather than starting from scratch.
     //
   public:
-    // Load the default values and type information for configuration
-    // variables of the package given a "tentative" dependent configuration.
-    //
-    // @@ TODO: if it will be more convenient to pass it as something other
-    //    than strings, then this can be accomodated.
+    // Reload the default values and type information for configuration
+    // variables using the values with the buildfile origin as a "tentative"
+    // dependent configuration.
     //
     void
-    load_defaults (const strings& dependent_vars);
+    reload_defaults (package_configuration&);
 
-    // Verify the specified "tentative" dependents configuration is sensible,
-    // that is, acceptable to the package. If it is not, then the second half
-    // of the result contains the diagnostics.
-    //
-    // @@ TODO: if it will be more convenient to pass it as something other
-    //    than strings, then this can be accomodated.
+    // Verify the specified "tentative" dependent configuration is sensible,
+    // that is, acceptable to the dependency itself. If it is not, then the
+    // second half of the result contains the diagnostics.
     //
     pair<bool, string>
-    verify_sensible (const strings& dependent_vars);
+    verify_sensible (const package_configuration&);
 
     // Incorporate the "final" dependent configuration into subsequent
     // evaluations. Dependent configuration variables are expected not to
     // clash with user.
     //
     void
-    dependent_config (strings&&);
+    dependent_config (const package_configuration&);
 
     // For the following evaluate_*() functions assume that the clause belongs
     // to the specified (by index) depends value (used to print its location
@@ -119,6 +119,23 @@ namespace bpkg
     void
     evaluate_reflect (const string&, size_t depends_index);
 
+    // Evaluate the prefer/accept or require clauses on the specified
+    // dependency configurations (serves as both input and output).
+    //
+    // Return true is acceptable and false otherwise. If acceptable, the
+    // passed configuration is updated with new values, if any.
+    //
+    using dependency_configurations =
+      small_vector<reference_wrapper<package_configuration>, 1>;
+
+    bool
+    evaluate_prefer_accept (const dependency_configurations&,
+                            const string&, const string&, size_t depends_index);
+
+    bool
+    evaluate_require (const dependency_configurations&,
+                      const string&, size_t depends_index);
+
     // Return the accumulated configuration variables (first) and project
     // configuration variable sources (second). Note that the arrays are not
     // necessarily parallel (config_vars may contain non-project variables).
@@ -161,8 +178,11 @@ namespace bpkg
     //
     // Call this function before evaluating every clause.
     //
+    // If dependency configurations are specified, then set their values and
+    // save the resulting versions in config_variable_value::version.
+    //
     build2::scope&
-    load ();
+    load (const dependency_configurations& = {});
 
     // Merge command line variable overrides into one list (normally to be
     // passed to bootstrap()).
@@ -171,7 +191,9 @@ namespace bpkg
     // calls.
     //
     const strings&
-    merge_cmd_vars (const strings& dependent_vars, bool cache = false);
+    merge_cmd_vars (const strings& dependent_vars,
+                    const strings& dependency_vars = {},
+                    bool cache = false);
 
     // Implementation details (public for bootstrap()).
     //
@@ -180,7 +202,6 @@ namespace bpkg
     //
     const common_options* co_;
     database* db_;
-    const available_package* available_;
 
     strings config_vars_;
     bool disfigure_;
diff --git a/bpkg/package.hxx b/bpkg/package.hxx
index 6ae2109..dc5031c 100644
--- a/bpkg/package.hxx
+++ b/bpkg/package.hxx
@@ -1590,6 +1590,12 @@ namespace bpkg
     }
 
     bool
+    operator!= (const package_key& v) const
+    {
+      return !(*this == v);
+    }
+
+    bool
     operator< (const package_key&) const;
 
     // Return the package string representation in the form:
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 43a8084..4194e3e 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -37,6 +37,7 @@
 #include <bpkg/pkg-disfigure.hxx>
 #include <bpkg/package-skeleton.hxx>
 #include <bpkg/system-repository.hxx>
+#include <bpkg/package-configuration.hxx>
 
 using namespace std;
 using namespace butl;
@@ -1332,6 +1333,15 @@ namespace bpkg
     dependents_map dependents;
     packages       dependencies;
 
+    // Dependency configuration.
+    //
+    // Note that this container may not yet contain some entries that are
+    // already in the dependencies member above. And it may already contain
+    // entries that are not yet in dependencies due to the retry_configuration
+    // logic.
+    //
+    package_configurations dependency_configurations;
+
     // Shadow dependencies and clusters.
     //
     // See the collect lambda in collect_build_prerequisites() for details.
@@ -6011,6 +6021,10 @@ namespace bpkg
 
         // Negotiate the configuration.
         //
+        // The overall plan is as follows: continue refining the configuration
+        // until there are no more changes by giving each dependent a chance
+        // to make further adjustments.
+        //
         for (auto b (pcfg->dependents.begin ()),
                i (b),
                e (pcfg->dependents.end ()); i != e; )
@@ -6023,11 +6037,15 @@ namespace bpkg
           // recursively collecting it). But we could be re-resolving the same
           // dependency multiple times.
           //
+          package_skeleton* dept;
           {
             build_package* b (entered_build (i->first));
             assert (b != nullptr && b->skeleton);
+            dept = &*b->skeleton;
           }
 
+          pair<size_t, size_t> pos;
+          small_vector<reference_wrapper<package_skeleton>, 1> depcs;
           {
             // A non-negotiated cluster must only have one depends position
             // for each dependent.
@@ -6037,14 +6055,29 @@ namespace bpkg
             const postponed_configuration::dependency& ds (
               i->second.dependencies.front ());
 
+            pos = ds.position;
+
+            depcs.reserve (ds.size ());
             for (const package_key& pk: ds)
             {
               build_package* b (entered_build (pk));
-              assert (b != nullptr && !b->skeleton);
+              assert (b != nullptr);
+
+              depcs.push_back (b->skeleton
+                               ? *b->skeleton
+                               : b->init_skeleton (o /* options */));
             }
           }
 
-          // up_negotiate (...);
+          if (up_negotiate_configuration (
+                pcfg->dependency_configurations, *dept, pos, depcs))
+          {
+            if (i != b)
+            {
+              i = b; // Restart from the beginning.
+              continue;
+            }
+          }
 
           ++i;
         }
@@ -6086,8 +6119,33 @@ namespace bpkg
           //
           if (!b->recursive_collection)
           {
-            build_package_refs dep_chain;
+            // Verify and set the dependent configuration for this dependency.
+            //
+            {
+              assert (b->skeleton); // Should have been init'ed above.
+
+              const package_configuration& cfg (
+                pcfg->dependency_configurations[p]);
+
+              pair<bool, string> pr (b->skeleton->verify_sensible (cfg));
+
+              if (!pr.first)
+              {
+                // @@ TODO: improve (print dependencies, dependents, config).
+                //
+                // Note that the diagnostics from the dependent will most
+                // likely be in the "error ..." form (potentially with
+                // additional info lines) and by printing it with a two-space
+                // indentation we make it "fit" into our diag record.
+                //
+                fail << "unable to negotiate sensible configuration\n"
+                     << "  " << pr.second;
+              }
 
+              b->skeleton->dependent_config (cfg);
+            }
+
+            build_package_refs dep_chain;
             collect_build_prerequisites (o,
                                          *b,
                                          fdb,
@@ -6434,7 +6492,8 @@ namespace bpkg
               // dependent/dependencies (i.e., we add them to the cluster but
               // they never get re-visited). @@ While seems harmless, maybe we
               // need to prune the part of the configuration that was added
-              // by them?
+              // by them? Maybe we should have the confirmed flag which is
+              // set when the originating dependent confirms the value?!
               //
               pc->add_shadow (move (e.dependent), e.position);
             }
@@ -6461,9 +6520,10 @@ namespace bpkg
 
               assert (!pc->negotiated);
 
-              // Drop any accumulated shadow dependents (which could be
-              // carried over from retry_configuration logic).
+              // Drop any accumulated configuration and shadow dependents
+              // (which could be carried over from retry_configuration logic).
               //
+              pc->dependency_configurations.clear ();
               pc->shadow_dependents.clear ();
 
               l5 ([&]{trace << "cfg-negotiation of " << *pc << " failed due "