Fix pkg-build to re-evaluate buildfile clauses in dependencies if configuration variables are specified for dependent

4 files changed, 154 insertions, 16 deletions

diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index e77b21b..3d0c47f 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -1202,18 +1202,29 @@ namespace bpkg
     // Collect prerequisites of the package being built recursively.
     //
     // But first "prune" this process if the package we build is a system one
-    // or is already configured and is not a repointed dependent, since that
-    // would mean all its prerequisites are configured as well. Note that this
-    // is not merely an optimization: the package could be an orphan in which
-    // case the below logic will fail (no repository fragment in which to
-    // search for prerequisites). By skipping the prerequisite check we are
-    // able to gracefully handle configured orphans.
-    //
-    // For the repointed dependent, we still need to collect its prerequisite
-    // replacements to make sure its constraints over them are satisfied. Note
-    // that, as it was said above, we can potentially fail if the dependent is
-    // an orphan, but this is exactly what we need to do in that case, since
-    // we won't be able to be reconfigure it anyway.
+    // or is already configured, since that would mean all its prerequisites
+    // are configured as well. Note that this is not merely an optimization:
+    // the package could be an orphan in which case the below logic will fail
+    // (no repository fragment in which to search for prerequisites). By
+    // skipping the prerequisite check we are able to gracefully handle
+    // configured orphans.
+    //
+    // There are, however, some cases when we still need to re-collect
+    // prerequisites of a configured package:
+    //
+    // - For the repointed dependent we still need to collect its prerequisite
+    //   replacements to make sure its dependency constraints are satisfied.
+    //
+    // - If configuration variables are specified for the dependent which has
+    //   any buildfile clauses in the dependencies, then we need to
+    //   re-evaluate them. This can result in a different set of dependencies
+    //   required by this dependent (due to conditional dependencies, etc)
+    //   and, potentially, for its reconfigured existing prerequisites,
+    //   recursively.
+    //
+    // Note that for these cases, as it was said above, we can potentially
+    // fail if the dependent is an orphan, but this is exactly what we need to
+    // do in that case, since we won't be able to re-collect its dependencies.
     //
     // Only a single true dependency alternative can be selected per function
     // call. Such an alternative can only be selected if its index in the
@@ -1259,6 +1270,8 @@ namespace bpkg
       //
       const map<config_package, bool>* rpt_prereq_flags (nullptr);
 
+      assert (ap != nullptr);
+
       // Bail out if this is a configured non-system package and no
       // up/down-grade nor collecting prerequisite replacements are required.
       //
@@ -1274,12 +1287,13 @@ namespace bpkg
         if (i != rpt_depts.end ())
           rpt_prereq_flags = &i->second;
 
-        if (!ud && rpt_prereq_flags == nullptr)
+        if (!ud                         &&
+            rpt_prereq_flags == nullptr &&
+            (pkg.config_vars.empty () ||
+             !has_buildfile_clause (ap->dependencies)))
           return;
       }
 
-      assert (ap != nullptr);
-
       // Iterate over dependencies, trying to unambiguously select a
       // satisfactory dependency alternative for each of them. Fail or
       // postpone the collection if unable to do so.
@@ -3269,7 +3283,10 @@ namespace bpkg
         // not as a system package, then that means we can use its
         // prerequisites list. Otherwise, we use the manifest data.
         //
