aboutsummaryrefslogtreecommitdiff
path: root/libbuild2
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2023-07-20 07:44:36 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2023-07-20 07:44:36 +0200
commitb61e0de250d522ec9a8e16146ef979a65c181db1 (patch)
tree93cf8e451a5c605f9d0159c6474e77d47b6f2de4 /libbuild2
parent7aabdc2ccfea23c93e3b94290df59708aa179104 (diff)
Change inner rule/prerequisites match order in install::file_rule
The old order messed up the for-install signaling logic. See the long comment in install::file_rule::apply_impl() for background and details.
Diffstat (limited to 'libbuild2')
-rw-r--r--libbuild2/algorithm.cxx14
-rw-r--r--libbuild2/algorithm.hxx7
-rw-r--r--libbuild2/bash/rule.cxx10
-rw-r--r--libbuild2/in/rule.cxx11
-rw-r--r--libbuild2/in/rule.hxx5
-rw-r--r--libbuild2/install/rule.cxx33
-rw-r--r--libbuild2/version/rule.cxx6
7 files changed, 82 insertions, 4 deletions
diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx
index 4db3d72..ad4c406 100644
--- a/libbuild2/algorithm.cxx
+++ b/libbuild2/algorithm.cxx
@@ -1265,6 +1265,20 @@ namespace build2
return ct.try_matched_state (a, false);
}
+ void
+ match_only_sync (action a, const target& t)
+ {
+ assert (t.ctx.phase == run_phase::match);
+
+ target_lock l (lock_impl (a, t, scheduler::work_none));
+
+ if (l.target != nullptr && l.offset < target::offset_matched)
+ {
+ if (match_impl (l, true /* step */).second == target_state::failed)
+ throw failed ();
+ }
+ }
+
// Note: lock is a reference to avoid the stacking overhead.
//
static group_view
diff --git a/libbuild2/algorithm.hxx b/libbuild2/algorithm.hxx
index 8bdf737..7cc42c2 100644
--- a/libbuild2/algorithm.hxx
+++ b/libbuild2/algorithm.hxx
@@ -376,6 +376,13 @@ namespace build2
pair<bool, target_state>
match_sync (action, const target&, unmatch);
+ // As above but only match the target (unless already matched) without
+ // applying the match (which is normally done with match_sync()). You will
+ // most likely regret using this function.
+ //
+ LIBBUILD2_SYMEXPORT void
+ match_only_sync (action, const target&);
+
// As above but without incrementing the target's dependents count. Should
// be executed with execute_direct_*().
//
diff --git a/libbuild2/bash/rule.cxx b/libbuild2/bash/rule.cxx
index 29c6a2a..502a206 100644
--- a/libbuild2/bash/rule.cxx
+++ b/libbuild2/bash/rule.cxx
@@ -63,10 +63,12 @@ namespace build2
// in_rule
//
bool in_rule::
- match (action a, target& t, const string& hint, match_extra&) const
+ match (action a, target& xt, const string& hint, match_extra&) const
{
tracer trace ("bash::in_rule::match");
+ file& t (xt.as<file> ()); // Only registered for exe{} and bash{}.
+
// Note that for bash{} and for exe{} with hint we match even if the
// target does not depend on any modules (while it could have been
// handled by the in module, that would require loading it).
@@ -89,6 +91,12 @@ namespace build2
l4 ([&]{trace << "no bash module prerequisite or hint for target "
<< t;});
+ // If we match, derive the file name early as recommended by the in
+ // rule.
+ //
+ if (fi && fm)
+ t.derive_path ();
+
return fi && fm;
}
diff --git a/libbuild2/in/rule.cxx b/libbuild2/in/rule.cxx
index 74bc2a7..31a9d94 100644
--- a/libbuild2/in/rule.cxx
+++ b/libbuild2/in/rule.cxx
@@ -47,6 +47,13 @@ namespace build2
if (!fi)
l5 ([&]{trace << "no in file prerequisite for target " << t;});
+ // If we match, derive the file name here instead of in apply() to make
+ // it available early for the in{} prerequisite search (see
+ // install::file_rule::apply_impl() for background).
+ //
+ if (fi)
+ t.derive_path ();
+
return fi;
}
@@ -55,9 +62,9 @@ namespace build2
{
file& t (xt.as<file> ());
- // Derive the file name.
+ // Make sure derived rules assign the path in match().
//
- t.derive_path ();
+ assert (!t.path ().empty ());
// Inject dependency on the output directory.
//
diff --git a/libbuild2/in/rule.hxx b/libbuild2/in/rule.hxx
index 369fd93..67c2509 100644
--- a/libbuild2/in/rule.hxx
+++ b/libbuild2/in/rule.hxx
@@ -22,6 +22,11 @@ namespace build2
// cache data (e.g., in match() or apply()) to be used in substitute() and
// lookup() calls.
//
+ // A derived rule is also required to derive the target file name in
+ // match() instead of apply() to make it available early for the in{}
+ // prerequisite search (see install::file_rule::apply_impl() for
+ // background).
+ //
// Note also that currently this rule ignores the dry-run mode (see
// perform_update() for the rationale).
//
diff --git a/libbuild2/install/rule.cxx b/libbuild2/install/rule.cxx
index 20a4bc3..4dd11e8 100644
--- a/libbuild2/install/rule.cxx
+++ b/libbuild2/install/rule.cxx
@@ -378,11 +378,36 @@ namespace build2
// (actual update). We used to do this after matching the prerequisites
// but the inner rule may provide some rule-specific information (like
// the target extension for exe{}) that may be required during the
- // prerequisite search (like the base name for in{}).
+ // prerequisite search (like the base name for in{}; this no longer
+ // reproduces likely due to the changes to exe{} extension derivation
+ // but a contrived arrangement can still be made to trigger this).
//
+ // But then we discovered that doing this before the prerequisites messes
+ // up with the for-install signaling. Specifically, matching the
+ // prerequisites may signal that they are being updated for install,
+ // for example, for a library via a metadata library used in a moc
+ // recipe. While matching the inner rule may trigger updating during
+ // match of such prerequisites, for example, a source file generated by
+ // that moc recipe that depends on this metadata library. If we match
+ // prerequisites before, then the library that is pulled by the metadata
+ // library will be updated before we had a chance to signal that it
+ // should be updated for install.
+ //
+ // To try to accommodate both cases (as best as we can) we now split the
+ // inner rule match into two steps: we do the match before and apply
+ // after. This allows rules that deal with tricky prerequisites like
+ // in{} to assign the target path in match() instead of apply() (see
+ // in::rule, for example).
+ //
+#if 0
optional<bool> unchanged;
if (a.operation () == update_id)
unchanged = match_inner (a, t, unmatch::unchanged).first;
+#else
+ action ia (a.inner_action ());
+ if (a.operation () == update_id)
+ match_only_sync (ia, t);
+#endif
optional<const scope*> is; // Installation scope (resolve lazily).
@@ -454,6 +479,12 @@ namespace build2
pts.push_back (prerequisite_target (pt, pi));
}
+#if 1
+ optional<bool> unchanged;
+ if (a.operation () == update_id)
+ unchanged = match_sync (ia, t, unmatch::unchanged).first;
+#endif
+
if (a.operation () == update_id)
{
return *unchanged
diff --git a/libbuild2/version/rule.cxx b/libbuild2/version/rule.cxx
index 98dc2da..65c1117 100644
--- a/libbuild2/version/rule.cxx
+++ b/libbuild2/version/rule.cxx
@@ -93,6 +93,12 @@ namespace build2
if (!fi)
l5 ([&]{trace << "no in file prerequisite for target " << t;});
+ // If we match, derive the file name early as recommended by the in
+ // rule.
+ //
+ if (fm && fi)
+ t.derive_path ();
+
return fm && fi;
}