Initial support for negotiation with system dependencies

6 files changed, 353 insertions, 90 deletions

diff --git a/bpkg/package-configuration.cxx b/bpkg/package-configuration.cxx
index 3c4fd44..688d202 100644
--- a/bpkg/package-configuration.cxx
+++ b/bpkg/package-configuration.cxx
@@ -118,6 +118,8 @@ namespace bpkg
     pair<size_t, size_t> pos,
     const small_vector<reference_wrapper<package_skeleton>, 1>& depcs)
   {
+    assert (!dept.system);
+
     pos.first--; pos.second--; // Convert to 0-base.
 
     const dependency_alternative& da (
@@ -197,6 +199,11 @@ namespace bpkg
     // based on the values of other configuration variables (we have a note
     // in the manual instructing the user not to do this).
     //
+    // The dependency could also be system in which case there could be no
+    // skeleton information to load the types/defaults from. In this case we
+    // can handle require in the "lax mode" (based on the above assumptions)
+    // but not prefer.
+    //
     // While at it, also collect the configurations to pass to dependent's
     // evaluate_*() calls.
     //
@@ -228,6 +235,31 @@ namespace bpkg
         }
       }
 
+      if (depc.available == nullptr)
+      {
+        assert (depc.system);
+
+        if (da.prefer)
+          fail << "unable to negotiate configuration for system dependency "
+               << depc.key << " without configuration information" <<
+            info << "consider specifying system dependency version that has "
+               << "corresponding available package" <<
+            info << "dependent " << dept.key << " has prefer/accept clauses "
+               << "that cannot be evaluated without configuration information";
+
+        if (!cfg.system)
+        {
+          // Note that we still need the overrides.
+          //
+          depc.load_overrides (cfg);
+          cfg.system = true;
+        }
+
+        continue;
+      }
+      else
+        assert (!cfg.system);
+
       if (da.prefer || cfg.empty ())
         depc.reload_defaults (cfg);
     }
diff --git a/bpkg/package-configuration.hxx b/bpkg/package-configuration.hxx
index afc3dba..6995857 100644
--- a/bpkg/package-configuration.hxx
+++ b/bpkg/package-configuration.hxx
@@ -113,6 +113,7 @@ namespace bpkg
   {
   public:
     package_key package;
+    bool system = false; // True if system package without skeleton info.
 
     explicit
     package_configuration (package_key p): package (move (p)) {}
diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx
index b4cb23c..b565bc1 100644
--- a/bpkg/package-skeleton.cxx
+++ b/bpkg/package-skeleton.cxx
@@ -145,6 +145,9 @@ namespace bpkg
     optional<size_t> dependency_var_prefixes_pending_;
   };
 
+  static void
+  create_context (package_skeleton&, const strings&);
+
   // Note: cannot be package_skeleton member function due to iterator return
   // (build2 stuff is only forward-declared in the header).
   //
@@ -159,7 +162,8 @@ namespace bpkg
   package_skeleton::
   package_skeleton (package_skeleton&& v)
       : key (move (v.key)),
-        available (v.available),
+        system (v.system),
+        available (move (v.available)),
         co_ (v.co_),
         db_ (v.db_),
         var_prefix_ (move (v.var_prefix_)),
