From 2dbac9c6d08697e28af28178b2ce041140164842 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Mon, 7 Feb 2022 14:02:11 +0300 Subject: Introduce package_skeleton for evaluating dependency clauses --- bpkg/package-skeleton.hxx | 139 +++++++++++++++++++++++++++++++++++ bpkg/package.cxx | 33 +++++++++ bpkg/package.hxx | 24 +++--- bpkg/pkg-build.cxx | 182 +++++++++++++++++++++++++++++----------------- bpkg/pkg-configure.cxx | 59 +++++++-------- bpkg/pkg-configure.hxx | 4 +- 6 files changed, 330 insertions(+), 111 deletions(-) create mode 100644 bpkg/package-skeleton.hxx (limited to 'bpkg') diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx new file mode 100644 index 0000000..5fcb151 --- /dev/null +++ b/bpkg/package-skeleton.hxx @@ -0,0 +1,139 @@ +// file : bpkg/package-skeleton.hxx -*- C++ -*- +// license : MIT; see accompanying LICENSE file + +#ifndef BPKG_PACKAGE_SKELETON_HXX +#define BPKG_PACKAGE_SKELETON_HXX + +#include +#include + +#include + +namespace bpkg +{ + // A build system skeleton of a package used to evaluate buildfile clauses + // during dependency resolution (enable, reflect, require or prefer/accept). + // + class package_skeleton + { + public: + // Note that the database and available_package are expected to outlive + // this object. + // + // Note also that this creates an "unloaded" skeleton and is therefore + // cheap. + // + // @@ Note that storing the list of configuration variables by reference + // complicates its use in pkg-build, where both the configuration and + // the optional skeleton are parts of the same copyable/moveable + // build_package object. We could probably move the configuration into + // the skeleton if create it, complicating an access to the + // configuration for the users (if the skeleton is present then get + // configuration from it, etc). Let's however keep it simple for now + // and just copy the configuration. + // + package_skeleton (database& db, + const available_package& ap, + const strings& cvs) + : db_ (db), available_ (ap), config_vars_ (cvs) {} + + // Evaluate the enable clause. + // + // @@ What can we pass as location? Maybe somehow point to manifest in the + // skeleton (will need to re-acquire the position somehow)? + // + bool + evaluate_enable (const string&) + { + // @@ TMP + // + fail << "conditional dependency for package " << name () << + info << "conditional dependencies are not yet supported"; + + load (); + + // TODO + + return true; // @@ TMP + } + + // Evaluate the reflect clause. + // + void + evaluate_reflect (const string& r) + { + load (); + + // TODO + + // @@ DEP For now we assume that the reflection, if present, contains + // a single configuration variable that assigns a literal value. + // + reflect_.push_back (r); + + dirty (); + } + + // Return the accumulated reflect values. + // + strings + collect_reflect () + { + return reflect_; + } + + const package_name& + name () const {return available_.get ().id.name;} + + private: + // Create the skeleton if necessary and (re)load the build system state. + // + // Call this function before evaluating every clause. + // + void + load () + { + if (loaded_ && !dirty_) + return; + + // Plan: + // + // 0. Create filesystem state if necessary (could have been created by + // another instance, e.g., during simulation). + // + // @@ build/ vs build2/ -- probably doesn't matter unless in the + // future we allow specifying additional files. + // + // 1. If loaded but dirty, save the accumulated reflect state, and + // destroy the old state. + // + // 2. Load the state potentially with accumulated reflect state. + + loaded_ = true; + dirty_ = false; + } + + // Mark the build system state as needing reloading. + // + // Call this function after evaluating the reflect clause (since some + // computed values in root.build may depend on the new value). + // + void + dirty () + { + dirty_ = true; + } + + private: + reference_wrapper db_; + reference_wrapper available_; + strings config_vars_; + + bool loaded_ = false; + bool dirty_ = false; + + strings reflect_; + }; +} + +#endif // BPKG_PACKAGE_SKELETON_HXX diff --git a/bpkg/package.cxx b/bpkg/package.cxx index b544773..c8ad9d6 100644 --- a/bpkg/package.cxx +++ b/bpkg/package.cxx @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -539,6 +540,38 @@ namespace bpkg return lazy_shared_ptr (ddb, move (prerequisite)); } + shared_ptr + make_available (const common_options& options, + database& db, + const shared_ptr& sp) + { + assert (sp != nullptr && sp->state != package_state::broken); + + if (sp->system ()) + return make_shared (sp->name, sp->version); + + // The package is in at least fetched state, which means we should + // be able to get its manifest. + // + const optional& a (sp->archive); + + package_manifest m ( + sp->state == package_state::fetched + ? pkg_verify (options, + a->absolute () ? *a : db.config_orig / *a, + true /* ignore_unknown */, + false /* expand_values */, + true /* load_buildfiles */) + : pkg_verify (options, + sp->effective_src_root (db.config_orig), + true /* ignore_unknown */, + true /* load_buildfiles */, + // Copy potentially fixed up version from selected package. + [&sp] (version& v) {v = sp->version;})); + + return make_shared (move (m)); + } + pair, database*> find_dependency (database& db, const package_name& pn, bool buildtime) { diff --git a/bpkg/package.hxx b/bpkg/package.hxx index db6b2b3..93fcc9c 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -592,20 +592,6 @@ namespace bpkg const dependency_alternatives_ex&, const package_name&); - // Return true if the dependency alternative enable condition is not - // specified or evaluates to true. Note that the package argument is used - // for diagnostics only. - // - // @@ DEP We will also need to pass some additional information here for the - // actual evaluation (reflect clauses of already selected dependency - // alternatives, etc). - // - bool - evaluate_enabled (const dependency_alternative&, - const string& bootstrap_build, - const optional& root_build, - const package_name&); - // tests // #pragma db value(test_dependency) definition @@ -1249,6 +1235,16 @@ namespace bpkg return os << p.string (); } + // Create a transient (or fake, if you prefer) available_package object + // corresponding to the specified selected object, which is expected to not + // be in the broken state. Note that the package locations list is left + // empty. + // + shared_ptr + make_available (const common_options&, + database&, + const shared_ptr&); + // Try to find a dependency in the dependency configurations (see // database::dependency_configs() for details). Return pointers to the found // package and the configuration it belongs to. Return a pair of NULLs if no diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 02e4253..63910e8 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -32,6 +32,7 @@ #include #include #include +#include #include using namespace std; @@ -378,22 +379,21 @@ namespace bpkg // locations list is left empty and that the returned repository fragment // could be NULL if the package is an orphan. // - // Note also that in our model we assume that make_available() is only - // called if there is no real available_package. This makes sure that if - // the package moves (e.g., from testing to stable), then we will be using - // stable to resolve its dependencies. + // Note also that in our model we assume that make_available_fragment() is + // only called if there is no real available_package. This makes sure that + // if the package moves (e.g., from testing to stable), then we will be + // using stable to resolve its dependencies. // static pair, lazy_shared_ptr> - make_available (const common_options& options, - database& db, - const shared_ptr& sp) + make_available_fragment (const common_options& options, + database& db, + const shared_ptr& sp) { - assert (sp != nullptr && sp->state != package_state::broken); + shared_ptr ap (make_available (options, db, sp)); if (sp->system ()) - return make_pair (make_shared (sp->name, sp->version), - nullptr); + return make_pair (move (ap), nullptr); // First see if we can find its repository fragment. // @@ -421,26 +421,7 @@ namespace bpkg } } - // The package is in at least fetched state, which means we should - // be able to get its manifest. - // - const optional& a (sp->archive); - - package_manifest m ( - sp->state == package_state::fetched - ? pkg_verify (options, - a->absolute () ? *a : db.config_orig / *a, - true /* ignore_unknown */, - false /* expand_values */, - true /* load_buildfiles */) - : pkg_verify (options, - sp->effective_src_root (db.config_orig), - true /* ignore_unknown */, - true /* load_buildfiles */, - // Copy potentially fixed up version from selected package. - [&sp] (version& v) {v = sp->version;})); - - return make_pair (make_shared (move (m)), move (rf)); + return make_pair (move (ap), move (rf)); } // Return true if the version constraint represents the wildcard version. @@ -570,6 +551,16 @@ namespace bpkg // optional dependencies; + // If we end up collecting the prerequisite builds for this package, then + // this member stores the skeleton of the package being built. + // + // Initially nullopt. Can potentially be loaded but with the reflection + // configuration variables collected only partially if the package + // prerequisite builds collection is postponed for any reason. Can also be + // unloaded if the package has no conditional dependencies. + // + optional skeleton; + // If the package prerequisite builds collection is postponed, then this // member stores the references to the enabled alternatives (in available // package) of a dependency being the cause of the postponement. This, in @@ -798,6 +789,19 @@ namespace bpkg if (p.checkout_purge) checkout_purge = p.checkout_purge; + // Note that configuration can only be specified for packages on the + // command line and such packages get collected/pre-entered early, + // before any prerequisites get collected. Thus, it doesn't seem + // possible that a package configuration may change after we have + // created the package skeleton. + // + // Also note that if it wouldn't be true, we would potentially need to + // re-collect the package prerequisites, since configuration change + // could affect the enable condition evaluation and, as a result, the + // dependency alternative choice. + // + assert (!skeleton || p.config_vars == config_vars); + if (!p.config_vars.empty ()) config_vars = move (p.config_vars); @@ -1195,6 +1199,10 @@ namespace bpkg // const dependencies& deps (ap->dependencies); + // Must both be either present or not. + // + assert (pkg.dependencies.has_value () == pkg.skeleton.has_value ()); + // Note that the selected alternatives list can be filled partially (see // build_package::dependencies for details). In this case we continue // collecting where we stopped previously. @@ -1205,6 +1213,8 @@ namespace bpkg if (size_t n = deps.size ()) pkg.dependencies->reserve (n); + + pkg.skeleton = package_skeleton (pdb, *ap, pkg.config_vars); } dependencies& sdeps (*pkg.dependencies); @@ -1239,6 +1249,8 @@ namespace bpkg assert (sdeps.size () < deps.size ()); + package_skeleton& skel (*pkg.skeleton); + for (size_t i (sdeps.size ()); i != deps.size (); ++i) { const dependency_alternatives_ex& das (deps[i]); @@ -1269,11 +1281,7 @@ namespace bpkg { for (const dependency_alternative& da: das) { - assert (ap->bootstrap_build); // Can't be a stub. - - if (evaluate_enabled (da, - *ap->bootstrap_build, ap->root_build, - pkg.name ())) + if (!da.enable || skel.evaluate_enable (*da.enable)) edas.push_back (da); } } @@ -1545,7 +1553,7 @@ namespace bpkg // (returning stub as an available package feels wrong). // if (dap == nullptr || dap->stub ()) - rp = make_available (options, *ddb, dsp); + rp = make_available_fragment (options, *ddb, dsp); } else // Remember that we may be forcing up/downgrade; we will deal @@ -1949,6 +1957,7 @@ namespace bpkg b.available, move (b.repository_fragment), nullopt, // Dependencies. + nullopt, // Package skeleton. nullopt, // Enabled dependency alternatives. nullopt, // Hold package. nullopt, // Hold version. @@ -2055,22 +2064,30 @@ namespace bpkg }; // Select a dependency alternative, copying it alone into the - // resulting dependencies list and collecting its dependency builds. + // resulting dependencies list, evaluating its reflect clause, if + // present, and collecting its dependency builds. // bool selected (false); - auto select = [&sdeps, &sdas, &collect, &selected] + auto select = [&sdeps, &sdas, &skel, &collect, &selected] (const dependency_alternative& da, prebuilds&& bs) { assert (sdas.empty ()); - sdas.push_back (da); - // Make sure that the alternative's enable condition is not - // evaluated repeatedly. + // Avoid copying enable/reflect not to evaluate them repeatedly. // - sdas.back ().enable = nullopt; + sdas.emplace_back (nullopt /* enable */, + nullopt /* reflect */, + da.prefer, + da.accept, + da.require, + da /* dependencies */); + sdeps.push_back (move (sdas)); + if (da.reflect) + skel.evaluate_reflect (*da.reflect); + collect (move (bs)); selected = true; @@ -2330,7 +2347,7 @@ namespace bpkg // The repointed dependent can be an orphan, so just create the // available package from the selected package. // - auto rp (make_available (o, db, sp)); + auto rp (make_available_fragment (o, db, sp)); // Add the prerequisite replacements as the required-by packages. // @@ -2351,6 +2368,7 @@ namespace bpkg move (rp.first), move (rp.second), nullopt, // Dependencies. + nullopt, // Package skeleton. nullopt, // Enabled dependency alternatives. nullopt, // Hold package. nullopt, // Hold version. @@ -2395,6 +2413,7 @@ namespace bpkg nullptr, nullptr, nullopt, // Dependencies. + nullopt, // Package skeleton. nullopt, // Enable dependency alternatives. nullopt, // Hold package. nullopt, // Hold version. @@ -2448,6 +2467,7 @@ namespace bpkg nullptr, nullptr, nullopt, // Dependencies. + nullopt, // Package skeleton. nullopt, // Enabled dependency alternatives. nullopt, // Hold package. nullopt, // Hold version. @@ -2882,6 +2902,7 @@ namespace bpkg nullptr, // No available pkg/repo fragment. nullptr, nullopt, // Dependencies. + nullopt, // Package skeleton. nullopt, // Enabled dependency alternatives. nullopt, // Hold package. nullopt, // Hold version. @@ -5825,7 +5846,7 @@ namespace bpkg { assert (sp != nullptr && sp->system () == arg_sys (pa)); - auto rp (make_available (o, *pdb, sp)); + auto rp (make_available_fragment (o, *pdb, sp)); ap = move (rp.first); af = move (rp.second); // Could be NULL (orphan). } @@ -5851,6 +5872,7 @@ namespace bpkg move (ap), move (af), nullopt, // Dependencies. + nullopt, // Package skeleton. nullopt, // Enabled dependency alternatives. true, // Hold package. pa.constraint.has_value (), // Hold version. @@ -5961,6 +5983,7 @@ namespace bpkg move (ap), move (apr.second), nullopt, // Dependencies. + nullopt, // Package skeleton. nullopt, // Enabled dependency alternatives. true, // Hold package. false, // Hold version. @@ -6241,6 +6264,7 @@ namespace bpkg nullptr, // Available package/repo fragment. nullptr, nullopt, // Dependencies. + nullopt, // Package skeleton. nullopt, // Enabled dependency alternatives. false, // Hold package. p.constraint.has_value (), // Hold version. @@ -6454,6 +6478,7 @@ namespace bpkg d.available, d.repository_fragment, nullopt, // Dependencies. + nullopt, // Package skeleton. nullopt, // Enabled dependency alternatives. nullopt, // Hold package. nullopt, // Hold version. @@ -8012,32 +8037,52 @@ namespace bpkg // Note that pkg_configure() commits the transaction. // if (p.system) + { sp = pkg_configure_system (ap->id.name, p.available_version (), pdb, t); + } else if (ap != nullptr) { // If the package prerequisites builds are collected, then use the - // resulting dependency list for optimization (not to re-evaluate - // enable conditions, etc). + // resulting package skeleton and dependency list for optimization + // (not to re-evaluate enable conditions, etc). // // @@ 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. // - assert (ap->bootstrap_build); // Can't be a stub. - pkg_configure (o, - pdb, - t, - sp, - p.dependencies ? *p.dependencies : ap->dependencies, - *ap->bootstrap_build, ap->root_build, - p.config_vars, - simulate, - fdb); + if (p.skeleton) + { + assert (p.dependencies); + + pkg_configure (o, + pdb, + t, + sp, + *p.dependencies, + *p.skeleton, + p.config_vars, + simulate, + fdb); + } + else + { + package_skeleton ps (pdb, *ap, p.config_vars); + + pkg_configure (o, + pdb, + t, + sp, + ap->dependencies, + ps, + p.config_vars, + simulate, + fdb); + } } else // Dependent. { @@ -8046,12 +8091,18 @@ namespace bpkg // assert (sp->state == package_state::unpacked); - package_manifest m ( - pkg_verify (o, - sp->effective_src_root (pdb.config_orig), - true /* ignore_unknown */, - true /* load_buildfiles */, - [&sp] (version& v) {v = sp->version;})); + // First try to find an existing available package for the selected + // package and, if not found, create a transient one. + // + shared_ptr dap ( + find_available_one (dependent_repo_configs (pdb), + sp->name, + version_constraint (sp->version)).first); + + if (dap == nullptr) + dap = make_available (o, pdb, sp); + + package_skeleton ps (pdb, *dap, p.config_vars); // @@ Note that on reconfiguration the dependent looses the potential // configuration variables specified by the user on some previous @@ -8059,12 +8110,11 @@ namespace bpkg // information in the database? // pkg_configure (o, - p.db, + pdb, t, sp, - convert (move (m.dependencies)), - *m.bootstrap_build, - m.root_build, + dap->dependencies, + ps, p.config_vars, simulate, fdb); diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index 05073a7..23cd85c 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -24,27 +24,24 @@ namespace bpkg // 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). - // Note that the package argument is used for diagnostics only. // // Note: loads selected packages. // - static pair> + static pair> pkg_configure_prerequisites (const common_options& o, database& db, transaction&, const dependencies& deps, - const string& bootstrap_build, - const optional& root_build, - const package_name& package, + package_skeleton& ps, bool simulate, const function& fdb) { package_prerequisites pps; - small_vector cvs; + vector cvs; for (const dependency_alternatives_ex& das: deps) { - if (das.empty () || toolchain_buildtime_dependency (o, das, package)) + if (das.empty () || toolchain_buildtime_dependency (o, das, ps.name ())) continue; // Pick the first alternative with dependencies that can all be resolved @@ -54,7 +51,7 @@ namespace bpkg bool enabled (false); // True if there is an enabled alternative. for (const dependency_alternative& da: das) { - if (!evaluate_enabled (da, bootstrap_build, root_build, package)) + if (da.enable && !ps.evaluate_enable (*da.enable)) continue; enabled = true; @@ -175,14 +172,10 @@ namespace bpkg } } - // Add the dependency alternative reflection configuration variable, - // if present. + // Evaluate the dependency alternative reflect clause, if present. // - // @@ DEP For now we assume that the reflection, if present, contains - // a single configuration variable that assigns a literal value. - // - if (!simulate && da.reflect) - cvs.push_back (*da.reflect); + if (da.reflect) + ps.evaluate_reflect (*da.reflect); satisfied = true; break; @@ -192,6 +185,15 @@ namespace bpkg fail << "unable to satisfy dependency on " << das; } + // Add the configuration variables collected from the reflect clauses, if + // any. + // + if (!simulate) + { + for (string& cv: ps.collect_reflect ()) + cvs.push_back (move (cv)); + } + return make_pair (move (pps), move (cvs)); } @@ -201,8 +203,7 @@ namespace bpkg transaction& t, const shared_ptr& p, const dependencies& deps, - const string& bootstrap_build, - const optional& root_build, + package_skeleton& ps, const strings& vars, bool simulate, const function& fdb) @@ -232,14 +233,12 @@ namespace bpkg // assert (p->prerequisites.empty ()); - pair> cpr ( + pair> cpr ( pkg_configure_prerequisites (o, db, t, deps, - bootstrap_build, - root_build, - p->name, + ps, simulate, fdb)); @@ -423,19 +422,21 @@ namespace bpkg l4 ([&]{trace << *p;}); - package_manifest m (pkg_verify (o, - p->effective_src_root (c), - true /* ignore_unknown */, - true /* load_buildfiles */, - [&p] (version& v) {v = p->version;})); + // Let's not bother trying to find an available package for this + // selected package, which may potentially not be present in this + // configuration (but instead be present in the configuration we are + // linked to, etc) and create a transient available package outright. + // + shared_ptr ap (make_available (o, db, p)); + + package_skeleton ps (db, *ap, vars); pkg_configure (o, db, t, p, - convert (move (m.dependencies)), - *m.bootstrap_build, - m.root_build, + ap->dependencies, + ps, vars, false /* simulate */); } diff --git a/bpkg/pkg-configure.hxx b/bpkg/pkg-configure.hxx index 4fc99fa..1b92281 100644 --- a/bpkg/pkg-configure.hxx +++ b/bpkg/pkg-configure.hxx @@ -13,6 +13,7 @@ #include // package_prerequisites, // dependencies. +#include #include namespace bpkg @@ -42,8 +43,7 @@ namespace bpkg transaction&, const shared_ptr&, const dependencies&, - const string& bootstrap_build, - const optional& root_build, + package_skeleton&, const strings& config_vars, bool simulate, const function& = {}); -- cgit v1.1