-        if (src_conf && sp->version == p.available_version ())
+        if (src_conf                              &&
+            sp->version == p.available_version () &&
+            (p.config_vars.empty () ||
+             !has_buildfile_clause (ap->dependencies)))
         {
           for (const auto& p: sp->prerequisites)
           {
diff --git a/tests/common/dependency-alternatives/t8a/fux-1.0.0.tar.gz b/tests/common/dependency-alternatives/t8a/fux-1.0.0.tar.gz
new file mode 100644
index 0000000..7764719
--- /dev/null
+++ b/tests/common/dependency-alternatives/t8a/fux-1.0.0.tar.gz
diff --git a/tests/common/dependency-alternatives/t8a/libbiz-0.1.0.tar.gz b/tests/common/dependency-alternatives/t8a/libbiz-0.1.0.tar.gz
new file mode 100644
index 0000000..575e346
--- /dev/null
+++ b/tests/common/dependency-alternatives/t8a/libbiz-0.1.0.tar.gz
diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript
index 30bbe96..36ffb9d 100644
--- a/tests/pkg-build.testscript
+++ b/tests/pkg-build.testscript
@@ -140,6 +140,7 @@
 # |   |-- libbar-1.0.0.tar.gz
 # |   |-- libbaz-1.0.0.tar.gz
 # |   |-- libbaz-1.1.0.tar.gz
+# |   |-- libbiz-0.1.0.tar.gz
 # |   |-- libbiz-1.0.0.tar.gz
 # |   |-- libbox-1.0.0.tar.gz
 # |   |-- foo-1.0.0.tar.gz      -> {libbar libbaz} ^1.0.0
@@ -149,6 +150,7 @@
 # |   |                            libbaz ^1.0.0 ? ($cxx.target.class != 'windows') config.fax.backend=libbaz,
 # |   |                            libbiz ? ($config.fax.libbiz) config.fax.extras='[b\i$z]',
 # |   |                            libbox ? ($config.fax.libbox && $config.fax.backend == libbaz && $config.fax.extras == '[b\i$z]')
+# |   |-- fux                   -> libbiz ? (!$config.fux.libbiz_old) | libbiz ^0.1.0 ? ($config.fux.libbiz_old)
 # |   `-- repositories.manifest
 # |
 # |-- t9
@@ -3670,6 +3672,125 @@ test.options += --no-progress
         $pkg_drop fax
       }
 
+      : reevaluate-alternatives
+      :
+      {
+        +$clone_cfg
+
+        : add-dependency
+        :
+        {
+          $clone_cfg;
+
+          $* fax 2>!;
+
+          $pkg_status -r >>"EOO";
+            !fax configured 1.0.0
+              $backend_configured
+            EOO
+
+          $* config.fax.libbiz=true -- fax 2>>~%EOE%;
+            disfigured fax/1.0.0
+            fetched libbiz/1.0.0
+            unpacked libbiz/1.0.0
+            configured libbiz/1.0.0
+            configured fax/1.0.0
+            %info: .+fax-1.0.0.+ is up to date%
+            updated fax/1.0.0
+            EOE
+
+          $pkg_status -r >>"EOO";
+            !fax configured 1.0.0
+              $backend_configured
+              libbiz configured 1.0.0
+            EOO
+
+          # While at it, tests that the dependency is properly removed.
+          #
+          $* config.fax.libbiz=false -- fax 2>>~%EOE%;
+            disfigured fax/1.0.0
+            disfigured libbiz/1.0.0
+            purged libbiz/1.0.0
+            configured fax/1.0.0
+            %info: .+fax-1.0.0.+ is up to date%
+            updated fax/1.0.0
+            EOE
+
+          $pkg_status -r >>"EOO";
+            !fax configured 1.0.0
+              $backend_configured
+            EOO
+
+          $pkg_drop fax
+        }
+
+        : downgrade-dependency
+        :
+        {
+          $clone_cfg;
+
+          $* fux 2>!;
+
+          $pkg_status -r >>"EOO";
+            !fux configured 1.0.0
+              libbiz configured 1.0.0
+            EOO
+
+          $* config.fux.libbiz_old=true -- fux 2>>~%EOE%;
+            disfigured fux/1.0.0
+            disfigured libbiz/1.0.0
+            fetched libbiz/0.1.0
+            unpacked libbiz/0.1.0
+            configured libbiz/0.1.0
+            configured fux/1.0.0
+            %info: .+fux-1.0.0.+ is up to date%
+            updated fux/1.0.0
+            EOE
+
+          $pkg_status -r >>"EOO";
+            !fux configured 1.0.0
+              libbiz configured 0.1.0 available 1.0.0
+            EOO
+
+          # While at it, test that the dependency is properly upgraded.
+          #
+          # Note that, unless requested, libbiz is not upgraded, since 0.1.0
+          # is still good for the selected alternative.
+          #
+          $* config.fux.libbiz_old=false -- fux 2>>~%EOE%;
+            disfigured fux/1.0.0
+            configured fux/1.0.0
+            %info: .+fux-1.0.0.+ is up to date%
+            updated fux/1.0.0
+            EOE
+
+          $pkg_status -r >>"EOO";
+            !fux configured 1.0.0
+              libbiz configured 0.1.0 available 1.0.0
+            EOO
+
+          $* fux +{ config.fux.libbiz_old=false } ?libbiz 2>>~%EOE%;
+            disfigured fux/1.0.0
+            disfigured libbiz/0.1.0
+            fetched libbiz/1.0.0
+            unpacked libbiz/1.0.0
+            configured libbiz/1.0.0
+            configured fux/1.0.0
+            %info: .+libbiz-1.0.0.+ is up to date%
+            %info: .+fux-1.0.0.+ is up to date%
+            updated libbiz/1.0.0
+            updated fux/1.0.0
+            EOE
+
+          $pkg_status -r >>"EOO";
+            !fux configured 1.0.0
+              libbiz configured 1.0.0
+            EOO
+
+          $pkg_drop fux
+        }
+      }
+
       : external-package
       :
       if! $remote