@@ -196,7 +200,8 @@ namespace bpkg
     if (this != &v)
     {
       key = move (v.key);
-      available = v.available;
+      system = v.system;
+      available = move (v.available);
       co_ = v.co_;
       db_ = v.db_;
       var_prefix_ = move (v.var_prefix_);
@@ -233,6 +238,7 @@ namespace bpkg
   package_skeleton::
   package_skeleton (const package_skeleton& v)
       : key (v.key),
+        system (v.system),
         available (v.available),
         co_ (v.co_),
         db_ (v.db_),
@@ -298,25 +304,28 @@ namespace bpkg
 
   package_skeleton::
   package_skeleton (const common_options& co,
-                    database& db,
+                    package_key pk,
+                    bool sys,
                     shared_ptr<const available_package> ap,
                     strings cvs,
                     bool df,
                     const vector<config_variable>* css,
                     optional<dir_path> src_root,
                     optional<dir_path> out_root)
-      : key (db, ap->id.name),
+      : key (move (pk)),
+        system (sys),
         available (move (ap)),
         co_ (&co),
-        db_ (&db),
+        db_ (&key.db.get ()),
         var_prefix_ ("config." + key.name.variable ()),
         config_vars_ (move (cvs)),
         disfigure_ (df),
         config_srcs_ (df ? nullptr : css)
   {
-    // Should not be created for stubs.
-    //
-    assert (available != nullptr && available->bootstrap_build);
+    if (available != nullptr)
+      assert (available->bootstrap_build); // Should have skeleton info.
+    else
+      assert (system);
 
     // We are only interested in old user configuration variables.
     //
@@ -333,25 +342,32 @@ namespace bpkg
     // We don't need to load old user configuration if there isn't any and
     // there is no new project configuration specified by the user.
     //
-    loaded_old_config_ =
-      (config_srcs_ == nullptr) &&
-      find_if (config_vars_.begin (), config_vars_.end (),
-               [this] (const string& v)
-               {
-                 // For now tighten it even further so that we can continue
-                 // using repositories without package skeleton information
-                 // (bootstrap.build, root.build). See load_old_config() for
-                 // details.
-                 //
+    // Note that at first it may seem like we shouldn't do this for any system
+    // packages but if we want to verify the user configuration, why not do so
+    // for system if we can (i.e., have skeleton info)?
+    //
+    if (available == nullptr)
+      loaded_old_config_ = true;
+    else
+      loaded_old_config_ =
+        (config_srcs_ == nullptr) &&
+        find_if (config_vars_.begin (), config_vars_.end (),
+                 [this] (const string& v)
+                 {
+                   // For now tighten it even further so that we can continue
+                   // using repositories without package skeleton information
+                   // (bootstrap.build, root.build). See load_old_config() for
+                   // details.
+                   //
 #if 1 // @@ TMP
-                 return project_override (v, var_prefix_);
+                   return project_override (v, var_prefix_);
 #else
-                 size_t vn;
-                 size_t pn (var_prefix_.size ());
-                 return (project_override (v, var_prefix_, &vn) &&
-                         v.compare (pn, vn - pn, ".develop") == 0);
+                   size_t vn;
+                   size_t pn (var_prefix_.size ());
+                   return (project_override (v, var_prefix_, &vn) &&
+                           v.compare (pn, vn - pn, ".develop") == 0);
 #endif
-               }) == config_vars_.end ();
+                 }) == config_vars_.end ();
 
     if (src_root)
     {
@@ -476,6 +492,7 @@ namespace bpkg
     assert (dependent_vars_.empty ()     &&
             reflect_vars_.empty ()       &&
             dependency_reflect_.empty () &&
+            available != nullptr         &&
             ctx_ == nullptr);
 
     if (!loaded_old_config_)
@@ -599,6 +616,96 @@ namespace bpkg
     }
   }
 
+  void package_skeleton::
+  load_overrides (package_configuration& cfg)
+  {
+    // Should only be called before dependent_config()/evaluate_*() and only
+    // on system package without skeleton info.
+    //
+    assert (dependent_vars_.empty ()     &&
+            reflect_vars_.empty ()       &&
+            dependency_reflect_.empty () &&
+            available == nullptr         &&
+            system);
+
+    if (find_if (config_vars_.begin (), config_vars_.end (),
+                 [this] (const string& v)
+                 {
+                   return project_override (v, var_prefix_);
+                 }) == config_vars_.end ())
+      return;
+
+    try
+    {
+      using namespace build2;
+      using build2::fail;
+      using build2::endf;
+
+      using config::variable_origin;
+
+      // Create the build context.
+      //
+      create_context (*this, strings {});
+      context& ctx (*ctx_);
+
+      scope& gs (ctx.global_scope.rw ());
+      auto& vp (ctx.var_pool.rw ());
+
+      for (const string& v: config_vars_)
+      {
+        size_t vn;
+        if (!project_override (v, var_prefix_, &vn))
+          continue;
+
+        const variable& var (vp.insert (string (v, 0, vn)));
+
+        // Parse the value part (note that all evaluate_require() cares about
+        // is whether the value is true or not, but we do need accurate values
+        // for diagnostics).
+        //
+        size_t p (v.find ('=', vn));
+        assert (p != string::npos);
+        if (v[p + 1] == '+')
+          ++p;
+
+        optional<names> val;
+        {
+          // Similar to context() ctor.
+          //
+          istringstream is (string (v, p + 1));
+          is.exceptions (istringstream::failbit | istringstream::badbit);
+
+          path_name in ("<cmdline>");
+          lexer lex (is, in, 1 /* line */, "\'\"\\$("); // Effective.
+
+          parser par (ctx);
+          pair<value, token> r (
+            par.parse_variable_value (lex, gs, &build2::work, var));
+
+          if (r.second.type != token_type::eos)
+            fail << "invalid command line variable override '" << v << "'";
+
+          val = reverse_value (r.first);
+        }
+
+        // @@ Should we do anything with append/prepend?
+        //
+        if (config_variable_value* v = cfg.find (var.name))
+          v->value = move (val);
+        else
+          cfg.push_back (
+            config_variable_value {
+              var.name, variable_origin::override_, {}, move (val), {}, false});
+      }
+
+      ctx_ = nullptr; // Free.
+    }
+    catch (const build2::failed&)
+    {
+      throw failed (); // Assume the diagnostics has already been issued.
+    }
+  }
+
   pair<bool, string> package_skeleton::
   verify_sensible (const package_configuration& cfg)
   {
@@ -607,6 +714,7 @@ namespace bpkg
     assert (dependent_vars_.empty ()     &&
             reflect_vars_.empty ()       &&
             dependency_reflect_.empty () &&
+            available != nullptr         &&
             ctx_ == nullptr);
 
     if (!loaded_old_config_)
@@ -1477,8 +1585,9 @@ namespace bpkg
       // to true and may not have any conditions on other configuration
       // variables (including their origin). As a result, we don't need to set
       // the default (or other dependent) values, but will need the type
-      // information as well as overrides (see negotiate_configuration()
-      // for details).
+      // information as well as overrides. Except that the type information
+      // may not be available for system packages, so we must deal with
+      // that. See negotiate_configuration() for details.
       //
       strings dvps;
       scope& rs (load (cfgs, &dvps, false /* defaults */));
@@ -1537,27 +1646,47 @@ namespace bpkg
             if (var.override ())
               continue;
 
-            const config_variable_value* v (cfg.find (var.name));
+            const value& val (p.first->second);
 
-            if (v == nullptr)
+            // Deal with a system package that has no type information.
+            //
+            if (!cfg.system)
             {
-              fail << "package " << cfg.package.name << " has no configuration "
-                   << "variable " << var.name <<
-                info << var.name << " set in require clause of dependent "
-                   << key.string ();
+              const config_variable_value* v (cfg.find (var.name));
+
+              if (v == nullptr)
+              {
+                fail << "package " << cfg.package.name << " has no "
+                     << "configuration variable " << var.name <<
+                  info << var.name << " set in require clause of dependent "
+                     << key.string ();
+              }
+
+              if (!v->type || *v->type != "bool")
+              {
+                fail << "configuration variable " << var.name << " is not of "
+                     << "bool type" <<
+                  info << var.name << " set in require clause of dependent "
+                     << key.string ();
+              }
             }
 
-            if (!v->type || *v->type != "bool")
+            bool r;
+            if (cfg.system)
             {
-              fail << "configuration variable " << var.name << " is not of "
-                   << "bool type" <<
-                info << var.name << " set in require clause of dependent "
-                   << key.string ();
+              try
+              {
+                r = build2::convert<bool> (val);
+              }
+              catch (const invalid_argument&)
+              {
+                r = false;
+              }
             }
+            else
+              r = cast_false<bool> (val);
 
-            const value& val (p.first->second);
-
-            if (!cast_false<bool> (val))
+            if (!r)
             {
               fail << "configuration variable " << var.name << " is not set "
                    << "to true" <<
@@ -1587,7 +1716,9 @@ namespace bpkg
 
           const value& val (p.first->second);
 
-          const config_variable_value& v (*cfg.find (var.name));
+          // Note: could be NULL if cfg.system.
+          //
+          const config_variable_value* v (cfg.find (var.name));
 
           // The only situation where the result would not be acceptable is if
           // one of the values were overridden to false.
@@ -1602,12 +1733,12 @@ namespace bpkg
           // cannot become an override. Except that the dependency override
           // could be specified (only) for the dependent.
           //
-          if (v.origin == variable_origin::override_)
+          if (v != nullptr && v->origin == variable_origin::override_)
           {
             assert (ol.first == variable_origin::override_);
           }
           else if (ol.first == variable_origin::override_ &&
-                   v.origin != variable_origin::override_)
+                   (v == nullptr || v->origin != variable_origin::override_))
           {
             fail << "dependency override " << var.name << " specified for "
                  << "dependent " << key.string () << " but not dependency" <<
@@ -1617,8 +1748,23 @@ namespace bpkg
 
           if (ol.first == variable_origin::override_)
           {
-            if (!cast_false<bool> (*ol.second))
-              r = false;
+            if (cfg.system)
+            {
+              try
+              {
+                if (!build2::convert<bool> (*ol.second))
+                  r = false;
+              }
+              catch (const invalid_argument&)
+              {
+                r = false;
+              }
+            }
+            else
+            {
+              if (!cast_false<bool> (*ol.second))
+                r = false;
+            }
           }
         }
       }
@@ -1650,7 +1796,15 @@ namespace bpkg
             if (var.override ())
               continue;
 
-            config_variable_value& v (*cfg.find (var.name));
+            config_variable_value* v (cfg.find (var.name));
+
+            if (v == nullptr) // cfg.system
+            {
+              cfg.push_back (
+                config_variable_value {
+                  var.name, variable_origin::undefined, {}, {}, {}, false});
+              v = &cfg.back ();
+            }
 
             // This value was set so save it as a dependency reflect.
             //
@@ -1660,10 +1814,18 @@ namespace bpkg
             //
             optional<names> ns (names {build2::name ("true")});
 
+            // Note: force bool type if system.
+            //
             dependency_reflect_.push_back (
-              reflect_variable_value {v.name, v.origin, v.type, ns});
-
-            if (v.origin != variable_origin::override_)
+              reflect_variable_value {
+                v->name,
+                (v->origin == variable_origin::override_
+                 ? v->origin
+                 : variable_origin::buildfile),
+                cfg.system ? optional<string> ("bool") : v->type,
+                ns});
+
+            if (v->origin != variable_origin::override_)
             {
               // Possible transitions:
               //
@@ -1671,21 +1833,21 @@ namespace bpkg
               // buildfile        -> buildfile -- override other dependent
               //
 
-              if (v.origin == variable_origin::buildfile)
+              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 == ns)
+                if (v->value == ns)
                   continue;
               }
               else
