Add support for build-warning-email and build-error-email package manifest values

11 files changed, 149 insertions, 95 deletions

diff --git a/libbrep/package.cxx b/libbrep/package.cxx
index d10186c..0a711ba 100644
--- a/libbrep/package.cxx
+++ b/libbrep/package.cxx
@@ -66,6 +66,8 @@ namespace brep
            optional<email_type> em,
            optional<email_type> pe,
            optional<email_type> be,
+           optional<email_type> bwe,
+           optional<email_type> bee,
            dependencies_type dp,
            requirements_type rq,
            build_constraints_type bc,
@@ -91,6 +93,8 @@ namespace brep
         email (move (em)),
         package_email (move (pe)),
         build_email (move (be)),
+        build_warning_email (move (bwe)),
+        build_error_email (move (bee)),
         dependencies (move (dp)),
         requirements (move (rq)),
         build_constraints (version.compare (wildcard_version, true) != 0
diff --git a/libbrep/package.hxx b/libbrep/package.hxx
index fb44bf3..97ea864 100644
--- a/libbrep/package.hxx
+++ b/libbrep/package.hxx
@@ -21,7 +21,7 @@
 //
 #define LIBBREP_PACKAGE_SCHEMA_VERSION_BASE 7
 
-#pragma db model version(LIBBREP_PACKAGE_SCHEMA_VERSION_BASE, 9, open)
+#pragma db model version(LIBBREP_PACKAGE_SCHEMA_VERSION_BASE, 10, open)
 
 namespace brep
 {
@@ -372,6 +372,8 @@ namespace brep
              optional<email_type>,
              optional<email_type> package_email,
              optional<email_type> build_email,
+             optional<email_type> build_warning_email,
+             optional<email_type> build_error_email,
              dependencies_type,
              requirements_type,
              build_constraints_type,
@@ -418,6 +420,8 @@ namespace brep
     optional<email_type> email;
     optional<email_type> package_email;
     optional<email_type> build_email;
+    optional<email_type> build_warning_email;
+    optional<email_type> build_error_email;
     dependencies_type dependencies;
     requirements_type requirements;
 
diff --git a/libbrep/package.xml b/libbrep/package.xml
index 2b9ebab..dff6a47 100644
--- a/libbrep/package.xml
+++ b/libbrep/package.xml
@@ -1,4 +1,13 @@
 <changelog xmlns="http://www.codesynthesis.com/xmlns/odb/changelog" database="pgsql" schema-name="package" version="1">
+  <changeset version="10">
+    <alter-table name="package">
+      <add-column name="build_warning_email" type="TEXT" null="true"/>
+      <add-column name="build_warning_email_comment" type="TEXT" null="true"/>
+      <add-column name="build_error_email" type="TEXT" null="true"/>
+      <add-column name="build_error_email_comment" type="TEXT" null="true"/>
+    </alter-table>
+  </changeset>
+
   <changeset version="9">
     <add-table name="tenant" kind="object">
       <column name="id" type="TEXT" null="false"/>
diff --git a/load/load.cxx b/load/load.cxx
index 3036daf..97cd133 100644
--- a/load/load.cxx
+++ b/load/load.cxx
@@ -487,6 +487,8 @@ load_packages (const shared_ptr<repository>& rp, database& db)
           move (pm.email),
           move (pm.package_email),
           move (pm.build_email),
+          move (pm.build_warning_email),
+          move (pm.build_error_email),
           move (ds),
           move (pm.requirements),
           move (pm.build_constraints),
diff --git a/migrate/migrate.cxx b/migrate/migrate.cxx
index ad5ea17..be955ea 100644
--- a/migrate/migrate.cxx
+++ b/migrate/migrate.cxx
@@ -218,8 +218,8 @@ struct package_migration_entry: package_migration_entry_base<v>
       : package_migration_entry_base<v> (f, "package") {}
 };
 
-// Don't forget to drop the repository_tenant view when stop supporting
-// data migration for this schema version.
+// Don't forget to drop the repository_tenant view when stop supporting data
+// migration for this schema version.
 //
 static const package_migration_entry<9>
 package_migrate_v9 ([] (database& db)
diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx
index 1ba1bec..7f73172 100644
--- a/mod/mod-build-result.cxx
+++ b/mod/mod-build-result.cxx
@@ -240,14 +240,14 @@ handle (request& rq, response&)
   // worth it here: email members are really secondary and we don't need to
   // switch transactions back and forth.
   //
-  shared_ptr<package> p;
+  shared_ptr<package> pkg;
   {
     transaction t (package_db_->begin ());
-    p = package_db_->find<package> (id.package);
+    pkg = package_db_->find<package> (id.package);
     t.commit ();
   }
 
-  if (p == nullptr)
+  if (pkg == nullptr)
   {
     warn_expired ("no package");
     return true;
@@ -260,19 +260,24 @@ handle (request& rq, response&)
 
   // Load and update the package build configuration (if present).
   //
-  shared_ptr<build> b;
+  // NULL if the package build doesn't exist or is not updated for any reason
+  // (authentication failed, etc).
+  //
+  shared_ptr<build> bld;
+
   optional<result_status> prev_status;
-  bool notify (false);
+  bool build_notify (false);
   bool unforced (true);
 
   {
     transaction t (build_db_->begin ());
 
     package_build pb;
+    shared_ptr<build> b;
     if (!build_db_->query_one<package_build> (
           query<package_build>::build::id == id, pb))
       warn_expired ("no package build");
-    else if ((b = pb.build)->state != build_state::building)
+    else if ((b = move (pb.build))->state != build_state::building)
       warn_expired ("package configuration state is " + to_string (b->state));
     else if (b->timestamp != session_timestamp)
       warn_expired ("non-matching timestamp");
@@ -357,11 +362,13 @@ handle (request& rq, response&)
       {
         unforced = b->force == force_state::unforced;
 
-        // Don's send email for the success-to-success status change, unless
-        // the build was forced.
+        // Don't send email to the build-email address for the
+        // success-to-success status change, unless the build was forced.
         //
-        notify = !(rqm.result.status == result_status::success &&
-                   b->status && *b->status == rqm.result.status && unforced);
+        build_notify = !(rqm.result.status == result_status::success &&
+                         b->status                                   &&
+                         *b->status == rqm.result.status             &&
+                         unforced);
 
         prev_status = move (b->status);
 
@@ -382,88 +389,111 @@ handle (request& rq, response&)
         b->timestamp = system_clock::now ();
 
         build_db_->update (b);
+
+        bld = move (b);
       }
     }
 
     t.commit ();
   }
 
-  // Don't send the notification email if the empty package build email is
-  // specified.
-  //
-  const optional<email>& build_email (p->build_email);
-  if (!notify || (build_email && build_email->empty ()))
+  if (bld == nullptr)
     return true;
 
-  // If the package build address is not specified, then it is assumed to be
-  // the same as the package email address, if specified, otherwise as the
-  // project email address, if specified, otherwise the notification email is
-  // not sent.
-  //
-  const optional<email>& to (build_email ? build_email
-                             : p->package_email
-                               ? p->package_email
-                               : p->email);
-  if (!to)
-    return true;
-
-  assert (b != nullptr);
+  string subj ((unforced ? "build " : "rebuild ") +
+               to_string (*bld->status) + ": " +
+               bld->package_name.string () + '/' +
+               bld->package_version.string () + '/' +
+               bld->configuration + '/' +
+               bld->toolchain_name + '-' + bld->toolchain_version.string ());
 
-  // Send email to the package owner.
+  // Send notification emails to the interested parties.
   //
-  try
+  auto send_email = [&pkg, &bld, &subj, &error, &trace, &print_args, this]
+                    (const string& to)
   {
-    string subj ((unforced ? "build " : "rebuild ") +
-                 to_string (*b->status) + ": " +
-                 b->package_name.string () + '/' +
-                 b->package_version.string () + '/' + b->configuration + '/' +
-                 b->toolchain_name + '-' + b->toolchain_version.string ());
-
-    // Redirect the diagnostics to webserver error log.
-    //
-    // Note: if using this somewhere else, then need to factor out all this
-    // exit status handling code.
-    //
-    sendmail sm (print_args,
-                 2,
-                 options_->email (),
-                 subj,
-                 {*to});
-
-    if (b->results.empty ())
-      sm.out << "No operation results available." << endl;
-    else
+    try
     {
-      const string& host (options_->host ());
-      const dir_path& root (options_->root ());
+      l2 ([&]{trace << "email '" << subj << "' to " << to;});
+
+      // Redirect the diagnostics to webserver error log.
+      //
+      // Note: if using this somewhere else, then need to factor out all this
+      // exit status handling code.
+      //
+      sendmail sm (print_args,
+                   2,
+                   options_->email (),
+                   subj,
+                   {to});
+
+      if (bld->results.empty ())
+        sm.out << "No operation results available." << endl;
+      else
+      {
+        const string& host (options_->host ());
+        const dir_path& root (options_->root ());
 
-      ostream& os (sm.out);
+        ostream& os (sm.out);
 
-      assert (b->status);
-      os << "combined: " << *b->status << endl << endl
-         << "  " << build_log_url (host, root, *b) << endl << endl;
+        assert (bld->status);
+        os << "combined: " << *bld->status << endl << endl
+           << "  " << build_log_url (host, root, *bld) << endl << endl;
 
-      for (const auto& r: b->results)
-        os << r.operation << ": " << r.status << endl << endl
-           << "  " << build_log_url (host, root, *b, &r.operation)
-           << endl << endl;
+        for (const auto& r: bld->results)
+          os << r.operation << ": " << r.status << endl << endl
+             << "  " << build_log_url (host, root, *bld, &r.operation)
+             << endl << endl;
 
-      os << "Force rebuild (enter the reason, use '+' instead of spaces):"
-         << endl << endl
-         << "  " << force_rebuild_url (host, root, *b) << endl;
-    }
+        os << "Force rebuild (enter the reason, use '+' instead of spaces):"
+           << endl << endl
+           << "  " << force_rebuild_url (host, root, *bld) << endl;
+      }
 
-    sm.out.close ();
+      sm.out.close ();
 
-    if (!sm.wait ())
-      error << "sendmail " << *sm.exit;
-  }
-  // Handle process_error and io_error (both derive from system_error).
+      if (!sm.wait ())
+        error << "sendmail " << *sm.exit;
+    }
+    // Handle process_error and io_error (both derive from system_error).
+    //
+    catch (const system_error& e)
+    {
+      error << "sendmail error: " << e;
+    }
+  };
+
+  // Don't send the build notification email if the empty package build email
+  // is specified.
   //
