Package object model refactoring

8 files changed, 369 insertions, 286 deletions

diff --git a/brep/package b/brep/package
index c81e652..c383b82 100644
--- a/brep/package
+++ b/brep/package
@@ -104,32 +104,33 @@ namespace brep
   #pragma db value
   struct package_version_id
   {
-    std::string repository;
     std::string package;
     std::uint16_t epoch;
     std::string canonical_upstream;
+    std::uint16_t revision;
 
     // Database mapping.
     //
-    #pragma db member(repository) points_to(repository) //on_delete(cascade)
     #pragma db member(package) points_to(package) //on_delete(cascade)
   };
 
   inline bool
   operator< (const package_version_id& x, const package_version_id& y)
   {
-    int r (x.repository.compare (y.repository));
+    int r (x.package.compare (y.package));
 
     if (r != 0)
       return r < 0;
 
-    r  = x.package.compare (y.package);
+    if (x.epoch != y.epoch)
+      return x.epoch < y.epoch;
+
+    r = x.canonical_upstream.compare (y.canonical_upstream);
 
     if (r != 0)
       return r < 0;
 
-    return x.epoch < y.epoch ||
-           (x.epoch == y.epoch && x.canonical_upstream < y.canonical_upstream);
+    return x.revision < y.revision;
   }
 
   using priority = bpkg::priority;
@@ -229,13 +230,6 @@ namespace brep
   class repository
   {
   public:
-    using path_type = brep::path;
-    using timestamp_type = brep::timestamp;
-    using package_versions_type =
-      std::vector<odb::lazy_weak_ptr<package_version>>;
-    using prerequisite_repositories_type =
-      std::vector<odb::lazy_weak_ptr<repository>>;
-
     // Create internal repository.
     //
     repository (repository_location,
@@ -258,16 +252,14 @@ namespace brep
 
     // Initialized with timestamp_nonexistent by default.
     //
-    timestamp_type packages_timestamp;
+    timestamp packages_timestamp;
 
     // Initialized with timestamp_nonexistent by default.
     // For external repositories stays timestamp_nonexistent.
     //
-    timestamp_type repositories_timestamp;
+    timestamp repositories_timestamp;
 
     bool internal;
-    package_versions_type package_versions;
-    prerequisite_repositories_type prerequisite_repositories;
 
     // Database mapping.
     //
@@ -289,10 +281,6 @@ namespace brep
     #pragma db member(id) virtual(_id_type) before id(canonical_name)   \
       get(_id) set(_id (std::move (?))) column("")
 
-    #pragma db member(package_versions) inverse(id.data.repository)
-    #pragma db member(prerequisite_repositories) id_column("repository") \
-      value_column("prerequisite_repository") value_not_null
-
   private:
     friend class odb::access;
     repository () = default;
@@ -333,7 +321,7 @@ namespace brep
     //
     #pragma db member(name) id
     #pragma db member(tags) id_column("package") value_column("tag")
-    #pragma db member(versions) inverse(id.data.package)
+    #pragma db member(versions) inverse(id.data.package) value_not_null
 
   private:
     friend class odb::access;
@@ -352,19 +340,18 @@ namespace brep
     using dependencies_type = brep::dependencies;
     using requirements_type = brep::requirements;
 
-    package_version (odb::lazy_shared_ptr<repository_type>,
-                     odb::lazy_shared_ptr<package_type>,
+    package_version (odb::lazy_shared_ptr<package_type>,
                      version_type,
                      priority_type,
                      license_alternatives_type,
                      std::string changes,
                      dependencies_type,
                      requirements_type,
-                     optional<path> location);
+                     optional<path> location,
+                     odb::lazy_shared_ptr<repository_type>);
 
     // Manifest data.
     //
-    odb::lazy_shared_ptr<repository_type> repository;
     odb::lazy_shared_ptr<package_type> package;
     version_type version;
     priority_type priority;
@@ -372,8 +359,15 @@ namespace brep
     std::string changes;
     dependencies_type dependencies;
     requirements_type requirements;
+    odb::lazy_shared_ptr<repository_type> internal_repository;
+
+    // Path to the package file. Set only for package versions present in
+    // internal repository.
+    //
     optional<path> location;
 
+    std::vector<odb::lazy_shared_ptr<repository_type>> external_repositories;
+
     // Database mapping.
     //
 
@@ -385,7 +379,6 @@ namespace brep
       #pragma db column("")
       package_version_id data;
       std::string upstream;
-      std::uint16_t revision;
     };
 
     _id_type
@@ -396,7 +389,6 @@ namespace brep
 
     #pragma db member(version) transient
     #pragma db member(package) transient
-    #pragma db member(repository) transient
 
     #pragma db member(id) virtual(_id_type) before id(data) \
       get(_id) set(_id (std::move (?), (!))) column("")
@@ -454,6 +446,11 @@ namespace brep
       set(_set (this.requirements, (?)))              \
       id_column("") key_column("") value_column("id")
 
+    #pragma db member(external_repositories) \
+      id_column("")                          \
+      value_column("repository")             \
+      value_not_null
+
   private:
     friend class odb::access;
     package_version () = default;
@@ -462,7 +459,7 @@ namespace brep
   #pragma db view object(package_version)                               \
     query((?) + "ORDER BY" + package_version::id.data.epoch + "DESC," + \
           package_version::id.data.canonical_upstream + "DESC," +       \
-          package_version::id.revision + "DESC LIMIT 1")
+          package_version::id.data.revision + "DESC LIMIT 1")
   struct max_package_version
   {
     using version_type = brep::version;
diff --git a/brep/package-version-details.cxx b/brep/package-version-details.cxx
index 094c3a3..0faecd2 100644
--- a/brep/package-version-details.cxx
+++ b/brep/package-version-details.cxx
@@ -9,7 +9,6 @@
 #include <cassert>
 #include <stdexcept> // invalid_argument
 
-
 #include <xml/serializer>
 
 #include <odb/database.hxx>
diff --git a/brep/package-version-search.cxx b/brep/package-version-search.cxx
index d7fea2d..bd01a46 100644
--- a/brep/package-version-search.cxx
+++ b/brep/package-version-search.cxx
@@ -164,7 +164,7 @@ namespace brep
       db_->query<package_version> ((query::id.data.package == name) +
         "ORDER BY" + query::id.data.epoch + "DESC," +
         query::id.data.canonical_upstream + "DESC," +
-        query::id.revision + "DESC " +
+        query::id.data.revision + "DESC " +
         "OFFSET" + to_string (pr.page () * rop) +
         "LIMIT" + to_string (rop)));
 
