Recognize -pthread as king of -l in *.libs

4 files changed, 56 insertions, 38 deletions

diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx
index f7f15aa..cd89c79 100644
--- a/libbuild2/cc/common.cxx
+++ b/libbuild2/cc/common.cxx
@@ -49,19 +49,19 @@ namespace build2
     // array that contains the current library dependency chain all the way to
     // the library passed to process_libraries(). The first element of this
     // array is NULL. If this argument is NULL, then this is a library without
-    // a target (e.g., -lpthread) and its name is in the second argument
-    // (which could be resolved to an absolute path or passed as an -l<name>
-    // option). Otherwise, (the first argument is not NULL), the second
-    // argument contains the target path (which can be empty in case of the
-    // unknown DLL path).
+    // a target (e.g., -lm, -pthread, etc) and its name is in the second
+    // argument (which could be resolved to an absolute path or passed as an
+    // -l<name>/-pthread option). Otherwise, (the first argument is not NULL),
+    // the second argument contains the target path (which can be empty in
+    // case of the unknown DLL path).
     //
-    // Initially, the second argument (library name) was a string (e.g.,
-    // -lpthread) but there are cases where the library is identified with
-    // multiple options, such as -framework CoreServices (there are also cases
-    // like -Wl,--whole-archive -lfoo -lbar -Wl,--no-whole-archive). So now it
-    // is a vector_view that contains a fragment of options (from one of the
-    // *.libs variables) that corresponds to the library (or several
-    // libraries, as in the --whole-archive example above).
+    // Initially, the second argument (library name) was a string (e.g., -lm)
+    // but there are cases where the library is identified with multiple
+    // options, such as -framework CoreServices (there are also cases like
+    // -Wl,--whole-archive -lfoo -lbar -Wl,--no-whole-archive). So now it is a
+    // vector_view that contains a fragment of options (from one of the *.libs
+    // variables) that corresponds to the library (or several libraries, as in
+    // the --whole-archive example above).
     //
     // Storing a reference to elements of library name in proc_lib is legal
     // (they come either from the target's path or from one of the *.libs
@@ -419,8 +419,8 @@ namespace build2
           // Determine the length of the library name fragment as well as
           // whether it is a system library. Possible length values are:
           //
-          // 1 - just the argument itself (-lpthread)
-          // 2 - argument and next element (-l pthread, -framework CoreServices)
+          // 1 - just the argument itself (-lm, -pthread)
+          // 2 - argument and next element (-l m, -framework CoreServices)
           // 0 - unrecognized/until the end (-Wl,--whole-archive ...)
           //
           // See similar code in find_system_library().
