aboutsummaryrefslogtreecommitdiff
path: root/libbuild2/dist
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2022-11-30 09:08:53 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2022-11-30 09:08:53 +0200
commit3bbbe09e8629ab5311a1bcbb9f56aa6a33e36f55 (patch)
treef3f6ab4fc633d98ae5a215ba4529b07849ba6792 /libbuild2/dist
parentd593b19735eec451b091fd46e4cb066e3478d6c9 (diff)
Deal with order dependence in dist rule
Diffstat (limited to 'libbuild2/dist')
-rw-r--r--libbuild2/dist/init.cxx14
-rw-r--r--libbuild2/dist/module.hxx14
-rw-r--r--libbuild2/dist/operation.cxx129
-rw-r--r--libbuild2/dist/rule.cxx43
-rw-r--r--libbuild2/dist/rule.hxx11
-rw-r--r--libbuild2/dist/types.hxx40
6 files changed, 191 insertions, 60 deletions
diff --git a/libbuild2/dist/init.cxx b/libbuild2/dist/init.cxx
index 26ff86d..9de84ce 100644
--- a/libbuild2/dist/init.cxx
+++ b/libbuild2/dist/init.cxx
@@ -21,8 +21,6 @@ namespace build2
{
namespace dist
{
- static const rule rule_;
-
void
boot (scope& rs, const location&, module_boot_extra& extra)
{
@@ -196,7 +194,7 @@ namespace build2
const location& l,
bool first,
bool,
- module_init_extra&)
+ module_init_extra& extra)
{
tracer trace ("dist::init");
@@ -208,13 +206,19 @@ namespace build2
l5 ([&]{trace << "for " << rs;});
+ module& mod (extra.module_as<module> ());
+
auto& vp (rs.var_pool (true /* public */)); // All qualified.
// Register our wildcard rule. Do it explicitly for the alias to prevent
// something like insert<target>(dist_id, test_id) taking precedence.
//
- rs.insert_rule<target> (dist_id, 0, "dist", rule_);
- rs.insert_rule<alias> (dist_id, 0, "dist.alias", rule_);
+ {
+ const rule& r (mod);
+
+ rs.insert_rule<target> (dist_id, 0, "dist", r);
+ rs.insert_rule<alias> (dist_id, 0, "dist.alias", r);
+ }
// We need this rule for out-of-any-project dependencies (for example,
// executables imported from /usr/bin, etc). We are registering it on
diff --git a/libbuild2/dist/module.hxx b/libbuild2/dist/module.hxx
index 9c682d0..dbe2a3e 100644
--- a/libbuild2/dist/module.hxx
+++ b/libbuild2/dist/module.hxx
@@ -10,14 +10,19 @@
#include <libbuild2/module.hxx>
#include <libbuild2/variable.hxx>
+#include <libbuild2/dist/types.hxx>
+#include <libbuild2/dist/rule.hxx>
+
#include <libbuild2/export.hxx>
namespace build2
{
namespace dist
{
- struct LIBBUILD2_SYMEXPORT module: build2::module
+ class LIBBUILD2_SYMEXPORT module: public build2::module,
+ public rule
{
+ public:
static const string name;
const variable& var_dist_package;
@@ -38,6 +43,10 @@ namespace build2
adhoc.push_back (move (f));
}
+ // List of postponed prerequisites.
+ //
+ postponed_prerequisites postponed;
+
// Distribution post-processing callbacks.
//
// Only the last component in the pattern may contain wildcards. If the
@@ -69,8 +78,9 @@ namespace build2
// Implementation details.
//
+ public:
module (const variable& v_d_p)
- : var_dist_package (v_d_p) {}
+ : rule (postponed), var_dist_package (v_d_p) {}
public:
bool distributed = false; // True if this project is being distributed.
diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx
index c7ce7f2..e5c9307 100644
--- a/libbuild2/dist/operation.cxx
+++ b/libbuild2/dist/operation.cxx
@@ -15,6 +15,7 @@
#include <libbuild2/filesystem.hxx>
#include <libbuild2/diagnostics.hxx>
+#include <libbuild2/dist/types.hxx>
#include <libbuild2/dist/module.hxx>
using namespace std;
@@ -236,6 +237,11 @@ namespace build2
fail << "unknown distribution package name" <<
info << "did you forget to set dist.package?";
+ // Need non-const in order to clear postponed. This is safe since only
+ // doing it when running serially.
+ //
+ module& mod (const_cast<module&> (*rs.find_module<module> (module::name)));
+
const string& dist_package (cast<string> (l));
const process_path& dist_cmd (cast<process_path> (rs.vars["dist.cmd"]));
@@ -272,60 +278,91 @@ namespace build2
values params;
path_name pn ("<dist>");
const location loc (pn); // Dummy location.
+ action_targets ts {tgt};
+
+ auto process_postponed = [&ctx, &mod] ()
{
- action_targets ts {tgt};
+ if (!mod.postponed.list.empty ())
+ {
+ // Re-grab the phase lock similar to perform_match().
+ //
+ phase_lock l (ctx, run_phase::match);
+
+ // Note that we don't need to bother with the mutex since we do
+ // all of this serially. But we can end up with new elements at
+ // the end.
+ //
+ // Strictly speaking, to handle this correctly we would need to do
+ // multiple passes over this list and only give up when we cannot
+ // make any progress since earlier entries that we cannot resolve
+ // could be "fixed" by later entries. But this feels far-fetched
+ // and so let's wait for a real example before complicating this.
+ //
+ for (auto i (mod.postponed.list.begin ());
+ i != mod.postponed.list.end ();
+ ++i)
+ {
+ rule::match_postponed (i->action, i->target, i->prereq);
+ }
+ }
+ };
- auto mog = make_guard ([&ctx] () {ctx.match_only = false;});
- ctx.match_only = true;
+ auto mog = make_guard ([&ctx] () {ctx.match_only = false;});
+ ctx.match_only = true;
- const operations& ops (rs.root_extra->operations);
- for (operations::size_type id (default_id + 1); // Skip default_id.
- id < ops.size ();
- ++id)
+ const operations& ops (rs.root_extra->operations);
+ for (operations::size_type id (default_id + 1); // Skip default_id.
+ id < ops.size ();
+ ++id)
+ {
+ if (const operation_info* oif = ops[id])
{
- if (const operation_info* oif = ops[id])
- {
- // Skip aliases (e.g., update-for-install). In fact, one can
- // argue the default update should be sufficient since it is
- // assumed to update all prerequisites and we no longer support
- // ad hoc stuff like test.input. Though here we are using the
- // dist meta-operation, not perform.
- //
- if (oif->id != id)
- continue;
+ // Skip aliases (e.g., update-for-install). In fact, one can argue
+ // the default update should be sufficient since it is assumed to
+ // update all prerequisites and we no longer support ad hoc stuff
+ // like test.input. Though here we are using the dist
+ // meta-operation, not perform.
+ //
+ if (oif->id != id)
+ continue;
- // Use standard (perform) match.
- //
- if (auto pp = oif->pre_operation)
+ // Use standard (perform) match.
+ //
+ if (auto pp = oif->pre_operation)
+ {
+ if (operation_id pid = pp (ctx, params, dist_id, loc))
{
- if (operation_id pid = pp (ctx, params, dist_id, loc))
- {
- const operation_info* poif (ops[pid]);
- ctx.current_operation (*poif, oif, false /* diag_noise */);
- action a (dist_id, poif->id, oif->id);
- perform_match (params, a, ts,
- 1 /* diag (failures only) */,
- false /* progress */);
- }
+ const operation_info* poif (ops[pid]);
+ ctx.current_operation (*poif, oif, false /* diag_noise */);
+ action a (dist_id, poif->id, oif->id);
+ mod.postponed.list.clear ();
+ perform_match (params, a, ts,
+ 1 /* diag (failures only) */,
+ false /* progress */);
+ process_postponed ();
}
+ }
- ctx.current_operation (*oif, nullptr, false /* diag_noise */);
- action a (dist_id, oif->id);
- perform_match (params, a, ts,
- 1 /* diag (failures only) */,
- false /* progress */);
+ ctx.current_operation (*oif, nullptr, false /* diag_noise */);
+ action a (dist_id, oif->id);
+ mod.postponed.list.clear ();
+ perform_match (params, a, ts,
+ 1 /* diag (failures only) */,
+ false /* progress */);
+ process_postponed ();
- if (auto po = oif->post_operation)
+ if (auto po = oif->post_operation)
+ {
+ if (operation_id pid = po (ctx, params, dist_id))
{
- if (operation_id pid = po (ctx, params, dist_id))
- {
- const operation_info* poif (ops[pid]);
- ctx.current_operation (*poif, oif, false /* diag_noise */);
- action a (dist_id, poif->id, oif->id);
- perform_match (params, a, ts,
- 1 /* diag (failures only) */,
- false /* progress */);
- }
+ const operation_info* poif (ops[pid]);
+ ctx.current_operation (*poif, oif, false /* diag_noise */);
+ action a (dist_id, poif->id, oif->id);
+ mod.postponed.list.clear ();
+ perform_match (params, a, ts,
+ 1 /* diag (failures only) */,
+ false /* progress */);
+ process_postponed ();
}
}
}
@@ -381,7 +418,7 @@ namespace build2
dir_path out_nroot (out_root / pd);
const scope& nrs (ctx.scopes.find_out (out_nroot));
- if (nrs.out_path () != out_nroot) // This subproject not loaded.
+ if (nrs.out_path () != out_nroot) // This subproject is not loaded.
continue;
if (!nrs.src_path ().sub (src_root)) // Not a strong amalgamation.
@@ -508,8 +545,6 @@ namespace build2
// Copy over all the files. Apply post-processing callbacks.
//
- const module& mod (*rs.find_module<module> (module::name));
-
prog = prog && show_progress (1 /* max_verb */);
size_t prog_percent (0);
diff --git a/libbuild2/dist/rule.cxx b/libbuild2/dist/rule.cxx
index 0c72ff5..7233eba 100644
--- a/libbuild2/dist/rule.cxx
+++ b/libbuild2/dist/rule.cxx
@@ -72,6 +72,8 @@ namespace build2
// Search for an existing target or existing file in src.
//
+ // Note: see also similar code in match_postponed() below.
+ //
const prerequisite_key& k (p.key ());
pt = k.tk.type->search (t, k);
@@ -85,12 +87,13 @@ namespace build2
!p.dir.sub (out_root))
continue;
- // @@ TODO: this can actually be order-dependent: for example
- // libs{} prerequisite may be unknown because we haven't
- // matched the lib{} group yet.
+ // This can be order-dependent: for example libs{} prerequisite
+ // may be unknown because we haven't matched the lib{} group
+ // yet. So we postpone this for later (see match_postponed()).
//
- fail << "prerequisite " << k << " is not existing source file "
- << "nor known output target" << endf;
+ mlock l (postponed_.mutex);
+ postponed_.list.push_back (postponed_prerequisite {a, t, p,});
+ continue;
}
search_custom (p, *pt); // Cache.
@@ -107,5 +110,35 @@ namespace build2
return noop_recipe; // We will never be executed.
}
+
+ void rule::
+ match_postponed (action a, const target& t, const prerequisite& p)
+ {
+ const prerequisite_key& k (p.key ());
+ const target* pt (k.tk.type->search (t, k));
+
+ if (pt == nullptr)
+ {
+ // Note that we do loose the diag frame that we normally get when
+ // failing during match. So let's mention the target manually.
+ //
+ fail << "prerequisite " << k << " is not existing source file nor "
+ << "known output target" <<
+ info << "while applying rule dist to " << diag_do (a, t);
+ }
+
+ search_custom (p, *pt); // Cache.
+
+ // It's theoretically possible that the target gets entered but nobody
+ // else depends on it but us. So we need to make sure it's matched
+ // (since it, in turns, can pull in other targets). Note that this could
+ // potentially add new postponed prerequisites to the list.
+ //
+ if (!pt->matched (a))
+ {
+ if (pt->dir.sub (t.root_scope ().out_path ()))
+ match_direct_sync (a, *pt);
+ }
+ }
}
}
diff --git a/libbuild2/dist/rule.hxx b/libbuild2/dist/rule.hxx
index a864015..ec6c41a 100644
--- a/libbuild2/dist/rule.hxx
+++ b/libbuild2/dist/rule.hxx
@@ -11,6 +11,8 @@
#include <libbuild2/action.hxx>
#include <libbuild2/target.hxx>
+#include <libbuild2/dist/types.hxx>
+
namespace build2
{
namespace dist
@@ -26,13 +28,20 @@ namespace build2
class rule: public simple_rule
{
public:
- rule () {}
+ explicit
+ rule (postponed_prerequisites& p): postponed_ (p) {}
virtual bool
match (action, target&) const override;
virtual recipe
apply (action, target&) const override;
+
+ static void
+ match_postponed (action, const target&, const prerequisite&);
+
+ private:
+ postponed_prerequisites& postponed_;
};
}
}
diff --git a/libbuild2/dist/types.hxx b/libbuild2/dist/types.hxx
new file mode 100644
index 0000000..f3b1009
--- /dev/null
+++ b/libbuild2/dist/types.hxx
@@ -0,0 +1,40 @@
+// file : libbuild2/dist/types.hxx -*- C++ -*-
+// license : MIT; see accompanying LICENSE file
+
+#ifndef LIBBUILD2_DIST_TYPES_HXX
+#define LIBBUILD2_DIST_TYPES_HXX
+
+#include <libbuild2/types.hxx>
+#include <libbuild2/forward.hxx>
+
+#include <libbuild2/prerequisite-key.hxx>
+
+namespace build2
+{
+ namespace dist
+ {
+ // List of prerequisites that could not be searched to a target and were
+ // postponed for later re-search. This can happen, for example, because a
+ // prerequisite would resolve to a member of a group that hasn't been
+ // matched yet (for example, libs{} of lib{}). See rule::apply() for
+ // details.
+ //
+ // Note that we are using list instead of vector because new elements can
+ // be added at the end while we are iterating over the list.
+ //
+ struct postponed_prerequisite
+ {
+ build2::action action;
+ reference_wrapper<const build2::target> target;
+ reference_wrapper<const prerequisite> prereq;
+ };
+
+ struct postponed_prerequisites
+ {
+ build2::mutex mutex;
+ build2::list<postponed_prerequisite> list;
+ };
+ }
+}
+
+#endif // LIBBUILD2_DIST_TYPES_HXX