From 856c55d0f8c50d0f371a546e4231e0f415975690 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 16 Jun 2022 09:43:32 +0200 Subject: Initial support for negotiation with system dependencies --- bpkg/package-configuration.cxx | 32 ++++ bpkg/package-configuration.hxx | 1 + bpkg/package-skeleton.cxx | 339 ++++++++++++++++++++++++++++++++--------- bpkg/package-skeleton.hxx | 19 ++- bpkg/pkg-build.cxx | 49 ++++-- bpkg/pkg-configure.cxx | 3 +- 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 pos, const small_vector, 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 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 ap, strings cvs, bool df, const vector* css, optional src_root, optional 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 val; + { + // Similar to context() ctor. + // + istringstream is (string (v, p + 1)); + is.exceptions (istringstream::failbit | istringstream::badbit); + + path_name in (""); + lexer lex (is, in, 1 /* line */, "\'\"\\$("); // Effective. + + parser par (ctx); + pair 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 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 (val); + } + catch (const invalid_argument&) + { + r = false; + } } + else + r = cast_false (val); - const value& val (p.first->second); - - if (!cast_false (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 (*ol.second)) - r = false; + if (cfg.system) + { + try + { + if (!build2::convert (*ol.second)) + r = false; + } + catch (const invalid_argument&) + { + r = false; + } + } + else + { + if (!cast_false (*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 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 ("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, strings config_vars, bool disfigure, @@ -65,6 +72,7 @@ namespace bpkg package_key key; + bool system; shared_ptr 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 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& override = nullptr) { - const shared_ptr& ap (override != nullptr - ? override - : available); + shared_ptr ap (override != nullptr + ? override + : available); assert (!skeleton && ap != nullptr); - optional src_root (external_dir ()); + package_key pk (db, ap->id.name); - optional out_root ( - src_root && !disfigure - ? dir_path (db.get ().config) /= name ().string () - : optional ()); + 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 src_root, out_root; + + if (ap != nullptr) + { + src_root = external_dir (); + out_root = (src_root && !disfigure + ? dir_path (db.get ().config) /= name ().string () + : optional ()); + } skeleton = package_skeleton ( options, - db, - ap, + move (pk), + system, + move (ap), config_vars, // @@ Maybe make optional and move? disfigure, (selected != nullptr ? &selected->config_variables : nullptr), @@ -4620,7 +4638,9 @@ namespace bpkg const package_configuration& pc ( cfg.dependency_configurations[p]); - pair pr (b->skeleton->verify_sensible (pc)); + pair 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 pr (b->skeleton->verify_sensible (pc)); + // Skip the verification if this is a system package + // without skeleton info. + // + pair 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 */, -- cgit v1.1