aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2024-05-16 17:53:47 +0300
committerKaren Arutyunov <karen@codesynthesis.com>2024-05-17 10:08:06 +0300
commit801593731e61efba2141cdad1ff0559e501f97ae (patch)
treef4f320aa794bb4a561961a200075cd07cbe20ec1
parent3271ee28840ff1ab905943649acbcea262cd9b2b (diff)
Prefer version replacement over greater version in pkg-build's collect_build()
Note that this fixes hanging described in the GH issue #318. The fixed bpkg now fails with the 'unable to downgrade package' error instead, while the dependency versions resolution is possible.
-rw-r--r--bpkg/pkg-build-collect.cxx124
1 files changed, 71 insertions, 53 deletions
diff --git a/bpkg/pkg-build-collect.cxx b/bpkg/pkg-build-collect.cxx
index 6f1195c..436d629 100644
--- a/bpkg/pkg-build-collect.cxx
+++ b/bpkg/pkg-build-collect.cxx
@@ -1490,78 +1490,87 @@ namespace bpkg
// applied. Ignore the replacement if its version doesn't satisfy the
// dependency constraints specified by the caller. Also ignore if this is
// a drop and the required-by package names of the specified build package
- // object have the "required by dependents" semantics
+ // object have the "required by dependents" semantics.
//
auto vi (replaced_vers.find (pk));
+ const version* replacement_version (nullptr);
- if (vi != replaced_vers.end () && !vi->second.replaced)
+ if (vi != replaced_vers.end ())
{
- l5 ([&]{trace << "apply version replacement for "
- << pkg.available_name_version_db ();});
-
replaced_version& v (vi->second);
if (v.available != nullptr)
+ replacement_version = (v.system
+ ? v.available->system_version (pk.db)
+ : &v.available->version);
+
+ if (!vi->second.replaced)
{
- const version& rv (v.system
- ? *v.available->system_version (pk.db)
- : v.available->version);
+ l5 ([&]{trace << "apply version replacement for "
+ << pkg.available_name_version_db ();});
- bool replace (true);
- for (const constraint_type& c: pkg.constraints)
+ if (v.available != nullptr)
{
- if (!satisfies (rv, c.value))
+ assert (replacement_version != nullptr);
+
+ const version& rv (*replacement_version);
+
+ bool replace (true);
+ for (const constraint_type& c: pkg.constraints)
{
- replace = false;
+ if (!satisfies (rv, c.value))
+ {
+ replace = false;
- l5 ([&]{trace << "replacement to " << rv << " is denied since "
- << c.dependent << " depends on (" << pk.name << ' '
- << c.value << ')';});
+ l5 ([&]{trace << "replacement to " << rv << " is denied since "
+ << c.dependent << " depends on (" << pk.name << ' '
+ << c.value << ')';});
+ }
}
- }
- if (replace)
- {
- v.replaced = true;
+ if (replace)
+ {
+ v.replaced = true;
- pkg.available = v.available;
- pkg.repository_fragment = v.repository_fragment;
- pkg.system = v.system;
+ pkg.available = v.available;
+ pkg.repository_fragment = v.repository_fragment;
+ pkg.system = v.system;
- l5 ([&]{trace << "replacement: "
- << pkg.available_name_version_db ();});
+ l5 ([&]{trace << "replacement: "
+ << pkg.available_name_version_db ();});
+ }
}
- }
- else
- {
- if (!pkg.required_by_dependents)
+ else
{
- v.replaced = true;
+ if (!pkg.required_by_dependents)
+ {
+ v.replaced = true;
- l5 ([&]{trace << "replacement: drop";});
+ l5 ([&]{trace << "replacement: drop";});
- // We shouldn't be replacing a package build with the drop if someone
- // depends on this package.
- //
- assert (pkg.selected != nullptr);
+ // We shouldn't be replacing a package build with the drop if someone
+ // depends on this package.
+ //
+ assert (pkg.selected != nullptr);
- collect_drop (options, pkg.db, pkg.selected, replaced_vers);
- return nullptr;
- }
- else
- {
- assert (!pkg.required_by.empty ());
+ collect_drop (options, pkg.db, pkg.selected, replaced_vers);
+ return nullptr;
+ }
+ else
+ {
+ assert (!pkg.required_by.empty ());
- l5 ([&]
- {
- diag_record dr (trace);
- dr << "replacement to drop is denied since " << pk
- << " is required by ";
- for (auto b (pkg.required_by.begin ()), i (b);
- i != pkg.required_by.end ();
- ++i)
- dr << (i != b ? ", " : "") << *i;
- });
+ l5 ([&]
+ {
+ diag_record dr (trace);
+ dr << "replacement to drop is denied since " << pk
+ << " is required by ";
+ for (auto b (pkg.required_by.begin ()), i (b);
+ i != pkg.required_by.end ();
+ ++i)
+ dr << (i != b ? ", " : "") << *i;
+ });
+ }
}
}
}
@@ -1629,19 +1638,28 @@ namespace bpkg
build_package* p2 (&pkg);
// Pick with the following preference order: user selection over
- // implicit one, source package over a system one, newer version over
- // an older one. So get the preferred into p1 and the other into p2.
+ // implicit one, source package over a system one, replacement version
+ // over a non-replacement one, newer version over an older one. So get
+ // the preferred into p1 and the other into p2.
//
{
+ const version& v1 (p1->available_version ());
+ const version& v2 (p2->available_version ());
+
int us (p1->user_selection () - p2->user_selection ());
int sf (p1->system - p2->system);
+ int rv (replacement_version != nullptr
+ ? (v1 == *replacement_version) - (v2 == *replacement_version)
+ : 0);
if (us < 0 ||
(us == 0 && sf > 0) ||
(us == 0 &&
sf == 0 &&
- p2->available_version () > p1->available_version ()))
+ (rv < 0 || (rv == 0 && v2 > v1))))
+ {
swap (p1, p2);
+ }
}
// If the versions differ, pick the satisfactory one and if both are