-  catch (const system_error& e)
+  optional<email>& build_email (pkg->build_email);
+  if (build_notify && (!build_email || !build_email->empty ()))
   {
-    error << "sendmail error: " << e;
+    // If none of the package build-* addresses is specified, then the build
+    // email address is assumed to be the same as the package email address,
+    // if specified, otherwise as the project email address, if specified,
+    // otherwise the notification email is not sent.
+    //
+    optional<email> to;
+
+    if (build_email)
+      to = move (build_email);
+    else if (!pkg->build_warning_email && !pkg->build_error_email)
+      to = move (pkg->package_email ? pkg->package_email : pkg->email);
+
+    if (to)
+      send_email (*to);
   }
 
+  assert (bld->status);
+
+  // Send the build warning/error notification emails, if requested.
+  //
+  if (pkg->build_warning_email && *bld->status >= result_status::warning)
+    send_email (*pkg->build_warning_email);
+
+  if (pkg->build_error_email && *bld->status >= result_status::error)
+    send_email (*pkg->build_error_email);
+
   return true;
 }
diff --git a/mod/mod-package-version-details.cxx b/mod/mod-package-version-details.cxx
index b548b32..f9d346e 100644
--- a/mod/mod-package-version-details.cxx
+++ b/mod/mod-package-version-details.cxx
@@ -248,10 +248,6 @@ handle (request& rq, response& rs)
   if (pe && pe != em)
     s << TR_EMAIL (*pe, "package-email");
 
