From 30e5f6677c6ad8246419b2392791f2664d48bf05 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 12 Dec 2023 10:37:34 +0200 Subject: Work around unexecuted member for installed libraries issue See comment for the long-term plan. --- libbuild2/cc/common.cxx | 38 ++++++++++++++++++++++++++++++++++---- libbuild2/cc/link-rule.cxx | 29 ++++++++++++++++++++++++++--- 2 files changed, 60 insertions(+), 7 deletions(-) (limited to 'libbuild2/cc') 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 { -- cgit v1.1