Add support for loading package version reviews

7 files changed, 562 insertions, 20 deletions

diff --git a/load/.gitignore b/load/.gitignore
index 035e847..314cf8f 100644
--- a/load/.gitignore
+++ b/load/.gitignore
@@ -1,2 +1,3 @@
 *-options.?xx
 brep-load
+brep-load-with-metadata
diff --git a/load/buildfile b/load/buildfile
index 4278f20..51b374d 100644
--- a/load/buildfile
+++ b/load/buildfile
@@ -6,11 +6,17 @@ import libs += libodb-pgsql%lib{odb-pgsql}
 import libs += libbutl%lib{butl}
 import libs += libbpkg%lib{bpkg}
 
+import mods = bpkg-util%bash{utility}
+
 include ../libbrep/
 
+./: exe{brep-load} exe{brep-load-with-metadata}
+
 exe{brep-load}: {hxx ixx cxx}{* -load-options} {hxx ixx cxx}{load-options} \
                 ../libbrep/lib{brep} $libs
 
+exe{brep-load-with-metadata}: in{load-with-metadata} $mods
+
 # Build options.
 #
 obj{load}: cxx.poptions += -DBREP_COPYRIGHT=\"$copyright\"
diff --git a/load/load-with-metadata.in b/load/load-with-metadata.in
new file mode 100644
index 0000000..a99709e
--- /dev/null
+++ b/load/load-with-metadata.in
@@ -0,0 +1,130 @@
+#!/usr/bin/env bash
+
+# file      : load/load-with-metadata.in
+# license   : MIT; see accompanying LICENSE file
+
+# The wrapper around brep-load, which pulls the package metadata from a git
+# repository and runs brep-load, passing the metadata directory to it.
+#
+# Specifically, pull a pre-cloned (read-only) git repository with the contents
+# of an archive-based bpkg repository. Run brep-load with the `--metadata
+# <dir>/owners` option, the --metadata-changed option, if the current snapshot
+# of the repository has not yet been processed by brep-load, and forward any
+# further arguments to brep-load.
+#
+# --timeout <seconds>
+#
+#   Git operation timeout. Specifically, the operation will be aborted if
+#   there is no network activity for the specified time. Default is 60
+#   seconds. Note that currently the git timeout is only supported for the
+#   http(s) transport.
+#
+# --brep-load <path>
+#
+#   The brep-load program to be used. This should be the path to the brep-load
+#   executable.
+#
+# Note also that this script maintains the <dir>.load file which contains the
+# last successfully processed commit.
+#
+usage="usage: $0 [<options>] <dir> [<brep-load-args>]"
+
+owd="$(pwd)"
+trap "{ cd '$owd'; exit 1; }" ERR
+set -o errtrace   # Trap in functions and subshells.
+set -o pipefail   # Fail if any pipeline command fails.
+shopt -s lastpipe # Execute last pipeline command in the current shell.
+shopt -s nullglob # Expand no-match globs to nothing rather than themselves.
+
+@import bpkg-util/utility@ # check_git_connectivity()
+
+# The script's own options.
+#
+timeout=60
+brep_load=
+
+while [[ "$#" -gt 0 ]]; do
+  case "$1" in
+    --timeout)
+      shift
+      timeout="$1"
+      shift || true
+      ;;
+    --brep-load)
+      shift
+      brep_load="$1"
+      shift || true
+      ;;
+    *)
+      break
+      ;;
+  esac
+done
+
+# The repository directory.
+#
+repo_dir="${1%/}"
+
+# Validate options and arguments.
+#
+if [[ -z "$repo_dir" ]]; then
+  error "$usage"
+fi
+
+if [[ ! -d "$repo_dir" ]]; then
+  error "'$repo_dir' does not exist or is not a directory"
+fi
+
+shift # repo_dir
+
+# If brep-load path is not specified, then use the brep-load program from the
+# script directory, if present. Otherwise, use the 'brep-load' path.
+#
+if [[ -z "$brep_load" ]]; then
+  brep_load="$(dirname "$(realpath "${BASH_SOURCE[0]}")")/brep-load"
+
+  if [[ ! -x "$brep_load" ]]; then
+    brep_load=brep-load
+  fi
+fi
+
+# Make sure the commit file is present.
+#
+load_commit="$repo_dir.load"
+touch "$load_commit"
+
+# Pull the repository.
+#
+if ! remote_url="$(git -C "$repo_dir" config --get remote.origin.url)"; then
+  error "'$repo_dir' is not a git repository"
+fi
+
+# Git doesn't support the connection timeout option. The options we use are
+# just an approximation of the former, that, in particular, don't cover the
+# connection establishing. To work around this problem, before running a git
+# command that assumes the remote repository communication we manually check
+# connectivity with the remote repository.
+#
+check_git_connectivity "$remote_url" "$timeout"
+
+# Fail if no network activity happens during the time specified.
+#
+git -c http.lowSpeedLimit=1 -c "http.lowSpeedTime=$timeout" \
+  -C "$repo_dir" pull -q
+
+# Match the HEAD commit id to the one stored in the file. If it matches, then
+# nothing changed in the repository since it has been processed by brep-load
+# last time and we should not pass the --metadata-changed option to brep-load.
+#
+commit="$(git -C "$repo_dir" rev-parse HEAD)"
+pc="$(cat "$load_commit")"
+
+loader_options=(--metadata "$repo_dir/owners")
+
+if [[ "$commit" != "$pc" ]]; then
+  loader_options+=(--metadata-changed)
+fi
+
+"$brep_load" "${loader_options[@]}" "$@"
+
+echo "$commit" >"$load_commit"
diff --git a/load/load.cli b/load/load.cli
index bda186a..fbdfbd8 100644
--- a/load/load.cli
+++ b/load/load.cli
@@ -126,6 +126,29 @@ class options
      \cb{--service-id} option to be specified."
   };
 
