From 9fea995ec4eff7d9aad66d2391329ea3bd4dbad5 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 9 Jan 2024 21:40:42 +0300 Subject: Always reconfigure dependency if configuration is specified on command line for it (GH issue #354) --- bpkg/pkg-build.cxx | 118 +++++++-- .../dependency-alternatives/t8a/dox-1.0.0.tar.gz | Bin 0 -> 351 bytes .../dependency-alternatives/t8a/foz-1.0.0.tar.gz | Bin 0 -> 353 bytes .../dependency-alternatives/t8a/fuz-1.0.0.tar.gz | Bin 0 -> 412 bytes tests/pkg-build.testscript | 269 ++++++++++++++++++++- 5 files changed, 360 insertions(+), 27 deletions(-) create mode 100644 tests/common/dependency-alternatives/t8a/dox-1.0.0.tar.gz create mode 100644 tests/common/dependency-alternatives/t8a/foz-1.0.0.tar.gz create mode 100644 tests/common/dependency-alternatives/t8a/fuz-1.0.0.tar.gz diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 89239cc..b430c29 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -403,7 +403,8 @@ namespace bpkg using existing_dependencies = vector; static evaluate_result - evaluate_dependency (database&, + evaluate_dependency (const common_options&, + database&, const shared_ptr&, const optional& desired, bool desired_sys, @@ -417,6 +418,7 @@ namespace bpkg const dependent_constraints&, const existing_dependencies&, const deorphaned_dependencies&, + const build_packages&, bool ignore_unsatisfiable); // If there are no user expectations regarding this dependency, then we give @@ -427,12 +429,14 @@ namespace bpkg // have dependents in the current configurations. // static optional - evaluate_dependency (database& db, + evaluate_dependency (const common_options& o, + database& db, const shared_ptr& sp, const dependency_packages& deps, bool no_move, const existing_dependencies& existing_deps, const deorphaned_dependencies& deorphaned_deps, + const build_packages& pkgs, bool ignore_unsatisfiable) { tracer trace ("evaluate_dependency"); @@ -648,7 +652,8 @@ namespace bpkg dpt_constrs.emplace_back (ddb, move (p), move (dep.constraint)); } - return evaluate_dependency (db, + return evaluate_dependency (o, + db, sp, dvc, dsys, @@ -662,6 +667,7 @@ namespace bpkg dpt_constrs, existing_deps, deorphaned_deps, + pkgs, ignore_unsatisfiable); } @@ -689,7 +695,8 @@ namespace bpkg }; static evaluate_result - evaluate_dependency (database& db, + evaluate_dependency (const common_options& o, + database& db, const shared_ptr& sp, const optional& dvc, bool dsys, @@ -703,6 +710,7 @@ namespace bpkg const dependent_constraints& dpt_constrs, const existing_dependencies& existing_deps, const deorphaned_dependencies& deorphaned_deps, + const build_packages& pkgs, bool ignore_unsatisfiable) { tracer trace ("evaluate_dependency"); @@ -916,6 +924,39 @@ namespace bpkg *dov}; }; + auto build_result = [&ddb, dsys, existing, upgrade] (available&& a) + { + return evaluate_result { + ddb, move (a.first), move (a.second), + false /* unused */, + dsys, + existing, + upgrade, + nullopt /* orphan */}; + }; + + // Note that if the selected dependency is the best that we can get, we + // normally issue the "no change" recommendation. However, if the + // configuration variables are specified for this dependency on the + // command line, then we issue the "reconfigure" recommendation instead. + // + // Return true, if the already selected dependency has been specified on + // the command line with the configuration variables, but has not yet been + // built on this pkg-build run. + // + auto reconfigure = [&ddb, &dsp, &nm, dsys, &pkgs] () + { + assert (dsp != nullptr); + + if (!dsys) + { + const build_package* p (pkgs.entered_build (ddb, nm)); + return p != nullptr && !p->action && !p->config_vars.empty (); + } + else + return false; + }; + for (available& af: afs) { shared_ptr& ap (af.first); @@ -937,8 +978,16 @@ namespace bpkg // if (!dsp->system () && !deorphan) { - l5 ([&]{trace << *dsp << ddb << ": best";}); - return no_change (); + if (reconfigure ()) + { + l5 ([&]{trace << *dsp << ddb << ": reconfigure (best)";}); + return build_result (find_available_fragment (o, ddb, dsp)); + } + else + { + l5 ([&]{trace << *dsp << ddb << ": best";}); + return no_change (); + } } // We can not upgrade the package to a stub version, so just skip it. @@ -1081,20 +1130,24 @@ namespace bpkg find (existing_deps.begin (), existing_deps.end (), package_key (ddb, nm)) != existing_deps.end ())) { - l5 ([&]{trace << *dsp << ddb << ": unchanged";}); - return no_change (); + if (reconfigure ()) + { + l5 ([&]{trace << *dsp << ddb << ": reconfigure";}); + return build_result (move (af)); + } + else + { + l5 ([&]{trace << *dsp << ddb << ": unchanged";}); + return no_change (); + } } + else + { + l5 ([&]{trace << *sp << db << ": update to " + << package_string (nm, av, dsys) << ddb;}); - l5 ([&]{trace << *sp << db << ": update to " - << package_string (nm, av, dsys) << ddb;}); - - return evaluate_result { - ddb, move (ap), move (af.second), - false /* unused */, - dsys, - existing, - upgrade, - nullopt /* orphan */}; + return build_result (move (af)); + } } } @@ -1128,8 +1181,16 @@ namespace bpkg { assert (!dsys); // Version cannot be empty for the system package. - l5 ([&]{trace << *dsp << ddb << ": only";}); - return no_change (); + if (reconfigure ()) + { + l5 ([&]{trace << *dsp << ddb << ": reconfigure (only)";}); + return build_result (find_available_fragment (o, ddb, dsp)); + } + else + { + l5 ([&]{trace << *dsp << ddb << ": only";}); + return no_change (); + } } // If the version satisfying the desired dependency version constraint is @@ -1370,11 +1431,13 @@ namespace bpkg // evaluate_dependency() function description for details). // static optional - evaluate_recursive (database& db, + evaluate_recursive (const common_options& o, + database& db, const shared_ptr& sp, const recursive_packages& recs, const existing_dependencies& existing_deps, const deorphaned_dependencies& deorphaned_deps, + const build_packages& pkgs, bool ignore_unsatisfiable, upgrade_dependencies_cache& cache) { @@ -1437,7 +1500,8 @@ namespace bpkg find_existing (db, sp->name, nullopt /* version_constraint */)); optional r ( - evaluate_dependency (db, + evaluate_dependency (o, + db, sp, nullopt /* desired */, false /* desired_sys */, @@ -1451,6 +1515,7 @@ namespace bpkg dpt_constrs, existing_deps, deorphaned_deps, + pkgs, ignore_unsatisfiable)); // Translate the "no change" result into nullopt. @@ -6219,6 +6284,7 @@ namespace bpkg &o, &existing_deps, &deorphaned_deps, + &pkgs, cache = upgrade_dependencies_cache {}] ( database& db, const shared_ptr& sp, @@ -6230,12 +6296,14 @@ namespace bpkg // See if there is an optional dependency upgrade recommendation. // if (!sp->hold_package) - r = evaluate_dependency (db, + r = evaluate_dependency (o, + db, sp, dep_pkgs, o.no_move (), existing_deps, deorphaned_deps, + pkgs, ignore_unsatisfiable); // If none, then see for the recursive dependency upgrade @@ -6245,11 +6313,13 @@ namespace bpkg // configured as such for a reason. // if (!r && !sp->system () && !rec_pkgs.empty ()) - r = evaluate_recursive (db, + r = evaluate_recursive (o, + db, sp, rec_pkgs, existing_deps, deorphaned_deps, + pkgs, ignore_unsatisfiable, cache); diff --git a/tests/common/dependency-alternatives/t8a/dox-1.0.0.tar.gz b/tests/common/dependency-alternatives/t8a/dox-1.0.0.tar.gz new file mode 100644 index 0000000..475f7d6 Binary files /dev/null and b/tests/common/dependency-alternatives/t8a/dox-1.0.0.tar.gz differ diff --git a/tests/common/dependency-alternatives/t8a/foz-1.0.0.tar.gz b/tests/common/dependency-alternatives/t8a/foz-1.0.0.tar.gz new file mode 100644 index 0000000..90506c6 Binary files /dev/null and b/tests/common/dependency-alternatives/t8a/foz-1.0.0.tar.gz differ diff --git a/tests/common/dependency-alternatives/t8a/fuz-1.0.0.tar.gz b/tests/common/dependency-alternatives/t8a/fuz-1.0.0.tar.gz new file mode 100644 index 0000000..03f8f1a Binary files /dev/null and b/tests/common/dependency-alternatives/t8a/fuz-1.0.0.tar.gz differ diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript index 443ce5f..9e25f5f 100644 --- a/tests/pkg-build.testscript +++ b/tests/pkg-build.testscript @@ -217,6 +217,7 @@ # | | libbaz # | |-- dix-0.1.0.tar.gz # | |-- dix-1.0.0.tar.gz -> dax require {config.dax.extras=true} +# | |-- dox-1.0.0.tar.gz -> dax # | |-- dux-1.0.0.tar.gz -> dix # | |-- fax-1.0.0.tar.gz -> libbar ^1.0.0 ? ($cxx.target.class == 'windows') config.fax.backend=libbar | # | | libbaz ^1.0.0 ? ($cxx.target.class != 'windows') config.fax.backend=libbaz, @@ -226,6 +227,8 @@ # | |-- foo-1.0.0.tar.gz -> {libbar libbaz} ^1.0.0 # | |-- fox-1.0.0.tar.gz -> libbar ^1.0.0 | libbaz ^1.0.0 # | |-- fux-1.0.0.tar.gz -> libbiz ? (!$config.fux.libbiz_old) | libbiz ^0.1.0 ? ($config.fux.libbiz_old) +# | |-- fuz-1.0.0.tar.gz -> libfoo +# | |-- foz-1.0.0.tar.gz -> fuz # | |-- tax-1.0.0.tar.gz -> libfoo == 1.0.0 | libfoo == 2.0.0 # | |-- tex-1.0.0.tar.gz -> libfoo prefer{} accept(true) reflect {...} # | |-- tix-1.0.0.tar.gz -> libfoo >= 2.0.0 reflect {...} | libfoo >= 1.0.0 reflect {...} @@ -3212,7 +3215,7 @@ test.arguments += --sys-no-query : satisfy-dependents : : Test resolving a conflict when libfix and libbiz have selected such - : versions of their dependency libbaz, that do not satisfy each other + : versions of their dependency libbaz, that don't satisfy each other : constraints. We resolve the conflict automatically as if by specifying : ?libbaz/0.0.3 on the command line, which satisfies both constraints. : @@ -3470,6 +3473,266 @@ test.arguments += --sys-no-query } } + : reconfigure + : + { + test.arguments += --yes --configure-only + + +$clone_root_cfg + +$rep_fetch $rep/t8a + + : deps-with-buildfile-clause + : + { + $clone_cfg; + + $* dox 2>!; + + $pkg_status -r >>EOO; + !dox configured 1.0.0 + dax configured 1.0.0 + libbaz configured 1.1.0 + EOO + + $* ?dax; # Noop. + + $* { config.dax.extras=true }+ ?dax 2>>EOE; + disfigured dox/1.0.0 + disfigured dax/1.0.0 + fetched libbar/1.0.0 + unpacked libbar/1.0.0 + configured libbar/1.0.0 + configured dax/1.0.0 + configured dox/1.0.0 + EOE + + cat cfg/dax-1.0.0/build/config.build >>~%EOO%; + %.* + config.dax.extras = true + %.* + EOO + + $* ?dax; # Noop. + + cat cfg/dax-1.0.0/build/config.build >>~%EOO%; + %.* + config.dax.extras = true + %.* + EOO + + $pkg_status -r >>EOO; + !dox configured 1.0.0 + dax configured 1.0.0 + libbar configured 1.0.0 + libbaz configured 1.1.0 + EOO + + $* { config.dax.extras=true }+ ?dax 2>>EOE; + disfigured dox/1.0.0 + disfigured dax/1.0.0 + configured dax/1.0.0 + configured dox/1.0.0 + EOE + + $pkg_status -r >>EOO; + !dox configured 1.0.0 + dax configured 1.0.0 + libbar configured 1.0.0 + libbaz configured 1.1.0 + EOO + + $* { config.dax.extras=false }+ ?dax 2>>EOE; + disfigured dox/1.0.0 + disfigured dax/1.0.0 + disfigured libbar/1.0.0 + purged libbar/1.0.0 + configured dax/1.0.0 + configured dox/1.0.0 + EOE + + cat cfg/dax-1.0.0/build/config.build >>~%EOO%; + %.* + config.dax.extras = false + %.* + EOO + + $pkg_status -r >>EOO; + !dox configured 1.0.0 + dax configured 1.0.0 + libbaz configured 1.1.0 + EOO + + # While at it, test that an attempt to reconfigure an orphan dependency + # which has its own dependencies with buildfile clauses fails. + # + $rep_remove $rep/t8a; + + $* { config.dax.extras=true }+ ?dax 2>>/EOE != 0; + error: unknown package dax + info: configuration cfg/ has no repositories + info: use 'bpkg rep-add' to add a repository + EOE + + cp -rp cfg/dax-1.0.0/ dax; + + $rep_add --type dir "$~/dax"; + $rep_fetch; + + $* { config.dax.extras=true }+ ?dax 2>>EOE != 0; + error: package dax/1.0.0 is orphaned + info: explicitly upgrade it to a new version + info: while satisfying dax/1.0.0 + EOE + + $pkg_drop dox + } + + : deps-without-buildfile-clause + : + { + $clone_cfg; + + $* foz 2>!; + + $pkg_status -r >>EOO; + !foz configured 1.0.0 + fuz configured 1.0.0 + libfoo configured 2.0.0 + EOO + + $* ?fuz; # Noop. + + cat cfg/fuz-1.0.0/build/config.build >>~%EOO%; + %.* + config.fuz.extras = false + %.* + EOO + + $* { config.fuz.extras=true }+ ?fuz 2>>EOE; + disfigured foz/1.0.0 + disfigured fuz/1.0.0 + configured fuz/1.0.0 + configured foz/1.0.0 + EOE + + cat cfg/fuz-1.0.0/build/config.build >>~%EOO%; + %.* + config.fuz.extras = true + %.* + EOO + + $* ?fuz; # Noop. + + cat cfg/fuz-1.0.0/build/config.build >>~%EOO%; + %.* + config.fuz.extras = true + %.* + EOO + + $* { config.fuz.extras=false }+ ?fuz 2>>EOE; + disfigured foz/1.0.0 + disfigured fuz/1.0.0 + configured fuz/1.0.0 + configured foz/1.0.0 + EOE + + cat cfg/fuz-1.0.0/build/config.build >>~%EOO%; + %.* + config.fuz.extras = false + %.* + EOO + + # While at it, test that we can also reconfigure an orphan with its own + # dependencies but without buildfile clauses. + # + $rep_remove $rep/t8a; + + cp -rp cfg/fuz-1.0.0/ fuz; + sed -i -e 's/(version:) 1.0.0/\1 2.0.0/' fuz/manifest; + + $rep_add --type dir "$~/fuz"; + $rep_fetch; + + $* { config.fuz.extras=true }+ ?fuz 2>>EOE; + disfigured foz/1.0.0 + disfigured fuz/1.0.0 + configured fuz/1.0.0 + configured foz/1.0.0 + EOE + + cat cfg/fuz-1.0.0/build/config.build >>~%EOO%; + %.* + config.fuz.extras = true + %.* + EOO + + $pkg_status -r >>EOO; + !foz configured 1.0.0 + fuz configured 1.0.0 available 2.0.0 + libfoo configured 2.0.0 + EOO + + $pkg_drop foz + } + + : no-deps + : + { + $clone_cfg; + + $* fuz 2>!; + + $* ?libfoo; # Noop. + + cat cfg/libfoo-2.0.0/build/config.build >>~%EOO%; + %.* + config.libfoo.protocol = 2 + %.* + EOO + + $* { config.libfoo.protocol=1 }+ ?libfoo 2>>EOE; + disfigured fuz/1.0.0 + disfigured libfoo/2.0.0 + configured libfoo/2.0.0 + configured fuz/1.0.0 + EOE + + cat cfg/libfoo-2.0.0/build/config.build >>~%EOO%; + %.* + config.libfoo.protocol = 1 + %.* + EOO + + $* ?libfoo; # Noop. + + # While at it, test that we can also reconfigure an orphan without + # dependencies. + # + $rep_remove $rep/t8a; + + cp -rp cfg/libfoo-2.0.0/ libfoo; + sed -i -e 's/(version:) 2.0.0/\1 3.0.0/' libfoo/manifest; + + $rep_add --type dir "$~/libfoo"; + $rep_fetch; + + $* { config.libfoo.protocol=3 }+ ?libfoo 2>>EOE; + disfigured fuz/1.0.0 + disfigured libfoo/2.0.0 + configured libfoo/2.0.0 + configured fuz/1.0.0 + EOE + + cat cfg/libfoo-2.0.0/build/config.build >>~%EOO%; + %.* + config.libfoo.protocol = 3 + %.* + EOO + + $pkg_drop fuz + } + } + : refine : { @@ -24132,7 +24395,7 @@ test.arguments += --sys-no-query # cps = for p: $ps - timeout 60 + timeout 120 if $* $p 2>&1 | $warn_to_info 2>! cps += $p end @@ -24142,7 +24405,7 @@ test.arguments += --sys-no-query # them into dependencies. # for p: $cps - timeout 60 + timeout 120 $* ?$p 2>! end end -- cgit v1.1