aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2022-05-23 16:33:54 +0300
committerKaren Arutyunov <karen@codesynthesis.com>2022-05-23 18:15:21 +0300
commita895b2f17e2b1f8aed3f7b5ca3a9a6c521443645 (patch)
treecdf58bafdf14b5f4901e99d37e3240625c80194f
parent597d509ff07513ce1f90abf13c21f0e8d44fc8e1 (diff)
Treat replacement of existing dependent as version replacement as well
-rw-r--r--bpkg/pkg-build.cxx113
-rw-r--r--tests/pkg-build.testscript170
2 files changed, 224 insertions, 59 deletions
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 91f32c3..fc049a6 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -1354,11 +1354,14 @@ namespace bpkg
ddi.add (dependency (position, move (deps)));
- // Conceptually we can only move from existing to non-existing (e.g.,
- // due to a upgrade/downgrade later). But that case is handled via
- // the version replacement rollback.
- //
- assert (ddi.existing == existing);
+ // Conceptually, on the first glance, we can only move from existing
+ // to non-existing (e.g., due to a upgrade/downgrade later) and that
+ // case is handled via the version replacement rollback. However,
+ // after re-evaluation the existing dependent is handled similar to
+ // the new dependent and we can potentially up-negotiate the
+ // dependency configuration for it.
+ //
+ assert (ddi.existing || !existing);
}
else
{
@@ -1845,9 +1848,14 @@ namespace bpkg
if (i != end ())
{
// Note that the cluster with a shadow cluster is by definition
- // either being negotiated or has been negotiated.
+ // either being negotiated or has been negotiated. Actually, there
+ // is also a special case when we didn't negotiate the configuration
+ // yet and are in the process of re-evaluating existing dependents.
+ // Note though, that in this case we have already got the try/catch
+ // frame corresponding to the cluster negotiation (see
+ // collect_build_postponed() for details).
//
- assert (i->negotiated);
+ assert (i->depth != 0);
ri = i;
@@ -2216,6 +2224,8 @@ namespace bpkg
// instead. Add entry to replaced_vers and throw replace_version if the
// existing version needs to be replaced but the new version cannot be
// re-collected recursively in-place (see replaced_versions for details).
+ // Also add entry and throw if the existing dependent needs to be
+ // replaced.
//
// Optionally, pass the function which verifies the chosen package
// version. It is called before replace_version is potentially thrown or
@@ -2255,11 +2265,11 @@ namespace bpkg
const function<add_priv_cfg_function>& apc,
bool initial_collection,
replaced_versions& replaced_vers,
+ postponed_configurations& postponed_cfgs,
build_package_refs* dep_chain = nullptr,
postponed_packages* postponed_repo = nullptr,
postponed_packages* postponed_alts = nullptr,
postponed_dependencies* postponed_deps = nullptr,
- postponed_configurations* postponed_cfgs = nullptr,
const function<verify_package_build_function>& vpb = nullptr)
{
using std::swap; // ...and not list::swap().
@@ -2271,8 +2281,7 @@ namespace bpkg
bool recursive (dep_chain != nullptr);
assert ((postponed_repo != nullptr) == recursive &&
(postponed_alts != nullptr) == recursive &&
- (postponed_deps != nullptr) == recursive &&
- (postponed_cfgs != nullptr) == recursive);
+ (postponed_deps != nullptr) == recursive);
// Only builds are allowed here.
//
@@ -2315,6 +2324,25 @@ namespace bpkg
}
}
+ // Add the version replacement entry, call the verification function if
+ // specified, and throw replace_version.
+ //
+ auto replace_ver = [&cp, &vpb, &vi, &replaced_vers]
+ (const build_package& p)
+ {
+ replaced_version rv (p.available, p.repository_fragment, p.system);
+
+ if (vi != replaced_vers.end ())
+ vi->second = move (rv);
+ else
+ replaced_vers.emplace (move (cp), move (rv));
+
+ if (vpb)
+ vpb (p, true /* scratch */);
+
+ throw replace_version ();
+ };
+
auto i (map_.find (cp));
// If we already have an entry for this package name, then we have to
@@ -2481,21 +2509,7 @@ namespace bpkg
<< p1->available_name_version_db ();});
if (scratch)
- {
- replaced_version rv (p1->available,
- p1->repository_fragment,
- p1->system);
-
- if (vi != replaced_vers.end ())
- vi->second = move (rv);
- else
- replaced_vers.emplace (move (cp), move (rv));
-
- if (vpb)
- vpb (*p1, true /* scratch */);
-
- throw replace_version ();
- }
+ replace_ver (*p1);
}
else
{
@@ -2514,6 +2528,27 @@ namespace bpkg
}
else
{
+ // Treat the replacement of the existing dependent as a version
+ // replacement as well. This way we will not be treating the dependent
+ // as an existing on the re-collection (see
+ // query_existing_dependents() for details).
+ //
+ // Note: an existing dependent may not be configured as system.
+ //
+ if (pkg.selected != nullptr &&
+ (pkg.selected->version != pkg.available_version () ||
+ pkg.system))
+ {
+
+ for (const postponed_configuration& cfg: postponed_cfgs)
+ {
+ auto i (cfg.dependents.find (cp));
+
+ if (i != cfg.dependents.end () && i->second.existing)
+ replace_ver (pkg);
+ }
+ }
+
// This is the first time we are adding this package name to the map.
//
l4 ([&]{trace << "add " << pkg.available_name_version_db ();});
@@ -2559,7 +2594,7 @@ namespace bpkg
postponed_alts,
0 /* max_alt_index */,
*postponed_deps,
- *postponed_cfgs);
+ postponed_cfgs);
return &p;
}
@@ -3836,7 +3871,15 @@ namespace bpkg
bool f (prq.selected->hold_version);
bool w (!f && prq.selected->hold_package);
- if (f || w || verb >= 2)
+ // Note that there is no sense to warn or inform the user if
+ // we are about to start re-collection from scratch.
+ //
+ // @@ It seems that we may still warn/inform multiple times
+ // about the same package if we start from scratch. The
+ // intermediate diagnostics can probably be irrelevant to
+ // the final result.
+ //
+ if (f || ((w || verb >= 2) && !scratch))
{
const version& av (p.available_version ());
@@ -3888,11 +3931,11 @@ namespace bpkg
apc,
initial_collection,
replaced_vers,
+ postponed_cfgs,
nullptr /* dep_chain */,
nullptr /* postponed_repo */,
nullptr /* postponed_alts */,
nullptr /* postponed_deps */,
- nullptr /* postponed_cfgs */,
verify));
config_package dcp (b.db, b.available->id.name);
@@ -4723,11 +4766,11 @@ namespace bpkg
apc,
true /* initial_collection */,
replaced_vers,
+ postponed_cfgs,
&dep_chain,
&postponed_repo,
&postponed_alts,
- &postponed_deps,
- &postponed_cfgs);
+ &postponed_deps);
}
}
@@ -5234,7 +5277,8 @@ namespace bpkg
rpt_depts,
apc,
true /* initial_collection */,
- replaced_vers);
+ replaced_vers,
+ postponed_cfgs);
build_package* b (entered_build (cp));
assert (b != nullptr);
@@ -9738,7 +9782,8 @@ namespace bpkg
rpt_depts,
add_priv_cfg,
true /* initial_collection */,
- replaced_vers);
+ replaced_vers,
+ postponed_cfgs);
// Collect all the prerequisites of the user selection.
//
@@ -9894,11 +9939,11 @@ namespace bpkg
add_priv_cfg,
true /* initial_collection */,
replaced_vers,
+ postponed_cfgs,
&dep_chain,
&postponed_repo,
&postponed_alts,
- &postponed_deps,
- &postponed_cfgs);
+ &postponed_deps);
}
}
diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript
index 2458937..e40c0f4 100644
--- a/tests/pkg-build.testscript
+++ b/tests/pkg-build.testscript
@@ -5473,7 +5473,6 @@ test.options += --no-progress
: postpone-existing
:
- if false
{
$clone_cfg;
@@ -5487,17 +5486,7 @@ test.options += --no-progress
#
$* fix 2>!;
- # @@ So here we should have been in a situation that after libfoo is
- # postponed with {foo^ | libfoo->{foo/1,1}} creation and we got
- # to negotiating it, by that time foo has stopped to be an
- # "existing" dependent since it was downgraded via fix/0.1.0. The
- # natural fix would be adding a record somewhere and start from
- # scratch (maybe add into postponed_poss with position {0,0}). We
- # however crash on assertion failure earlier (see below). Sounds
- # like we need to scratch earlier or maybe get rid of assertion
- # and deal with the existing flag there.
- #
- $* libfoo/0.1.0 fix/0.1.0 2>>~%EOE%
+ $* libfoo/0.1.0 fix/0.1.0 2>>~%EOE%;
%.*
trace: pkg_build: refine package collection/plan execution from scratch
%.*
@@ -5509,6 +5498,40 @@ test.options += --no-progress
%.*
trace: collect_build_prerequisites: begin fix/0.1.0
%.*
+ trace: pkg_build: refine package collection/plan execution from scratch
+ %.*
+ trace: collect_build: add libfoo/0.1.0
+ trace: collect_build: add fix/0.1.0
+ %.*
+ trace: collect_build_prerequisites: skip expected to be built existing dependent foo of dependency libfoo
+ trace: collect_build_prerequisites: begin libfoo/0.1.0
+ trace: collect_build_prerequisites: end libfoo/0.1.0
+ %.*
+ trace: collect_build_prerequisites: begin fix/0.1.0
+ %.*
+ trace: collect_build: apply version replacement for foo/0.1.0
+ trace: collect_build: replacement: foo/0.1.0
+ trace: collect_build: add foo/0.1.0
+ info: package fix dependency on (foo == 0.1.0) is forcing downgrade of foo/1.0.0 to 0.1.0
+ trace: collect_build_prerequisites: no cfg-clause for dependency foo/0.1.0 of dependent fix/0.1.0
+ %.*
+ trace: collect_build_prerequisites: skip being built existing dependent fix of dependency foo
+ trace: collect_build_prerequisites: begin foo/0.1.0
+ %.*
+ trace: collect_build: pick libfoo/0.1.0 over libfoo/1.0.0
+ trace: collect_build_prerequisites: cannot cfg-postpone dependency libfoo/0.1.0 of dependent foo/0.1.0 (collected prematurely), throwing postpone_dependency
+ trace: pkg_build: collection failed due to prematurely collected dependency (libfoo), retry from scratch
+ %.*
+ trace: pkg_build: refine package collection/plan execution from scratch
+ %.*
+ trace: collect_build: add libfoo/0.1.0
+ trace: collect_build: add fix/0.1.0
+ trace: pkg_build: dep-postpone user-specified libfoo
+ %.*
+ trace: collect_build_prerequisites: begin fix/0.1.0
+ %.*
+ trace: collect_build: apply version replacement for foo/0.1.0
+ trace: collect_build: replacement: foo/0.1.0
trace: collect_build: add foo/0.1.0
info: package fix dependency on (foo == 0.1.0) is forcing downgrade of foo/1.0.0 to 0.1.0
trace: collect_build_prerequisites: no cfg-clause for dependency foo/0.1.0 of dependent fix/0.1.0
@@ -5518,9 +5541,37 @@ test.options += --no-progress
%.*
trace: collect_build: pick libfoo/0.1.0 over libfoo/1.0.0
trace: collect_build_prerequisites: cfg-postpone dependency libfoo/0.1.0 of dependent foo/0.1.0
- trace: postponed_configurations::add: add {foo 1,1: libfoo} to {foo^ | libfoo->{foo/1,1}}
- bpkg: /home/karen/work/build2/bpkg/bpkg/pkg-build.cxx:1361: void bpkg::postponed_configuration::add(bpkg::config_package&&, bool, std::pair<long unsigned int, long unsigned int>, bpkg::postponed_configuration::packages&&): Assertion 'ddi.existing == existing' failed.
+ trace: postponed_configurations::add: create {foo | libfoo->{foo/1,1}}
+ trace: collect_build_prerequisites: postpone foo/0.1.0
+ trace: collect_build_prerequisites: end fix/0.1.0
+ trace: collect_build_postponed (0): begin
+ trace: collect_build_postponed (1): begin {foo | libfoo->{foo/1,1}}
+ %.*
+ trace: collect_build_postponed (1): skip being built existing dependent foo of dependency libfoo
+ trace: collect_build_postponed (1): cfg-negotiate begin {foo | libfoo->{foo/1,1}}
+ trace: collect_build_postponed (1): recursively collect cfg-negotiated dependencies
+ trace: collect_build_prerequisites: begin libfoo/0.1.0
+ trace: collect_build_prerequisites: end libfoo/0.1.0
+ trace: collect_build_postponed (1): recursively collect cfg-negotiated dependents
+ trace: collect_build_postponed (1): select cfg-negotiated dependency alternative for dependent foo/0.1.0
+ trace: collect_build_prerequisites: resume foo/0.1.0
+ trace: collect_build_prerequisites: end foo/0.1.0
+ trace: collect_build_postponed (1): cfg-negotiate end {foo | libfoo->{foo/1,1}}!
+ trace: collect_build_postponed (1): end {foo | libfoo->{foo/1,1}}
+ trace: collect_build_postponed (0): end
+ %.*
+ trace: execute_plan: simulate: yes
+ %.*
EOE
+
+ $pkg_status -r >>EOO;
+ !libfoo configured !0.1.0 available 1.0.0
+ !fix configured !0.1.0 available 1.0.0
+ foo configured 0.1.0 available 1.0.0
+ !libfoo configured !0.1.0 available 1.0.0
+ EOO
+
+ $pkg_drop fix libfoo --drop-dependent
}
}
@@ -5562,22 +5613,14 @@ test.options += --no-progress
EOE
}
- : re-evaluating-dependent
+ : up-negotiate
:
- if false
{
$clone_cfg;
$* tex 2>!;
- # @@ Seems we should get rid of this assertion (as suggested above)
- # since the situation is pretty valid here: we just should
- # up-negotiate 'tex: depends: libfoo(c)' after existing dependent
- # tex was re-evaluated and become a regular dependent in a sence
- # (we still need to keep the existing flag for it; see
- # collect_build_prerequisites()).
- #
- $* baz/0.1.0 2>>~%EOE%
+ $* baz/0.1.0 2>>~%EOE%;
%.*
trace: pkg_build: refine package collection/plan execution from scratch
%.*
@@ -5620,8 +5663,85 @@ test.options += --no-progress
trace: collect_build: pick libfoo/0.1.0 over libfoo/1.0.0
trace: collect_build_prerequisites: cfg-postpone dependency libfoo/0.1.0 of dependent tex/1.0.0
trace: postponed_configurations::add: add {tex 2,1: libfoo} to {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1}}?
- bpkg: /home/karen/work/build2/bpkg/bpkg/pkg-build.cxx:1361: void bpkg::postponed_configuration::add(bpkg::config_package&&, bool, std::pair<long unsigned int, long unsigned int>, bpkg::postponed_configuration::packages&&): Assertion 'ddi.existing == existing' failed.
+ trace: collect_build_prerequisites: cfg-postponing dependent tex/1.0.0 involves negotiated configurations and results in {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1 tex/2,1}}?, throwing retry_configuration
+ trace: collect_build_postponed (0): cfg-negotiation of {baz | libbar->{baz/1,1} libfoo->{baz/1,1}} failed due to dependent tex, adding shadow dependent and re-negotiating
+ trace: collect_build_postponed (1): begin {baz | libbar->{baz/1,1} libfoo->{baz/1,1}}
+ %.*
+ trace: collect_build_postponed (1): re-evaluate existing dependents for {baz | libbar->{baz/1,1} libfoo->{baz/1,1}}
+ trace: collect_build: add tex/1.0.0
+ trace: collect_build_prerequisites: reeval tex/1.0.0
+ %.*
+ trace: collect_build: pick libbar/0.1.0 over libbar/1.0.0
+ trace: postponed_configurations::add: add {tex^ 1,1: libbar} to {baz | libbar->{baz/1,1} libfoo->{baz/1,1}}
+ trace: collect_build_prerequisites: re-evaluating dependent tex/1.0.0 results in {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1}}
+ trace: collect_build_prerequisites: re-evaluated tex/1.0.0
+ trace: collect_build_postponed (1): cfg-negotiate begin {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1}}
+ trace: collect_build_postponed (1): recursively collect cfg-negotiated dependencies
+ trace: collect_build_prerequisites: begin libbar/0.1.0
+ trace: collect_build_prerequisites: end libbar/0.1.0
+ trace: collect_build_prerequisites: begin libfoo/0.1.0
+ trace: collect_build_prerequisites: end libfoo/0.1.0
+ trace: collect_build_postponed (1): recursively collect cfg-negotiated dependents
+ trace: collect_build_postponed (1): select cfg-negotiated dependency alternative for dependent baz/0.1.0
+ trace: collect_build_prerequisites: resume baz/0.1.0
+ trace: collect_build_prerequisites: end baz/0.1.0
+ trace: collect_build_postponed (1): select cfg-negotiated dependency alternative for dependent tex/1.0.0
+ trace: collect_build_prerequisites: resume tex/1.0.0
+ %.*
+ trace: collect_build: pick libfoo/0.1.0 over libfoo/1.0.0
+ trace: collect_build_prerequisites: cfg-postpone dependency libfoo/0.1.0 of dependent tex/1.0.0
+ trace: postponed_configurations::add: add {tex 2,1: libfoo} to {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1}}?
+ trace: collect_build_prerequisites: dependent tex/1.0.0 is a shadow dependent for {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1 tex/2,1}}?
+ trace: collect_build_prerequisites: cfg-postponing dependent tex/1.0.0 involves non-negotiated configurations and results in {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1 tex/2,1}}?, throwing merge_configuration
+ trace: collect_build_postponed (0): cfg-negotiation of {baz | libbar->{baz/1,1} libfoo->{baz/1,1}} failed due to non-negotiated clusters, force-merging based on shadow cluster {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1 tex/2,1}}?
+ trace: collect_build_postponed (1): begin {baz | libbar->{baz/1,1} libfoo->{baz/1,1}}
+ %.*
+ trace: collect_build_postponed (1): re-evaluate existing dependents for {baz | libbar->{baz/1,1} libfoo->{baz/1,1}}
+ trace: collect_build: add tex/1.0.0
+ trace: collect_build_prerequisites: reeval tex/1.0.0
+ %.*
+ trace: collect_build: pick libbar/0.1.0 over libbar/1.0.0
+ trace: postponed_configurations::add: add {tex^ 1,1: libbar} to {baz | libbar->{baz/1,1} libfoo->{baz/1,1}} (shadow cluster-based)
+ trace: collect_build_prerequisites: re-evaluating dependent tex/1.0.0 results in {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1}}
+ trace: collect_build_prerequisites: re-evaluated tex/1.0.0
+ trace: collect_build_postponed (1): cfg-negotiate begin {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1}}
+ trace: collect_build_postponed (1): recursively collect cfg-negotiated dependencies
+ trace: collect_build_prerequisites: begin libbar/0.1.0
+ trace: collect_build_prerequisites: end libbar/0.1.0
+ trace: collect_build_prerequisites: begin libfoo/0.1.0
+ trace: collect_build_prerequisites: end libfoo/0.1.0
+ trace: collect_build_postponed (1): recursively collect cfg-negotiated dependents
+ trace: collect_build_postponed (1): select cfg-negotiated dependency alternative for dependent baz/0.1.0
+ trace: collect_build_prerequisites: resume baz/0.1.0
+ trace: collect_build_prerequisites: end baz/0.1.0
+ trace: collect_build_postponed (1): select cfg-negotiated dependency alternative for dependent tex/1.0.0
+ trace: collect_build_prerequisites: resume tex/1.0.0
+ %.*
+ trace: collect_build: pick libfoo/0.1.0 over libfoo/1.0.0
+ trace: collect_build_prerequisites: cfg-postpone dependency libfoo/0.1.0 of dependent tex/1.0.0
+ trace: postponed_configurations::add: add {tex 2,1: libfoo} to {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1}}? (shadow cluster-based)
+ trace: collect_build_prerequisites: dependent tex/1.0.0 is a shadow dependent for {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1 tex/2,1}}?
+ trace: collect_build_prerequisites: configuration for cfg-postponed dependencies of dependent tex/1.0.0 is negotiated
+ trace: collect_build_prerequisites: dependency libfoo/0.1.0 of dependent tex/1.0.0 is already (being) recursively collected, skipping
+ trace: collect_build_prerequisites: end tex/1.0.0
+ trace: collect_build_postponed (1): cfg-negotiate end {baz tex^ | libbar->{baz/1,1 tex/1,1} libfoo->{baz/1,1 tex/2,1}}!
+ trace: collect_build_postponed (1): end {baz | libbar->{baz/1,1} libfoo->{baz/1,1}}
+ trace: collect_build_postponed (0): end
+ %.*
+ trace: execute_plan: simulate: yes
+ %.*
EOE
+
+ $pkg_status -r >>EOO;
+ !tex configured 1.0.0
+ libbar configured 0.1.0 available 1.0.0
+ libfoo configured 0.1.0 available 1.0.0
+ !baz configured !0.1.0 available 1.0.0
+ libbar configured 0.1.0 available 1.0.0
+ libfoo configured 0.1.0 available 1.0.0
+ EOO
+
+ $pkg_drop tex baz
}
}
}