aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--libbuild2/algorithm.hxx6
-rw-r--r--libbuild2/cc/common.cxx38
-rw-r--r--libbuild2/cc/link-rule.cxx29
-rw-r--r--libbuild2/target.ixx12
4 files changed, 64 insertions, 21 deletions
diff --git a/libbuild2/algorithm.hxx b/libbuild2/algorithm.hxx
index c8f1d81..c6fb1c2 100644
--- a/libbuild2/algorithm.hxx
+++ b/libbuild2/algorithm.hxx
@@ -434,7 +434,8 @@ namespace build2
bool fail = true);
// Apply the specified recipe directly and without incrementing the
- // dependency counts. The target must be locked.
+ // dependency counts. The target must be locked (and it remains locked
+ // after this function returns).
//
// Note that there will be no way to rematch on options change (since there
// is no rule), so passing anything other than all_options is most likely a
@@ -446,7 +447,8 @@ namespace build2
uint64_t options = match_extra::all_options);
// Match (but do not apply) the specified rule directly and without
- // incrementing the dependency counts. The target must be locked.
+ // incrementing the dependency counts. The target must be locked (and it
+ // remains locked after this function returns).
//
void
match_rule (target_lock&,
diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx
index 6d36099..99c8f5d 100644
--- a/libbuild2/cc/common.cxx
+++ b/libbuild2/cc/common.cxx
@@ -1472,7 +1472,7 @@ namespace build2
// @@ TODO: we currently always reload pkgconfig for lt (and below).
//
mark_cc (*lt);
- lt->mtime (mt);
+ lt->mtime (mt); // Note: problematic, see below for details.
// We can only load metadata from here since we can only do this
// during the load phase. But it's also possible that a racing match
@@ -1523,6 +1523,9 @@ namespace build2
return l;
};
+ target_lock al (lock (a));
+ target_lock sl (lock (s));
+
target_lock ll (lock (lt));
// Set lib{} group members to indicate what's available. Note that we
@@ -1550,9 +1553,6 @@ namespace build2
ll.unlock ();
}
- target_lock al (lock (a));
- target_lock sl (lock (s));
-
if (!al) a = nullptr;
if (!sl) s = nullptr;
@@ -1586,6 +1586,34 @@ namespace build2
if (s != nullptr) match_rule (sl, file_rule::rule_match);
if (ll)
{
+ // @@ Turns out this has a problem: file_rule won't match/execute
+ // group members. So what happens is that if we have two installed
+ // libraries, say lib{build2} that depends on lib{butl}, then
+ // lib{build2} will have lib{butl} as a prerequisite and file_rule
+ // that matches lib{build2} will update lib{butl} (also matched by
+ // file_rule), but not its members. Later, someone (for example,
+ // the newer() call in append_libraries()) will pick one of the
+ // members assuming it is executed and things will go sideways.
+ //
+ // For now we hacked around the issue but the long term solution is
+ // probably to add to the bin module a special rule that is
+ // registered on the global scope and matches the installed lib{}
+ // targets. This rule will have to both update prerequisites like
+ // the file_rule and group members like the lib_rule (or maybe it
+ // can skip prerequisites since one of the member will do that; in
+ // which case maybe we will be able to reuse lib_rule maybe with
+ // the "all members" flag or some such). A few additional
+ // notes/thoughts:
+ //
+ // - Will be able to stop inheriting lib{} from mtime_target.
+ //
+ // - Will need to register for perform_update/clean like in context
+ // as well as for configure as in the config module (feels like
+ // shouldn't need to register for dist).
+ //
+ // - Will need to test batches, immediate import thoroughly (this
+ // stuff is notoriously tricky to get right in all situations).
+ //
match_rule (ll, file_rule::rule_match);
// Also bless the library group with a "trust me it exists" timestamp.
@@ -1594,6 +1622,8 @@ namespace build2
// won't match.
//
lt->mtime (mt);
+
+ ll.unlock (); // Unlock group before members, for good measure.
}
return r;
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 499f867..1991eda 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -2265,6 +2265,29 @@ namespace build2
<< "for install";
}
+ auto newer = [&d, l] ()
+ {
+ // @@ Work around the unexecuted member for installed libraries
+ // issue (see search_library() for details).
+ //
+ // Note that the member may not even be matched, let alone
+ // executed, so we have to go through the group to detect this
+ // case (if the group is not matched, then the member got to be).
+ //
+#if 0
+ return l->newer (d.mt);
+#else
+ const target* g (l->group);
+ target_state s (g != nullptr &&
+ g->matched (d.a, memory_order_acquire) &&
+ g->state[d.a].rule == &file_rule::rule_match
+ ? target_state::unchanged
+ : l->executed_state (d.a));
+
+ return l->newer (d.mt, s);
+#endif
+ };
+
if (d.li.type == otype::a)
{
// Linking a utility library to a static library.
@@ -2292,7 +2315,7 @@ namespace build2
// Check if this library renders us out of date.
//
if (d.update != nullptr)
- *d.update = *d.update || l->newer (d.mt);
+ *d.update = *d.update || newer ();
for (const target* pt: l->prerequisite_targets[d.a])
{
@@ -2331,7 +2354,7 @@ namespace build2
// Check if this library renders us out of date.
//
if (d.update != nullptr)
- *d.update = *d.update || l->newer (d.mt);
+ *d.update = *d.update || newer ();
// On Windows a shared library is a DLL with the import library as
// an ad hoc group member. MinGW though can link directly to DLLs
@@ -3497,7 +3520,7 @@ namespace build2
&cs, &update, mt,
bs, a, *f, la, p.data, li,
for_install, true, true, &lc);
- f = nullptr; // Timestamp checked by hash_libraries().
+ f = nullptr; // Timestamp checked by append_libraries().
}
else
{
diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx
index 2e1c65e..39b81e7 100644
--- a/libbuild2/target.ixx
+++ b/libbuild2/target.ixx
@@ -769,19 +769,7 @@ namespace build2
inline bool mtime_target::
newer (timestamp mt, target_state s) const
{
-#ifndef NDEBUG
- // @@ TMP
- //
- if (s == target_state::unknown)
- {
- text << "unknown target_state in newer(): " << *this <<
- info << "phase: " << ctx.phase;
-
- terminate (true /* trace */);
- }
-
assert (s != target_state::unknown); // Should be executed.
-#endif
timestamp mp (mtime ());