Reimplement pkg-config generation with more conservative approach for now

13 files changed, 92 insertions, 93 deletions

diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx
index 29bae6d..2ce34e3 100644
--- a/build2/algorithm.cxx
+++ b/build2/algorithm.cxx
@@ -179,7 +179,7 @@ namespace build2
       {
       case target::offset_executed:
         {
-          if (t.action == a || t.action > a)
+          if (t.action >= a)
           {
             // We don't lock already executed targets.
             //
@@ -210,7 +210,7 @@ namespace build2
           }
           else
           {
-            assert (t.action > a || a == t.action);
+            assert (t.action >= a);
 
             // Release the lock if already applied for this action. This is
             // necessary no to confuse execute since otherwise it might see
diff --git a/build2/algorithm.hxx b/build2/algorithm.hxx
index 931b396..76e9d76 100644
--- a/build2/algorithm.hxx
+++ b/build2/algorithm.hxx
@@ -208,13 +208,12 @@ namespace build2
   match_recipe (target_lock&, recipe);
 
   // Match a "delegate rule" from withing another rules' apply() function
-  // avoiding recursive matches (thus the third argument). Return recipe and
-  // recipe action (if any). Unless fail is false, fail if not rule is found.
-  // Otherwise return empty recipe. Note that unlike match(), this function
-  // does not increment the dependents count. See also the companion
-  // execute_delegate().
+  // avoiding recursive matches (thus the third argument). Unless fail is
+  // false, fail if not rule is found. Otherwise return empty recipe. Note
+  // that unlike match(), this function does not increment the dependents
+  // count. See also the companion execute_delegate().
   //
-  pair<recipe, action>
+  recipe
   match_delegate (action, target&, const rule&, bool fail = true);
 
   // The standard prerequisite search and match implementations. They call
diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx
index 5680288..03b4604 100644
--- a/build2/algorithm.ixx
+++ b/build2/algorithm.ixx
@@ -274,15 +274,14 @@ namespace build2
     l.offset = target::offset_applied;
   }
 
-  inline pair<recipe, action>
+  inline recipe
   match_delegate (action a, target& t, const rule& r, bool fail)
   {
     assert (phase == run_phase::match);
     auto mr (match_impl (a, t, &r, fail));
-    return make_pair (mr.first != nullptr
-                      ? apply_impl (t, *mr.first, mr.second)
-                      : empty_recipe,
-                      mr.second);
+    return mr.first != nullptr
+      ? apply_impl (t, *mr.first, mr.second)
+      : empty_recipe;
   }
 
   group_view
diff --git a/build2/bin/rule.cxx b/build2/bin/rule.cxx
index 7d32e26..bb9036b 100644
--- a/build2/bin/rule.cxx
+++ b/build2/bin/rule.cxx
@@ -38,7 +38,7 @@ namespace build2
     // our prerequisites.
     //
     match_result lib_rule::
-    match (action, target& xt, const string&) const
+    match (action act, target& xt, const string&) const
     {
       lib& t (xt.as<lib> ());
 
@@ -57,7 +57,15 @@ namespace build2
       t.a = a ? &search<liba> (t, t.dir, t.out, t.name) : nullptr;
       t.s = s ? &search<libs> (t, t.dir, t.out, t.name) : nullptr;
 
-      return true;
+      match_result mr (true);
+
+      // If there is an outer operation, indicate that we match
+      // unconditionally so that we don't override ourselves.
+      //
+      if (act.outer_operation () != 0)
+        mr.recipe_action = action (act.meta_operation (), act.operation ());
+
+      return mr;
     }
 
     recipe lib_rule::
diff --git a/build2/cc/install.cxx b/build2/cc/install.cxx
index c65e7f6..72d9218 100644
--- a/build2/cc/install.cxx
+++ b/build2/cc/install.cxx
@@ -107,30 +107,6 @@ namespace build2
       return r;
     }
 
-    target_state file_install::
-    update_extra (action act, const target& t) const
-    {
-      // (Re)generate pkg-config's .pc file. While the target itself might be
-      // up-to-date from a previous run, there is no guarantee that .pc exists
-      // or also up-to-date. So to keep things simple we just regenerate it
-      // unconditionally.
-      //
-      // Also, if you are wondering why don't we just always produce this .pc,
-      // install or no install, the reason is unless and until we are updating
-      // for install, we have no idea where to things will be installed.
-      //
-      bool a;
-      const file* f;
-
-      if ((a = (f = t.is_a<liba> ())) ||
-          (     f = t.is_a<libs> ()))
-      {
-        pkgconfig_save (act, *f, a);
-      }
-
-      return target_state::unchanged;
-    }
-
     void file_install::
     install_extra (const file& t, const install_dir& id) const
     {
diff --git a/build2/cc/install.hxx b/build2/cc/install.hxx
index 2dd2b71..a846fc8 100644
--- a/build2/cc/install.hxx
+++ b/build2/cc/install.hxx
@@ -41,15 +41,6 @@ namespace build2
       virtual bool
       uninstall_extra (const file&, const install_dir&) const override;
 
-      virtual target_state
-      update_extra (action, const target&) const override;
-
-    private:
-      // pkg-config's .pc file generation (in pkgconfig.cxx).
-      //
-      void
-      pkgconfig_save (action, const file&, bool) const;
-
     private:
       const link& link_;
     };
diff --git a/build2/cc/link.cxx b/build2/cc/link.cxx
index 08b5314..41455e5 100644
--- a/build2/cc/link.cxx
+++ b/build2/cc/link.cxx
@@ -1564,6 +1564,31 @@ namespace build2
 
       dd.close ();
 
+      // (Re)generate pkg-config's .pc file. While the target itself might be
+      // up-to-date from a previous run, there is no guarantee that .pc exists
+      // or also up-to-date. So to keep things simple we just regenerate it
+      // unconditionally.
+      //
+      // Also, if you are wondering why don't we just always produce this .pc,
+      // install or no install, the reason is unless and until we are updating
+      // for install, we have no idea where to things will be installed.
+      //
+      if (for_install)
+      {
+        bool a;
+        const file* f;
+
+        if ((a = (f = t.is_a<liba> ())) ||
+            (     f = t.is_a<libs> ()))
+        {
+          // @@ Hack: this should really be in install:update_extra() where we
+          // (should) what we are installing and what not.
+          //
+          if (rs["install.root"])
+            pkgconfig_save (act, *f, a);
+        }
+      }
+
       // If nothing changed, then we are done.
       //
       if (!update)
@@ -1945,7 +1970,7 @@ namespace build2
 
       if (tclass == "windows")
       {
-        // For Windows generate rpath-emulating assembly (unless updaing for
+        // For Windows generate rpath-emulating assembly (unless updating for
         // install).
         //
         if (lt.executable () && !for_install)
diff --git a/build2/cc/link.hxx b/build2/cc/link.hxx
index cadc82c..dc5f70a 100644
--- a/build2/cc/link.hxx
+++ b/build2/cc/link.hxx
@@ -122,6 +122,11 @@ namespace build2
       pair<path, bool>
       windows_manifest (const file&, bool rpath_assembly) const;
 
+      // pkg-config's .pc file generation (pkgconfig.cxx).
+      //
+      void
+      pkgconfig_save (action, const file&, bool) const;
+
     private:
       const string rule_id;
     };
diff --git a/build2/cc/pkgconfig.cxx b/build2/cc/pkgconfig.cxx
index 3fc40ad..0a7bf8f 100644
--- a/build2/cc/pkgconfig.cxx
+++ b/build2/cc/pkgconfig.cxx
@@ -19,7 +19,7 @@
 #include <build2/cc/utility.hxx>
 
 #include <build2/cc/common.hxx>
-#include <build2/cc/install.hxx>
+#include <build2/cc/link.hxx>
 
 using namespace std;
 using namespace butl;
@@ -537,7 +537,7 @@ namespace build2
       return true;
     }
 