-  const auto& be (pkg->build_email);
-  if (be && ((pe && be != pe) || (!pe && be != em)))
-    s << TR_EMAIL (*be, "build-email");
-
   s <<     TR_TAGS (pkg->tags, root, tenant)
     <<   ~TBODY
     << ~TABLE;
diff --git a/tests/load/1/math/libexp-+2-1.2+1.tar.gz b/tests/load/1/math/libexp-+2-1.2+1.tar.gz
index 56f877e..e8b9183 100644
--- a/tests/load/1/math/libexp-+2-1.2+1.tar.gz
+++ b/tests/load/1/math/libexp-+2-1.2+1.tar.gz
diff --git a/tests/load/1/math/libstudxml-1.0.0+1.tar.gz b/tests/load/1/math/libstudxml-1.0.0+1.tar.gz
index c351fa5..41c9637 100644
--- a/tests/load/1/math/libstudxml-1.0.0+1.tar.gz
+++ b/tests/load/1/math/libstudxml-1.0.0+1.tar.gz
diff --git a/tests/load/1/math/packages.manifest b/tests/load/1/math/packages.manifest
index 3ac5efd..fea801f 100644
--- a/tests/load/1/math/packages.manifest
+++ b/tests/load/1/math/packages.manifest
@@ -8,15 +8,15 @@ summary: The exponent
 license: MIT
 tags: mathlab, c++, exponent
 description: The exponent math function.
-url: http://www.exp.com
-email: users@exp.com
-build-email: builds@exp.com
+url: http://exp.example.com
+email: users@exp.example.com
+build-email: builds@exp.example.com
 depends: libmisc
 depends: libpq >= 9.0.0
 build-exclude: *; Only supported on Linux.
 build-include: linux*
 location: libexp-+2-1.2+1.tar.gz
