From df79f83187ecb91569b254ecbd90f46edabe8826 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 18 Mar 2022 21:01:55 +0300 Subject: Fix pkg-build to re-evaluate buildfile clauses in dependencies if configuration variables are specified for dependent --- bpkg/pkg-build.cxx | 49 ++++++--- .../dependency-alternatives/t8a/fux-1.0.0.tar.gz | Bin 0 -> 466 bytes .../t8a/libbiz-0.1.0.tar.gz | Bin 0 -> 350 bytes tests/pkg-build.testscript | 121 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 16 deletions(-) create mode 100644 tests/common/dependency-alternatives/t8a/fux-1.0.0.tar.gz create mode 100644 tests/common/dependency-alternatives/t8a/libbiz-0.1.0.tar.gz 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* 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 Binary files /dev/null and b/tests/common/dependency-alternatives/t8a/fux-1.0.0.tar.gz differ 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 Binary files /dev/null and b/tests/common/dependency-alternatives/t8a/libbiz-0.1.0.tar.gz differ 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 -- cgit v1.1