diff --git a/brep/package.cxx b/brep/package.cxx
index 215754f..59efbfb 100644
--- a/brep/package.cxx
+++ b/brep/package.cxx
@@ -55,17 +55,16 @@ namespace brep
   // package_version
   //
   package_version::
-  package_version (lazy_shared_ptr<repository_type> rp,
-                   lazy_shared_ptr<package_type> pk,
+  package_version (lazy_shared_ptr<package_type> pk,
                    version_type vr,
                    priority_type pr,
                    license_alternatives_type la,
                    string ch,
                    dependencies_type dp,
                    requirements_type rq,
-                   optional<path> lc)
-      : repository (move (rp)),
-        package (move (pk)),
+                   optional<path> lc,
+                   lazy_shared_ptr<repository_type> rp)
+      : package (move (pk)),
         version (move (vr)),
         priority (move (pr)),
         license_alternatives (move (la)),
@@ -74,6 +73,10 @@ namespace brep
         requirements (move (rq)),
         location (move (lc))
   {
+    if (rp.load ()->internal)
+      internal_repository = move (rp);
+    else
+      external_repositories.emplace_back (move (rp));
   }
 
   package_version::_id_type package_version::
@@ -81,21 +84,19 @@ namespace brep
   {
     return _id_type {
       {
-        repository.object_id (),
         package.object_id (),
         version.epoch (),
-        version.canonical_upstream ()
+        version.canonical_upstream (),
+        version.revision ()
       },
-      version.upstream (),
-      version.revision ()};
+      version.upstream ()};
   }
 
   void package_version::
   _id (_id_type&& v, database& db)
   {
-    repository = lazy_shared_ptr<repository_type> (db, v.data.repository);
     package = lazy_shared_ptr<package_type> (db, v.data.package);
-    version = version_type (v.data.epoch, move (v.upstream), v.revision);
+    version = version_type (v.data.epoch, move (v.upstream), v.data.revision);
     assert (version.canonical_upstream () == v.data.canonical_upstream);
   }
 
@@ -104,7 +105,7 @@ namespace brep
   void max_package_version::
   _id (package_version::_id_type&& v)
   {
-    version = version_type (v.data.epoch, move (v.upstream), v.revision);
+    version = version_type (v.data.epoch, move (v.upstream), v.data.revision);
     assert (version.canonical_upstream () == v.data.canonical_upstream);
   }
 