@@ -451,9 +451,9 @@ namespace build2
             {
               if (l[0] == '-')
               {
-                // -l<name>, -l <name>
+                // -l<name>, -l <name>, -pthread
                 //
-                if (l[1] == 'l')
+                if (l[1] == 'l' || l == "-pthread")
                 {
                   n = l.size () == 2 ? 2 : 1;
                 }
@@ -502,10 +502,10 @@ namespace build2
               //
               if (n.simple ())
               {
-                // This is something like -lpthread or shell32.lib so should
-                // be a valid path. But it can also be an absolute library
-                // path (e.g., something that may come from our
-                // .{static/shared}.pc files).
+                // This is something like -lm or shell32.lib so should be a
+                // valid path. But it can also be an absolute library path
+                // (e.g., something that may come from our .{static/shared}.pc
+                // files).
                 //
                 if (proc_lib)
                 {
@@ -628,8 +628,8 @@ namespace build2
 
             for (auto i (ns->begin ()), e (ns->end ()); i != e; )
             {
-              // This is something like -lpthread or shell32.lib so should be
-              // a valid path.
+              // This is something like -lm or shell32.lib so should be a
+              // valid path.
               //
               pair<size_t, bool> r (sense_fragment (*i));
 
@@ -1317,7 +1317,7 @@ namespace build2
         // (or custom ones) from *.export.poptions.
         //
         // @@ Should we add .pc files as ad hoc members so pkconfig_save() can
-        // use their names when deriving -l-names (this would be expecially
+        // use their names when deriving -l-names (this would be especially
         // helpful for binless libraries to get hold of prefix/suffix, etc).
         //
         if (pc.first.empty () && pc.second.empty ())
diff --git a/libbuild2/cc/functions.cxx b/libbuild2/cc/functions.cxx
index 6b64677..9b7a3d9 100644
--- a/libbuild2/cc/functions.cxx
+++ b/libbuild2/cc/functions.cxx
@@ -222,9 +222,10 @@ namespace build2
       // Note that passing multiple targets at once is not a mere convenience:
       // this also allows for more effective duplicate suppression.
       //
-      // Note also that this function can only be called during execution
-      // after all the specified library targets have been matched. Normally
-      // it is used in ad hoc recipes to implement custom compilation.
+      // Note also that this function can only be called during execution (or,
+      // carefully, during match) after all the specified library targets have
+      // been matched. Normally it is used in ad hoc recipes to implement
+      // custom compilation.
       //
       // Note that this function is not pure.
       //
@@ -303,9 +304,10 @@ namespace build2
       // Note that passing multiple targets at once is not a mere convenience:
       // this also allows for more effective duplicate suppression.
       //
-      // Note also that this function can only be called during execution
-      // after all the specified library targets have been matched. Normally
-      // it is used in ad hoc recipes to implement custom linking.
+      // Note also that this function can only be called during execution (or,
+      // carefully, during match) after all the specified library targets have
+      // been matched. Normally it is used in ad hoc recipes to implement
+      // custom linking.
       //
       // Note that this function is not pure.
       //
diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index f8414c9..0c7c0c2 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -156,7 +156,7 @@ namespace build2
         {
           if (s[0] == '-')
           {
-            // -l<name>, -l <name>
+            // -l<name>, -l <name> (Note: not -pthread, which is system)
             //
             if (s[1] == 'l')
             {
@@ -1288,7 +1288,7 @@ namespace build2
           }
 
           // Another thing we must check is for the presence of any simple
-          // libraries (-lpthread, shell32.lib, etc) in *.export.libs. See
+          // libraries (-lm, shell32.lib, etc) in *.export.libs. See
           // process_libraries() for details.
           //
           if (rec_binless)
@@ -1522,6 +1522,16 @@ namespace build2
             // represent it as an exe{} member to make sure it gets installed
             // next to the main .js file.
             //
+            // @@ Note that our recommendation is to pass -pthread in *.libs
+            // but checking that is not straightforward (it could come from
+            // one of the libraries that we are linking). We could have called
+            // append_libraries() (similar to $x.lib_libs()) and then looked
+            // there. But this is quite heavy handed and it's not clear this
+            // is worth the trouble since the -pthread support in Emscripten
+            // is quite high-touch (i.e., it's not like we can write a library
+            // that starts some threads and then run its test as on any other
+            // POSIX platform).
+            //
             if (find_option ("-pthread", cmode)         ||
                 find_option ("-pthread", t, c_loptions) ||
                 find_option ("-pthread", t, x_loptions))
diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx
index 6df36fe..a48d99c 100644
--- a/libbuild2/cc/pkgconfig.cxx
+++ b/libbuild2/cc/pkgconfig.cxx
@@ -741,8 +741,7 @@ namespace build2
           // We only keep -I, -D and -U.
           //
           if (n >= 2 &&
-              o[0] == '-' &&
-              (o[1] == 'I' || o[1] == 'D' || o[1] == 'U'))
+              o[0] == '-' && (o[1] == 'I' || o[1] == 'D' || o[1] == 'U'))
           {
             pops.push_back (move (o));
             arg = (n == 2);
@@ -788,7 +787,8 @@ namespace build2
         // library is binless. But sometimes we may have other linker options,
         // for example, -Wl,... or -pthread. It's probably a bad idea to
         // ignore them. Also, theoretically, we could have just the library
-        // name/path.
+        // name/path. Note that (after some meditation) we consider -pthread
+        // a special form of -l.
         //
         // The tricky part, of course, is to know whether what follows after
         // an option we don't recognize is its argument or another option or
@@ -819,10 +819,10 @@ namespace build2
             continue;
           }
 
-          // See if that's -l or just the library name/path.
+          // See if that's -l, -pthread, or just the library name/path.
           //
           if ((known && o[0] != '-') ||
-              (n > 2 && o[0] == '-' && o[1] == 'l'))
+              (n > 2 && o[0] == '-' && (o[1] == 'l' || o == "-pthread")))
           {
             // Unless binless, the first one is the library itself, which we
             // skip. Note that we don't verify this and theoretically it could
@@ -892,8 +892,8 @@ namespace build2
         // import installed, or via a .pc file (which we could have generated
         // from the export stub). The exception is "runtime libraries" (which
         // are really the extension of libc or the operating system in case of
-        // Windows) such as -lm, -ldl, -lpthread, etc. Those we will detect
-        // and leave as -l*.
+        // Windows) such as -lm, -ldl, -lpthread (or its -pthread variant),
+        // etc. Those we will detect and leave as -l*.
         //
         // If we managed to resolve all the -l's (sans runtime), then we can
         // omit -L's for a nice and tidy command line.
@@ -970,6 +970,11 @@ namespace build2
               }
               continue;
             }
+            else if (tsys == "mingw32")
+            {
+              if (l == "-pthread")
+                continue;
+            }
           }
           else
           {
@@ -979,6 +984,7 @@ namespace build2
                 l == "-lm"       ||
                 l == "-ldl"      ||
                 l == "-lrt"      ||
+                l == "-pthread"  ||
                 l == "-lpthread")
               continue;