aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--bpkg/package-skeleton.cxx20
-rw-r--r--bpkg/package-skeleton.hxx14
-rw-r--r--bpkg/pkg-build.cxx255
3 files changed, 188 insertions, 101 deletions
diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx
index 80df830..4813651 100644
--- a/bpkg/package-skeleton.cxx
+++ b/bpkg/package-skeleton.cxx
@@ -138,6 +138,26 @@ namespace bpkg
// that it can be loaded if necessary.
}
+ void package_skeleton::
+ reset ()
+ {
+ assert (db_ != nullptr); // Cannot be called after collect_config().
+
+ rs_ = nullptr;
+ ctx_ = nullptr; // Free.
+
+ cmd_vars_.clear ();
+ cmd_vars_cache_ = false;
+
+ dependent_vars_.clear ();
+ reflect_vars_.clear ();
+ reflect_frag_.clear ();
+
+ dependency_reflect_.clear ();
+ dependency_reflect_index_ = 0;
+ dependency_reflect_pending_ = 0;
+ }
+
package_skeleton::
package_skeleton (const common_options& co,
database& db,
diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx
index 0fc371d..d7cafe9 100644
--- a/bpkg/package-skeleton.hxx
+++ b/bpkg/package-skeleton.hxx
@@ -73,14 +73,13 @@ namespace bpkg
// The following functions should be called in the following sequence
// (* -- zero or more, ? -- zero or one):
//
- // * reload_defaults()
- // * verify_sensible()
+ // * reload_defaults() | verify_sensible()
// ? dependent_config()
- // * evaluate_enable() | evaluate_reflect()
+ // * evaluate_*()
// collect_config()
//
// Note that a copy of the skeleton is expected to continue with the
- // sequence rather than starting from scratch.
+ // sequence rather than starting from scratch, unless reset() is called.
//
public:
// Reload the default values and type information for configuration
@@ -136,6 +135,13 @@ namespace bpkg
evaluate_require (const dependency_configurations&,
const string&, size_t depends_index);
+ // Reset the skeleton to the start of the call sequence.
+ //
+ // Note that this function cannot be called after collect_config().
+ //
+ void
+ reset ();
+
// Return the accumulated configuration variables (first) and project
// configuration variable sources (second). Note that the arrays are not
// necessarily parallel (config_vars may contain non-project variables).
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx
index 4194e3e..40228db 100644
--- a/bpkg/pkg-build.cxx
+++ b/bpkg/pkg-build.cxx
@@ -4478,7 +4478,7 @@ namespace bpkg
// everything in this cluster is already negotiated) and we can
// return true (no need to postpone any further steps).
//
- // But if we do have clusters not yet negotiated, or, worse,
+ // But if we merged clusters not yet negotiated, or, worse,
// being in the middle of negotiation, then we need to get this
// merged cluster into the fully negotiated state. The we do it
// is by throwing merge_configuration (see below).
@@ -4543,115 +4543,165 @@ namespace bpkg
throw merge_configuration {cfg.depth};
}
- if (r.second && !cfg.contains_shadow_dependent (pk, dp))
+ // Up-negotiate the configuration and if it has changed, throw
+ // retry_configuration to the try/catch frame corresponding to
+ // the negotiation of the outermost merged cluster in order to
+ // retry the same steps (potentially refining the configuration
+ // as we go along) and likely (but not necessarily) ending up
+ // here again, at which point we up-negotiate again with the
+ // expectation that the configuration won't change (but if it
+ // does, then we throw again and do another refinement pass).
+ //
+ // In a sense, semantically, we should act like a one more
+ // iteration of the initial negotiation loop with the exception
+ // acting like a request to restart the refinement process from
+ // the beginning.
+ //
+ bool changed;
{
- assert (*r.second); // Would't be here otherwise.
-
- // The "first time" case.
+ // Similar to initial negotiation, resolve package skeletons
+ // for this dependent and its dependencies.
//
+ package_skeleton* dept;
+ {
+ build_package* b (entered_build (pk));
+ assert (b != nullptr && b->skeleton);
+ dept = &*b->skeleton;
+ }
- // @@ TODO: if configuration has not changed, then we don't
- // need to throw. Do it at the end (there is no harm in
- // throwing even if not needed and it will change test
- // trace).
- //
- // Also, we may have to throw multiple times if the
- // configuration gets refined... In a sense, semantically,
- // we should act like a one more iteration of the initial
- // negotiation loop with the exception acting like a
- // request to restart the refinement process from the
- // beginning.
- //
- // It feels like the unchanged case semantics should be
- // like falling through to the "second time" case. Though
- // I wonder if there could be some missing steps, like
- // collecting dependencies? Doesn't feel like it since
- // we are re-using existing configuration which presumably
- // means all this has already been done.
- //
- // Hm, won't we need to "reset" dependency package
- // skeletons (which have by now potentially seen
- // evaluate_*() calls) in order to negotiate. And if the
- // configuration hasn't changed, then how will we restore
- // them? Maybe we should make copies (that are "reset" to
- // the initial state) and use that?
+ // If a dependency has already been recursively collected,
+ // then we can no longer call reload_defaults() or
+ // verify_sensible() on its skeleton. We could reset it, but
+ // then we wouldn't be able to continue using it if
+ // up_negotiate_configuration() below returns false. So it
+ // seems the most sensible approach is to make a temporary
+ // copy and reset that.
//
+ small_vector<reference_wrapper<package_skeleton>, 1> depcs;
+ forward_list<package_skeleton> depcs_storage; // Ref stability.
+ {
+ depcs.reserve (cfg_deps.size ());
+ for (const package_key& pk: cfg_deps)
+ {
+ build_package* b (entered_build (pk));
+ assert (b != nullptr);
+
+ package_skeleton* depc;
+ if (b->recursive_collection)
+ {
+ assert (b->skeleton);
+ depcs_storage.push_front (*b->skeleton);
+ depc = &depcs_storage.front ();
+ depc->reset ();
+ }
+ else
+ depc = &(b->skeleton
+ ? *b->skeleton
+ : b->init_skeleton (options));
+
+ depcs.push_back (*depc);
+ }
+ }
+
+ changed = up_negotiate_configuration (
+ cfg.dependency_configurations, *dept, dp, depcs);
+ }
+
+ // If the configuration hasn't changed, then we carry on.
+ // Otherwise, retry the negotiation from the beginning to
+ // refine the resulting configuration (see the catch block
+ // for retry_configuration).
+ //
+ if (changed)
+ {
l5 ([&]{trace << "cfg-postponing dependent "
<< pkg.available_name_version_db ()
<< " involves (being) negotiated configurations "
<< "and results in " << cfg
<< ", throwing retry_configuration";});
- // up_negotiate (...);
-
// Don't print the "while satisfying..." chain.
//
dep_chain.clear ();
throw retry_configuration {cfg.depth, move (pk), dp};
}
- else
+
+ l5 ([&]{trace << "configuration for cfg-postponed "
+ << "dependencies of dependent "
+ << pkg.available_name_version_db () << " "
+ << (r.second ? "" : "shadow-") << "negotiated";});
+
+ // Note that even in the fully negotiated case we may still add
+ // extra dependencies to this cluster which we still need to
+ // configure and recursively collect before indicating to the
+ // caller (returning true) that we are done with this depends
+ // value and the dependent is not postponed.
+ //
+ for (const package_key& p: cfg_deps)
{
- // The "second time" case.
- //
- assert (!r.second || // Shadow cluster was/is being negotiated.
- *r.second); // Everything already negotiated.
-
- l5 ([&]{trace << "configuration for cfg-postponed "
- << "dependencies of dependent "
- << pkg.available_name_version_db () << " is "
- << (r.second ? "" : "shadow-") << "negotiated";});
-
- // Note that even in the fully negotiated case we may still
- // add extra dependencies to this cluster which we still need
- // to configure and recursively collect before indicating to
- // the caller (returning true) that we are done with this
- // depends value and the dependent is not postponed.
- //
- // @@ TODO call verify_sensible(), etc. DO NOW.
- //
- for (const package_key& p: cfg_deps)
+ build_package* b (entered_build (p));
+ assert (b != nullptr);
+
+ if (!b->recursive_collection)
{
- build_package* b (entered_build (p));
- assert (b != nullptr);
+ l5 ([&]{trace << "collecting cfg-postponed dependency "
+ << b->available_name_version_db ()
+ << " of dependent "
+ << pkg.available_name_version_db ();});
- if (!b->recursive_collection)
+ // Similar to the inital negotiation case, verify and set
+ // the dependent configuration for this dependency.
+ //
{
- l5 ([&]{trace << "collecting cfg-postponed dependency "
- << b->available_name_version_db ()
- << " of dependent "
- << pkg.available_name_version_db ();});
-
- collect_build_prerequisites (options,
- *b,
- fdb,
- rpt_depts,
- apc,
- initial_collection,
- replaced_vers,
- dep_chain,
- postponed_repo,
- postponed_alts,
- 0 /* max_alt_index */,
- postponed_deps,
- postponed_cfgs,
- postponed_poss,
- true /* force_configured */);
+ assert (b->skeleton); // Should have been init'ed above.
+
+ const package_configuration& pc (
+ cfg.dependency_configurations[p]);
+
+ pair<bool, string> pr (b->skeleton->verify_sensible (pc));
+
+ if (!pr.first)
+ {
+ // @@ TODO: improve (see the other case).
+ //
+ fail << "unable to negotiate sensible configuration\n"
+ << " " << pr.second;
+ }
+
+ b->skeleton->dependent_config (pc);
}
- else
- l5 ([&]{trace << "dependency "
- << b->available_name_version_db ()
- << " of dependent "
- << pkg.available_name_version_db ()
- << " is already (being) recursively "
- << "collected, skipping";});
- }
- return true;
+ collect_build_prerequisites (options,
+ *b,
+ fdb,
+ rpt_depts,
+ apc,
+ initial_collection,
+ replaced_vers,
+ dep_chain,
+ postponed_repo,
+ postponed_alts,
+ 0 /* max_alt_index */,
+ postponed_deps,
+ postponed_cfgs,
+ postponed_poss,
+ true /* force_configured */);
+ }
+ else
+ l5 ([&]{trace << "dependency "
+ << b->available_name_version_db ()
+ << " of dependent "
+ << pkg.available_name_version_db ()
+ << " is already (being) recursively "
+ << "collected, skipping";});
}
+
+ return true;
#else
- //@@ TMP: get rid of shadow_dependents if pans out.
+ //@@ TMP: get rid of shadow_dependents (including in
+ // retry_configuration) if pans out.
// Case (3).
//
@@ -6124,14 +6174,15 @@ namespace bpkg
{
assert (b->skeleton); // Should have been init'ed above.
- const package_configuration& cfg (
+ const package_configuration& pc (
pcfg->dependency_configurations[p]);
- pair<bool, string> pr (b->skeleton->verify_sensible (cfg));
+ pair<bool, string> pr (b->skeleton->verify_sensible (pc));
if (!pr.first)
{
// @@ TODO: improve (print dependencies, dependents, config).
+ // (also in the up-negotiation case).
//
// Note that the diagnostics from the dependent will most
// likely be in the "error ..." form (potentially with
@@ -6142,7 +6193,7 @@ namespace bpkg
<< " " << pr.second;
}
- b->skeleton->dependent_config (cfg);
+ b->skeleton->dependent_config (pc);
}
build_package_refs dep_chain;
@@ -6464,6 +6515,9 @@ namespace bpkg
if (e.depth != pcd)
throw;
+ package_configurations cfgs (
+ move (pc->dependency_configurations));
+
// Restore the state from snapshot.
//
// Note: postponed_cfgs is re-assigned.
@@ -6480,22 +6534,29 @@ namespace bpkg
<< "to dependent " << e.dependent << ", adding "
<< "shadow dependent and re-negotiating";});
- // @@ TODO: need to copy dependency_configuration to the
- // restored state. DO NOW.
+ // @@ TODO
+ //
+ // Note that there is also a possibility of having a "bogus"
+ // dependent/dependencies (i.e., we add them to the cluster but
+ // they never get re-visited). While seems harmless (or not),
+ // maybe we need to prune the part of the configuration that was
+ // added by them? Maybe we should have the confirmed flag which
+ // is set when the originating dependent confirms the value?!
+
+ // Copy over the configuration for further refinement.
+ //
+ pc->dependency_configurations = move (cfgs);
+
+#if 0
+ // @@ TMP drop if pans out.
// Record the dependent/dependencies/position that triggered
// this so that we don't repeat this up-negotiate/throw/catch
// dance when we retry collect_build_postponed(). @@ Hm. could
// this not be re-done via "unchanged configuration"?
//
- // Note that there is also a possibility of having a "bogus"
- // dependent/dependencies (i.e., we add them to the cluster but
- // they never get re-visited). @@ While seems harmless, maybe we
- // need to prune the part of the configuration that was added
- // by them? Maybe we should have the confirmed flag which is
- // set when the originating dependent confirms the value?!
- //
pc->add_shadow (move (e.dependent), e.position);
+#endif
}
catch (merge_configuration& e)
{