diff --git a/loader/loader.cxx b/loader/loader.cxx
index e666804..7d60da6 100644
--- a/loader/loader.cxx
+++ b/loader/loader.cxx
@@ -8,11 +8,11 @@
 #include <utility>   // move()
 #include <cstdint>   // uint64_t
 #include <sstream>
+#include <cassert>
 #include <fstream>
 #include <iostream>
 #include <stdexcept> // runtime_error, invalid_argument
 
-#include <odb/session.hxx>
 #include <odb/database.hxx>
 #include <odb/transaction.hxx>
 
@@ -233,152 +233,123 @@ changed (const internal_repositories& repos, database& db)
   return !rs.empty ();
 }
 
-// Load the repository state (including of its prerequsite repositories)
-// from the 'packages' file.
+static timestamp
+manifest_stream (const path& p, ifstream& f)
+{
+  f.open (p.string ());
+  if (!f.is_open ())
+    throw ifstream::failure (p.string () + ": unable to open");
+
+  f.exceptions (ifstream::badbit | ifstream::failbit);
+  return file_mtime (p);
+}
+
+// Loads the repository packages from the 'packages' file and persists the
+// repository. Should be called once per repository.
 //
 static void
-load_repository (const shared_ptr<repository>& rp, database& db)
+load_packages (const shared_ptr<repository>& rp, database& db)
 {
-  if (rp->packages_timestamp != timestamp_nonexistent)
-    return; // The repository is already loaded.
+  // packages_timestamp different from timestamp_nonexistent signals the
+  // repository packages are already loaded.
+  //
+  assert (rp->packages_timestamp == timestamp_nonexistent);
 
   // Only locally accessible repositories allowed until package manager API is
   // ready.
   //
   assert (!rp->local_path.empty ());
 
-  auto mstream ([](const path& p, ifstream& f) -> timestamp
-    {
-      f.open (p.string ());
-      if (!f.is_open ())
-        throw ifstream::failure (p.string () + ": unable to open");
-      f.exceptions (ifstream::badbit | ifstream::failbit);
-      return file_mtime (p);
-    });
-
-  // Don't add prerequisite repositories for external repositories.
-  //
-  if (rp->internal)
-  {
-    repository_manifests rpm;
-
-    {
-      ifstream ifs;
-      path p (rp->local_path / path ("repositories"));
-      rp->repositories_timestamp = mstream (p, ifs);
-
-      manifest_parser mp (ifs, p.string ());
-      rpm = repository_manifests (mp);
-    }
-
-    for (auto& rm: rpm)
-    {
-      if (rm.location.empty ())
-        continue; // Ignore entry for this repository.
-
-      repository_location rl;
-
-      auto bad_location (
-        [&rp, &rm]()
-        {
-          ostringstream o;
-          o << "invalid location '" << rm.location.string ()
-            << "' of the prerequisite repository for internal "
-            "repository '" << rp->location.string () << "'";
-
-          throw runtime_error (o.str ());
-        });
-
-      try
-      {
-        // Absolute path location make no sense for the web interface.
-        //
-        if (rm.location.absolute ())
-          bad_location ();
-
-        // Convert the relative repository location to remote one, leave remote
-        // location unchanged.
-        //
-        rl = repository_location (rm.location.string (), rp->location);
-      }
-      catch (const invalid_argument&)
-      {
-        bad_location ();
-      }
-
-      shared_ptr<repository> pr (db.find<repository> (rl.canonical_name ()));
-
-      if (pr == nullptr)
-      {
-        pr = make_shared<repository> (move (rl));
-
-        // If the prerequsite repository location is a relative path, then
-        // calculate its absolute local path.
-        //
-        if (rm.location.relative ())
-        {
-          dir_path& lp (pr->local_path);
-          lp = rp->local_path / rm.location.path ();
-
-          try
-          {
-            lp.normalize ();
-          }
-          catch (const invalid_path&)
-          {
-            ostringstream o;
-            o << "can't normalize local path'" << lp.string ()
-              << "' of the prerequisite repository for internal "
-              "repository '" << rp->location.string () << "'";
-
-            throw runtime_error (o.str ());
-          }
-        }
-
-        db.persist (pr);
-      }
-
-      load_repository (pr, db);
-
-      rp->prerequisite_repositories.emplace_back (pr);
-    }
-  }
-
-  // Temporary reset ODB session for the current thread while persisting
-  // package and package_version objects to decrease memory consumption.
-  //
-  session& s (session::current ());
-  session::reset_current ();
-
   package_manifests pkm;
 
   {
     ifstream ifs;
     path p (rp->local_path / path ("packages"));
-
-    // Mark as loaded. This is important in case we try to load this
-    // repository again recursively.
-    //
-    rp->packages_timestamp = mstream (p, ifs);
+    rp->packages_timestamp = manifest_stream (p, ifs);
 
     manifest_parser mp (ifs, p.string ());
     pkm = package_manifests (mp);
   }
 
+  // Let's establish the terminology which will be used in comments appearing
+  // in the body of this function.
+  // * Will call a package manifest internal if corresponding 'packages' file
+  //   is located in the internal repository, otherwise call a package manifest
+  //   external.
+  // * Will call a package version internal if it is described by internal
+  //   package manifest, otherwise call a package version external.
+  // * Will call a package internal if there is an internal package version,
+  //   otherwise call it external.
+  //
+
   for (auto& pm: pkm)
   {
+    // The code below ensures that the package object get updated with a
+    // package manifest info of the highest version. It should also be assured
+    // that for the internal package only an internal package manifests are
+    // considered for an update purpose.
+    //
+
     max_package_version mv;
 
-    // If there are no package_version objects persisted yet for this
-    // package, then query_one() will leave mv unchanged in which case
-    // the version member remains empty. The empty version value is
-    // less than any non-empty one so the condition below evaluates
-    // to true and the package object gets persisted.
+    // If there are no package_version objects meeting query condition,
+    // then query_one() will leave mv unchanged, in which case the version
+    // member remains empty. The empty version value is less than any non-empty
+    // one, so version comparisons below evaluate to true and the package
+    // object gets persisted.
+    //
+    // Get maximum internal version of the package.
     //
+    using query = query<max_package_version>;
     db.query_one<max_package_version> (
-      query<max_package_version>::id.data.package == pm.name, mv);
+      query::id.data.package == pm.name &&
+      query::internal_repository.is_not_null (),
+      mv);
+
+    bool update (false);
+
+    if (mv.version.empty ())
+    {
+      // The package is external or not persisted yet.
+      //
+
+      // Get maximum external version of the package.
+      //
+      db.query_one<max_package_version> (
+        query::id.data.package == pm.name, mv);
+
+      if (rp->internal)
+        // Unconditionally update external package with internal package
+        // manifest info. Persist not yet persisted package.
+        //
+        update = true;
+      else
+        // Update external package with external package manifest info
+        // of a higher version. Version of not persisted package is empty and
+        // therefore less then any package manifest version, so the package
+        // will be persisted.
+        //
+        update = mv.version < pm.version;
+    }
+    else
+    {
+      // The package is internal.
+      //
+
+      if (rp->internal)
+        // Update internal package with the internal package manifest info
+        // of a higher version.
+        //
+        update = mv.version < pm.version;
+      else
+      {
+        // Should not update internal package with an external package
+        // manifest info.
+        //
+      }
+    }
 
-    if (mv.version < pm.version)
+    if (update)
     {
       // Create the package object.
       //
@@ -412,55 +383,196 @@ load_repository (const shared_ptr<repository>& rp, database& db)
         db.update (p);
     }
 
-    // Create package version object.
-    //
-    dependencies dep;
-    requirements req;
-    brep::optional<path> loc; // Ambiguity with butl::optional.
-    string chn;
+    shared_ptr<package_version> pv (
+      db.find<package_version> (
+        package_version_id
+        {
+          pm.name,
+          pm.version.epoch (),
+          pm.version.canonical_upstream (),
+          pm.version.revision ()
+        }));
 
-    // Don't add dependencies, requirements and changes for external
-    // repository packages.
-    //
-    if (rp->internal)
+    if (pv == nullptr)
     {
-      dep = move (pm.dependencies);
-      req = move (pm.requirements);
-      loc = move (pm.location);
+      // Create package version object.
+      //
+      dependencies dep;
+      requirements req;
+      brep::optional<path> loc; // Ambiguity with butl::optional.
+      string chn;
 
-      for (auto& c: pm.changes)
+      // Don't add dependencies, requirements and changes for external
+      // repository packages.
+      //
+      if (rp->internal)
       {
-        if (c.file)
-        {
-          // @@ Pull change notes from the file when package manager
-          // API is ready.
-        }
-        else
+        dep = move (pm.dependencies);
+        req = move (pm.requirements);
+        loc = move (pm.location);
+
+        for (auto& c: pm.changes)
         {
-          if (chn.empty ())
-            chn = move (c);
+          if (c.file)
+          {
+            // @@ Pull change notes from the file when package manager
+            //    API is ready.
+          }
           else
-            chn += "\n" + c;
+          {
+            if (chn.empty ())
+              chn = move (c);
+            else
+              chn += "\n" + c;
+          }
         }
       }
+
+      package_version pv (lazy_shared_ptr<package> (db, pm.name),
+                          move (pm.version),
+                          pm.priority ? move (*pm.priority) : priority (),
+                          move (pm.license_alternatives),
+                          move (chn),
+                          move (dep),
+                          move (req),
+                          move (loc),
+                          rp);
+
+      db.persist (pv);
     }
+    else
+    {
+      // @@ Need to ensure that the same package versions coming from
+      //    different repositories are equal. Probably will invent hashsum at
+      //    some point for this purpose.
+      //
+
+      if (rp->internal)
+      {
+        // As soon as internal repositories get loaded first, the internal
+        // package version can duplicate an internal package version only.
+        //
+        assert (pv->internal_repository != nullptr);
 
-    package_version pv (rp,
-                        lazy_shared_ptr<package> (db, pm.name),
-                        move (pm.version),
-                        pm.priority ? move (*pm.priority) : priority (),
-                        move (pm.license_alternatives),
-                        move (chn),
-                        move (dep),
-                        move (req),
-                        move (loc));
-
-    db.persist (pv);
+        // Just skip the duplicate.
+        //
+      }
+      else
+      {
+        pv->external_repositories.push_back (rp);
+        db.update (pv);
+      }
+    }
   }
 