-sha256sum: bc68940a1b3b7e345cbceac35d308b4e04b304f49ff2087340949f2879709967
+sha256sum: 9449cb008ca8cc3b91fbe5c44ae87f0e10fd24ff453bb88cf4504dabe2068eb3
 :
 name: libfoo
 version: +0-X.Y
@@ -113,10 +113,12 @@ summary: Modern C++ XML API
 license: MIT
 tags: c++, xml, parser, serializer, pull, streaming, modern
 url: http://www.codesynthesis.com/projects/libstudxml/
-email: studxml-users@codesynthesis.com; Public mailing list, posts by\
- non-members are allowed but moderated.
-package-email: boris@codesynthesis.com; Direct email to the author.
+email: studxml-users@example.com; Public mailing list, posts by non-members\
+ are allowed but moderated.
+package-email: studxml-package@example.com; Direct email to the packager.
+build-warning-email: studxml-warnings@example.com
+build-error-email: studxml-errors@example.com
 depends: libexpat >= 2.0.0
 depends: libgenx
 location: libstudxml-1.0.0+1.tar.gz
-sha256sum: cfa4b1f89f8e903d48eff1e1d14628c32aa4d126d09b0b056d2cd80f8dc78580
+sha256sum: 1833906dd93ccc0cda832d6a1b3ef9ed7877bb9958b46d9b2666033d4a7919c9
diff --git a/tests/load/driver.cxx b/tests/load/driver.cxx
index a36653c..7009d9e 100644
--- a/tests/load/driver.cxx
+++ b/tests/load/driver.cxx
@@ -628,13 +628,20 @@ test_pkg_repos (const cstrings& loader_args,
     assert (xpv->url &&
             *xpv->url == "http://www.codesynthesis.com/projects/libstudxml/");
     assert (!xpv->package_url);
-    assert (xpv->email && *xpv->email ==
-            email ("studxml-users@codesynthesis.com",
+    assert (xpv->email && *xpv->email == email ("studxml-users@example.com",
                    "Public mailing list, posts by  non-members "
                    "are allowed but moderated."));
+
     assert (xpv->package_email &&
-            *xpv->package_email == email ("boris@codesynthesis.com",
-                                          "Direct email to the author."));
+            *xpv->package_email == email ("studxml-package@example.com",
+                                          "Direct email to the packager."));
+    assert (xpv->build_warning_email &&
+            *xpv->build_warning_email ==
+            email ("studxml-warnings@example.com"));
+
+    assert (xpv->build_error_email &&
+            *xpv->build_error_email ==
+            email ("studxml-errors@example.com"));
 
     assert (xpv->internal_repository.load () == mr);
     assert (xpv->other_repositories.empty ());
@@ -660,7 +667,7 @@ test_pkg_repos (const cstrings& loader_args,
     assert (check_location (xpv));
 
     assert (xpv->sha256sum && *xpv->sha256sum ==
-            "cfa4b1f89f8e903d48eff1e1d14628c32aa4d126d09b0b056d2cd80f8dc78580");
+            "1833906dd93ccc0cda832d6a1b3ef9ed7877bb9958b46d9b2666033d4a7919c9");
 
     // Verify libfoo package versions.
     //
@@ -790,11 +797,11 @@ test_pkg_repos (const cstrings& loader_args,
     assert (epv->tags == strings ({"mathlab", "c++", "exponent"}));
     assert (epv->description && *epv->description ==
             "The exponent math function.");
-    assert (epv->url && *epv->url == "http://www.exp.com");
+    assert (epv->url && *epv->url == "http://exp.example.com");
     assert (!epv->package_url);
-    assert (epv->email && *epv->email == email ("users@exp.com"));
+    assert (epv->email && *epv->email == email ("users@exp.example.com"));
     assert (!epv->package_email);
-    assert (epv->build_email && *epv->build_email == "builds@exp.com");
+    assert (epv->build_email && *epv->build_email == "builds@exp.example.com");
 
     assert (epv->internal_repository.load () == mr);
     assert (epv->other_repositories.empty ());
@@ -827,7 +834,7 @@ test_pkg_repos (const cstrings& loader_args,
 
     assert (check_location (epv));
     assert (epv->sha256sum && *epv->sha256sum ==
-            "bc68940a1b3b7e345cbceac35d308b4e04b304f49ff2087340949f2879709967");
+            "9449cb008ca8cc3b91fbe5c44ae87f0e10fd24ff453bb88cf4504dabe2068eb3");
 
     // Verify libpq package version.
     //