+  brep::dir_path --metadata
+  {
+    "<dir>",
+    "Directory where the package metadata manifest files are located. If
+     specified, then (re-)load the metadata if the package information is also
+     (re-)loaded or update it if the \cb{--metadata-changed} option is
+     specified.
+
+     The subdirectory hierarchy under this directory is expected to be in the
+     following form:
+
+     \
+     <project>/<package>/<version>/
+     \
+     "
+  }
+
+  bool --metadata-changed
+  {
+    "Update the package metadata even if the package information is not
+     reloaded."
+  };
+
   brep::path --overrides-file
   {
     "<file>",
@@ -186,7 +209,7 @@ class options
      this option to specify multiple package manager options."
   }
 
-  brep::path openssl = "openssl"
+  brep::path --openssl = "openssl"
   {
     "<path>",
     "The openssl program to be used for crypto operations. You can also
@@ -195,7 +218,7 @@ class options
      specified, then \cb{brep-load} will use \cb{openssl} by default."
   }
 
-  brep::strings openssl-option
+  brep::strings --openssl-option
   {
     "<opt>",
     "Additional option to be passed to the openssl program (see \cb{openssl}
diff --git a/load/load.cxx b/load/load.cxx
index 2b2cd56..f79b606 100644
--- a/load/load.cxx
+++ b/load/load.cxx
@@ -3,6 +3,7 @@
 
 #include <signal.h> // signal()
 
+#include <map>
 #include <cerrno>
 #include <chrono>
 #include <thread>   // this_thread::sleep_for()
@@ -31,6 +32,7 @@
 #include <libbrep/package.hxx>
 #include <libbrep/package-odb.hxx>
 #include <libbrep/database-lock.hxx>
+#include <libbrep/review-manifest.hxx>
 
 #include <load/load-options.hxx>
 #include <load/options-types.hxx>
@@ -56,6 +58,7 @@ static const char* help_info (
 
 static const path packages     ("packages.manifest");
 static const path repositories ("repositories.manifest");
+static const path reviews      ("reviews.manifest");
 
 // Retry executing bpkg on recoverable errors for about 10 seconds.
 //
@@ -362,6 +365,56 @@ repository_info (const options& lo, const string& rl, const cstrings& options)
   }
 }
 
+// Map of package versions to their metadata information in the form it is
+// stored in the database (reviews summary, etc).
+//
+// This map is filled by recursively traversing the metadata directory and
+// parsing the encountered metadata manifest files (reviews.manifest, etc; see
+// --metadata option for background on metadata). Afterwards, this map is used
+// as a data source for the being persisted/updated package objects.
+//
+struct package_version_key
+{
+  package_name  name;
+  brep::version version;
+
+  package_version_key (package_name n, brep::version v)
+      : name (move (n)), version (move (v)) {}
+
+  bool
+  operator< (const package_version_key& k) const
+  {
+    if (int r = name.compare (k.name))
+      return r < 0;
+
+    return version < k.version;
+  }
+};
+
+class package_version_metadata
+{
+public:
+  // Extracted from the package metadata directory. Must match the respective
+  // package manifest information.
+  //
+  package_name project;
+
+  optional<reviews_summary> reviews;
+
+  // The directory the metadata manifest files are located. It has the
+  // <project>/<package>/<version> form and is only used for diagnostics.
+  //
+  dir_path
+  directory () const
+  {
+    assert (reviews); // At least one kind of metadata must be present.
+    return reviews->manifest_file.directory ();
+  }
+};
+
+using package_version_metadata_map = std::map<package_version_key,
+                                              package_version_metadata>;
+
 // Load the repository packages from the packages.manifest file and persist
 // the repository. Should be called once per repository.
 //