-  session::current (s); // Restore current session.
+  db.persist (rp); // Save the repository state.
+}
+
+// Loads the prerequsite repositories state from the 'repositories' file.
+// Updates the repository persistent state to save repositories_timestamp
+// member. Should be called once per internal repository.
+//
+static void
+load_prerequsites (const shared_ptr<repository>& rp, database& db)
+{
+  // repositories_timestamp different from timestamp_nonexistent signals the
+  // repository prerequsites are already loaded.
+  //
+  assert (rp->repositories_timestamp == timestamp_nonexistent);
+
+  // Load prerequsites for internal repositories only.
+  //
+  assert (rp->internal);
+
+  // Only locally accessible repositories allowed until package manager API is
+  // ready.
+  //
+  assert (!rp->local_path.empty ());
+
+  repository_manifests rpm;
 
-  db.update (rp); // Save the repository state.
+  {
+    ifstream ifs;
+    path p (rp->local_path / path ("repositories"));
+    rp->repositories_timestamp = manifest_stream (p, ifs);
+
+    manifest_parser mp (ifs, p.string ());
+    rpm = repository_manifests (mp);
+  }
+
+  for (auto& rm: rpm)
+  {
+    if (rm.location.empty ())
+      continue; // Ignore entry for this repository.
+
+    repository_location rl;
+
+    auto bad_location (
+      [&rp, &rm]()
+      {
+        ostringstream o;
+        o << "invalid location '" << rm.location.string ()
+          << "' of the prerequisite repository for internal "
+          "repository '" << rp->location.string () << "'";
+
+        throw runtime_error (o.str ());
+      });
+
+    try
+    {
+      // Absolute path location make no sense for the web interface.
+      //
+      if (rm.location.absolute ())
+        bad_location ();
+
+      // Convert the relative repository location to remote one, leave remote
+      // location unchanged.
+      //
+      rl = repository_location (rm.location.string (), rp->location);
+    }
+    catch (const invalid_argument&)
+    {
+      bad_location ();
+    }
+
+    shared_ptr<repository> pr (db.find<repository> (rl.canonical_name ()));
+
+    if (pr != nullptr)
+      // The prerequisite repository is already loaded.
+      //
+      continue;
+
+    pr = make_shared<repository> (move (rl));
+
+    // If the prerequsite repository location is a relative path, then
+    // calculate its absolute local path.
+    //
+    if (rm.location.relative ())
+    {
+      dir_path& lp (pr->local_path);
+      lp = rp->local_path / rm.location.path ();
+
+      try
+      {
+        lp.normalize ();
+      }
+      catch (const invalid_path&)
+      {
+        ostringstream o;
+        o << "can't normalize local path'" << lp.string ()
+          << "' of the prerequisite repository for internal "
+          "repository '" << rp->location.string () << "'";
+
+        throw runtime_error (o.str ());
+      }
+    }
+
+    load_packages (pr, db);
+  }
+
+  // Updates repositories_timestamp member.
+  //
+  db.update (rp);
 }
 
 int
