Make sure we only build one package config per interactive tennant

6 files changed, 310 insertions, 151 deletions

diff --git a/doc/manual.cli b/doc/manual.cli
index d0a1923..53dcb28 100644
--- a/doc/manual.cli
+++ b/doc/manual.cli
@@ -359,8 +359,21 @@ being applied. Currently, only the following value groups can be overridden:
 \
 build-email build-{warning,error}-email
 builds build-{include,exclude}
+*-builds *-build-{include,exclude}
 \
 
+Note that the build constraints group values (both common and build package
+configuration-specific) are overridden hierarchically so that the
+\c{[\b{*-}]\b{build-}{\b{include},\b{exclude}\}} overrides don't affect the
+respective \c{[\b{*-}]\b{builds}} values.
+
+Note also that the common and build package configuration-specific build
+constraints group value overrides are mutually exclusive. If the common build
+constraints are overridden, then all the configuration-specific constraints
+are removed. Otherwise, if any configuration-specific constraints are
+overridden, then for the remaining configurations the build constraints are
+reset to \cb{builds:\ none}.
+
 See \l{bpkg#manifest-package Package Manifest} for details on these values.
 
 
diff --git a/libbrep/build-package.hxx b/libbrep/build-package.hxx
index 79fb6a4..6ca8702 100644
--- a/libbrep/build-package.hxx
+++ b/libbrep/build-package.hxx
@@ -25,7 +25,7 @@ namespace brep
   //
   // Foreign object that is mapped to a subset of the tenant object.
   //
-  #pragma db object table("build_tenant") pointer(shared_ptr) readonly
+  #pragma db object table("build_tenant") pointer(shared_ptr)
   class build_tenant
   {
   public:
diff --git a/libbrep/package.hxx b/libbrep/package.hxx
index 4a68a07..cf6ae64 100644
--- a/libbrep/package.hxx
+++ b/libbrep/package.hxx
@@ -18,7 +18,7 @@
 
 // Used by the data migration entries.
 //
-#define LIBBREP_PACKAGE_SCHEMA_VERSION_BASE 25
+#define LIBBREP_PACKAGE_SCHEMA_VERSION_BASE 26
 
 #pragma db model version(LIBBREP_PACKAGE_SCHEMA_VERSION_BASE, 26, closed)
 
diff --git a/libbrep/package.xml b/libbrep/package.xml
index 4e5544a..8f32284 100644
--- a/libbrep/package.xml
+++ b/libbrep/package.xml
@@ -1,139 +1,5 @@
 <changelog xmlns="http://www.codesynthesis.com/xmlns/odb/changelog" database="pgsql" schema-name="package" version="1">
-  <changeset version="26">
-    <add-table name="package_build_configs" kind="container">
-      <column name="tenant" type="TEXT" null="false"/>
-      <column name="name" type="CITEXT" null="false"/>
-      <column name="version_epoch" type="INTEGER" null="false"/>
-      <column name="version_canonical_upstream" type="TEXT" null="false"/>
-      <column name="version_canonical_release" type="TEXT" null="false" options="COLLATE &quot;C&quot;"/>
-      <column name="version_revision" type="INTEGER" null="false"/>
-      <column name="index" type="BIGINT" null="false"/>
-      <column name="config_name" type="TEXT" null="false"/>
-      <column name="config_arguments" type="TEXT" null="false"/>
-      <column name="config_comment" type="TEXT" null="false"/>
-      <foreign-key name="tenant_fk" deferrable="DEFERRED">
-        <column name="tenant"/>
-        <references table="tenant">
-          <column name="id"/>
-        </references>
-      </foreign-key>
-      <foreign-key name="object_id_fk" on-delete="CASCADE">
-        <column name="tenant"/>
-        <column name="name"/>
-        <column name="version_epoch"/>
-        <column name="version_canonical_upstream"/>
-        <column name="version_canonical_release"/>
-        <column name="version_revision"/>
-        <references table="package">
-          <column name="tenant"/>
-          <column name="name"/>
-          <column name="version_epoch"/>
-          <column name="version_canonical_upstream"/>
-          <column name="version_canonical_release"/>
-          <column name="version_revision"/>
-        </references>
-      </foreign-key>
-      <index name="package_build_configs_object_id_i">
-        <column name="tenant"/>
-        <column name="name"/>
-        <column name="version_epoch"/>
-        <column name="version_canonical_upstream"/>
-        <column name="version_canonical_release"/>
-        <column name="version_revision"/>
-      </index>
-      <index name="package_build_configs_index_i">
-        <column name="index"/>
-      </index>
-    </add-table>
-    <add-table name="package_build_config_builds" kind="container">
-      <column name="tenant" type="TEXT" null="false"/>
-      <column name="name" type="CITEXT" null="false"/>
-      <column name="version_epoch" type="INTEGER" null="false"/>
-      <column name="version_canonical_upstream" type="TEXT" null="false"/>
-      <column name="version_canonical_release" type="TEXT" null="false" options="COLLATE &quot;C&quot;"/>
-      <column name="version_revision" type="INTEGER" null="false"/>
-      <column name="config_index" type="BIGINT" null="false"/>
-      <column name="index" type="BIGINT" null="false"/>
-      <column name="expression" type="TEXT" null="false"/>
-      <column name="comment" type="TEXT" null="false"/>
-      <foreign-key name="tenant_fk" deferrable="DEFERRED">
-        <column name="tenant"/>
-        <references table="tenant">
-          <column name="id"/>
-        </references>
-      </foreign-key>
-      <foreign-key name="object_id_fk" on-delete="CASCADE">
-        <column name="tenant"/>
-        <column name="name"/>
-        <column name="version_epoch"/>
-        <column name="version_canonical_upstream"/>
-        <column name="version_canonical_release"/>
-        <column name="version_revision"/>
-        <references table="package">
-          <column name="tenant"/>
-          <column name="name"/>
-          <column name="version_epoch"/>
-          <column name="version_canonical_upstream"/>
-          <column name="version_canonical_release"/>
-          <column name="version_revision"/>
-        </references>
-      </foreign-key>
-      <index name="package_build_config_builds_object_id_i">
-        <column name="tenant"/>
-        <column name="name"/>
-        <column name="version_epoch"/>
-        <column name="version_canonical_upstream"/>
-        <column name="version_canonical_release"/>
-        <column name="version_revision"/>
-      </index>
-    </add-table>
-    <add-table name="package_build_config_constraints" kind="container">
-      <column name="tenant" type="TEXT" null="false"/>
-      <column name="name" type="CITEXT" null="false"/>
-      <column name="version_epoch" type="INTEGER" null="false"/>
-      <column name="version_canonical_upstream" type="TEXT" null="false"/>
-      <column name="version_canonical_release" type="TEXT" null="false" options="COLLATE &quot;C&quot;"/>
-      <column name="version_revision" type="INTEGER" null="false"/>
-      <column name="config_index" type="BIGINT" null="false"/>
-      <column name="index" type="BIGINT" null="false"/>
-      <column name="exclusion" type="BOOLEAN" null="false"/>
-      <column name="config" type="TEXT" null="false"/>
-      <column name="target" type="TEXT" null="true"/>
-      <column name="comment" type="TEXT" null="false"/>
-      <foreign-key name="tenant_fk" deferrable="DEFERRED">
-        <column name="tenant"/>
-        <references table="tenant">
-          <column name="id"/>
-        </references>
-      </foreign-key>
-      <foreign-key name="object_id_fk" on-delete="CASCADE">
-        <column name="tenant"/>
-        <column name="name"/>
-        <column name="version_epoch"/>
-        <column name="version_canonical_upstream"/>
-        <column name="version_canonical_release"/>
-        <column name="version_revision"/>
-        <references table="package">
-          <column name="tenant"/>
-          <column name="name"/>
-          <column name="version_epoch"/>
-          <column name="version_canonical_upstream"/>
-          <column name="version_canonical_release"/>
-          <column name="version_revision"/>
-        </references>
-      </foreign-key>
-      <index name="package_build_config_constraints_object_id_i">
-        <column name="tenant"/>
-        <column name="name"/>
-        <column name="version_epoch"/>
-        <column name="version_canonical_upstream"/>
-        <column name="version_canonical_release"/>
-        <column name="version_revision"/>
-      </index>
-    </add-table>
-  </changeset>
-
-  <model version="25">
+  <model version="26">
     <table name="tenant" kind="object">
       <column name="id" type="TEXT" null="false"/>
       <column name="private" type="BOOLEAN" null="false"/>
@@ -986,6 +852,137 @@
         <column name="index"/>
       </index>
     </table>
+    <table name="package_build_configs" kind="container">
+      <column name="tenant" type="TEXT" null="false"/>
+      <column name="name" type="CITEXT" null="false"/>
+      <column name="version_epoch" type="INTEGER" null="false"/>
+      <column name="version_canonical_upstream" type="TEXT" null="false"/>
+      <column name="version_canonical_release" type="TEXT" null="false" options="COLLATE &quot;C&quot;"/>
+      <column name="version_revision" type="INTEGER" null="false"/>
+      <column name="index" type="BIGINT" null="false"/>
+      <column name="config_name" type="TEXT" null="false"/>
+      <column name="config_arguments" type="TEXT" null="false"/>
+      <column name="config_comment" type="TEXT" null="false"/>
+      <foreign-key name="tenant_fk" deferrable="DEFERRED">
+        <column name="tenant"/>
+        <references table="tenant">
+          <column name="id"/>
+        </references>
+      </foreign-key>
+      <foreign-key name="object_id_fk" on-delete="CASCADE">
+        <column name="tenant"/>
+        <column name="name"/>
+        <column name="version_epoch"/>
+        <column name="version_canonical_upstream"/>
+        <column name="version_canonical_release"/>
+        <column name="version_revision"/>
+        <references table="package">
+          <column name="tenant"/>
+          <column name="name"/>
+          <column name="version_epoch"/>
+          <column name="version_canonical_upstream"/>
+          <column name="version_canonical_release"/>
+          <column name="version_revision"/>
+        </references>
+      </foreign-key>
+      <index name="package_build_configs_object_id_i">
+        <column name="tenant"/>
+        <column name="name"/>
+        <column name="version_epoch"/>
+        <column name="version_canonical_upstream"/>
+        <column name="version_canonical_release"/>
+        <column name="version_revision"/>
+      </index>
+      <index name="package_build_configs_index_i">
+        <column name="index"/>
+      </index>
+    </table>
+    <table name="package_build_config_builds" kind="container">
+      <column name="tenant" type="TEXT" null="false"/>
+      <column name="name" type="CITEXT" null="false"/>
+      <column name="version_epoch" type="INTEGER" null="false"/>
+      <column name="version_canonical_upstream" type="TEXT" null="false"/>
+      <column name="version_canonical_release" type="TEXT" null="false" options="COLLATE &quot;C&quot;"/>
+      <column name="version_revision" type="INTEGER" null="false"/>
+      <column name="config_index" type="BIGINT" null="false"/>
+      <column name="index" type="BIGINT" null="false"/>
+      <column name="expression" type="TEXT" null="false"/>
+      <column name="comment" type="TEXT" null="false"/>
+      <foreign-key name="tenant_fk" deferrable="DEFERRED">
+        <column name="tenant"/>
+        <references table="tenant">
+          <column name="id"/>
+        </references>
+      </foreign-key>
+      <foreign-key name="object_id_fk" on-delete="CASCADE">
+        <column name="tenant"/>
+        <column name="name"/>
+        <column name="version_epoch"/>
+        <column name="version_canonical_upstream"/>
+        <column name="version_canonical_release"/>
+        <column name="version_revision"/>
+        <references table="package">
+          <column name="tenant"/>
+          <column name="name"/>
+          <column name="version_epoch"/>
+          <column name="version_canonical_upstream"/>
+          <column name="version_canonical_release"/>
+          <column name="version_revision"/>
+        </references>
+      </foreign-key>
+      <index name="package_build_config_builds_object_id_i">
+        <column name="tenant"/>
+        <column name="name"/>
+        <column name="version_epoch"/>
+        <column name="version_canonical_upstream"/>
+        <column name="version_canonical_release"/>
+        <column name="version_revision"/>
+      </index>
+    </table>
+    <table name="package_build_config_constraints" kind="container">
+      <column name="tenant" type="TEXT" null="false"/>
+      <column name="name" type="CITEXT" null="false"/>
+      <column name="version_epoch" type="INTEGER" null="false"/>
+      <column name="version_canonical_upstream" type="TEXT" null="false"/>
+      <column name="version_canonical_release" type="TEXT" null="false" options="COLLATE &quot;C&quot;"/>
+      <column name="version_revision" type="INTEGER" null="false"/>
+      <column name="config_index" type="BIGINT" null="false"/>
+      <column name="index" type="BIGINT" null="false"/>
+      <column name="exclusion" type="BOOLEAN" null="false"/>
+      <column name="config" type="TEXT" null="false"/>
+      <column name="target" type="TEXT" null="true"/>
+      <column name="comment" type="TEXT" null="false"/>
+      <foreign-key name="tenant_fk" deferrable="DEFERRED">
+        <column name="tenant"/>
+        <references table="tenant">
+          <column name="id"/>
+        </references>
+      </foreign-key>
+      <foreign-key name="object_id_fk" on-delete="CASCADE">
+        <column name="tenant"/>
+        <column name="name"/>
+        <column name="version_epoch"/>
+        <column name="version_canonical_upstream"/>
+        <column name="version_canonical_release"/>
+        <column name="version_revision"/>
+        <references table="package">
+          <column name="tenant"/>
+          <column name="name"/>
+          <column name="version_epoch"/>
+          <column name="version_canonical_upstream"/>
+          <column name="version_canonical_release"/>
+          <column name="version_revision"/>
+        </references>
+      </foreign-key>
+      <index name="package_build_config_constraints_object_id_i">
+        <column name="tenant"/>
+        <column name="name"/>
+        <column name="version_epoch"/>
+        <column name="version_canonical_upstream"/>
+        <column name="version_canonical_release"/>
+        <column name="version_revision"/>
+      </index>
+    </table>
     <table name="package_other_repositories" kind="container">
       <column name="tenant" type="TEXT" null="false"/>
       <column name="name" type="CITEXT" null="false"/>
diff --git a/load/load.cxx b/load/load.cxx
index 000743c..ed48910 100644
--- a/load/load.cxx
+++ b/load/load.cxx
@@ -368,7 +368,8 @@ load_packages (const shared_ptr<repository>& rp,
                const repository_location& cl,
                database& db,
                bool ignore_unknown,
-               const manifest_name_values& overrides)
+               const manifest_name_values& overrides,
+               const string& overrides_name)
 {
   // packages_timestamp other than timestamp_nonexistent signals the
   // repository packages are already loaded.
@@ -434,15 +435,17 @@ load_packages (const shared_ptr<repository>& rp,
     {
       if (rp->internal)
       {
+        if (!overrides.empty ())
         try
         {
-          pm.override (overrides, "" /* name */);
+          pm.override (overrides, overrides_name);
         }
-        catch (const manifest_parsing&)
+        catch (const manifest_parsing& e)
         {
-          // Overrides are already validated (see below).
-          //
-          assert (false);
+          cerr << "error: unable to override " << p << " manifest: " << e
+               << endl;
+
+          throw failed ();
         }
 
         // Create internal package object.
@@ -580,8 +583,7 @@ load_packages (const shared_ptr<repository>& rp,
           move (ts),
           move (pm.builds),
           move (pm.build_constraints),
-          build_package_configs (make_move_iterator (pm.build_configs.begin ()),
-                                 make_move_iterator (pm.build_configs.end ())),
+          move (pm.build_configs),
           move (pm.location),
           move (pm.fragment),
           move (pm.sha256sum),
@@ -979,7 +981,8 @@ load_repositories (const options& lo,
                    !pr->cache_location.empty () ? pr->cache_location : cl,
                    db,
                    ignore_unknown,
-                   manifest_name_values () /* overrides */);
+                   manifest_name_values () /* overrides */,
+                   "" /* overrides_name */);
 
     load_repositories (lo,
                        pr,
@@ -1453,6 +1456,10 @@ try
 
   // Parse and validate overrides, if specified.
   //
+  // Note that here we make sure that the overrides manifest is valid.
+  // Applying overrides to a specific package manifest may still fail (see
+  // package_manifest::validate_overrides() for details).
+  //
   manifest_name_values overrides;
 
   if (ops.overrides_file_specified ())
@@ -1579,7 +1586,8 @@ try
                      r->cache_location,
                      db,
                      ops.ignore_unknown (),
-                     overrides);
+                     overrides,
+                     ops.overrides_file ().string ());
     }
 
     // On the second pass over the internal repositories we load their
diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx
index a15ff90..30bff0d 100644
--- a/mod/mod-build-task.cxx
+++ b/mod/mod-build-task.cxx
@@ -182,7 +182,7 @@ handle (request& rq, response& rs)
                         dir_path () /* start */,
                         path_match_flags::match_absent))
           conf_machines.emplace (build_target_config_id {c.target, c.name},
-                                 config_machine  {&c, &m});
+                                 config_machine {&c, &m});
       }
       catch (const invalid_path&) {}
     }
