From 1a4a071568116f1131507f692f5618d5e5748b71 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 9 Sep 2020 15:58:01 +0300 Subject: Fix loader assertion failure for external repository without local cache Now loader fetches external repositories into temporary local caches. --- load/load.cxx | 180 ++++++++++++++++++++++++++++----- tests/load/1/testing/packages.manifest | 2 +- tests/load/driver.cxx | 16 ++- tests/load/loadtab | 7 +- 4 files changed, 168 insertions(+), 37 deletions(-) diff --git a/load/load.cxx b/load/load.cxx index e6591f5..53f1f25 100644 --- a/load/load.cxx +++ b/load/load.cxx @@ -352,6 +352,7 @@ repository_info (const options& lo, const string& rl, const cstrings& options) // static void load_packages (const shared_ptr& rp, + const repository_location& cl, database& db, bool ignore_unknown, const manifest_name_values& overrides) @@ -361,13 +362,9 @@ load_packages (const shared_ptr& rp, // assert (rp->packages_timestamp == timestamp_nonexistent); - // Only locally accessible repositories allowed until package manager API is - // ready. - // - assert (!rp->cache_location.empty ()); - vector pms; - const repository_location& cl (rp->cache_location); + + assert (!cl.empty ()); path p (cl.path () / packages); @@ -621,7 +618,9 @@ load_packages (const shared_ptr& rp, // changed members. Should be called once per persisted internal repository. // static void -load_repositories (const shared_ptr& rp, +load_repositories (const options& lo, + const shared_ptr& rp, + const repository_location& cl, database& db, bool ignore_unknown, bool shallow) @@ -631,11 +630,6 @@ load_repositories (const shared_ptr& rp, // assert (rp->repositories_timestamp == timestamp_nonexistent); - // Only locally accessible repositories allowed until package manager API is - // ready. - // - assert (!rp->cache_location.empty ()); - const string& tenant (rp->tenant); // Repository is already persisted by the load_packages() function call. @@ -645,7 +639,9 @@ load_repositories (const shared_ptr& rp, pkg_repository_manifests rpm; - path p (rp->cache_location.path () / repositories); + assert (!cl.empty ()); + + path p (cl.path () / repositories); try { @@ -780,15 +776,19 @@ load_repositories (const shared_ptr& rp, pr = make_shared (tenant, move (rl)); - // If the prerequsite repository location is a relative path, then - // calculate its cache location. + // If the base repository is internal and the prerequsite repository + // location is a relative path, then calculate its cache location. // - if (rm.location.relative ()) + if (rp->internal && rm.location.relative ()) { + // For an internal repository the cache location always comes from the + // loadtab file. + // + assert (cl.path () == rp->cache_location.path ()); + try { - pr->cache_location = - repository_location (rm.location, rp->cache_location); + pr->cache_location = repository_location (rm.location, cl); } catch (const invalid_argument&) { @@ -796,21 +796,141 @@ load_repositories (const shared_ptr& rp, << "repository '" << rm.location << "'" << endl << " info: base (internal) repository location is " << rp->location << endl - << " info: base repository cache location is " - << rp->cache_location << endl; + << " info: base repository cache location is " << cl << endl; throw failed (); } } + // If the (external) prerequisite repository cache location is empty, then + // check if the repository is local and, if that's the case, use its + // location as a cache location. Otherwise, fetch the repository + // information creating a temporary cache for it. + // + auto_rmdir cdr; // Remove the temporary cache after the repo load. + repository_location cl; // Repository temporary cache location. + + if (pr->cache_location.empty ()) + { + if (pr->location.local ()) + { + pr->cache_location = pr->location; + } + else + { + dir_path cd; + + try + { + cd = dir_path::temp_path ("brep-load-cache"); + } + catch (const system_error& e) + { + cerr << "unable to obtain temporary directory: " << e; + throw failed (); + } + + // It's highly unlikely but still possible that the temporary cache + // directory already exists. This can only happen due to the unclean + // loader termination. Let's remove it and retry. + // + try + { + if (try_mkdir (cd) == mkdir_status::already_exists) + { + try_rmdir_r (cd); + + if (try_mkdir (cd) == mkdir_status::already_exists) + throw_generic_error (EEXIST); + } + } + catch (const system_error& e) + { + cerr << "unable to create directory '" << cd << "': " << e; + throw failed (); + } + + cdr = auto_rmdir (cd); + + path rf (cd / repositories); + path pf (cd / packages); + + // Note that the fetch timeout can be overridden via --bpkg-option. + // + cstrings args { + "--fetch-timeout", "60", // 1 minute. + "--deep", + "--manifest", + "--repositories", + "--repositories-file", rf.string ().c_str (), + "--packages", + "--packages-file", pf.string ().c_str ()}; + + if (rm.trust) + { + args.push_back ("--trust"); + args.push_back (rm.trust->c_str ()); + } + + // Always add it, so bpkg won't try to prompt for a certificate + // authentication if the fingerprint doesn't match. + // + args.push_back ("--trust-no"); + + process p (repository_info (lo, pr->location.string (), args)); + + try + { + // @@ Note that bpkg-rep-info can fail with the 'repository manifest + // file checksum mismatch' error if the repository was changed + // during the fetch. We will need to retry when bpkg is fixed to + // exit with some recognizable code on this failure. + // + if (!p.wait ()) + { + // Assume the child issued diagnostics if terminated normally. + // + if (!p.exit->normal ()) + cerr << "process " << lo.bpkg () << " " << *p.exit << endl; + + cerr << "error: unable to fetch manifests for " + << pr->canonical_name << endl + << " info: base repository location is " + << rp->location << endl; + + throw failed (); + } + } + catch (const process_error& e) + { + cerr << "error: unable to fetch manifests for " + << pr->canonical_name << ": " << e << endl; + + throw failed (); + } + + // Note that this is a non-pkg repository cache and so we create the + // dir repository location (see load_repositories(path) for details). + // + cl = repository_location (repository_url (cd.string ()), + repository_type::dir); + } + } + // We don't apply overrides to the external packages. // load_packages (pr, + !pr->cache_location.empty () ? pr->cache_location : cl, db, ignore_unknown, manifest_name_values () /* overrides */); - load_repositories (pr, db, ignore_unknown, false /* shallow */); + load_repositories (lo, + pr, + !pr->cache_location.empty () ? pr->cache_location : cl, + db, + ignore_unknown, + false /* shallow */); } db.update (rp); @@ -1149,8 +1269,11 @@ certificate_info (const options& lo, pr.wait (); } - // Assume the child issued diagnostics. + // Assume the child issued diagnostics if terminated normally. // + if (!pr.exit->normal ()) + cerr << "process " << lo.bpkg () << " " << *pr.exit << endl; + cerr << "error: unable to fetch certificate information for " << rl.canonical_name () << endl; @@ -1358,7 +1481,11 @@ try ir.buildable, priority++)); - load_packages (r, db, ops.ignore_unknown (), overrides); + load_packages (r, + r->cache_location, + db, + ops.ignore_unknown (), + overrides); } // On the second pass over the internal repositories we load their @@ -1371,7 +1498,12 @@ try db.load ( repository_id (tnt, ir.location.canonical_name ()))); - load_repositories (r, db, ops.ignore_unknown (), ops.shallow ()); + load_repositories (ops, + r, + r->cache_location, + db, + ops.ignore_unknown (), + ops.shallow ()); } // Resolve internal packages dependencies and, unless this is a shallow diff --git a/tests/load/1/testing/packages.manifest b/tests/load/1/testing/packages.manifest index a903878..2d458f0 100644 --- a/tests/load/1/testing/packages.manifest +++ b/tests/load/1/testing/packages.manifest @@ -1,5 +1,5 @@ : 1 -sha256sum: 5f2b297be1eafd70fe55f179a0cf062baf8405e08b3854600801420132a206b1 +sha256sum: fac618ab9d8132777a7d2f10e338266957de02b87f233714bce2331e692f4ab6 : name: libmisc version: 2.4.0 diff --git a/tests/load/driver.cxx b/tests/load/driver.cxx index b4c43cc..8192827 100644 --- a/tests/load/driver.cxx +++ b/tests/load/driver.cxx @@ -975,8 +975,8 @@ test_pkg_repos (const cstrings& loader_args, assert (tr->location.canonical_name () == "pkg:dev.cppget.org/testing"); assert (tr->location.string () == "http://dev.cppget.org/1/testing"); - assert (tr->display_name.empty ()); - assert (tr->priority == 0); + assert (tr->display_name == "testing"); + assert (tr->priority == 3); assert (tr->interface_url && *tr->interface_url == "http://test.cppget.org/hello/"); assert (!tr->email); @@ -994,7 +994,7 @@ test_pkg_repos (const cstrings& loader_args, assert (tr->repositories_timestamp == file_mtime (tr->cache_location.path () / repositories)); - assert (!tr->internal); + assert (tr->internal); assert (tr->prerequisites.empty ()); assert (tr->complements.size () == 1); assert (tr->complements[0].load () == gr); @@ -1007,9 +1007,8 @@ test_pkg_repos (const cstrings& loader_args, db.load ( package_id (tenant, package_name ("libmisc"), version ("2.4.0")))); - assert (check_external (*mpv0)); - assert (mpv0->other_repositories.size () == 1); - assert (mpv0->other_repositories[0].load () == tr); + assert (mpv0->internal_repository.load () == tr); + assert (mpv0->other_repositories.empty ()); assert (check_location (mpv0)); assert (!mpv0->buildable); @@ -1019,9 +1018,8 @@ test_pkg_repos (const cstrings& loader_args, db.load ( package_id (tenant, package_name ("libmisc"), version ("2.3.0+1")))); - assert (check_external (*mpv1)); - assert (mpv1->other_repositories.size () == 1); - assert (mpv1->other_repositories[0].load () == tr); + assert (mpv1->internal_repository.load () == tr); + assert (mpv1->other_repositories.empty ()); assert (check_location (mpv1)); assert (!mpv1->buildable); diff --git a/tests/load/loadtab b/tests/load/loadtab index 96e1f00..b6ce020 100644 --- a/tests/load/loadtab +++ b/tests/load/loadtab @@ -1,4 +1,5 @@ -http://dev.cppget.org/1/stable stable cache:1/stable buildable:no +http://dev.cppget.org/1/stable stable cache:1/stable buildable:no http://dev.cppget.org/1/math math cache:1/math -http://dev.cppget.org/1/signed signed cache:pkg/1/dev.cppget.org/signed fingerprint:C3:EC:12:53:AD:64:41:0E:35:3A:9A:A6:EE:57:BF:E6:05:40:42:2B:FF:AF:2C:B0:99:AD:E9:4A:9C:48:40:22 -http://dev.cppget.org/1/unsigned unsigned cache:pkg/1/dev.cppget.org/unsigned fingerprint: +http://dev.cppget.org/1/testing testing cache:1/testing buildable:no +http://dev.cppget.org/1/signed signed cache:pkg/1/dev.cppget.org/signed fingerprint:C3:EC:12:53:AD:64:41:0E:35:3A:9A:A6:EE:57:BF:E6:05:40:42:2B:FF:AF:2C:B0:99:AD:E9:4A:9C:48:40:22 +http://dev.cppget.org/1/unsigned unsigned cache:pkg/1/dev.cppget.org/unsigned fingerprint: -- cgit v1.1