From e85e618a1b918f7279133eb1f446c1af871f5dd2 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 3 Dec 2020 11:00:24 +0200 Subject: Fix modules support for installed libraries --- libbuild2/bin/target.cxx | 2 +- libbuild2/bin/target.hxx | 8 ++------ libbuild2/cc/common.cxx | 28 +++++++++++++++++++++++----- libbuild2/cc/compile-rule.cxx | 24 +++++++++++++++++------- libbuild2/cc/pkgconfig.cxx | 15 +++++++-------- 5 files changed, 50 insertions(+), 27 deletions(-) diff --git a/libbuild2/bin/target.cxx b/libbuild2/bin/target.cxx index bf701c9..94851cd 100644 --- a/libbuild2/bin/target.cxx +++ b/libbuild2/bin/target.cxx @@ -53,7 +53,7 @@ namespace build2 const target_type libx::static_type { "libx", - &mtime_target::static_type, + &target::static_type, nullptr, nullptr, nullptr, diff --git a/libbuild2/bin/target.hxx b/libbuild2/bin/target.hxx index 5e7f445..8be8c23 100644 --- a/libbuild2/bin/target.hxx +++ b/libbuild2/bin/target.hxx @@ -200,14 +200,10 @@ namespace build2 // Common base for lib{} and libul{} groups. // - // We use mtime_target as a base for the "trust me it exists" functionality - // which we use, for example, to have installed lib{} prerequisites that - // are matched by the fallback file rule. - // - class LIBBUILD2_BIN_SYMEXPORT libx: public mtime_target + class LIBBUILD2_BIN_SYMEXPORT libx: public target { public: - using mtime_target::mtime_target; + using target::target; public: static const target_type static_type; diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx index 7d2fd63..a1bee8c 100644 --- a/libbuild2/cc/common.cxx +++ b/libbuild2/cc/common.cxx @@ -855,8 +855,20 @@ namespace build2 if (s != nullptr) lt->s = s; } - target_lock al (a != nullptr ? lock (act, *a) : target_lock ()); - target_lock sl (s != nullptr ? lock (act, *s) : target_lock ()); + target_lock al (a != nullptr ? lock (act, *a, false) : target_lock ()); + target_lock sl (s != nullptr ? lock (act, *s, false) : target_lock ()); + + if (al && al.offset == target::offset_matched) + { + assert ((*a)[act].rule == &file_rule::rule_match); + al.unlock (); + } + + if (sl && sl.offset == target::offset_matched) + { + assert ((*s)[act].rule == &file_rule::rule_match); + sl.unlock (); + } if (!al) a = nullptr; if (!sl) s = nullptr; @@ -953,11 +965,17 @@ namespace build2 } // If we have the lock (meaning this is the first time), set the - // traget's recipe to noop. Failed that we will keep re-locking it, + // traget's rule/recipe. Failed that we will keep re-locking it, // updating its members, etc. // - if (al) match_recipe (al, noop_recipe); - if (sl) match_recipe (sl, noop_recipe); + // For members, use the fallback file rule instead of noop since we may + // need their prerequisites matched (used for modules support; see + // pkgconfig_load(), search_modules() for details). + // + // Note also that these calls clear target data. + // + if (al) match_rule (al, file_rule::rule_match); + if (sl) match_rule (sl, file_rule::rule_match); if (ll) match_recipe (ll, noop_recipe); return r; diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index eba58fd..7cea9d6 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -768,18 +768,23 @@ namespace build2 { // Handle (phase two) imported libraries. We know that for such // libraries we don't need to do match() in order to get options - // (if any, they would be set by search_library()). + // (if any, they would be set by search_library()). But we do need + // to match it if we may need its modules (see search_modules() + // for details). // if (p.proj ()) { - if (search_library (a, - sys_lib_dirs, - usr_lib_dirs, - p.prerequisite) != nullptr) + pt = search_library (a, + sys_lib_dirs, + usr_lib_dirs, + p.prerequisite); + + if (pt != nullptr && !modules) continue; } - pt = &p.search (t); + if (pt == nullptr) + pt = &p.search (t); if (const libx* l = pt->is_a ()) pt = link_member (*l, a, li); @@ -836,7 +841,7 @@ namespace build2 // resolved and prerequisite_targets populated. So we match it but // then unmatch if it is safe. And thanks to the two-pass prerequisite // match in link::apply() it will be safe unless someone is building - // an obj?{} target directory. + // an obj?{} target directly. // pair mr ( build2::match ( @@ -5588,6 +5593,11 @@ namespace build2 // The module names should be specified but if not assume // something else is going on and ignore. // + // Note also that besides modules, prerequisite_targets may + // contain libraries which are interface dependencies of this + // library and which may be called to resolve its module + // dependencies. + // const string* n (cast_null (bt->vars[c_module_name])); if (n == nullptr) diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx index bbcddf9..35ba2d8 100644 --- a/libbuild2/cc/pkgconfig.cxx +++ b/libbuild2/cc/pkgconfig.cxx @@ -1209,6 +1209,11 @@ namespace build2 // liba{} would require weeding out duplicates that are already in // lib{}. // + // Currently, this information is only used by the modules machinery to + // resolve module names to module files (but we cannot only do this if + // modules are enabled since the same installed library can be used by + // multiple builds). + // prerequisites prs; pkgconf apc; @@ -1262,8 +1267,8 @@ namespace build2 parse_cflags (*st, spc, false); // For now we assume static and shared variants export the same set of - // modules. While technically possible, having a different set will - // most likely lead to all sorts of trouble (at least for installed + // modules. While technically possible, having a different set will most + // likely lead to all sorts of complications (at least for installed // libraries) and life is short. // if (modules) @@ -1272,12 +1277,6 @@ namespace build2 assert (!lt.has_prerequisites ()); if (!prs.empty ()) lt.prerequisites (move (prs)); - - // Bless the library group with a "trust me it exists" timestamp. Failed - // that, if we add it as a prerequisite (like we do above), the fallback - // file rule won't match. - // - lt.mtime (mtime (ipc.path)); } #else -- cgit v1.1