Fix collect_order_dependents() not to verify constraints of being dropped dependent

5 files changed, 97 insertions, 31 deletions

diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index b58a25d..9ca992a 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -2146,10 +2146,8 @@ namespace bpkg
       const map<config_package, bool>* rpt_prereq_flags (nullptr);
 
       // Bail out if this is a configured non-system package and no
-      // up/down-grade nor collecting prerequisite replacements are required.
-      //
-      // NOTE: remember to update the check condition in
-      // collect_order_dependents() if changing anything here.
+      // up/down-grade, reconfiguration, nor collecting prerequisite
+      // replacements are required.
       //
       bool src_conf (sp != nullptr &&
                      sp->state == package_state::configured &&
@@ -4720,8 +4718,8 @@ namespace bpkg
           auto i (map_.find (ddb, dn));
 
           // Make sure the up/downgraded package still satisfies this
-          // dependent. But first "prune" if this is a replaced prerequisite
-          // of the repointed dependent.
+          // dependent. But first "prune" if the dependent is being dropped or
+          // this is a replaced prerequisite of the repointed dependent.
           //
           // Note that the repointed dependents are always collected and have
           // all their collected prerequisites ordered (including new and old
@@ -4731,6 +4729,13 @@ namespace bpkg
 
           if (i != map_.end () && i->second.position != end ())
           {
+            build_package& dp (i->second.package);
+
+            // Skip the droped dependent.
+            //
+            if (dp.action && *dp.action == build_package::drop)
+              continue;
+
             repointed_dependents::const_iterator j (
               rpt_depts.find (config_package {ddb, dn}));
 
@@ -4744,24 +4749,14 @@ namespace bpkg
                 continue;
             }
 
-            build_package& dp (i->second.package);
-            const shared_ptr<selected_package>& dsp (dp.selected);
-
             // There is one tricky aspect: the dependent could be in the
-            // process of being up/downgraded as well. In this case all we
-            // need to do is detect this situation and skip the test since all
-            // the (new) contraints of this package have been satisfied in
-            // collect_build().
+            // process of being reconfigured or up/downgraded as well. In this
+            // case all we need to do is detect this situation and skip the
+            // test since all the (new) constraints of this package have been
+            // satisfied in collect_build().
             //
             if (check)
-            {
-              check = dp.available == nullptr ||
-                      (dsp->system () == dp.system &&
-                       dsp->version == dp.available_version () &&
-                       (dp.system               ||
-                        dp.config_vars.empty () ||
-                        !has_buildfile_clause (dp.available->dependencies)));
-            }
+              check = !dp.dependencies;
           }
 
           if (check)
@@ -4845,11 +4840,10 @@ namespace bpkg
           // list, the package is in the map (but not on the list) and it
           // is in neither.
           //