-                v.origin = variable_origin::buildfile;
+                v->origin = variable_origin::buildfile;
 
-              v.value = move (ns);
-              v.dependent = key; // We are the originating dependent.
-              v.confirmed = true;
+              v->value = move (ns);
+              v->dependent = key; // We are the originating dependent.
+              v->confirmed = true;
             }
           }
         }
@@ -1777,7 +1939,8 @@ namespace bpkg
             continue;
         }
 
-        print (v) << " (user configuration)";
+        print (v) << " (" << (system ? "expected " : "")
+                  << "user configuration)";
       }
     }
 
@@ -1788,14 +1951,16 @@ namespace bpkg
       const string& v (dependent_vars_[i]);
       const package_key& d (dependent_orgs_[i]); // Parallel.
 
-      print (v) << " (set by " << d << ')';
+      print (v) << " (" << (system ? "expected" : "set") << " by "
+                << d << ')';
     }
 
     // Finally reflect.
     //
     for (const string& v: reflect_vars_)
     {
-      print (v) << " (set by " << key.name << ')';
+      print (v) << " (" << (system ? "expected" : "set") << " by "
+                << key.name << ')';
     }
   }
 
@@ -2286,12 +2451,50 @@ namespace bpkg
     }
   }
 
+  // Create the build context.
+  //
+  static void
+  create_context (package_skeleton& skl, const strings& cmd_vars)
+  {
+    assert (skl.db_ != nullptr && skl.ctx_ == nullptr);
+
+    // Initialize the build system.
+    //
+    if (!build2_sched.started ())
+      build2_init (*skl.co_);
+
+    try
+    {
+      using namespace build2;
+      using build2::fail;
+      using build2::endf;
+
+      // Create build context.
+      //
+      skl.ctx_.reset (
+        new context (build2_sched,
+                     build2_mutexes,
+                     build2_fcache,
+                     false /* match_only */,          // Shouldn't matter.
+                     false /* no_external_modules */,
+                     false /* dry_run */,             // Shouldn't matter.
+                     false /* keep_going */,          // Shouldnt' matter.
+                     cmd_vars));
+    }
+    catch (const build2::failed&)
+    {
+      throw failed (); // Assume the diagnostics has already been issued.
+    }
+  }
+
   // Bootstrap the package skeleton.
   //
   static build2::scope_map::iterator
   bootstrap (package_skeleton& skl, const strings& cmd_vars)
   {
-    assert (skl.db_ != nullptr && skl.ctx_ == nullptr);
+    assert (skl.db_ != nullptr       &&
+            skl.ctx_ == nullptr      &&
+            skl.available != nullptr);
 
     // The overall plan is as follows:
     //
@@ -2435,29 +2638,15 @@ namespace bpkg
       skl.created_ = true;
     }
 
