From ca387611165403835655db1f3620ef8e65cb92b5 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Thu, 10 May 2018 21:28:26 +0300 Subject: Fix package checksum mismatch for all repositories being fetched --- bpkg/package.hxx | 10 +---- bpkg/rep-fetch.cxx | 106 ++++++++++++++++++++++++++++++++++------------------ bpkg/rep-remove.hxx | 2 + 3 files changed, 73 insertions(+), 45 deletions(-) diff --git a/bpkg/package.hxx b/bpkg/package.hxx index 1986e1f..94d5136 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -247,17 +247,11 @@ namespace bpkg // fragmented. For example, a git repository consists of multiple commits // (fragments) which could contain different sets of packages and even // prerequisite/complement repositories. Note also that the same fragment - // could be shared by multiple repository objects. We assume a fragment to - // be immutable, so it's complement, prerequisite and package sets can never - // change. + // could be shared by multiple repository objects. // // For repository types that do not support fragmentation, there should // be a single repository_fragment with the name and location equal to the - // ones of the containing repository. Such a fragment can not be shared but - // can be changed. - // - // One of the consequences of the above is that a fragment can either be - // shared or be mutable. + // ones of the containing repository. Such a fragment cannot be shared. // class repository; diff --git a/bpkg/rep-fetch.cxx b/bpkg/rep-fetch.cxx index 6e7ec88..412a745 100644 --- a/bpkg/rep-fetch.cxx +++ b/bpkg/rep-fetch.cxx @@ -469,11 +469,11 @@ namespace bpkg } // Return an existing repository fragment or create a new one. Update the - // existing object unless it is immutable (see repository_fragment class - // description for details). Don't fetch the complement and prerequisite - // repositories. + // existing object unless it is already fetched. Don't fetch the complement + // and prerequisite repositories. // - using repository_trust = map, optional>; + using repository_fragments = set>; + using repository_trust = map, optional>; static shared_ptr rep_fragment (const common_options& co, @@ -481,6 +481,8 @@ namespace bpkg transaction& t, const repository_location& rl, rep_fetch_data::fragment&& fr, + repository_fragments& parsed_fragments, + bool full_fetch, repository_trust& repo_trust) { tracer trace ("rep_fragment"); @@ -488,8 +490,6 @@ namespace bpkg database& db (t.database ()); tracer_guard tg (db, trace); - bool mut (fr.id.empty ()); // Is the fragment mutable? - // Calculate the fragment location. // repository_location rfl; @@ -499,15 +499,11 @@ namespace bpkg case repository_type::pkg: case repository_type::dir: { - assert (mut); - rfl = rl; break; } case repository_type::git: { - assert (!mut); - repository_url url (rl.url ()); url.fragment = move (fr.id); @@ -569,7 +565,7 @@ namespace bpkg } }; - // Return the existing repository fragment if it is immutable. + // Create or update the repository fragment. // bool exists (rf != nullptr); @@ -586,25 +582,20 @@ namespace bpkg rf->location = move (rfl); db.update (rf); } - - if (!mut) - { - add_trust (); - return rf; - } } + else + rf = make_shared (move (rfl)); - // Create or update the repository fragment. - // - if (exists) + if (!parsed_fragments.insert (rf).second) // Is already parsed. { - assert (mut); + assert (exists); - rf->complements.clear (); - rf->prerequisites.clear (); + add_trust (); + return rf; } - else - rf = make_shared (move (rfl)); + + rf->complements.clear (); + rf->prerequisites.clear (); for (repository_manifest& rm: fr.repositories) { @@ -676,6 +667,9 @@ namespace bpkg } } + // Note that it relies on the prerequisite and complement repositories be + // already persisted. + // add_trust (); // For dir and git repositories that have neither prerequisites nor @@ -718,9 +712,11 @@ namespace bpkg session::reset_current (); // Remove this repository fragment from locations of the available - // packages it contains. + // packages it contains. Note that when we fetch all the repositories the + // available packages are cleaned up in advance (see rep_fetch() for + // details). // - if (exists) + if (exists && !full_fetch) rep_remove_package_locations (t, rf->name); for (package_manifest& pm: fr.packages) @@ -782,10 +778,24 @@ namespace bpkg const string& r1 (rl.canonical_name ()); const string& r2 (p->locations[0].repository_fragment.object_id ()); - fail << "checksum mismatch for " << pm.name << " " << pm.version << + diag_record dr (fail); + + dr << "checksum mismatch for " << pm.name << " " << pm.version << info << r1 << " has " << *pm.sha256sum << - info << r2 << " has " << *p->sha256sum << - info << "consider reporting this to the repository maintainers"; + info << r2 << " has " << *p->sha256sum; + + // If we fetch all the repositories then the mismatch is definitely + // caused by the broken repository. Otherwise, it may also happen + // due to the old available package that is not wiped out yet. + // Thus, we advice the user to perform the full fetch, unless the + // filesystem state is already changed and so this advice will be + // given anyway (see rep_fetch() for details). + // + if (full_fetch) + dr << info << "consider reporting this to the repository " + << "maintainers"; + else if (!filesystem_state_changed) + dr << info << "run 'bpkg rep-fetch' to update"; } } } @@ -805,8 +815,7 @@ namespace bpkg return rf; } - using repositories = set>; - using repository_fragments = set>; + using repositories = set>; // If reason is absent, then don't print the "fetching ..." progress line. // @@ -818,8 +827,10 @@ namespace bpkg const optional& dependent_trust, repositories& fetched_repositories, repositories& removed_repositories, + repository_fragments& parsed_fragments, repository_fragments& removed_fragments, bool shallow, + bool full_fetch, const optional& reason) { tracer trace ("rep_fetch(rep)"); @@ -926,8 +937,14 @@ namespace bpkg { string nm (fr.friendly_name); // Don't move, still may be used. - shared_ptr rf ( - rep_fragment (co, conf, t, rl, move (fr), repo_trust)); + shared_ptr rf (rep_fragment (co, + conf, + t, + rl, + move (fr), + parsed_fragments, + full_fetch, + repo_trust)); collect_deps (rf, new_complements, new_prerequisites); @@ -996,7 +1013,9 @@ namespace bpkg &t, &fetched_repositories, &removed_repositories, + &parsed_fragments, &removed_fragments, + full_fetch, &rl, &repo_trust] (const shared_ptr& r, const char* what) @@ -1011,8 +1030,10 @@ namespace bpkg i->second, fetched_repositories, removed_repositories, + parsed_fragments, removed_fragments, false /* shallow */, + full_fetch, what + rl.canonical_name ()); }; @@ -1042,6 +1063,7 @@ namespace bpkg transaction& t, const vector>& repos, bool shallow, + bool full_fetch, const optional& reason) { tracer trace ("rep_fetch(repos)"); @@ -1067,6 +1089,7 @@ namespace bpkg repositories fetched_repositories; repositories removed_repositories; + repository_fragments parsed_fragments; repository_fragments removed_fragments; // Fetch the requested repositories, recursively. @@ -1079,8 +1102,10 @@ namespace bpkg nullopt /* dependent_trust */, fetched_repositories, removed_repositories, + parsed_fragments, removed_fragments, shallow, + full_fetch, reason); // Remove dangling repositories. @@ -1195,7 +1220,7 @@ namespace bpkg repos.emplace_back (r); } - rep_fetch (o, conf, t, repos, shallow, reason); + rep_fetch (o, conf, t, repos, shallow, false /* full_fetch */, reason); t.commit (); } @@ -1223,8 +1248,9 @@ namespace bpkg repository_fragment::complements_type& ua (root->complements); optional reason; + bool full_fetch (!args.more ()); - if (!args.more ()) + if (full_fetch) { if (ua.empty ()) fail << "configuration " << c << " has no repositories" << @@ -1237,6 +1263,12 @@ namespace bpkg // there is only one. // reason = ""; + + // Cleanup the available packages in advance to avoid sha256sum mismatch + // for packages being fetched and the old available packages, that are + // not wiped out yet (see rep_fragment() for details). + // + db.erase_query (); } else { @@ -1296,7 +1328,7 @@ namespace bpkg } } - rep_fetch (o, c, t, repos, o.shallow (), reason); + rep_fetch (o, c, t, repos, o.shallow (), full_fetch, reason); size_t rcount (0), pcount (0); if (verb) diff --git a/bpkg/rep-remove.hxx b/bpkg/rep-remove.hxx index 477f308..2f8df16 100644 --- a/bpkg/rep-remove.hxx +++ b/bpkg/rep-remove.hxx @@ -43,6 +43,8 @@ namespace bpkg // // - Remove all repositories except the top-level ones and the root. // + // - Remove all repository fragments except the root. + // // - Remove all repository state directories (regardless of whether they // actually relate to any existing repositories). // -- cgit v1.1