-          // If the existing entry is a drop, then we skip it. If it is
-          // pre-entered, is an adjustment, or is a build that is not supposed
-          // to be built (not in the list), then we merge it into the new
-          // adjustment entry. Otherwise (is a build in the list), we just add
-          // the reconfigure adjustment flag to it.
+          // If the existing entry is pre-entered, is an adjustment, or is a
+          // build that is not supposed to be built (not in the list), then we
+          // merge it into the new adjustment entry. Otherwise (is a build in
+          // the list), we just add the reconfigure adjustment flag to it.
           //
           if (i != map_.end ())
           {
@@ -4860,11 +4854,6 @@ namespace bpkg
                 *dp.action != build_package::build || // Non-build.
                 dpos == end ())                       // Build not in the list.
             {
-              // Skip the droped package.
-              //
-              if (dp.action && *dp.action == build_package::drop)
-                continue;
-
               build_package bp (adjustment ());
               bp.merge (move (dp));
               dp = move (bp);
diff --git a/tests/common/satisfy/t12b/baz-0.1.0.tar.gz b/tests/common/satisfy/t12b/baz-0.1.0.tar.gz
new file mode 100644
index 0000000..2676c52
--- /dev/null
+++ b/tests/common/satisfy/t12b/baz-0.1.0.tar.gz
diff --git a/tests/common/satisfy/t12b/baz-1.0.0.tar.gz b/tests/common/satisfy/t12b/baz-1.0.0.tar.gz
new file mode 100644
index 0000000..1aec461
--- /dev/null
+++ b/tests/common/satisfy/t12b/baz-1.0.0.tar.gz
diff --git a/tests/common/satisfy/t12b/foo-0.1.0.tar.gz b/tests/common/satisfy/t12b/foo-0.1.0.tar.gz
new file mode 100644
index 0000000..a282f20
--- /dev/null
+++ b/tests/common/satisfy/t12b/foo-0.1.0.tar.gz
diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript
index 79cce38..1aa3a95 100644
--- a/tests/pkg-build.testscript
+++ b/tests/pkg-build.testscript
@@ -227,8 +227,11 @@
 # |-- t12b                      -> t12b (prerequisite repository)
 # |   |-- libbaz-0.1.0.tar.gz
 # |   |-- libbar-1.0.0.tar.gz   -> libbaz == 0.1.0
+# |   |-- foo-0.1.0.tar.gz
 # |   |-- foo-1.0.0.tar.gz      -> libbar
 # |   |-- bar-1.0.0.tar.gz      -> libbar == 0.1.0
+# |   |-- baz-0.1.0.tar.gz      -> libbaz
+# |   |-- baz-1.0.0.tar.gz      -> libbaz == 1.0.0
 # |   `-- repositories.manifest
 # |
 # `-- git
@@ -4630,6 +4633,80 @@ test.options += --no-progress
     }
   }
 
+  : drop-dependent
+  :
+  {
+    +$clone_root_cfg && $rep_add $rep/t12b && $rep_fetch
+
+    test.arguments += --yes
+
+    : unhold
+    :
+    : Test that the being dropped dependent does not constrain a dependency
+    : anymore.
+    :
+    {
+      $clone_cfg;
+
+      $* libbar 2>!;
+
+      $pkg_status -r >>EOO;
+        !libbar configured 1.0.0
+          libbaz configured 0.1.0 available [1.0.0]
+        EOO
+
+      $* baz/0.1.0 ?libbar ?libbaz 2>!;
+
+      $pkg_status -r >>EOO;
+        !baz configured !0.1.0 available 1.0.0
+          libbaz configured 1.0.0
+        EOO
+
+      $pkg_drop baz
+    }
+
+    : unuse
+    :
+    : Unlike the previous test, at the time we check the constraint applied
+    : by libbar on libbaz (== 0.1.0) there is no evidence that libbar will be
+    : dropped, which will happen some later execution plan refinement
+    : iteration.
+    :
+    : @@ This scenario is not supported yet and fails with:
+    :
+    :    error: unable to upgrade package libbaz/0.1.0 to 1.0.0
+    :      info: because package libbar depends on (libbaz == 0.1.0)
+    :      info: package libbaz/1.0.0 required by baz
+    :      info: explicitly request up/downgrade of package libbar
+    :      info: or explicitly specify package libbaz version to manually satisfy these constraints
+    :
+    :    We could probably fix this postponing the constraints check in
+    :    collect_order_dependents() until the final execution plan is produced
+    :    (after all that refinement iterations). We could have an additional
+    :    iteration after all the refinements which would enable the constraint
+    :    check in collect_order_dependents().
+    :
+    if false
+    {
+      $clone_cfg;
+
+      $* foo 2>!;
+
+      $pkg_status -r >>EOO;
+        !foo configured 1.0.0
+          libbar configured 1.0.0
+            libbaz configured 0.1.0 available [1.0.0]
+        EOO
+
+      $* baz foo/0.1.0 2>|;
+
+      $pkg_status -r >>EOO;
+        EOO
+
+      $pkg_drop foo
+    }
+  }
+
   : configuration-negotiation
   :
   {