Fix loader assertion failure for external repository without local cache

Now loader fetches external repositories into temporary local caches.

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<repository>& rp,
+               const repository_location& cl,
                database& db,
                bool ignore_unknown,
                const manifest_name_values& overrides)
@@ -361,13 +362,9 @@ load_packages (const shared_ptr<repository>& rp,
   //
   assert (rp->packages_timestamp == timestamp_nonexistent);
 
-  // Only locally accessible repositories allowed until package manager API is
-  // ready.
-  //
-  assert (!rp->cache_location.empty ());
-
   vector<package_manifest> 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<repository>& rp,
 // changed members. Should be called once per persisted internal repository.
 //
 static void
-load_repositories (const shared_ptr<repository>& rp,
+load_repositories (const options& lo,
+                   const shared_ptr<repository>& rp,
+                   const repository_location& cl,
                    database& db,
                    bool ignore_unknown,
                    bool shallow)
@@ -631,11 +630,6 @@ load_repositories (const shared_ptr<repository>& 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<repository>& 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<repository>& rp,
 
     pr = make_shared<repository> (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<repository>& 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> (
           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> (
         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> (
         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: