Add support for overrides parameter in CI request handler

10 files changed, 198 insertions, 39 deletions

diff --git a/brep/handler/ci/ci-load.in b/brep/handler/ci/ci-load.in
index 0a16e8c..915f9a6 100644
--- a/brep/handler/ci/ci-load.in
+++ b/brep/handler/ci/ci-load.in
@@ -288,11 +288,18 @@ manifest_serializer_finish
 loadtab="$data_dir/loadtab"
 run echo "$repository $display_name cache:cache" >"$loadtab"
 
+# Apply overrides, if uploaded.
+#
+if [ -f "$data_dir/overrides.manifest" ]; then
+  loader_options+=(--overrides-file "$data_dir/overrides.manifest")
+fi
+
 # Load the requested repository packages into the brep package database for
 # the tenant identified by the reference.
 #
-run "$loader" "${loader_options[@]}" --force --shallow --tenant "$reference" \
-"$loadtab"
+loader_options+=(--force --shallow --tenant "$reference")
+
+run "$loader" "${loader_options[@]}" "$loadtab"
 
 # Remove the no longer needed CI request data directory.
 #
diff --git a/doc/cli.sh b/doc/cli.sh
index 4f0547f..e32ee02 100755
--- a/doc/cli.sh
+++ b/doc/cli.sh
@@ -39,12 +39,14 @@ function compile ()
   cli -I .. -v project="brep" -v version="$version" -v date="$date" \
 --include-base-last "${o[@]}" --generate-html --html-prologue-file \
 man-prologue.xhtml --html-epilogue-file man-epilogue.xhtml --html-suffix .xhtml \
+--link-regex '%bpkg([-.].+)%../../bpkg/doc/bpkg$1%' \
 --link-regex '%brep(#.+)?%build2-repository-interface-manual.xhtml$1%' \
 ../$n.cli
 
   cli -I .. -v project="brep" -v version="$version" -v date="$date" \
 --include-base-last "${o[@]}" --generate-man --man-prologue-file \
 man-prologue.1 --man-epilogue-file man-epilogue.1 --man-suffix .1 \
+--link-regex '%bpkg(#.+)?%$1%' \
 --link-regex '%brep(#.+)?%$1%' \
 ../$n.cli
 }
@@ -71,6 +73,7 @@ cli -I .. \
 --html-epilogue-file doc-epilogue.xhtml \
 --link-regex '%b([-.].+)%../../build2/doc/b$1%' \
 --link-regex '%bpkg([-.].+)%../../bpkg/doc/bpkg$1%' \
+--link-regex '%bpkg(#.+)?%../../bpkg/doc/build2-package-manager-manual.xhtml$1%' \
 --output-prefix build2-repository-interface- manual.cli
 
 html2ps -f doc.html2ps:a4.html2ps -o build2-repository-interface-manual-a4.ps build2-repository-interface-manual.xhtml
diff --git a/doc/manual.cli b/doc/manual.cli
index 56ae05e..61ef1f8 100644
--- a/doc/manual.cli
+++ b/doc/manual.cli
@@ -221,6 +221,11 @@ case all the versions of this package present in the repository will be
 tested, or both the name and version in the \c{<name>/<version>} form (for
 example, \c{libhello/1.2.3}.|
 
+\li|Verify the optional \c{overrides} parameter.
+
+The overrides parameter, if specified, must be the CI overrides manifest
+upload.|
+
 \li|Verify other parameters are valid manifest name/value pairs.
 
 The value can only contain printable ASCII characters as well as tab
@@ -236,6 +241,10 @@ is created in the \c{ci-data} directory with this id as its name.|
 The CI request manifest is saved as \c{request.manifest} into the request
 subdirectory created on the previous step.|
 
+\li|Save the CI overrides manifest into the request directory.
+
+If the CI overrides manifest is uploaded, then it is saved as
+\c{overrides.manifest} into the request subdirectory.|
 
 \li|Invoke the CI handler program.
 
@@ -283,7 +292,8 @@ reference: <request-id>
 \li|Send the CI request email.
 
 If \c{ci-email} is configured, send an email to this address containing the CI
-request manifest and the CI result manifest.|
+request manifest, the potentially empty CI overrides manifest, and the CI
+result manifest.|
 
 \li|Respond to the client.
 
@@ -329,6 +339,21 @@ is in the ISO-8601 \c{<YYYY>-<MM>-<DD>T<hh>:<mm>:<ss>Z} form (always
 UTC). Note also that \c{client-ip} can be IPv4 or IPv6.
 
 
+\h#ci-overrides-manifest|CI Overrides Manifest|
+
+The CI overrides manifest is a package manifest fragment that should be
+applied to all the packages being tested. The contained values override the
+whole value groups they belong to, resetting all the group values prior to
+being applied. Currently, only the following value groups can be overridden:
+
+\
+build-email build-{warning,error}-email
+builds build-{include,exclude}
+\
+
+See \l{bpkg#manifest-package Package Manifest} for details on these values.
+
+
 \h#ci-result-manifest|CI Result Manifest|
 
 The CI result manifest starts with the below values and in that order
diff --git a/load/load.cli b/load/load.cli
index 6dc6e48..17aba5b 100644
--- a/load/load.cli
+++ b/load/load.cli
@@ -59,6 +59,13 @@ class options
      specified, then the single-tenant mode is assumed."
   };
 
+  brep::path --overrides-file
+  {
+    "<file>",
+    "Read package manifest overrides from the specified manifest fragment
+     file and apply them to packages being loaded."
+  }
+
   std::string --db-user|-u
   {
     "<user>",
diff --git a/load/load.cxx b/load/load.cxx
index 8e640ad..d00a1af 100644
--- a/load/load.cxx
+++ b/load/load.cxx
@@ -42,6 +42,8 @@ using namespace butl;
 using namespace bpkg;
 using namespace brep;
 
+using manifest_name_values = vector<manifest_name_value>;
+
 // Operation failed, diagnostics has already been issued.
 //
 struct failed {};
@@ -339,7 +341,9 @@ repository_info (const options& lo, const string& rl, const cstrings& options)
 // the repository. Should be called once per repository.
 //
 static void
-load_packages (const shared_ptr<repository>& rp, database& db)
+load_packages (const shared_ptr<repository>& rp,
+               database& db,
+               const manifest_name_values& overrides)
 {
   // packages_timestamp other than timestamp_nonexistent signals the
   // repository packages are already loaded.
@@ -391,7 +395,7 @@ load_packages (const shared_ptr<repository>& rp, database& db)
     throw failed ();
   }
 
-  for (auto& pm: pms)
+  for (package_manifest& pm: pms)
   {
     shared_ptr<package> p (
       db.find<package> (package_id (rp->tenant, pm.name, pm.version)));
@@ -405,6 +409,17 @@ load_packages (const shared_ptr<repository>& rp, database& db)
     {
       if (rp->internal)
       {
+        try
+        {
+          pm.override (overrides, "" /* name */);
+        }
+        catch (const manifest_parsing&)
+        {
+          // Overrides are already validated (see below).
+          //
+          assert (false);
+        }
+
         // Create internal package object.
         //
         optional<string> dsc;
@@ -727,7 +742,9 @@ load_repositories (const shared_ptr<repository>& rp,
       }
     }
 
-    load_packages (pr, db);
+    // We don't apply overrides to the external packages.
+    //
+    load_packages (pr, db, manifest_name_values () /* overrides */);
     load_repositories (pr, db, false /* shallow */);
   }
 
@@ -1054,7 +1071,7 @@ try
   {
     cerr << "error: unable to ignore broken pipe (SIGPIPE) signal: "
          << system_error (errno, generic_category ()) << endl; // Sanitize.
-    return 1;
+    throw failed ();
   }
 
   cli::argv_scanner scan (argc, argv, true);
@@ -1095,14 +1112,14 @@ try
   {
     cerr << "error: configuration file expected" << endl
          << help_info << endl;
-    return 1;
+    throw failed ();
   }
 
   if (argc > 2)
   {
     cerr << "error: unexpected argument encountered" << endl
          << help_info << endl;
-    return 1;
+    throw failed ();
   }
 
   // By default the tenant is empty and assumes a single-tenant mode. Let's
@@ -1114,7 +1131,35 @@ try
   {
     cerr << "error: empty tenant" << endl
          << help_info << endl;
-    return 1;
+    throw failed ();
+  }
+
+  // Parse and validate overrides, if specified.
+  //
+  manifest_name_values overrides;
+
+  if (ops.overrides_file_specified ())
+  try
+  {
+    const string& name (ops.overrides_file ().string ());
+
+    ifdstream is (ops.overrides_file ());
+    manifest_parser mp (is, name);
+    overrides = parse_manifest (mp);
+    is.close ();
+
+    package_manifest::validate_overrides (overrides, name);
+  }
+  catch (const manifest_parsing& e)
+  {
+    cerr << "error: unable to parse overrides: " << e << endl;
+    throw failed ();
+  }
+  catch (const io_error& e)
+  {
+    cerr << "error: unable to read '" << ops.overrides_file () << "': " << e
+         << endl;
+    throw failed ();
   }
 
   odb::pgsql::database db (
@@ -1139,7 +1184,7 @@ try
   {
     cerr << "error: package database schema differs from the current one"
          << endl << "  info: use brep-migrate to migrate the database" << endl;
-    return 1;
+    throw failed ();
   }
 
   // Load the description of all the internal repositories from the
@@ -1203,7 +1248,7 @@ try
                                  move (cert),
                                  priority++));
 
-      load_packages (r, db);
+      load_packages (r, db, overrides);
     }
 
     // On the second pass over the internal repositories we load their
diff --git a/mod/external-handler.cxx b/mod/external-handler.cxx
index e88e4b4..4237439 100644
--- a/mod/external-handler.cxx
+++ b/mod/external-handler.cxx
@@ -302,10 +302,6 @@ namespace brep
         //
         assert (n.empty () && v == "1");
 
-        // Save the format version pair.
-        //
-        r.values.push_back (move (nv));
-
         // Get and verify the HTTP status.
         //
         nv = p.next ();
@@ -329,10 +325,6 @@ namespace brep
         //
         for (nv = p.next (); !nv.empty (); nv = p.next ())
           r.values.push_back (move (nv));
-
-        // Save end of manifest.
-        //
-        r.values.push_back (move (nv));
       }
       catch (const parsing& e)
       {
diff --git a/mod/external-handler.hxx b/mod/external-handler.hxx
index 66172d0..5a1d731 100644
--- a/mod/external-handler.hxx
+++ b/mod/external-handler.hxx
@@ -34,8 +34,11 @@ namespace brep
     struct result_manifest
     {
       uint16_t status;
-      vector<butl::manifest_name_value> values; // Note: all values, including
-                                                // status.
+
+      // All values, including status but excluding format version and
+      // end-of-manifest.
+      //
+      vector<butl::manifest_name_value> values;
     };
 
     optional<result_manifest>
diff --git a/mod/mod-ci.cxx b/mod/mod-ci.cxx
index f5efaac..168b57c 100644
--- a/mod/mod-ci.cxx
+++ b/mod/mod-ci.cxx
@@ -103,6 +103,8 @@ handle (request& rq, response& rs)
   using namespace bpkg;
   using namespace xhtml;
 
+  using parser        = manifest_parser;
+  using parsing       = manifest_parsing;
   using serializer    = manifest_serializer;
   using serialization = manifest_serialization;
 
@@ -162,7 +164,10 @@ handle (request& rq, response& rs)
   if (!options_->ci_data_specified ())
     return respond_manifest (404, "CI request submission disabled");
 
-  // Parse the request form data.
+  // Parse the request form data and verify the submission size limit.
+  //
+  // Note that the submission may include the overrides upload that we don't
+  // expect to be large.
   //
   const name_values& rps (rq.parameters (64 * 1024));
 
@@ -280,6 +285,38 @@ handle (request& rq, response& rs)
       return respond_manifest (400, "invalid parameter " + nv.name);
   }
 
