Fix modules support for installed libraries

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<libx> ())
               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<bool, target_state> 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<string> (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