diff options
author | Boris Kolpackov <boris@codesynthesis.com> | 2023-07-20 07:44:36 +0200 |
---|---|---|
committer | Boris Kolpackov <boris@codesynthesis.com> | 2023-07-20 07:44:36 +0200 |
commit | b61e0de250d522ec9a8e16146ef979a65c181db1 (patch) | |
tree | 93cf8e451a5c605f9d0159c6474e77d47b6f2de4 /libbuild2 | |
parent | 7aabdc2ccfea23c93e3b94290df59708aa179104 (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.cxx | 14 | ||||
-rw-r--r-- | libbuild2/algorithm.hxx | 7 | ||||
-rw-r--r-- | libbuild2/bash/rule.cxx | 10 | ||||
-rw-r--r-- | libbuild2/in/rule.cxx | 11 | ||||
-rw-r--r-- | libbuild2/in/rule.hxx | 5 | ||||
-rw-r--r-- | libbuild2/install/rule.cxx | 33 | ||||
-rw-r--r-- | libbuild2/version/rule.cxx | 6 |
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; } |