+  // Parse and validate overrides, if present.
+  //
+  vector<manifest_name_value> overrides;
+
+  if (params.overrides_specified ())
+  try
+  {
+    istream& is (rq.open_upload ("overrides"));
+    parser mp (is, "overrides");
+    overrides = parse_manifest (mp);
+
+    package_manifest::validate_overrides (overrides, mp.name ());
+  }
+  // Note that invalid_argument (thrown by open_upload() function call) can
+  // mean both no overrides upload or multiple overrides uploads.
+  //
+  catch (const invalid_argument&)
+  {
+    return respond_manifest (400, "overrides upload expected");
+  }
+  catch (const parsing& e)
+  {
+    return respond_manifest (400,
+                             string ("unable to parse overrides: ") +
+                             e.what ());
+  }
+  catch (const io_error& e)
+  {
+    error << "unable to read overrides: " << e;
+    return respond_error ();
+  }
+
   try
   {
     // Note that from now on the result manifest we respond with will contain
@@ -340,7 +377,7 @@ handle (request& rq, response& rs)
       s.next ("id", request_id);
       s.next ("repository", rl.string ());
 
-      for (const string& p: params.package())
+      for (const string& p: params.package ())
       {
         if (!p.empty ()) // Skip empty package names (see above for details).
           s.next ("package", p);
@@ -385,6 +422,7 @@ handle (request& rq, response& rs)
         if (n != "repository" &&
             n != "_"          &&
             n != "package"    &&
+            n != "overrides"  &&
             n != "simulate")
           s.next (n, nv.value ? *nv.value : "");
       }
@@ -418,6 +456,39 @@ handle (request& rq, response& rs)
     return respond_error ();
   }
 