@@ -372,7 +425,8 @@ load_packages (const options& lo,
                database& db,
                bool ignore_unknown,
                const manifest_name_values& overrides,
-               const string& overrides_name)
+               const string& overrides_name,
+               optional<package_version_metadata_map>& metadata)
 {
   // packages_timestamp other than timestamp_nonexistent signals the
   // repository packages are already loaded.
@@ -728,6 +782,31 @@ load_packages (const options& lo,
               keys_to_objects (move (pm.build_configs[i].bot_keys));
         }
 
+        optional<reviews_summary> rvs;
+
+        if (metadata)
+        {
+          auto i (metadata->find (package_version_key {pm.name, pm.version}));
+
+          if (i != metadata->end ())
+          {
+            package_version_metadata& md (i->second);
+
+            if (md.project != project)
+            {
+              cerr << "error: project '" << project << "' of package "
+                   << pm.name << ' ' << pm.version << " doesn't match "
+                   << "metadata directory path "
+                   << lo.metadata () / md.directory ();
+
+              throw failed ();
+            }
+
+            if (md.reviews)
+              rvs = move (md.reviews);
+          }
+        }
+
         p = make_shared<package> (
           move (pm.name),
           move (pm.version),
@@ -758,6 +837,7 @@ load_packages (const options& lo,
           move (pm.build_auxiliaries),
           move (bot_keys),
           move (build_configs),
+          move (rvs),
           move (pm.location),
           move (pm.fragment),
           move (pm.sha256sum),
@@ -1153,13 +1233,16 @@ load_repositories (const options& lo,
 
     // We don't apply overrides to the external packages.
     //
+    optional<package_version_metadata_map> metadata;
+
     load_packages (lo,
                    pr,
                    !pr->cache_location.empty () ? pr->cache_location : cl,
                    db,
                    ignore_unknown,
                    manifest_name_values () /* overrides */,
-                   "" /* overrides_name */);
+                   "" /* overrides_name */,
+                   metadata);
 
     load_repositories (lo,
                        pr,
@@ -1778,6 +1861,11 @@ try
     }
   }
 
+  // Note: the interactive tenant implies private.
+  //
+  if (ops.interactive_specified ())
+    ops.private_ (true);
+
   // Parse and validate overrides, if specified.
   //
   // Note that here we make sure that the overrides manifest is valid.
@@ -1818,34 +1906,236 @@ try
     ops.db_port (),
     "options='-c default_transaction_isolation=serializable'");
 
+  // Load the description of all the internal repositories from the
+  // configuration file.
+  //
+  internal_repositories irs (load_repositories (path (argv[1])));
+
   // Prevent several brep utility instances from updating the package database
   // simultaneously.
   //
   database_lock l (db);
 
-  transaction t (db.begin ());
-
-  // Check that the package database schema matches the current one.
+  // Check that the package database schema matches the current one and if the
+  // package information needs to be (re-)loaded.
   //
-  const string ds ("package");
-  if (schema_catalog::current_version (db, ds) != db.schema_version (ds))
+  bool load_pkgs;
   {
-    cerr << "error: package database schema differs from the current one"
-         << endl << "  info: use brep-migrate to migrate the database" << endl;
-    throw failed ();
+    transaction t (db.begin ());
+
+    // Check the database schema match.
+    //
+    const string ds ("package");
+
+    if (schema_catalog::current_version (db, ds) != db.schema_version (ds))
+    {
+      cerr << "error: package database schema differs from the current one"
+           << endl << "  info: use brep-migrate to migrate the database" << endl;
+      throw failed ();
+    }
+
+    load_pkgs = (ops.force () || changed (tnt, irs, db));
+
+    t.commit ();
   }
 
-  // Note: the interactive tenant implies private.
+  // Check if the package versions metadata needs to be (re-)loaded and, if
+  // that's the case, stash it in the memory.
   //
-  if (ops.interactive_specified ())
-    ops.private_ (true);
+  optional<package_version_metadata_map> metadata;
+  if (ops.metadata_specified () && (load_pkgs || ops.metadata_changed ()))
+  {
+    metadata = package_version_metadata_map ();
 
-  // Load the description of all the internal repositories from the
-  // configuration file.
+    const dir_path& d (ops.metadata ());
+
+    // The first level are package projects.
+    //
+    try
+    {
+      for (const dir_entry& e: dir_iterator (d, dir_iterator::no_follow))
+      {
+        const string& n (e.path ().string ());
+
+        if (e.type () != entry_type::directory || n[0] == '.')
+          continue;
+
+        package_name project;
+
+        try
+        {
+          project = package_name (n);
+        }
+        catch (const invalid_argument& e)
+        {
+          cerr << "error: name of subdirectory '" << n << "' in " << d
+               << " is not a project name: " << e << endl;
+          throw failed ();
+        }
+
+        // The second level are package names.
+        //
+        dir_path pd (d / path_cast<dir_path> (e.path ()));
+
+        try
+        {
+          for (const dir_entry& e: dir_iterator (pd, dir_iterator::no_follow))
+          {
+            const string& n (e.path ().string ());
+
+            if (e.type () != entry_type::directory || n[0] == '.')
+              continue;
+
+            package_name name;
+
+            try
+            {
+              name = package_name (n);
+            }
+            catch (const invalid_argument& e)
+            {
+              cerr << "error: name of subdirectory '" << n << "' in " << pd
+                   << " is not a package name: " << e << endl;
+              throw failed ();
+            }
+
+            // The third level are package versions.
+            //
+            dir_path vd (pd / path_cast<dir_path> (e.path ()));
+
+            try
+            {
+              for (const dir_entry& e: dir_iterator (vd,
+                                                     dir_iterator::no_follow))
+              {
+                const string& n (e.path ().string ());
+
+                if (e.type () != entry_type::directory || n[0] == '.')
+                  continue;
+
+                version ver;
+
+                try
+                {
+                  ver = version (n);
+                }
+                catch (const invalid_argument& e)
+                {
+                  cerr << "error: name of subdirectory '" << n << "' in " << vd
+                       << " is not a package version: " << e << endl;
+                  throw failed ();
+                }
+
+                dir_path md (vd / path_cast<dir_path> (e.path ()));
+
+                // Parse the reviews.manifest file, if present.
+                //
+                // Note that semantically, the absent manifest file and the
+                // empty manifest list are equivalent and result in an absent
+                // reviews summary.
+                //
+                optional<reviews_summary> rs;
+                {
+                  path rf (md / reviews);
+
+                  try
+                  {
+                    if (file_exists (rf))
+                    {
+                      ifdstream ifs (rf);
+                      manifest_parser mp (ifs, rf.string ());
+
+                      // Count the passed and failed reviews.
+                      //
+                      size_t ps (0);
+                      size_t fl (0);
+
+                      for (review_manifest& m:
+                             review_manifests (mp, ops.ignore_unknown ()))
+                      {
+                        bool fail (false);
+
+                        for (const review_aspect& r: m.results)
+                        {
+                          switch (r.result)
+                          {
+                          case review_result::fail: fail = true; break;
+
+                          case review_result::unchanged:
+                            {
+                              cerr << "error: unsupported review result "
+                                   << "'unchanged' in " << rf << endl;
+                              throw failed ();
+                            }
+
+                          case review_result::pass: break; // Noop
+                          }
+                        }
+
+                        ++(fail ? fl : ps);
+                      }
+
+                      if (ps + fl != 0)
+                        rs = reviews_summary {ps, fl, rf.relative (d)};
+                    }
+                  }
+                  catch (const manifest_parsing& e)
+                  {
+                    cerr << "error: unable to parse reviews: " << e << endl;
+                    throw failed ();
+                  }
+                  catch (const io_error& e)
+                  {
+                    cerr << "error: unable to read " << rf << ": " << e << endl;
+                    throw failed ();
+                  }
+                  catch (const system_error& e)
+                  {
+                    cerr << "error: unable to stat " << rf << ": " << e << endl;
+                    throw failed ();
+                  }
+                }
+
+                // Add the package version metadata to the map if any kind of
+                // metadata is present.
+                //
+                if (rs)
+                {
+                  (*metadata)[package_version_key {name, move (ver)}] =
+                    package_version_metadata {project, move (rs)};
+                }
+              }
+            }
+            catch (const system_error& e)
+            {
+              cerr << "error: unable to iterate over " << vd << ": " << e
+                   << endl;
+              throw failed ();
+            }
+          }
+        }
+        catch (const system_error& e)
+        {
+          cerr << "error: unable to iterate over " << pd << ": " << e << endl;
+          throw failed ();
+        }
+      }
+    }
+    catch (const system_error& e)
+    {
+      cerr << "error: unable to iterate over " << d << ": " << e << endl;
+      throw failed ();
+    }
+  }
+
+  // Bail out if no package information nor metadata needs to be loaded.
   //
-  internal_repositories irs (load_repositories (path (argv[1])));
+  if (!load_pkgs && !metadata)
+    return 0;
+
+  transaction t (db.begin ());
 
-  if (ops.force () || changed (tnt, irs, db))
+  if (load_pkgs)
   {
     shared_ptr<tenant> t; // Not NULL in the --existing-tenant mode.
 
@@ -2015,7 +2305,8 @@ try
                      db,
                      ops.ignore_unknown (),
                      overrides,
-                     ops.overrides_file ().string ());
+                     ops.overrides_file ().string (),
+                     metadata);
     }
 
     // On the second pass over the internal repositories we load their
