From 110b8541dca943a5513ce2ad4e1e8863806aa56f Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 17 Jan 2023 17:35:27 +0300 Subject: Add support for multiple system package versions --- bpkg/package.cxx | 24 +++++++------- bpkg/package.hxx | 40 +++++++++++++++++++---- bpkg/pkg-build-collect.cxx | 61 +++++++++++++++++++++++++++------- bpkg/pkg-build-collect.hxx | 4 +++ bpkg/pkg-build.cxx | 79 +++++++++++++++++++++++++++++---------------- bpkg/system-repository.cxx | 14 +++++--- bpkg/system-repository.hxx | 29 +++++++++++++---- tests/pkg-system.testscript | 11 +++++-- 8 files changed, 191 insertions(+), 71 deletions(-) diff --git a/bpkg/package.cxx b/bpkg/package.cxx index fe04248..03416be 100644 --- a/bpkg/package.cxx +++ b/bpkg/package.cxx @@ -71,10 +71,10 @@ namespace bpkg // available_package // - const version* available_package:: - system_version (database& db) const + const system_package_versions* available_package:: + system_versions (database& db) const { - if (!system_version_) + if (system_versions_.empty ()) { assert (db.system_repository); @@ -83,36 +83,36 @@ namespace bpkg // Only cache if it is authoritative. // if (sp->authoritative) - system_version_ = sp->version; + system_versions_ = sp->versions; else - return &sp->version; + return &sp->versions; } } - return system_version_ ? &*system_version_ : nullptr; + return !system_versions_.empty () ? &system_versions_ : nullptr; } - pair available_package:: - system_version_authoritative (database& db) const + pair available_package:: + system_versions_authoritative (database& db) const { assert (db.system_repository); const system_package* sp (db.system_repository->find (id.name)); - if (!system_version_) + if (system_versions_.empty ()) { if (sp != nullptr) { // Only cache if it is authoritative. // if (sp->authoritative) - system_version_ = sp->version; + system_versions_ = sp->versions; else - return make_pair (&sp->version, false); + return make_pair (&sp->versions, false); } } - return make_pair (system_version_ ? &*system_version_ : nullptr, + return make_pair (!system_versions_.empty () ? &system_versions_ : nullptr, sp != nullptr ? sp->authoritative : false); } diff --git a/bpkg/package.hxx b/bpkg/package.hxx index 6afc624..8754534 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -22,6 +22,7 @@ #include #include +#include // Used by the data migration entries. // @@ -729,7 +730,7 @@ namespace bpkg private: #pragma db transient - mutable optional system_version_; + mutable system_package_versions system_versions_; public: // Note: version constraints must be complete and the bootstrap build must @@ -766,23 +767,48 @@ namespace bpkg // available_package (package_name n, version_type sysv) : id (move (n), wildcard_version), - version (wildcard_version), - system_version_ (sysv) {} + version (wildcard_version) + { + system_versions_.push_back (move (sysv)); + } bool stub () const {return version.compare (wildcard_version, true) == 0;} - // Return package system version if one has been discovered. Note that + // Return package system versions if any has been discovered. Note that // we do not implicitly assume a wildcard version. // - const version_type* - system_version (database&) const; + const system_package_versions* + system_versions (database&) const; // As above but also return an indication if the version information is // authoritative. // + pair + system_versions_authoritative (database&) const; + +#if 1 + const version_type* + system_version (database& db) const + { + const system_package_versions* vs (system_versions (db)); + assert (vs == nullptr || vs->size () == 1); + return vs != nullptr ? &vs->front () : nullptr; + } + + pair - system_version_authoritative (database&) const; + system_version_authoritative (database& db) const + { + pair r ( + system_versions_authoritative (db)); + + assert (r.first == nullptr || r.first->size () == 1); + return r.first != nullptr + ? make_pair (&r.first->front (), r.second) + : make_pair (nullptr, r.second); + } +#endif // Database mapping. // diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx index 12db3ba..506897f 100644 --- a/bpkg/pkg-build-collect.cxx +++ b/bpkg/pkg-build-collect.cxx @@ -100,6 +100,19 @@ namespace bpkg (*action == build && (flags & (build_repoint | build_reevaluate)) != 0); } + // @@ MULT-SYS Note that the semantics of this function to return the + // package version which this build should result with (should we rename + // it to just version()?). + // + // We should probably make `bool system` member as + // `optional system` and, if present, return it's value here. + // + // The creator of build_package object should provide the version for the + // system package build. It can, for example, be the latest version from + // the system repository when this is a build-to-hold object (specified + // on the command line). Or it can be a satisfying version from + // the system repository for a dependency build. + // const version& build_package:: available_version () const { @@ -107,7 +120,7 @@ namespace bpkg // assert (available != nullptr && (system - ? available->system_version (db) != nullptr + ? available->system_versions (db) != nullptr : !available->stub ())); return system ? *available->system_version (db) : available->version; @@ -1809,7 +1822,12 @@ namespace bpkg shared_ptr selected; shared_ptr available; lazy_shared_ptr repository_fragment; + + // @@ MULT-SYS Make it of optional type as + // build_package::system (see above for details). + // bool system; + bool specified_dependency; bool force; @@ -2115,6 +2133,11 @@ namespace bpkg // A stub satisfies any version constraint so we weed them out // (returning stub as an available package feels wrong). // + // For a system selected package we still create a stub, but + // with a fixed system version (see + // available_package(package_name, version_type) constructor + // for details). + // if (dap == nullptr || dap->stub ()) rp = make_available_fragment (options, *ddb, dsp); } @@ -2457,7 +2480,7 @@ namespace bpkg // version constraint). If it were, then the system version // wouldn't be NULL and would satisfy itself. // - if (dap->system_version (*ddb) == nullptr) + if (dap->system_versions (*ddb) == nullptr) { if (dr != nullptr) *dr << error << "dependency " << d << " of package " @@ -2468,6 +2491,10 @@ namespace bpkg return precollect_result (false /* postpone */); } + // @@ MULT-SYS Go through all sys versions and only if none + // satisfying found, only then fail. Otherwise save the + // satisfying version to the `system` variable. + // if (!satisfies (*dap->system_version (*ddb), d.constraint)) { if (dr != nullptr) @@ -2485,6 +2512,9 @@ namespace bpkg } else { + // @@ MULT-SYS Go through all sys versions and if there is a + // satisfying one, the save it into the `system` variable. + // auto p (dap->system_version_authoritative (*ddb)); if (p.first != nullptr && @@ -2500,14 +2530,14 @@ namespace bpkg reused = false; r.push_back (prebuild {d, - *ddb, - move (dsp), - move (dap), - move (rp.second), - system, - specified, - force, - ru}); + *ddb, + move (dsp), + move (dap), + move (rp.second), + system, + specified, + force, + ru}); } // Now, as we have pre-collected the dependency builds, go through @@ -2537,6 +2567,10 @@ namespace bpkg if (bp.action && *bp.action == build_package::build) { + // @@ MULT-SYS: + // + // const version& v1 (b.system ? *b.system : dap->version); + // const version& v1 (b.system ? *dap->system_version (b.db) : dap->version); @@ -3985,7 +4019,7 @@ namespace bpkg package_key pk (db, sp->name); // If there is an entry for building specific version of the package (the - // available member is not NULL), then it wasn't created to prevent out + // available member is not NULL), then it wasn't created to prevent our // drop (see replaced_versions for details). This rather mean that the // replacement version is not being built anymore due to the plan // refinement. Thus, just erase the entry in this case and continue. @@ -4000,6 +4034,11 @@ namespace bpkg { if (verb >= 5) { + // @@ MULT-SYS: + // + // const optional& s (v.system); + // const version& av (s ? *s : ap->version); + // bool s (v.system); const version& av (s ? *ap->system_version (db) : ap->version); diff --git a/bpkg/pkg-build-collect.hxx b/bpkg/pkg-build-collect.hxx index e47d9fa..ff7735c 100644 --- a/bpkg/pkg-build-collect.hxx +++ b/bpkg/pkg-build-collect.hxx @@ -673,6 +673,10 @@ namespace bpkg // shared_ptr available; lazy_shared_ptr repository_fragment; + + // @@ MULT-SYS Make it of optional type as build_package::system + // (see available_version() implementation for details). + // bool system; // Meaningless for the drop. // True if the entry has been inserted or used for the replacement during diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index f452d50..32a6fee 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -520,10 +520,8 @@ namespace bpkg // optional c; - if (!dvc) + if (!dsys && !dvc) // @@ For now we don't patch system packages. { - assert (!dsys); // The version can't be empty for the system package. - if (patch) { c = patch_constraint (sp, ignore_unsatisfiable); @@ -535,7 +533,7 @@ namespace bpkg } } } - else if (!dsys || !wildcard (*dvc)) + else if (!dsys || (dvc && !wildcard (*dvc))) c = dvc; available_packages afs (find_available (nm, c, rfs)); @@ -568,10 +566,8 @@ namespace bpkg // // Note that we also handle a package stub here. // - if (!dvc && dsp != nullptr && av < dsp->version) + if (!dsys && !dvc && dsp != nullptr && av < dsp->version) { - assert (!dsys); // Version can't be empty for the system package. - // For the selected system package we still need to pick a source // package version to downgrade to. // @@ -1533,7 +1529,14 @@ namespace bpkg package_scheme scheme; package_name name; + + // Note that currently a system package without constraint is always + // configured not to hold the version and can implicitly be + // up/downgraded. But in the future we can add support for holding the + // resolved version, for example, with the sys:libfoo/? syntax. + // optional constraint; + string value; pkg_options options; strings config_vars; @@ -1652,19 +1655,39 @@ namespace bpkg return r; }; - // Figure out the system package version unless explicitly specified and - // add the system package authoritative information to the database's + // Figure out the system package version(s) unless explicitly specified + // and add the system package authoritative information to the database's // system repository unless the database is NULL or it already contains - // authoritative information for this package. Return the figured out - // system package version as constraint. + // authoritative information for this package. + // + // Specifically, if the system package version is not specified its + // deduction is performed as follows: + // + // - If no system manager is available or --sys-no-query option is + // specified, then the version is the wildcard version. + // + // - Otherwise, the system manager is queried for available versions and + // the resulting versions are mapped to the downstream package + // versions. The system versions which cannot be mapped are not + // considered. + // + // - Fail if no version were deduced. // // Note that it is assumed that all the possible duplicates are handled // elsewhere/later. // + // @@ Since the only outcome of the function is adding the system package + // versions to the system repository, calling with the NULL database + // doesn't make much sense. This, however, can be plausable if we + // implement more accurate diagnostics (see below). So let's keep + // `database*` in favor of `database&` for now. + // auto add_system_package = [] (database* db, const package_name& nm, - optional vc) + const optional& vc) { + system_package_versions vs; + if (!vc) { // @@ Where do we check that this package should have available @@ -1713,15 +1736,23 @@ namespace bpkg //@@ TODO: if we extracted a version, then we need to add an entry // to the imaginary stubs (probably checking for duplicated), just // as if the user specified it explicitly. + + if (db != nullptr) + vs.push_back (wildcard_version); // @@ TMP } - else - vc = version_constraint (wildcard_version); + else if (db != nullptr) + vs.push_back (wildcard_version); } else + { // The system package may only have an exact/wildcard version // specified. // - assert (vc->min_version == vc->max_version); + assert (vc->min_version && vc->min_version == vc->max_version); + + if (db != nullptr) + vs.push_back (*vc->min_version); + } if (db != nullptr) { @@ -1730,9 +1761,7 @@ namespace bpkg const system_package* sp (db->system_repository->find (nm)); if (sp == nullptr || !sp->authoritative) - db->system_repository->insert (nm, - *vc->min_version, - true /* authoritative */); + db->system_repository->insert (nm, vs, true /* authoritative */); // @@ If it is authoritative, wouldn't it be a good idea to check that // the versions match? @@ -1766,8 +1795,6 @@ namespace bpkg // source of authoritative information is system package manager // and it caches its results. } - - return vc; }; // Create the parsed package argument. Issue diagnostics and fail if the @@ -1806,7 +1833,7 @@ namespace bpkg { case package_scheme::sys: { - r.constraint = add_system_package (db, r.name, move (r.constraint)); + add_system_package (db, r.name, r.constraint); break; } case package_scheme::none: break; // Nothing to do. @@ -2221,7 +2248,7 @@ namespace bpkg !compare_options (a.options, pa.options) || a.config_vars != pa.config_vars)) fail << "duplicate package " << pa.name << - info << "first mentioned as " << arg_string (r.first->second) << + info << "first mentioned as " << arg_string (a) << info << "second mentioned as " << arg_string (pa); return !r.second; @@ -2464,10 +2491,8 @@ namespace bpkg bool sys (arg_sys (pa)); - if (!pa.constraint) + if (!sys && !pa.constraint) { - assert (!sys); - if (pa.options.patch () && (sp = pdb->find (pa.name)) != nullptr) { @@ -2485,7 +2510,7 @@ namespace bpkg patch = true; } } - else if (!sys || !wildcard (*pa.constraint)) + else if (!sys || (pa.constraint && !wildcard (*pa.constraint))) c = pa.constraint; auto rp (find_available_one (pa.name, c, root)); @@ -2652,7 +2677,7 @@ namespace bpkg // if (pa.constraint) { - for (;;) + for (;;) // Breakout loop. { if (ap != nullptr) // Must be that version, see above. break; diff --git a/bpkg/system-repository.cxx b/bpkg/system-repository.cxx index d7a47b7..229ebd3 100644 --- a/bpkg/system-repository.cxx +++ b/bpkg/system-repository.cxx @@ -5,10 +5,14 @@ namespace bpkg { - const version& system_repository:: - insert (const package_name& name, const version& v, bool authoritative) + const system_package_versions& system_repository:: + insert (const package_name& name, + const system_package_versions& vs, + bool authoritative) { - auto p (map_.emplace (name, system_package {v, authoritative})); + assert (!vs.empty ()); + + auto p (map_.emplace (name, system_package {vs, authoritative})); if (!p.second) { @@ -21,10 +25,10 @@ namespace bpkg if (authoritative >= sp.authoritative) { sp.authoritative = authoritative; - sp.version = v; + sp.versions = vs; } } - return p.first->second.version; + return p.first->second.versions; } } diff --git a/bpkg/system-repository.hxx b/bpkg/system-repository.hxx index f33d622..d070c49 100644 --- a/bpkg/system-repository.hxx +++ b/bpkg/system-repository.hxx @@ -20,23 +20,38 @@ namespace bpkg // that are present in the database; in a sence it was authoritative but // on some previous run. // - // Note that in our model we assume that once an authoritative version has - // been discovered, it does not change (on this run; see caching logic in + // Note that in our model we assume that once an authoritative versions have + // been discovered, they does not change (on this run; see caching logic in // available package). // + using system_package_versions = small_vector; + struct system_package { - using version_type = bpkg::version; - - version_type version; + system_package_versions versions; bool authoritative; }; class system_repository { public: - const version& - insert (const package_name& name, const version&, bool authoritative); + // @@ Add move-insertion overloads (system_package_versions&& and + // version&&)? + // + + // Note: the system package versions are assumed to be provided in the + // preference descending order. + // + const system_package_versions& + insert (const package_name& name, + const system_package_versions&, + bool authoritative); + + const system_package_versions& + insert (const package_name& n, const version& v, bool a) + { + return insert (n, system_package_versions ({v}), a); + } const system_package* find (const package_name& name) diff --git a/tests/pkg-system.testscript b/tests/pkg-system.testscript index fc4f707..946a459 100644 --- a/tests/pkg-system.testscript +++ b/tests/pkg-system.testscript @@ -766,7 +766,7 @@ rep_remove += -d cfg 2>! EOE $pkg_status foo >'!foo configured 2'; - $pkg_status libbar >'libbar configured,system !*'; + $pkg_status libbar >'libbar configured,system *'; # Drop foo. # @@ -938,7 +938,14 @@ rep_remove += -d cfg 2>! EOE $pkg_status foo >'!foo configured 2'; - $pkg_status libbar >'libbar configured,system !*'; + $pkg_status libbar >'libbar configured,system *'; + + # Build ?sys:libbar/*. + # + # @@ Doesn't seems to reconfigure with the hold version flag. Need to fix. + # + #$pkg_build '?sys:libbar/*'; + #$pkg_status libbar >'libbar configured,system !*'; $pkg_drop foo } -- cgit v1.1