@@ -603,9 +603,9 @@ handle (request& rq, response& rs)
     // configurations that have already been acted upon (initially empty).
     //
     // This is why we query the database for configurations that should not be
-    // built (in the built state, or in the building state and not
-    // expired). Having such a list we will select the first build
-    // configuration that is not in the list (if available) for the response.
+    // built (in the built state, or in the building state and not expired).
+    // Having such a list we will select the first build configuration that is
+    // not in the list (if available) for the response.
     //
     using bld_query = query<build>;
     using prep_bld_query = prepared_query<build>;
@@ -726,6 +726,139 @@ handle (request& rq, response& rs)
 
         shared_ptr<build_package> p (build_db_->load<build_package> (id));
 
+        // Note that a request to interactively build a package in multiple
+        // configurations is most likely a mistake than a deliberate choice.
+        // Thus, for the interactive tenant let's check if the package can be
+        // built in multiple configurations. If that's the case then we will
+        // put all the potential builds into the aborted state and continue
+        // iterating looking for another package. Otherwise, just proceed for
+        // this package normally.
+        //
+        // It also feels like a good idea to archive an interactive tenant
+        // after a build object is created for it, regardless if the build
+        // task is issued or not. This way we make sure that an interactive
+        // build is never performed multiple times for such a tenant for any
+        // reason (multiple toolchains, buildtab change, etc). Note that the
+        // build result will still be accepted for an archived build.
+        //
+        shared_ptr<build_tenant> t (build_db_->load<build_tenant> (id.tenant));
+
+        if (t->interactive)
+        {
+          // Note that the tenant can be archived via some other package on
+          // some previous iteration. Skip the package if that's the case.
+          //
+          if (t->archived)
+            continue;
+
+          // Collect the potential build configurations as all combinations of
+          // the tenant's packages build configurations and the non-excluded
+          // (by the packages) build target configurations. Note that here we
+          // ignore the machines from the task request.
+          //
+          struct build_config
+          {
+            package_id                 pid;
+            string                     pc;
+            const build_target_config* tc;
+          };
+
+          small_vector<build_config, 1> build_configs;
+
+          // Note that we don't bother creating a prepared query here, since
+          // its highly unlikely to encounter multiple interactive tenants per
+          // task request. Given that we archive such tenants immediately, as
+          // a common case there will be none.
+          //
+          pkg_query pq (pkg_query::build_tenant::id == t->id);
+          for (auto& tp: build_db_->query<buildable_package> (pq))
+          {
+            shared_ptr<build_package> p (
+              build_db_->load<build_package> (tp.id));
+
+            for (build_package_config& pc: p->configs)
+            {
+              for (const auto& tc: *target_conf_)
+              {
+                if (!exclude (pc, p->builds, p->constraints, tc))
+                  build_configs.push_back (build_config {p->id, pc.name, &tc});
+              }
+            }
+          }
+
+          // If multiple build configurations are collected, then abort all
+          // the potential builds and continue iterating over the packages.
+          //
+          if (build_configs.size () > 1)
+          {
+            // Abort the builds.
+            //
+            for (build_config& c: build_configs)
+            {
+              const string& pc (c.pc);
+              const build_target_config& tc (*c.tc);
+
+              build_id bid (c.pid,
+                            tc.target,
+                            tc.name,
+                            pc,
+                            tqm.toolchain_name,
+                            toolchain_version);
+
+              // Can there be any existing builds for such a tenant? Doesn't
+              // seem so, unless due to some manual intervention into the
+              // database. Anyway, let's just leave such a build alone.
+              //
+              shared_ptr<build> b (build_db_->find<build> (bid));
+
+              if (b == nullptr)
+              {
+                b = make_shared<build> (
+                  move (bid.package.tenant),
+                  move (bid.package.name),
+                  bp.version,
+                  move (bid.target),
+                  move (bid.target_config_name),
+                  move (bid.package_config_name),
+                  move (bid.toolchain_name),
+                  toolchain_version,
+                  nullopt             /* interactive */,
+                  nullopt             /* agent_fp */,
+                  nullopt             /* agent_challenge */,
+                  "brep"              /* machine */,
+                  "build task module" /* machine_summary */,
+                  ""                  /* controller_checksum */,
+                  ""                  /* machine_checksum */);
+
+                b->state  = build_state::built;
+                b->status = result_status::abort;
+
+                b->soft_timestamp = b->timestamp;
+                b->hard_timestamp = b->soft_timestamp;
+
+                // Mark the section as loaded, so results are updated.
+                //
+                b->results_section.load ();
+
+                b->results.push_back (
+                  operation_result {
+                    "configure",
+                      result_status::abort,
+                      "error: multiple configurations for interactive build\n"});
+
+                build_db_->persist (b);
+              }
+            }
+
+            // Archive the tenant.
+            //
+            t->archived = true;
+            build_db_->update (t);
+
+            continue; // Skip the package.
+          }
+        }
+
         for (build_package_config& pc: p->configs)
         {
           pkg_config_name = pc.name;
@@ -870,6 +1003,14 @@ handle (request& rq, response& rs)
                 build_db_->update (b);
               }
 
+              // Archive an interactive tenant.
+              //
+              if (t->interactive)
+              {
+                t->archived = true;
+                build_db_->update (t);
+              }
+
               // Finally, prepare the task response manifest.
               //
               tsm = task (move (b), move (p), move (pc), move (t), cm);