@@ -547,40 +659,28 @@ main (int argc, char* argv[])
       db.erase_query<package> ();
       db.erase_query<package_version> ();
 
-      // We use repository object packages_timestamp as a flag to signal that
-      // we have already loaded this repo. The easiest way to make
-      // it work in case of cycles is to use a session. This way,
-      // the repository object on which we updated the packages_timestamp
-      // will be the same as the one we may check down the call
-      // stack.
+      // On the first pass over the internal repositories we load their
+      // packages.
       //
-      session s;
-
-      // On the first pass over the internal repositories list we
-      // persist empty repository objects, setting the interal flag
-      // to true and packages_timestamp to non-existent. The idea is to
-      // establish the "final" list of internal repositories.
-      //
-      for (auto& ir: irs)
+      for (const auto& ir: irs)
       {
         shared_ptr<repository> r (
           make_shared<repository> (ir.location,
                                    move (ir.display_name),
                                    move (ir.local_path)));
 
-        db.persist (r);
+        load_packages (r, db);
       }
 
       // On the second pass over the internal repositories we
-      // load them and all their (not yet loaded) prerequisite
-      // repositories.
+      // load their (not yet loaded) prerequisite repositories.
       //
       for (const auto& ir: irs)
       {
         shared_ptr<repository> r (
           db.load<repository> (ir.location.canonical_name ()));
 
-        load_repository (r, db);
+        load_prerequsites (r, db);
       }
     }
 
