diff options
| -rw-r--r-- | libbuild2/algorithm.hxx | 6 | ||||
| -rw-r--r-- | libbuild2/cc/common.cxx | 38 | ||||
| -rw-r--r-- | libbuild2/cc/link-rule.cxx | 29 | ||||
| -rw-r--r-- | libbuild2/target.ixx | 12 |
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 ()); |