diff options
| -rw-r--r-- | bpkg/cfg-create.cxx | 3 | ||||
| -rw-r--r-- | bpkg/package-skeleton.cxx | 57 | ||||
| -rw-r--r-- | bpkg/package-skeleton.hxx | 12 | ||||
| -rw-r--r-- | bpkg/package.cxx | 22 | ||||
| -rw-r--r-- | bpkg/package.hxx | 30 | ||||
| -rw-r--r-- | bpkg/package.xml | 21 | ||||
| -rw-r--r-- | bpkg/pkg-build.cxx | 17 | ||||
| -rw-r--r-- | bpkg/pkg-command.cxx | 4 | ||||
| -rw-r--r-- | bpkg/pkg-configure.cxx | 112 | ||||
| -rw-r--r-- | bpkg/pkg-disfigure.cxx | 3 | ||||
| -rw-r--r-- | tests/pkg-build.testscript | 89 |
11 files changed, 330 insertions, 40 deletions
diff --git a/bpkg/cfg-create.cxx b/bpkg/cfg-create.cxx index 29de01c..c31b197 100644 --- a/bpkg/cfg-create.cxx +++ b/bpkg/cfg-create.cxx @@ -11,6 +11,7 @@ #include <bpkg/cfg-link.hxx> using namespace std; +using namespace butl; namespace bpkg { @@ -249,7 +250,7 @@ namespace bpkg string a (args.next ()); if (a.find ('=') != string::npos) - vars.push_back (move (a)); + vars.push_back (move (trim (a))); else if (!a.empty ()) mods.push_back (move (a)); else diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx index 9bef69a..972d064 100644 --- a/bpkg/package-skeleton.cxx +++ b/bpkg/package-skeleton.cxx @@ -384,7 +384,10 @@ namespace bpkg // reflect_names_.clear (); reflect_frag_.clear (); + #if 0 + // NOTE: see also collect_config() if enabling this. + // reflect_vars_.clear (); #else reflect_vars_ = config_vars_; @@ -547,7 +550,7 @@ namespace bpkg // TODO: copy over config_vars_ that are not in the map (case #1). #endif - // Drop the build system state since it needd reloading (some computed + // Drop the build system state since it needs reloading (some computed // values in root.build may depend on the new configuration values). // ctx_ = nullptr; @@ -558,9 +561,53 @@ namespace bpkg } } - strings package_skeleton:: - collect_reflect () && + pair<strings, vector<optional<config_source>>> package_skeleton:: + collect_config () && { + // Note that if the reflect variables list is not empty, then it also + // contains the user-specified configuration variables, which all come + // first (see above). + // + size_t nc (config_vars_.size ()); + + strings vars (reflect_vars_.empty () + ? move (config_vars_) + : move (reflect_vars_)); + + vector<optional<config_source>> sources; + + if (!vars.empty ()) + { + sources.reserve (vars.size ()); + + // Assign the user source to only those user-specified configuration + // variables which are project variables (i.e., names start with + // config.<project>). Assign the reflect source to all variables that + // follow the user-specified configuration variables (which can only be + // project variables). + // + // Note that some user-specified variables may have qualifications + // (global, scope, etc) but there is no reason to expect any project + // configuration variables to use such qualifications (since they can + // only apply to one project). So we skip all qualified variables. + // + string p ("config." + name ().variable ()); + size_t n (p.size ()); + + for (const string& v: vars) + { + if (sources.size () < nc) + { + if (v.compare (0, n, p) == 0 && strchr (".=+ \t", v[n]) != nullptr) + sources.push_back (config_source::user); + else + sources.push_back (nullopt); + } + else + sources.push_back (config_source::reflect); + } + } + assert (db_ != nullptr); ctx_ = nullptr; // In case we only had conditions. @@ -568,9 +615,7 @@ namespace bpkg db_ = nullptr; available_ = nullptr; - return reflect_vars_.empty () - ? move (config_vars_) - : move (reflect_vars_); + return make_pair (move (vars), move (sources)); } build2::scope& package_skeleton:: diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx index d2a848c..aaa9b8c 100644 --- a/bpkg/package-skeleton.hxx +++ b/bpkg/package-skeleton.hxx @@ -56,15 +56,17 @@ namespace bpkg void evaluate_reflect (const string&, size_t depends_index); - // Return the accumulated reflect values. + // Return the accumulated reflect values together with their sources (the + // resulting vectors are parallel). // - // Note that the result is merged with config_vars and you should be used - // instead rather than in addition to config_vars. + // Note that the result is merged with config_vars and should be used + // instead rather than in addition to config_vars. The source of + // configuration variables which are not the project variables is absent. // // Note also that this should be the final call on this object. // - strings - collect_reflect () &&; + pair<strings, vector<optional<config_source>>> + collect_config () &&; const package_name& name () const {return available_->id.name;} diff --git a/bpkg/package.cxx b/bpkg/package.cxx index 4b9cdc3..3a356f8 100644 --- a/bpkg/package.cxx +++ b/bpkg/package.cxx @@ -529,6 +529,28 @@ namespace bpkg return lazy_shared_ptr<selected_package> (ddb, move (prerequisite)); } + string + to_string (config_source s) + { + switch (s) + { + case config_source::user: return "user"; + case config_source::dependent: return "dependent"; + case config_source::reflect: return "reflect"; + } + + return string (); // Should never reach. + } + + config_source + to_config_source (const string& s) + { + if (s == "user") return config_source::user; + else if (s == "dependent") return config_source::dependent; + else if (s == "reflect") return config_source::reflect; + else throw invalid_argument ("invalid config source '" + s + "'"); + } + shared_ptr<available_package> make_available (const common_options& options, database& db, diff --git a/bpkg/package.hxx b/bpkg/package.hxx index fbf8643..7dc8e11 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -27,7 +27,7 @@ // #define DB_SCHEMA_VERSION_BASE 7 -#pragma db model version(DB_SCHEMA_VERSION_BASE, 17, closed) +#pragma db model version(DB_SCHEMA_VERSION_BASE, 18, closed) namespace bpkg { @@ -1066,6 +1066,30 @@ namespace bpkg to(bpkg::_selected_package_ref (?)) \ from(std::move (?).to_ptr (*db)) + enum class config_source + { + user, // User configuration specified on command line. + dependent, // Dependent-imposed configuration from prefer/require clauses. + reflect // Package-reflected configuration from reflect clause. + }; + + string + to_string (config_source); + + config_source + to_config_source (const string&); // May throw std::invalid_argument. + + #pragma db map type(config_source) as(string) \ + to(to_string (?)) \ + from(bpkg::to_config_source (?)) + + #pragma db value + struct config_variable + { + string name; + config_source source; + }; + #pragma db object pointer(shared_ptr) session class selected_package { @@ -1154,6 +1178,8 @@ namespace bpkg package_prerequisites prerequisites; + vector<config_variable> config_variables; + public: bool system () const @@ -1229,6 +1255,8 @@ namespace bpkg #pragma db member(prerequisites) id_column("package") \ key_column("") value_column("") + #pragma db member(config_variables) id_column("package") value_column("") + // Explicit aggregate initialization for C++20 (private default ctor). // selected_package (package_name n, diff --git a/bpkg/package.xml b/bpkg/package.xml index d2006f9..904d0f6 100644 --- a/bpkg/package.xml +++ b/bpkg/package.xml @@ -1,4 +1,25 @@ <changelog xmlns="http://www.codesynthesis.com/xmlns/odb/changelog" database="sqlite" version="1"> + <changeset version="18"> + <add-table name="main.selected_package_config_variables" kind="container"> + <column name="package" type="TEXT" null="true" options="COLLATE NOCASE"/> + <column name="index" type="INTEGER" null="true"/> + <column name="name" type="TEXT" null="true"/> + <column name="source" type="TEXT" null="true"/> + <foreign-key name="package_fk" on-delete="CASCADE"> + <column name="package"/> + <references table="main.selected_package"> + <column name="name"/> + </references> + </foreign-key> + <index name="selected_package_config_variables_package_i"> + <column name="package"/> + </index> + <index name="selected_package_config_variables_index_i"> + <column name="index"/> + </index> + </add-table> + </changeset> + <changeset version="17"> <alter-table name="main.selected_package"> <add-column name="buildfiles_checksum" type="TEXT" null="true"/> diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 8f56fc8..60674b6 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -4578,7 +4578,7 @@ namespace bpkg fail << "unexpected options group for configuration variable '" << v << "'"; - cvars.push_back (move (v)); + cvars.push_back (move (trim (v))); } if (!cvars.empty () && !sep) @@ -4619,7 +4619,7 @@ namespace bpkg if (a.find ('=') == string::npos) fail << "unexpected group argument '" << a << "'"; - cvs.push_back (move (a)); + cvs.push_back (move (trim (a))); } } @@ -8177,12 +8177,21 @@ namespace bpkg // resulting package skeleton and dependency list for optimization // (not to re-evaluate enable conditions, etc). // + // Note that we may not collect the package prerequisites builds if + // the package is already configured but we still need to reconfigure + // it due, for example, to an upgrade of its dependency. In this case + // we pass to pkg_configure() the newly created package skeleton which + // contains the package configuration variables specified on the + // command line but (naturally) no reflection configuration variables. + // Note, however, that in this case pkg_configure() call will evaluate + // the reflect clauses itself and so the proper reflection variables + // will still end up in the package configuration. + // // @@ Note that if we ever allow the user to override the alternative // selection, this will break (and also if the user re-configures // the package manually). Maybe that a good reason not to allow // this? Or we could store this information in the database. // - if (p.skeleton) { assert (p.dependencies); @@ -8198,6 +8207,8 @@ namespace bpkg } else { + assert (sp != nullptr); // See above. + optional<dir_path> src_root (p.external_dir ()); optional<dir_path> out_root ( diff --git a/bpkg/pkg-command.cxx b/bpkg/pkg-command.cxx index 0ff6a0d..e07d801 100644 --- a/bpkg/pkg-command.cxx +++ b/bpkg/pkg-command.cxx @@ -213,7 +213,7 @@ namespace bpkg if (args.group ().more ()) fail << "unexpected options group for variable '" << a << "'"; - cvars.push_back (move (a)); + cvars.push_back (move (trim (a))); } else { @@ -228,7 +228,7 @@ namespace bpkg if (a.find ('=') == string::npos) fail << "unexpected group argument '" << a << "'"; - vars.push_back (move (a)); + vars.push_back (move (trim (a))); } pkg_args.push_back (pkg_arg {move (n), move (vars)}); diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index 772cc67..0519f7c 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -18,16 +18,32 @@ using namespace butl; namespace bpkg { - // Given dependencies of a package, return its prerequisite packages and + // Given dependencies of a package, return its prerequisite packages, // configuration variables that resulted from selection of these - // prerequisites (import, reflection, etc). See pkg_configure() for the - // semantics of the dependency list. Fail if for some of the dependency - // alternative lists there is no satisfactory alternative (all its - // dependencies are configured, satisfy the respective constraints, etc). + // prerequisites (import, reflection, etc), and sources of the configuration + // variables resulted from evaluating the reflect clauses. See + // pkg_configure() for the semantics of the dependency list. Fail if for + // some of the dependency alternative lists there is no satisfactory + // alternative (all its dependencies are configured, satisfy the respective + // constraints, etc). // + struct configure_prerequisites_result + { + package_prerequisites prerequisites; + strings config_variables; // Note: name and value. + + // Only contains sources of configuration variables collected using the + // package skeleton, excluding those user-specified variables which are + // not the project variables for the specified package (module + // configuration variables, etc). Thus, it is not parallel to the + // variables member. + // + vector<config_variable> config_sources; // Note: name and source. + }; + // Note: loads selected packages. // - static pair<package_prerequisites, vector<string>> + static configure_prerequisites_result pkg_configure_prerequisites (const common_options& o, database& db, transaction&, @@ -36,8 +52,9 @@ namespace bpkg bool simulate, const function<find_database_function>& fdb) { - package_prerequisites pps; - vector<string> cvs; + package_prerequisites prereqs; + strings vars; + vector<config_variable> srcs; for (size_t di (0); di != deps.size (); ++di) { @@ -114,7 +131,7 @@ namespace bpkg const package_name& pn (pr.first.object_id ()); const optional<version_constraint>& pc (pr.second); - auto p (pps.emplace (pr.first, pc)); + auto p (prereqs.emplace (pr.first, pc)); // Currently we can only capture a single constraint, so if we // already have a dependency on this package and one constraint is @@ -167,8 +184,8 @@ namespace bpkg // bootstrap` in their manifest. // dir_path od (sp->effective_out_root (pdb.config)); - cvs.push_back ("config.import." + sp->name.variable () + - "='" + od.representation () + "'"); + vars.push_back ("config.import." + sp->name.variable () + + "='" + od.representation () + "'"); } } } @@ -192,18 +209,36 @@ namespace bpkg // if (!simulate) { - strings rvs (move (ps).collect_reflect ()); + auto rvs (move (ps).collect_config ()); - if (cvs.empty ()) - cvs = move (rvs); - else + strings& vs (rvs.first); + vector<optional<config_source>>& ss (rvs.second); + + if (!vs.empty ()) { - for (string& cv: rvs) - cvs.push_back (move (cv)); + vars.reserve (vars.size () + vs.size ()); + + for (size_t i (0); i != vs.size (); ++i) + { + string& v (vs[i]); + const optional<config_source>& s (ss[i]); + + if (s) + { + size_t p (v.find_first_of ("=+ \t")); + assert (p != string::npos); + + srcs.push_back (config_variable {string (v, 0, p), *s}); + } + + vars.push_back (move (v)); + } } } - return make_pair (move (pps), move (cvs)); + return configure_prerequisites_result {move (prereqs), + move (vars), + move (srcs)}; } void @@ -241,7 +276,7 @@ namespace bpkg // assert (p->prerequisites.empty ()); - pair<package_prerequisites, vector<string>> cpr ( + configure_prerequisites_result cpr ( pkg_configure_prerequisites (o, db, t, @@ -250,7 +285,7 @@ namespace bpkg simulate, fdb)); - p->prerequisites = move (cpr.first); + p->prerequisites = move (cpr.prerequisites); if (!simulate) { @@ -269,11 +304,42 @@ namespace bpkg l4 ([&]{trace << "buildspec: " << bspec;}); + // Deduce the configuration variables which are not reflected anymore + // and disfigure them. + // + string dvar; + for (const config_variable& cv: p->config_variables) + { + if (cv.source == config_source::reflect) + { + const vector<config_variable>& ss (cpr.config_sources); + auto i (find_if (ss.begin (), ss.end (), + [&cv] (const config_variable& v) + { + return v.name == cv.name; + })); + + if (i == ss.end ()) + { + if (dvar.empty ()) + dvar = "config.config.disfigure="; + else + dvar += ' '; + + dvar += cv.name; + } + } + } + // Configure. // try { - run_b (o, verb_b::quiet, cpr.second, bspec); + run_b (o, + verb_b::quiet, + cpr.config_variables, + (!dvar.empty () ? dvar.c_str () : nullptr), + bspec); } catch (const failed&) { @@ -298,6 +364,8 @@ namespace bpkg false /* simulate */); throw; } + + p->config_variables = move (cpr.config_sources); } p->out_root = out_root.leaf (); @@ -368,7 +436,7 @@ namespace bpkg } if (!sep && a.find ('=') != string::npos) - vars.push_back (move (a)); + vars.push_back (move (trim (a))); else if (n.empty ()) n = move (a); else diff --git a/bpkg/pkg-disfigure.cxx b/bpkg/pkg-disfigure.cxx index bae2a4e..690c3e1 100644 --- a/bpkg/pkg-disfigure.cxx +++ b/bpkg/pkg-disfigure.cxx @@ -198,6 +198,9 @@ namespace bpkg << "use 'pkg-purge' to remove"; throw; } + + if (disfigure) + p->config_variables.clear (); } p->out_root = nullopt; diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript index 36ffb9d..8ee0ee1 100644 --- a/tests/pkg-build.testscript +++ b/tests/pkg-build.testscript @@ -4051,6 +4051,95 @@ test.options += --no-progress $pkg_drop fax } } + + : reconfigure-reflect-vars + : + { + $clone_cfg; + cp -rp ../fax/ ./; + + $* config.fax.libbiz=true -- fax/ 2>>~"%EOE%"; + fetched $backend_dep + unpacked $backend_dep + fetched libbiz/1.0.0 + unpacked libbiz/1.0.0 + using fax/1.0.0 \(external\) + configured $backend_dep + configured libbiz/1.0.0 + configured fax/1.0.0 + %info: .+fax.+ is up to date% + updated fax/1.0.0 + EOE + + $pkg_status -r >>"EOO"; + !fax configured !1.0.0 + $backend_configured + libbiz configured 1.0.0 + EOO + + cat cfg/fax/build/config.build >>~"%EOO%"; + %.* + config.fax.backend = $backend + config.fax.libbiz = true + %config.fax.extras = '.+'% + config.fax.libbox = false + EOO + + # While at it, make sure none of the reflect variables are + # unexpectedly wiped out on reconfiguration due to the dependency + # upgrade. + # + $* fax/ "?sys:$backend/*" 2>>~"%EOE%"; + disfigured fax/1.0.0 + %disfigured $backend/.+% + %purged $backend/.+% + configured sys:$backend/* + configured fax/1.0.0 + %info: .+fax.+ is up to date% + updated fax/1.0.0 + EOE + + $pkg_status -r >>~"%EOO%"; + !fax configured !1.0.0 + % $backend configured,system .+% + libbiz configured 1.0.0 + EOO + + cat cfg/fax/build/config.build >>~"%EOO%"; + %.* + config.fax.backend = $backend + config.fax.libbiz = true + %config.fax.extras = '.+'% + config.fax.libbox = false + EOO + + # Now make sure that dependency clauses re-evaluation is properly + # reflected in the configuration. + # + $* config.fax.libbiz=false -- fax/ 2>>~"%EOE%"; + disfigured fax/1.0.0 + disfigured libbiz/1.0.0 + purged libbiz/1.0.0 + configured fax/1.0.0 + %info: .+fax.+ is up to date% + updated fax/1.0.0 + EOE + + $pkg_status -r >>~"%EOO%"; + !fax configured !1.0.0 + % $backend configured,system .+% + EOO + + cat cfg/fax/build/config.build >>~"%EOO%"; + %.* + config.fax.backend = $backend + config.fax.libbiz = false + config.fax.extras = [null] + config.fax.libbox = false + EOO + + $pkg_drop fax + } } : evaluate-reflect-vars |