From 5754ef6aca5ccab95476a159a1da7b41effdd03d Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 19 May 2022 11:24:29 +0200 Subject: Drag configuration sources into package skeleton --- bpkg/package-skeleton.cxx | 157 ++++++++++++++++++++++++++++++++++++---------- bpkg/package-skeleton.hxx | 35 ++++++++--- bpkg/pkg-configure.cxx | 39 +++++------- 3 files changed, 166 insertions(+), 65 deletions(-) diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx index e0d85ed..385a4fd 100644 --- a/bpkg/package-skeleton.cxx +++ b/bpkg/package-skeleton.cxx @@ -59,6 +59,7 @@ namespace bpkg db_ = v.db_; available_ = v.available_; config_vars_ = move (v.config_vars_); + config_srcs_ = v.config_srcs_; src_root_ = move (v.src_root_); out_root_ = move (v.out_root_); created_ = v.created_; @@ -82,6 +83,7 @@ namespace bpkg db_ (v.db_), available_ (v.available_), config_vars_ (v.config_vars_), + config_srcs_ (v.config_srcs_), src_root_ (v.src_root_), out_root_ (v.out_root_), created_ (v.created_), @@ -99,14 +101,22 @@ namespace bpkg database& db, const available_package& ap, strings cvs, + /*const vector* css,*/ // @@ TMP optional src_root, optional out_root) - : co_ (&co), db_ (&db), available_ (&ap), config_vars_ (move (cvs)) + : co_ (&co), + db_ (&db), + available_ (&ap), + config_vars_ (move (cvs)), + config_srcs_ (/*css*/ nullptr) // @@ TMP { // Should not be created for stubs. // assert (available_->bootstrap_build); + if (config_srcs_ != nullptr && config_srcs_->empty ()) + config_srcs_ = nullptr; + if (src_root) { src_root_ = move (*src_root); @@ -561,61 +571,140 @@ namespace bpkg } } - pair>> package_skeleton:: + pair> 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). + assert (db_ != nullptr); // Must be called only once. + + strings vars; + vector srcs; + + // Check whether the user-specified configuration variable is a project + // variables (i.e., its name start with config.). // - size_t nc (config_vars_.size ()); + // 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 ignore all qualified variables. + // + auto prj_var = [this, p = optional ()] (const string& v) mutable + { + if (!p) + p = "config." + name ().variable (); - strings vars (reflect_vars_.empty () - ? move (config_vars_) - : move (reflect_vars_)); + size_t n (p->size ()); - vector> sources; + return v.compare (0, n, *p) == 0 && strchr (".=+ \t", v[n]) != nullptr; + }; - if (!vars.empty ()) + if (!reflect_vars_.empty ()) { - sources.reserve (vars.size ()); + assert (config_srcs_ == nullptr); // Should have been merged. - // Assign the user source to only those user-specified configuration - // variables which are project variables (i.e., names start with - // config.). 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. + vars = move (reflect_vars_); + + // 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). // - string p ("config." + name ().variable ()); - size_t n (p.size ()); + size_t nc (config_vars_.size ()); - for (const string& v: vars) + if (!vars.empty ()) { - if (sources.size () < nc) + srcs.reserve (vars.size ()); // At most that many. + + // Assign the user source only to user-specified configuration + // variables which are project variables (i.e., names start with + // config.). Assign the reflect source to all variables that + // follow the user-specified configuration variables (which can only + // be project variables). + // + for (size_t j (0); j != vars.size (); ++j) { - if (v.compare (0, n, p) == 0 && strchr (".=+ \t", v[n]) != nullptr) - sources.push_back (config_source::user); + const string& v (vars[j]); + + config_source s; + + if (j < nc) + { + if (prj_var (v)) + s = config_source::user; + else + continue; + } + else + s = config_source::reflect; + + size_t p (v.find_first_of ("=+ \t")); + assert (p != string::npos); + + string n (v, 0, p); + + // Check for a duplicate. + // + auto i (find_if (srcs.begin (), srcs.end (), + [&n] (const config_variable& cv) + { + return cv.name == n; + })); + + if (i != srcs.end ()) + assert (i->source == s); // See evaluate_reflect() for details. else - sources.push_back (nullopt); + srcs.push_back (config_variable {move (n), s}); } - else - sources.push_back (config_source::reflect); } } + else + { + vars = move (config_vars_); - assert (db_ != nullptr); + // If we don't have any reflect variables, then we don't really need to + // load user variables from config.build (or equivalent) and add them to + // config_vars since there is nothing for them to possibly override. So + // all we need to do is to add user variables from config_vars. + // + if (!vars.empty () || config_srcs_ != nullptr) + { + srcs.reserve ((config_srcs_ != nullptr ? config_srcs_->size () : 0) + + vars.size ()); // At most that many. - ctx_ = nullptr; // In case we only had conditions. + if (config_srcs_ != nullptr) + for (const config_variable& v: *config_srcs_) + if (v.source == config_source::user) + srcs.push_back (v); + + for (const string& v: vars) + { + // Similar logic to the above case. + // + if (prj_var (v)) + { + size_t p (v.find_first_of ("=+ \t")); + assert (p != string::npos); + + string n (v, 0, p); + // Check for a duplicate. + // + auto i (find_if (srcs.begin (), srcs.end (), + [&n] (const config_variable& cv) + { + return cv.name == n; + })); + + if (i == srcs.end ()) + srcs.push_back (config_variable {move (n), config_source::user}); + } + } + } + } + + ctx_ = nullptr; // In case we only had conditions. db_ = nullptr; available_ = nullptr; - return make_pair (move (vars), move (sources)); + return make_pair (move (vars), move (srcs)); } build2::scope& package_skeleton:: diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx index aaa9b8c..3e0da50 100644 --- a/bpkg/package-skeleton.hxx +++ b/bpkg/package-skeleton.hxx @@ -29,8 +29,23 @@ namespace bpkg // If the package is not external, then none of the root directories // should be specified. // - // Note that the options, database, and available_package are expected to - // outlive this object. + // Note that the options, database, available_package, and config_srcs are + // expected to outlive this object. + // + // The config_vars argument contains configuration variables specified by + // the user in this bpkg execution. Optional config_srcs is used to + // extract (from config.build or equivalent) configuration variables + // specified by the user in previous bpkg executions. It should be NULL if + // this is the first build of the package or if it is being disfigured. + // The extracted variables are merged with config_vars and the combined + // result is returned by collect_config() below. + // + // @@ TODO: speaking of the "config.build or equivalent" part, the + // equivalent is likely to be extracted configuration (probably saved + // to file in tmp somewhere) that we will load with config.config.load. + // It doesn't seem like a good idea to pass it as part of config_vars + // (because sometimes we may need to omit it) so most likely it will be + // passed as a separate arguments (likely a file path). // // Note also that this creates an "unloaded" skeleton and is therefore // relatively cheap. @@ -39,6 +54,7 @@ namespace bpkg database&, const available_package&, strings config_vars, + /*const vector* config_srcs,*/ // @@ TMP optional src_root, optional out_root); @@ -56,16 +72,16 @@ namespace bpkg void evaluate_reflect (const string&, size_t depends_index); - // Return the accumulated reflect values together with their sources (the - // resulting vectors are parallel). + // Return the accumulated configuration variables (first) and project + // configuration variable sources (second). Note that the arrays are + // not necessarily parallel. // - // 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 that the reflect variables are merged with config_vars/config_srcs + // and should be used instead rather than in addition to config_vars. // // Note also that this should be the final call on this object. // - pair>> + pair> collect_config () &&; const package_name& @@ -97,7 +113,10 @@ namespace bpkg const common_options* co_; database* db_; const available_package* available_; + strings config_vars_; + const vector* config_srcs_; // NULL if nothing to do or + // already done. dir_path src_root_; // Must be absolute and normalized. dir_path out_root_; // If empty, the same as src_root_. diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index ee123d5..49570c2 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -36,7 +36,7 @@ namespace bpkg // 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. + // config_variables member. // vector config_sources; // Note: name and source. }; @@ -54,9 +54,8 @@ namespace bpkg bool simulate, const function& fdb) { - package_prerequisites prereqs; - strings vars; - vector srcs; + package_prerequisites prereqs; + strings vars; // Alternatives argument must be parallel to the dependencies argument if // specified. @@ -286,34 +285,28 @@ namespace bpkg } } - // Add the configuration variables collected from the reflect clauses, if - // any. + // Add the rest of the configuration variables (user overrides, reflects, + // etc) as well as their sources. // + vector srcs; + if (!simulate) { - auto rvs (move (ps).collect_config ()); + pair> rvs (move (ps).collect_config ()); - strings& vs (rvs.first); - vector>& ss (rvs.second); + strings& vs (rvs.first); + srcs = move (rvs.second); if (!vs.empty ()) { - vars.reserve (vars.size () + vs.size ()); - - for (size_t i (0); i != vs.size (); ++i) + if (vars.empty ()) + vars = move (vs); + else { - string& v (vs[i]); - const optional& 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.reserve (vars.size () + vs.size ()); - vars.push_back (move (v)); + for (string& v: vs) + vars.push_back (move (v)); } } } -- cgit v1.1