Suppress duplicate libraries in pkg-config files

3 files changed, 89 insertions, 46 deletions

diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx
index 0482fcb..98b6b9e 100644
--- a/libbuild2/cc/link-rule.cxx
+++ b/libbuild2/cc/link-rule.cxx
@@ -1718,33 +1718,7 @@ namespace build2
         {
           // Hoist the elements corresponding to this library to the end.
           //
-          if (al->begin != al->end)
-          {
-            // Rotate to the left the subrange starting from the first element
-            // of this library and until the end so that the element after the
-            // last element of this library becomes the first element of this
-            // subrange. We also need to adjust begin/end of libraries
-            // affected by the rotation.
-            //
-            rotate (d.args.begin () + al->begin,
-                    d.args.begin () + al->end,
-                    d.args.end ());
-
-            size_t n (al->end - al->begin);
-
-            for (appended_library& al1: d.ls)
-            {
-              if (al1.begin >= al->end)
-              {
-                al1.begin -= n;
-                al1.end -= n;
-              }
-            }
-
-            al->end = d.args.size ();
-            al->begin = al->end - n;
-          }
-
+          d.ls.hoist (d.args, *al);
           return;
         }
 
diff --git a/libbuild2/cc/link-rule.hxx b/libbuild2/cc/link-rule.hxx
index c761d53..f990415 100644
--- a/libbuild2/cc/link-rule.hxx
+++ b/libbuild2/cc/link-rule.hxx
@@ -134,6 +134,40 @@ namespace build2
 
           return &back ();
         }
+
+        // Hoist the elements corresponding to the specified library to the
+        // end.
+        //
+        void
+        hoist (strings& args, appended_library& al)
+        {
+          if (al.begin != al.end)
+          {
+            // Rotate to the left the subrange starting from the first element
+            // of this library and until the end so that the element after the
+            // last element of this library becomes the first element of this
+            // subrange. We also need to adjust begin/end of libraries
+            // affected by the rotation.
+            //
+            rotate (args.begin () + al.begin,
+                    args.begin () + al.end,
+                    args.end ());
+
+            size_t n (al.end - al.begin);
+
+            for (appended_library& al1: *this)
+            {
+              if (al1.begin >= al.end)
+              {
+                al1.begin -= n;
+                al1.end -= n;
+              }
+            }
+
+            al.end = args.size ();
+            al.begin = al.end - n;
+          }
+        }
       };
 
       void
diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx
index 15c6e3f..7061491 100644
--- a/libbuild2/cc/pkgconfig.cxx
+++ b/libbuild2/cc/pkgconfig.cxx
@@ -1523,9 +1523,9 @@ namespace build2
           }
         };
 
-        // Given a library target, save its -l-style library name.
+        // Given a library target, return its -l-style library name.
         //
-        auto save_library_target = [&os, this] (const file& l)
+        auto save_library_target = [this] (const file& l) -> string
         {
           // If available (it may not, in case of import-installed libraris),
           // use the .pc file name to derive the -l library name (in case of
@@ -1574,13 +1574,13 @@ namespace build2
             }
           }
 
-          os << " -l" << n;
+          return "-l" + n;
         };
 
-        // Given a (presumably) compiler-specific library name, save its
+        // Given a (presumably) compiler-specific library name, return its
         // -l-style library name.
         //
-        auto save_library_name = [&os, this] (const string& n)
+        auto save_library_name = [this] (const string& n) -> string
         {
           if (tsys == "win32-msvc")
           {
@@ -1590,14 +1590,13 @@ namespace build2
 
             if (p != string::npos && icasecmp (n.c_str () + p + 1, "lib") == 0)
             {
-              os << " -l" << string (n, 0, p);
-              return;
+              return "-l" + string (n, 0, p);
             }
 
-            // Fall through and save as is.
+            // Fall through and return as is.
           }
 
-          os << ' ' << n;
+          return n;
         };
 
         // @@ TODO: support whole archive?
@@ -1619,11 +1618,6 @@ namespace build2
         // dependencies if we don't have the shared variant (see the load
         // logic for details). And also for the common .pc file, naturally.
         //
-        //@@ TODO: would be nice to weed out duplicates. But is it always
-        //   safe? Think linking archives: will have to keep duplicates in
-        //   the second position, not first. Gets even trickier with the
-        //   Libs.private split.
-        //
         {
           os << "Libs:";
 
@@ -1633,12 +1627,23 @@ namespace build2
           os << " -L" << escape (ldir.string ());
 
           // Now process ourselves as if we were being linked to something (so
-          // pretty similar to link_rule::append_libraries()).
+          // pretty similar to link_rule::append_libraries()). We also reuse
+          // the link_rule's machinery to suppress duplicates.
           //
+          appended_libraries ls;
+          strings args;
           bool priv (false);
+
+          struct data
+          {
+            ofdstream&           os;
+            appended_libraries&  ls;
+            strings&             args;
+          } d {os, ls, args};
+
           auto imp = [&priv] (const target&, bool la) {return priv && la;};
 
-          auto lib = [&save_library_target, &save_library_name] (
+          auto lib = [&d, &save_library_target, &save_library_name] (
             const target* const* lc,
             const small_vector<reference_wrapper<const string>, 2>& ns,
             lflags,
@@ -1646,27 +1651,49 @@ namespace build2
           {
             const file* l (lc != nullptr ? &(*lc)->as<file> () : nullptr);
 
+            // Suppress duplicates (see append_libraries() for details).
+            //
+            // Note that we use the original name for duplicate tracking.
+            //
+            appended_library* al (l != nullptr
+                                  ? &d.ls.append (*l, d.args.size ())
+                                  : d.ls.append (ns, d.args.size ()));
+
+            if (al != nullptr && al->end != appended_library::npos)
+            {
+              d.ls.hoist (d.args, *al);
+              return;
+            }
+
             if (l != nullptr)
             {
               if (l->is_a<libs> () || l->is_a<liba> ()) // See through libux.
-                save_library_target (*l);
+                d.args.push_back (save_library_target (*l));
             }
             else
             {
               // Something "system'y", save as is.
               //
               for (const string& n: ns)
-                save_library_name (n);
+                d.args.push_back (save_library_name (n));
             }
+
+            if (al != nullptr)
+              al->end = d.args.size (); // Close.
           };
 
-          auto opt = [] (const target&, const string&, bool, bool)
+          auto opt = [&d] (const target& lt, const string&, bool, bool)
           {
+            const file& l (lt.as<file> ());
+
             //@@ TODO: should we filter -L similar to -I?
             //@@ TODO: how will the Libs/Libs.private work?
             //@@ TODO: remember to use escape()
 
             // See link_rule::append_libraries().
+
+            if (d.ls.append (l, d.args.size ()).end != appended_library::npos)
+              return;
           };
 
           // Pretend we are linking an executable using what would be normal,
@@ -1677,16 +1704,24 @@ namespace build2
           process_libraries (a, bs, li, sys_lib_dirs,
                              l, la, 0, // Link flags.
                              imp, lib, opt, !binless);
+
+          for (const string& a: args)
+            os << ' ' << a;
           os << endl;
 
           if (la)
           {
             os << "Libs.private:";
 
+            ls.clear ();
+            args.clear ();
             priv = true;
             process_libraries (a, bs, li, sys_lib_dirs,
                                l, la, 0, // Link flags.
                                imp, lib, opt, false);
+
+            for (const string& a: args)
+              os << ' ' << a;
             os << endl;
           }
         }