diff --git a/tests/loader/driver.cxx b/tests/loader/driver.cxx
index 501ae80..1fc2521 100644
--- a/tests/loader/driver.cxx
+++ b/tests/loader/driver.cxx
@@ -38,12 +38,12 @@ operator== (const dependency& a, const dependency& b)
 static bool
 check_location (shared_ptr<package_version>& pv)
 {
-  if (pv->repository.load ()->internal)
+  if (pv->internal_repository == nullptr)
+    return !pv->location;
+  else
     return pv->location && *pv->location ==
       path (pv->package.load ()->name + "-" + pv->version.string () +
             ".tar.gz");
-  else
-    return !pv->location;
 }
 
 int
@@ -112,25 +112,8 @@ main (int argc, char* argv[])
       assert (sr->repositories_timestamp ==
               file_mtime (dir_path (sr->local_path) / path ("repositories")));
       assert (sr->internal);
-      assert (sr->prerequisite_repositories.size () == 2);
-
-      vector<shared_ptr<repository>> pr {mr, cr};
-
-      auto i (find (pr.begin (),
-                    pr.end (),
-                    sr->prerequisite_repositories[0].load ()));
-
-      assert (i != pr.end ());
-      pr.erase (i);
 
-      assert (find (pr.begin (),
-                    pr.end (),
-                    sr->prerequisite_repositories[1].load ()) != pr.end ());
-
-      auto& srv (sr->package_versions);
-      assert (srv.size () == 5);
-
-      using lv_t = decltype (srv[0]);
+      using lv_t = lazy_weak_ptr<package_version>;
       auto vc ([](const lv_t& a, const lv_t& b){
           auto v1 (a.load ());
           auto v2 (b.load ());
@@ -143,61 +126,54 @@ main (int argc, char* argv[])
           return v1->version < v2->version;
         });
 