+  // Serialize the CI overrides manifest to a stream. On the stream error pass
+  // through the io_error exception.
+  //
+  // Note that it can't throw the serialization exception as the override
+  // manifest is parsed from the stream and so verified.
+  //
+  auto ovm = [&overrides] (ostream& os, bool long_lines = false)
+  {
+    try
+    {
+      serializer s (os, "overrides", long_lines);
+      serialize_manifest (s, overrides);
+    }
+    catch (const serialization&) {assert (false);} // See above.
+  };
+
+  // Serialize the CI overrides manifest to the submission directory.
+  //
+  path ovf (dd / "overrides.manifest");
+
+  if (!overrides.empty ())
+  try
+  {
+    ofdstream os (ovf);
+    ovm (os);
+    os.close ();
+  }
+  catch (const io_error& e)
+  {
+    error << "unable to write to '" << ovf << "': " << e;
+    return respond_error ();
+  }
+
   // Given that the submission data is now successfully persisted we are no
   // longer in charge of removing it, except for the cases when the submission
   // handler terminates with an error (see below for details).
@@ -493,11 +564,9 @@ handle (request& rq, response& rs)
       rvs.emplace_back (move (nv));
     };
 
-    add ("", "1");                           // Start of manifest.
     add ("status", "200");
     add ("message", "CI request is queued");
     add ("reference", request_id);
-    add ("", "");                            // End of manifest.
   }
 
   assert (!rvs.empty ()); // Produced by the handler or is implied.
@@ -512,9 +581,7 @@ handle (request& rq, response& rs)
     try
     {
       serializer s (os, "result", long_lines);
-      for (const manifest_name_value& nv: rvs)
-        s.next (nv.name, nv.value);
-
+      serialize_manifest (s, rvs);
       return true;
     }
     catch (const serialization& e)
@@ -592,12 +659,18 @@ handle (request& rq, response& rs)
                  "CI request submission (" + request_id + ")",
                  {options_->ci_email ()});
 
-    // Write the submission request manifest.
+    // Write the CI request manifest.
     //
     bool r (rqm (sm.out, true /* long_lines */));
     assert (r); // The serialization succeeded once, so can't fail now.
 
-    // Write the submission result manifest.
+    // Write the CI overrides manifest.
+    //
+    sm.out << "\n\n";
+
+    ovm (sm.out, true /* long_lines */);
+
+    // Write the CI result manifest.
     //
     sm.out << "\n\n";
 
diff --git a/mod/mod-submit.cxx b/mod/mod-submit.cxx
index ac98268..ad1b716 100644
--- a/mod/mod-submit.cxx
+++ b/mod/mod-submit.cxx
@@ -12,7 +12,7 @@
 #include <libbutl/timestamp.mxx>
 #include <libbutl/filesystem.mxx>
 #include <libbutl/process-io.mxx>          // operator<<(ostream, process_args)
-#include <libbutl/manifest-parser.mxx>
+#include <libbutl/manifest-types.mxx>
 #include <libbutl/manifest-serializer.mxx>
 
 #include <web/xhtml.hxx>
@@ -295,8 +295,9 @@ handle (request& rq, response& rs)
   try
   {
     // Note that providing a meaningful prefix for temp_name() is not really
-    // required as the temporary directory is used by brep exclusively. However,
-    // using the abbreviated checksum can be helpful for troubleshooting.
+    // required as the temporary directory is used by brep exclusively.
+    // However, using the abbreviated checksum can be helpful for
+    // troubleshooting.
     //
     td = dir_path (options_->submit_temp () /
                    dir_path (path::traits::temp_name (ref)));
@@ -601,11 +602,9 @@ handle (request& rq, response& rs)
       rvs.emplace_back (move (nv));
     };
 
-    add ("", "1");                           // Start of manifest.
     add ("status", "200");
     add ("message", "package submission is queued");
     add ("reference", ref);
-    add ("", "");                            // End of manifest.
   }
 
   assert (!rvs.empty ()); // Produced by the handler or is implied.
@@ -620,9 +619,7 @@ handle (request& rq, response& rs)
     try
     {
       serializer s (os, "result", long_lines);
-      for (const manifest_name_value& nv: rvs)
-        s.next (nv.name, nv.value);
-
+      serialize_manifest (s, rvs);
       return true;
     }
     catch (const serialization& e)
diff --git a/mod/options.cli b/mod/options.cli
index 01309dc..00b88db 100644
--- a/mod/options.cli
+++ b/mod/options.cli
@@ -797,6 +797,13 @@ namespace brep
       //
       strings package;
 
+      // Overrides file name. Must be <input type="file"/>.
+      //
+      // Note that we don't really need this name and only check if this
+      // parameter is specified to detect presence of the upload.
+      //
+      string overrides;
+
       // Submission simulation outcome.
       //
       string simulate;