diff options
| -rw-r--r-- | bpkg/database.cxx | 95 | ||||
| -rw-r--r-- | bpkg/database.hxx | 22 | ||||
| -rw-r--r-- | bpkg/package.cxx | 50 | ||||
| -rw-r--r-- | tests/pkg-drop.testscript | 36 |
4 files changed, 155 insertions, 48 deletions
diff --git a/bpkg/database.cxx b/bpkg/database.cxx index 45f4b29..1b7612f 100644 --- a/bpkg/database.cxx +++ b/bpkg/database.cxx @@ -339,7 +339,22 @@ namespace bpkg schema_catalog::migrate (*this); for (auto& c: query<configuration> (odb::query<configuration>::id != 0)) - attach (c.make_effective_path (config)).migrate (); + { + dir_path d (c.effective_path (config)); + + // Remove the dangling implicit association. + // + if (!c.expl && !exists (d)) + { + warn << "implicit association " << c.path << " of configuration " + << config_orig << " does not exist, removing"; + + erase (c); + continue; + } + + attach (d).migrate (); + } } } @@ -516,54 +531,56 @@ namespace bpkg // if (implicit_associations_.empty () && ath) { + implicit_associations_.push_back (*this); + using q = odb::query<configuration>; - // Make sure the self-association (zero id) comes first. - // - for (const auto& ac: query<configuration> ("ORDER BY" + q::id)) + for (const auto& ac: query<configuration> (q::id != 0)) { - database& db (attach (ac.effective_path (config), sys_rep)); + dir_path d (ac.effective_path (config)); - // Verify the association integrity and pre-attach its explicit - // associations, if required. + // Skip the dangling implicit association. // - if (*ac.id != 0) + if (!ac.expl && !exists (d)) + continue; + + database& db (attach (d, sys_rep)); + + // Verify the association integrity. + // + verify_association (ac, db); + + // If the association is explicit, also check if it is also implicit + // (see cfg_add() for details) and skip if it is not. + // + if (ac.expl) { - verify_association (ac, db); + shared_ptr<configuration> cf ( + db.query_one<configuration> (q::uuid == uuid.string ())); - // If the association is explicit, also check if it is also implicit - // (see cfg_add() for details) and skip if it is not. - // - if (ac.expl) - { - shared_ptr<configuration> cf ( - db.query_one<configuration> (q::uuid == uuid.string ())); - - if (cf == nullptr) - fail << "configuration " << db.config_orig << " is associated " - << "with " << config_orig << " but latter is not " - << "implicitly associated with former"; - - // While at it, verify the integrity of the other end of the - // association. - // - db.verify_association (*cf, *this); - - if (!cf->expl) - continue; - } - - // If the explicitly associated databases are pre-attached, normally - // to make the selected packages loadable, then we also pre-attach - // explicit associations of the database being attached implicitly, - // by the same reason. Indeed, think of loading the package - // dependent from the implicitly associated database as a selected - // package. + if (cf == nullptr) + fail << "configuration " << db.config_orig << " is associated " + << "with " << config_orig << " but latter is not " + << "implicitly associated with former"; + + // While at it, verify the integrity of the other end of the + // association. // - if (!explicit_associations_.empty ()) - db.attach_explicit (sys_rep); + db.verify_association (*cf, *this); + + if (!cf->expl) + continue; } + // If the explicitly associated databases are pre-attached, normally + // to make the selected packages loadable, then we also pre-attach + // explicit associations of the database being attached implicitly, by + // the same reason. Indeed, think of loading the package dependent + // from the implicitly associated database as a selected package. + // + if (!explicit_associations_.empty ()) + db.attach_explicit (sys_rep); + implicit_associations_.push_back (db); } } diff --git a/bpkg/database.hxx b/bpkg/database.hxx index 53ca54a..735fe7f 100644 --- a/bpkg/database.hxx +++ b/bpkg/database.hxx @@ -143,11 +143,18 @@ namespace bpkg // By default attach and cache the implicitly associated configuration // databases on the first call and return them along with the self- - // association (comes first). If attach is false, then return an empty - // list if associations were not yet cached by this function's previous - // call. - // - // Note that for implicitly associated configurations the association + // association (comes first), silently skipping the dangling + // associations. If attach is false, then return an empty list if + // associations were not yet cached by this function's previous call. + // + // Note that we skip dangling associations without any warning since they + // can be quite common. Think of a shared host configuration with a bunch + // of implicitly associated configurations, which are removed and + // potentially recreated later during the host configuration lifetime. + // Note however, that we remove the dangling implicit associations during + // migration (see migrate() on details). + // + // Also note that for implicitly associated configurations the association // information (id, etc) is useless, thus we only return the databases // rather than the association information. // @@ -311,7 +318,10 @@ namespace bpkg // // Note that since the whole associated databases cluster is migrated at // once, it is assumed that if migration is unnecessary for this database - // then it is also unnecessary for its associated databases. + // then it is also unnecessary for its associated databases. By this + // reason, we also drop the dangling implicit associations rather than + // skip them, as we do for normal operations (see implicit_associations () + // for details). // void migrate (); diff --git a/bpkg/package.cxx b/bpkg/package.cxx index b1e9ab1..abfcd55 100644 --- a/bpkg/package.cxx +++ b/bpkg/package.cxx @@ -497,12 +497,56 @@ namespace bpkg lazy_shared_ptr<selected_package> _selected_package_ref:: to_ptr (odb::database& db) && { + database& pdb (static_cast<database&> (db)); + // Note that if this points to a different configuration, then it should // already be pre-attached since it must be explicitly associated. // - return lazy_shared_ptr<selected_package> ( - static_cast<database&> (db).find_dependency_config (configuration), - move (prerequisite)); + database& ddb (pdb.find_dependency_config (configuration)); + + // Make sure the prerequisite exists in the explicitly associated + // configuration, so that a subsequent load() call will not fail. This, + // for example, can happen in unlikely but possible situation when the + // implicitly associated configuration containing a dependent was + // temporarily renamed before its prerequisite was dropped. + // + // Note that the diagnostics lacks information about the dependent and its + // configuration. However, handling this situation at all the load() + // function call sites where this information is available, for example by + // catching the odb::object_not_persistent exception, feels a bit + // hairy. Given the situation is not common, let's keep it simple for now + // and see how it goes. + // + // @@ As a side note, the following code crashes. Is this a libodb bug or + // just lack of my understanding? + // +#if 0 + if (ddb != pdb) + { + shared_ptr<selected_package> p ( + ddb.find<selected_package> (prerequisite)); + + if (p != nullptr) + { + lazy_shared_ptr<selected_package> lp (ddb, move (p)); + lp.database ().string (); // Ok. + + lazy_shared_ptr<selected_package> lp2 (move (lp)); + lp2.database ().string (); // Ok. + + lazy_shared_ptr<selected_package> lp3; + + lp3 = move (lp2); // lp2.i_.db_ is not copied (odb/lazy-ptr-impl.ixx:83) + lp3.database ().string (); // Crashes since lp3.i_.db_ is NULL. + } + } +#endif + + if (ddb != pdb && ddb.find<selected_package> (prerequisite) == nullptr) + fail << "unable to find prerequisite package " << prerequisite + << " in associated configuration " << ddb.config_orig; + + return lazy_shared_ptr<selected_package> (ddb, move (prerequisite)); } pair<shared_ptr<selected_package>, database*> diff --git a/tests/pkg-drop.testscript b/tests/pkg-drop.testscript index ff42e58..a9e8f4f 100644 --- a/tests/pkg-drop.testscript +++ b/tests/pkg-drop.testscript @@ -693,4 +693,40 @@ $* libfoo/1.0.0 2>>~%EOE% != 0 $pkg_status -r libbar >'libbar available 1.0.0' } + + : skip-deleted-dependency + : + { + $clone_cfg; + cp -pr ../cfg2 ./; + + $pkg_build libbar --yes >! &cfg/lib*/*** &cfg/lib*; + + mv cfg cfg.tmp; + + $* -d cfg2 libbaz <<EOI 2>>/~%EOE%; + y + y + EOI + following dependent packages will have to be dropped as well: + foo (requires libbaz) + %drop dependent packages\? \[y.N\] drop foo% + drop libbaz + %continue\? \[Y.n\] disfigured foo% + disfigured libbaz + purged foo + purged libbaz + EOE + + # While at it, test that we properly handle the missing prerequisite + # situation. + # + mv cfg.tmp cfg; + + $* libbar 2>>/EOE != 0; + error: unable to find prerequisite package foo in associated configuration cfg2/ + EOE + + $pkg_status -d cfg2 -r 2>'info: no held packages in the configuration' + } } |