-      sort (srv.begin (), srv.end (), vc);
-
       version fv0 ("1.0");
       shared_ptr<package_version> fpv0 (
         db.load<package_version> (
           package_version_id {
-            "cppget.org/stable",
             "libfoo",
             fv0.epoch (),
-            fv0.canonical_upstream ()}));
-      assert (srv[0].load () == fpv0);
+            fv0.canonical_upstream (),
+            fv0.revision ()}));
       assert (check_location (fpv0));
 
       version fv1 ("1.2.2");
       shared_ptr<package_version> fpv1 (
         db.load<package_version> (
           package_version_id {
-            "cppget.org/stable",
             "libfoo",
             fv1.epoch (),
-            fv1.canonical_upstream ()}));
-      assert (srv[1].load () == fpv1);
+            fv1.canonical_upstream (),
+            fv1.revision ()}));
       assert (check_location (fpv1));
 
       version fv2 ("1.2.3-4");
       shared_ptr<package_version> fpv2 (
         db.load<package_version> (
           package_version_id {
-            "cppget.org/stable",
             "libfoo",
             fv2.epoch (),
-            fv2.canonical_upstream ()}));
-      assert (srv[2].load () == fpv2);
+            fv2.canonical_upstream (),
+            fv2.revision ()}));
       assert (check_location (fpv2));
 
       version fv3 ("1.2.4");
       shared_ptr<package_version> fpv3 (
         db.load<package_version> (
           package_version_id {
-            "cppget.org/stable",
             "libfoo",
             fv3.epoch (),
-            fv3.canonical_upstream ()}));
-      assert (srv[3].load () == fpv3);
+            fv3.canonical_upstream (),
+            fv3.revision ()}));
       assert (check_location (fpv3));
 
       version xv ("1.0.0-1");
       shared_ptr<package_version> xpv (
         db.load<package_version> (
           package_version_id {
-            "cppget.org/stable",
             "libstudxml",
             xv.epoch (),
-            xv.canonical_upstream ()}));
-      assert (srv[4].load () == xpv);
+            xv.canonical_upstream (),
+            xv.revision ()}));
       assert (check_location (xpv));
 
       // Verify libstudxml package.
@@ -223,7 +199,8 @@ main (int argc, char* argv[])
 
       // Verify libstudxml package version.
       //
-      assert (xpv->repository.load () == sr);
+      assert (xpv->internal_repository.load () == sr);
+      assert (xpv->external_repositories.empty ());
       assert (xpv->package.load () == px);
       assert (xpv->version == version ("1.0.0-1"));
       assert (xpv->priority == priority::low);
@@ -270,13 +247,12 @@ main (int argc, char* argv[])
       assert (fpv[1].load () == fpv1);
       assert (fpv[2].load () == fpv2);
       assert (fpv[3].load () == fpv3);
-      // Asserting fpv[3].load () == fpv4 goes later when fpv4, belonging to
-      // another repository, get introduced.
-      //
 
       // Verify libfoo package versions.
       //
-      assert (fpv0->repository.load () == sr);
+      assert (fpv0->internal_repository.load () == sr);
+      assert (fpv0->external_repositories.size () == 1);
+      assert (fpv0->external_repositories[0].load () == cr);
       assert (fpv0->package.load () == pf);
       assert (fpv0->version == version ("1.0"));
       assert (fpv0->priority == priority::low);
@@ -289,7 +265,8 @@ main (int argc, char* argv[])
       assert (fpv0->dependencies.empty ());
       assert (fpv0->requirements.empty ());
 
-      assert (fpv1->repository.load () == sr);
+      assert (fpv1->internal_repository.load () == sr);
+      assert (fpv1->external_repositories.empty ());
       assert (fpv1->package.load () == pf);
       assert (fpv1->version == version ("1.2.2"));
       assert (fpv1->priority == priority::low);
@@ -335,7 +312,8 @@ main (int argc, char* argv[])
       assert (fpvr1[3].comment ==
               "libc++ standard library if using Clang on Mac OS X.");
 
-      assert (fpv2->repository.load () == sr);
+      assert (fpv2->internal_repository.load () == sr);
+      assert (fpv2->external_repositories.empty ());
       assert (fpv2->package.load () == pf);
       assert (fpv2->version == version ("1.2.3-4"));
       assert (fpv2->priority == priority::low);
@@ -353,7 +331,8 @@ main (int argc, char* argv[])
                  brep::optional<version_comparison> (
                    version_comparison{version ("2.0.0"), comparison::ge})}));
 
-      assert (fpv3->repository.load () == sr);
+      assert (fpv3->internal_repository.load () == sr);
+      assert (fpv3->external_repositories.empty ());
       assert (fpv3->package.load () == pf);
       assert (fpv3->version == version ("1.2.4"));
       assert (fpv3->priority == priority::low);
@@ -386,39 +365,32 @@ main (int argc, char* argv[])
       assert (mr->repositories_timestamp ==
               file_mtime (dir_path (mr->local_path) / path ("repositories")));
       assert (mr->internal);