-    void file_install::
+    void link::
     pkgconfig_save (action act, const file& l, bool la) const
     {
       tracer trace (x, "pkgconfig_save");
diff --git a/build2/install/rule.cxx b/build2/install/rule.cxx
index 6b75cea..c76800a 100644
--- a/build2/install/rule.cxx
+++ b/build2/install/rule.cxx
@@ -199,36 +199,41 @@ namespace build2
       //
       if (a.operation () == update_id)
       {
-        // Save the prerequisite targets that we found since the call to
-        // match_delegate() below will wipe them out.
+        // Save the prerequisite targets that we found since the
+        // call to match_delegate() below will wipe them out.
         //
-        prerequisite_targets pts;
-        pts.swap (t.prerequisite_targets);
+        prerequisite_targets p;
 
-        // Find the "real" update rule, that is, the rule that would have been
-        // found if we signalled that we do not match from match() above.
+        if (!t.prerequisite_targets.empty ())
+          p.swap (t.prerequisite_targets);
+
+        // Find the "real" update rule, that is, the rule that would
+        // have been found if we signalled that we do not match from
+        // match() above.
         //
-        recipe d (match_delegate (a, t, *this).first);
+        recipe d (match_delegate (a, t, *this));
 
-        return [pts = move (pts), d = move (d), this]
-          (action a, const target& t) mutable -> target_state
-        {
-          // Do the target update first (we cannot call noop_recipe).
-          //
-          recipe_function** f (d.target<recipe_function*> ());
-          target_state r (f != nullptr && *f == &noop_action
-                          ? target_state::unchanged
-                          : execute_delegate (d, a, t));
+        // If we have no installable prerequisites, then simply redirect
+        // to it.
+        //
+        if (p.empty ())
+          return d;
 
-          // Then the extra hook.
+        // Ok, the worst case scenario: we need to cause update of
+        // prerequisite targets and also delegate to the real update.
+        //
+        return [pt = move (p), dr = move (d)] (
+          action a, const target& t) mutable -> target_state
+        {
+          // Do the target update first.
           //
-          r |= update_extra (a, t);
+          target_state r (execute_delegate (dr, a, t));
 
           // Swap our prerequisite targets back in and execute.
           //
-          t.prerequisite_targets.swap (pts);
+          t.prerequisite_targets.swap (pt);
           r |= straight_execute_prerequisites (a, t);
-          pts.swap (t.prerequisite_targets); // In case we get re-executed.
+          pt.swap (t.prerequisite_targets); // In case we get re-executed.
 
           return r;
         };
@@ -256,12 +261,6 @@ namespace build2
       return false;
     }
 
-    target_state file_rule::
-    update_extra (action, const target&) const
-    {
-      return target_state::unchanged;
-    }
-
     struct install_dir
     {
       dir_path dir;
diff --git a/build2/install/rule.hxx b/build2/install/rule.hxx
index 617beab..26f568b 100644
--- a/build2/install/rule.hxx
+++ b/build2/install/rule.hxx
@@ -70,12 +70,6 @@ namespace build2
       virtual bool
       uninstall_extra (const file&, const install_dir&) const;
 
-      // Extra update for install hooks. It is executed after the target has
-      // been updated but only for those that will actually be installed.
-      //
-      virtual target_state
-      update_extra (action, const target&) const;
-
       // Installation "commands".
       //
       // If verbose is false, then only print the command at verbosity level 2
diff --git a/build2/operation.hxx b/build2/operation.hxx
index 08bcccf..06a6781 100644
--- a/build2/operation.hxx
+++ b/build2/operation.hxx
@@ -76,6 +76,15 @@ namespace build2
     action_id outer_id;
   };
 
+  inline bool
+  operator== (action x, action y)
+  {
+    return x.inner_id == y.inner_id && x.outer_id == y.outer_id;
+  }
+
+  inline bool
+  operator!= (action x, action y) {return !(x == y);}
+
   // This is an "overrides" comparison, i.e., it returns true if the recipe
   // for x overrides recipe for y. The idea is that for the same inner
   // operation, action with an outer operation is "weaker" than the one
@@ -89,13 +98,7 @@ namespace build2
   }
 
   inline bool
-  operator== (action x, action y)
-  {
-    return x.inner_id == y.inner_id && x.outer_id == y.outer_id;
-  }
-
-  inline bool
-  operator!= (action x, action y) {return !(x == y);}
+  operator>= (action x, action y) {return x == y || x > y;}
 
   ostream&
   operator<< (ostream&, action);
diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx
index c9a1fb9..05f3988 100644
--- a/build2/test/rule.cxx
+++ b/build2/test/rule.cxx
@@ -182,7 +182,7 @@ namespace build2
       // standard alias rule.
       //
       if (a.operation () == update_id)
-        return match_delegate (a, t, *this).first;
+        return match_delegate (a, t, *this);
 
       // For the test operation we have to implement our own search and match
       // because we need to ignore prerequisites that are outside of our
@@ -219,7 +219,7 @@ namespace build2
       if (md.script)
       {
         if (a.operation () == update_id)
-          return match_delegate (a, t, *this).first;
+          return match_delegate (a, t, *this);
 
         // Collect all the testscript targets in prerequisite_targets.
         //
@@ -320,7 +320,7 @@ namespace build2
           // been found if we signalled that we do not match from match()
           // above.
           //
-          recipe d (match_delegate (a, t, *this).first);
+          recipe d (match_delegate (a, t, *this));
 
           // If we have no input/output that needs updating, then simply
           // redirect to it.