@@ -2070,6 +2361,83 @@ try
       }
     }
   }
+  else if (metadata)
+  {
+    // Iterate over the packages which contain metadata and apply the changes,
+    // if present. Erase the metadata map entries which introduce such
+    // changes, so at the end only the newly added metadata is left in the
+    // map.
+    //
+    using query = query<package>;
+
+    for (package& p: db.query<package> (query::reviews.pass.is_not_null ()))
+    {
+      bool u (false);
+      auto i (metadata->find (package_version_key {p.name, p.version}));
+
+      if (i == metadata->end ())
+      {
+        // Mark the section as loaded, so the reviews summary is updated.
+        //
+        p.reviews_section.load ();
+        p.reviews = nullopt;
+        u = true;
+      }
+      else
+      {
+        package_version_metadata& md (i->second);
+
+        if (md.project != p.project)
+        {
+          cerr << "error: project '" << p.project << "' of package "
+               << p.name << ' ' << p.version << " doesn't match metadata "
+               << "directory path " << ops.metadata () / md.directory ();
+
+          throw failed ();
+        }
+
+        db.load (p, p.reviews_section);
+
+        if (p.reviews != md.reviews)
+        {
+          p.reviews = move (md.reviews);
+          u = true;
+        }
+
+        metadata->erase (i);
+      }
+
+      if (u)
+        db.update (p);
+    }
+
+    // Add the newly added metadata to the packages.
+    //
+    for (auto& m: *metadata)
+    {
+      if (shared_ptr<package> p =
+          db.find<package> (package_id (tnt, m.first.name, m.first.version)))
+      {
+        package_version_metadata& md (m.second);
+
+        if (m.second.project != p->project)
+        {
+          cerr << "error: project '" << p->project << "' of package "
+               << p->name << ' ' << p->version << " doesn't match metadata "
+               << "directory path " << ops.metadata () / md.directory ();
+
+          throw failed ();
+        }
+
+        // Mark the section as loaded, so the reviews summary is updated.
+        //
+        p->reviews_section.load ();
+        p->reviews = move (md.reviews);
+
+        db.update (p);
+      }
+    }
+  }
 
   t.commit ();
   return 0;
diff --git a/load/types-parsers.cxx b/load/types-parsers.cxx
index a18330d..4f031df 100644
--- a/load/types-parsers.cxx
+++ b/load/types-parsers.cxx
@@ -40,6 +40,13 @@ namespace cli
     parse_path (x, s);
   }
 
+  void parser<dir_path>::
+  parse (dir_path& x, bool& xs, scanner& s)
+  {
+    xs = true;
+    parse_path (x, s);
+  }
+
   void parser<ignore_unresolved_conditional_dependencies>::
   parse (ignore_unresolved_conditional_dependencies& x, bool& xs, scanner& s)
   {
diff --git a/load/types-parsers.hxx b/load/types-parsers.hxx
index fcf5113..b79cca4 100644
--- a/load/types-parsers.hxx
+++ b/load/types-parsers.hxx
@@ -26,6 +26,13 @@ namespace cli
   };
 
   template <>
+  struct parser<brep::dir_path>
+  {
+    static void
+    parse (brep::dir_path&, bool&, scanner&);
+  };
+
+  template <>
   struct parser<brep::ignore_unresolved_conditional_dependencies>
   {
     static void