-    // Initialize the build system.
-    //
-    if (!build2_sched.started ())
-      build2_init (*skl.co_);
-
     try
     {
       using namespace build2;
       using build2::fail;
       using build2::endf;
 
-      // Create build context.
+      // Create the build context.
       //
-      skl.ctx_.reset (
-        new context (build2_sched,
-                     build2_mutexes,
-                     build2_fcache,
-                     false /* match_only */,          // Shouldn't matter.
-                     false /* no_external_modules */,
-                     false /* dry_run */,             // Shouldn't matter.
-                     false /* keep_going */,          // Shouldnt' matter.
-                     cmd_vars));
-
+      create_context (skl, cmd_vars);
       context& ctx (*skl.ctx_);
 
       // This is essentially a subset of the steps we perform in b.cxx. See
diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx
index 2250f57..88cba76 100644
--- a/bpkg/package-skeleton.hxx
+++ b/bpkg/package-skeleton.hxx
@@ -21,6 +21,12 @@ namespace bpkg
   class package_skeleton
   {
   public:
+    // If the package is system, then its available package should be NULL if
+    // it doesn't match the system package version "close enough" to be usable
+    // as the source of its configuration information (types, defaults). If it
+    // is NULL, then the skeleton can only be used to print and collect the
+    // configuration information.
+    //
     // If the package is external, then the existing package source root
     // directory needs to be specified (as absolute and normalized). In this
     // case, if output root is specified (as absolute and normalized; normally
@@ -55,7 +61,8 @@ namespace bpkg
     // relatively cheap.
     //
     package_skeleton (const common_options& co,
-                      database&,
+                      package_key,
+                      bool system,
                       shared_ptr<const available_package>,
                       strings config_vars,
                       bool disfigure,
@@ -65,6 +72,7 @@ namespace bpkg
 
 
     package_key key;
+    bool system;
     shared_ptr<const available_package> available;
 
     const package_name&
@@ -90,6 +98,13 @@ namespace bpkg
     void
     reload_defaults (package_configuration&);
 
+    // Load overrides for a system package without skeleton info. Note that
+    // this is done in an ad hoc manner and only to support evaluate_require()
+    // semantics (see the implementation for details).
+    //
+    void
+    load_overrides (package_configuration&);
+
     // 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.
@@ -242,7 +257,7 @@ namespace bpkg
     bool created_ = false;
     bool verified_ = false;
     bool loaded_old_config_;
-    bool develop_ = false;  // Package has config.*.develop.
+    bool develop_ = true;  // Package has config.*.develop.
 
     unique_ptr<build2::context> ctx_;
     build2::scope* rs_ = nullptr;
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 318ce91..a8976ad 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -1060,23 +1060,41 @@ namespace bpkg
     init_skeleton (const common_options& options,
                    const shared_ptr<available_package>& override = nullptr)
     {
-      const shared_ptr<available_package>& ap (override != nullptr
-                                               ? override
-                                               : available);
+      shared_ptr<available_package> ap (override != nullptr
+                                        ? override
+                                        : available);
 
       assert (!skeleton && ap != nullptr);
 
-      optional<dir_path> src_root (external_dir ());
+      package_key pk (db, ap->id.name);
 
-      optional<dir_path> out_root (
-        src_root && !disfigure
-        ? dir_path (db.get ().config) /= name ().string ()
-        : optional<dir_path> ());
+      if (system)
+      {
+        // @@ TODO
+        //
+        // Keep the available package if its version is "close enough" to the
+        // system package version. For now we will require the exact match
+        // but in the future we could relax this (e.g., allow the user to
+        // specify something like libfoo/^1.2.0 or some such).
+        //
+        ap = nullptr;
+      }
+
+      optional<dir_path> src_root, out_root;
+
+      if (ap != nullptr)
+      {
+        src_root = external_dir ();
+        out_root = (src_root && !disfigure
+                    ? dir_path (db.get ().config) /= name ().string ()
+                    : optional<dir_path> ());
+      }
 
       skeleton = package_skeleton (
         options,
-        db,
-        ap,
+        move (pk),
+        system,
+        move (ap),
         config_vars, // @@ Maybe make optional<strings> and move?
         disfigure,
         (selected != nullptr ? &selected->config_variables : nullptr),
@@ -4620,7 +4638,9 @@ namespace bpkg
                     const package_configuration& pc (
                       cfg.dependency_configurations[p]);
 
-                    pair<bool, string> pr (b->skeleton->verify_sensible (pc));
+                    pair<bool, string> pr (b->skeleton->available != nullptr
+                                           ? b->skeleton->verify_sensible (pc)
+                                           : make_pair (true, string ()));
 
                     if (!pr.first)
                     {
@@ -5961,7 +5981,12 @@ namespace bpkg
               const package_configuration& pc (
                 pcfg->dependency_configurations[p]);
 
-              pair<bool, string> pr (b->skeleton->verify_sensible (pc));
+              // Skip the verification if this is a system package
+              // without skeleton info.
+              //
+              pair<bool, string> pr (b->skeleton->available != nullptr
+                                     ? b->skeleton->verify_sensible (pc)
+                                     : make_pair (true, string ()));
 
               if (!pr.first)
               {
diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx
index 93f8602..834bdf9 100644
--- a/bpkg/pkg-configure.cxx
+++ b/bpkg/pkg-configure.cxx
@@ -619,7 +619,8 @@ namespace bpkg
                      ap->dependencies,
                      nullptr /* alternatives */,
                      package_skeleton (o,
-                                       db,
+                                       package_key (db, ap->id.name),
+                                       false /* system */,
                                        ap,
                                        move (vars),
                                        false /* disfigure */,