-      assert (mr->prerequisite_repositories.size () == 1);
-      assert (mr->prerequisite_repositories[0].load () == cr);
-
-      auto& mrv (mr->package_versions);
-      assert (mrv.size () == 2);
-      sort (mrv.begin (), mrv.end (), vc);
 
       version ev ("1+1.2");
       shared_ptr<package_version> epv (
         db.load<package_version> (
           package_version_id {
-            "cppget.org/math",
             "libexp",
             ev.epoch (),
-            ev.canonical_upstream ()}));
-      assert (mrv[0].load () == epv);
+            ev.canonical_upstream (),
+            ev.revision ()}));
       assert (check_location (epv));
 
       version fv4 ("1.2.4-1");
       shared_ptr<package_version> fpv4 (
         db.load<package_version> (
           package_version_id {
-            "cppget.org/math",
             "libfoo",
             fv4.epoch (),
-            fv4.canonical_upstream ()}));
-      assert (mrv[1].load () == fpv4);
+            fv4.canonical_upstream (),
+            fv4.revision ()}));
       assert (fpv[4].load () == fpv4);
       assert (check_location (fpv4));
 
       // Verify libfoo package versions.
       //
-      assert (fpv4->repository.load () == mr);
+      assert (fpv4->internal_repository.load () == mr);
+      assert (fpv4->external_repositories.empty ());
       assert (fpv4->package.load () == pf);
       assert (fpv4->version == version ("1.2.4-1"));
       assert (fpv4->priority == priority::high);
@@ -464,7 +436,8 @@ main (int argc, char* argv[])
 
       // Verify libexp package version.
       //
-      assert (epv->repository.load () == mr);
+      assert (epv->internal_repository.load () == mr);
+      assert (epv->external_repositories.empty ());
       assert (epv->package.load () == pe);
       assert (epv->version == version ("1+1.2"));
       assert (epv->priority == priority (priority::low));
@@ -495,20 +468,15 @@ main (int argc, char* argv[])
               file_mtime (dir_path (cr->local_path) / path ("packages")));
       assert (cr->repositories_timestamp == timestamp_nonexistent);
       assert (!cr->internal);
-      assert (cr->prerequisite_repositories.empty ());
-
-      auto& crv (cr->package_versions);
-      assert (crv.size () == 1);
 
       version bv ("2.3.5");
       shared_ptr<package_version> bpv (
         db.load<package_version> (
           package_version_id {
-            "cppget.org/misc",
             "libbar",
             bv.epoch (),
-            bv.canonical_upstream ()}));
-      assert (crv[0].load () == bpv);
+            bv.canonical_upstream (),
+            bv.revision ()}));
       assert (check_location (bpv));
 
       // Verify libbar package.
@@ -529,7 +497,9 @@ main (int argc, char* argv[])
 
       // Verify libbar package version.
       //
-      assert (bpv->repository.load () == cr);
+      assert (bpv->internal_repository == nullptr);
+      assert (bpv->external_repositories.size () == 1);
+      assert (bpv->external_repositories[0].load () == cr);
       assert (bpv->package.load () == pb);
       assert (bpv->version == version ("2.3.5"));
 
diff --git a/tests/loader/external/1/misc/packages b/tests/loader/external/1/misc/packages
index 718b30e..d6d34ea 100644
--- a/tests/loader/external/1/misc/packages
+++ b/tests/loader/external/1/misc/packages
@@ -13,3 +13,11 @@ depends: libmath >= 2.0.0
 requires: linux | windows | macosx
 changes: some changes
 location: libbar-2.3.5.tar.gz
+:
+name: libfoo
+version: 1.0
+summary: Foo Library
+license: MIT
+url: http://www.example.com/foo/
+email: foo-users@example.com
+location: libfoo-1.0.tar.gz
diff --git a/tests/loader/internal/1/math/packages b/tests/loader/internal/1/math/packages
index cbd8165..9ce4063 100644
--- a/tests/loader/internal/1/math/packages
+++ b/tests/loader/internal/1/math/packages
@@ -25,3 +25,11 @@ package-url: http://www.example.com/foo/pack
 package-email: pack@example.com
 depends: libmisc >= 2.3.0
 location: libfoo-1.2.4-1.tar.gz
+:
+name: libfoo
+version: 1.0
+summary: The Foo Lib
+license: MIT
+url: http://www.example.com/foo/
+email: foo-users@example.com
+location: libfoo-1.0.tar.gz