aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--bpkg/cfg-create.cxx3
-rw-r--r--bpkg/package-skeleton.cxx57
-rw-r--r--bpkg/package-skeleton.hxx12
-rw-r--r--bpkg/package.cxx22
-rw-r--r--bpkg/package.hxx30
-rw-r--r--bpkg/package.xml21
-rw-r--r--bpkg/pkg-build.cxx17
-rw-r--r--bpkg/pkg-command.cxx4
-rw-r--r--bpkg/pkg-configure.cxx112
-rw-r--r--bpkg/pkg-disfigure.cxx3
-rw-r--r--tests/pkg-build.testscript89
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