Fix issue with fallback rule priority in dist module

While at it, also remove workarounds for the same issue in the config and
test modules.

6 files changed, 48 insertions, 22 deletions

diff --git a/libbuild2/config/init.cxx b/libbuild2/config/init.cxx
index d42bace..38590ae 100644
--- a/libbuild2/config/init.cxx
+++ b/libbuild2/config/init.cxx
@@ -26,6 +26,8 @@ namespace build2
 {
   namespace config
   {
+    static const file_rule file_rule_ (true /* check_type */);
+
     void
     functions (function_map&); // functions.cxx
 
@@ -712,21 +714,23 @@ namespace build2
 
       // Register alias and fallback rule for the configure meta-operation.
       //
+      rs.insert_rule<alias> (configure_id, 0, "config.alias", alias_rule::instance);
+
+      // This allows a custom configure rule while doing nothing by default.
+      //
+      rs.insert_rule<target> (configure_id, 0, "config.noop", noop_rule::instance);
+
       // We need this rule for out-of-any-project dependencies (for example,
       // libraries imported from /usr/lib). We are registering it on the
       // global scope similar to builtin rules.
       //
-      // See a similar rule in the dist module.
+      // Note: use target instead of anything more specific (such as
+      // mtime_target) in order not to take precedence over the rules above.
       //
-      rs.global_scope ().insert_rule<mtime_target> (
-        configure_id, 0, "config.file", file_rule::instance);
-
-      rs.insert_rule<alias> (configure_id, 0, "config.alias", alias_rule::instance);
-
-      // This allows a custom configure rule while doing nothing by default.
+      // See a similar rule in the dist module.
       //
-      rs.insert_rule<target> (configure_id, 0, "config.noop", noop_rule::instance);
-      rs.insert_rule<file> (configure_id, 0, "config.noop", noop_rule::instance);
+      rs.global_scope ().insert_rule<target> (
+        configure_id, 0, "config.file", file_rule_);
 
       return true;
     }
diff --git a/libbuild2/dist/init.cxx b/libbuild2/dist/init.cxx
index 2a25992..48a3e15 100644
--- a/libbuild2/dist/init.cxx
+++ b/libbuild2/dist/init.cxx
@@ -22,6 +22,7 @@ namespace build2
   namespace dist
   {
     static const rule rule_;
+    static const file_rule file_rule_ (true /* check_type */);
 
     void
     boot (scope& rs, const location&, module_boot_extra& extra)
@@ -222,10 +223,14 @@ namespace build2
       // executables imported from /usr/bin, etc). We are registering it on
       // the global scope similar to builtin rules.
       //
+      // Note: use target instead of anything more specific (such as
+      // mtime_target) in order not to take precedence over the "dist" rule
+      // above.
+      //
       // See a similar rule in the config module.
       //
-      rs.global_scope ().insert_rule<mtime_target> (
-        dist_id, 0, "dist.file", file_rule::instance);
+      rs.global_scope ().insert_rule<target> (
+        dist_id, 0, "dist.file", file_rule_);
 
       // Configuration.
       //
diff --git a/libbuild2/install/init.cxx b/libbuild2/install/init.cxx
index 0b33475..3df912f 100644
--- a/libbuild2/install/init.cxx
+++ b/libbuild2/install/init.cxx
@@ -396,6 +396,9 @@ namespace build2
         // Note: use mtime_target (instead of target) to take precedence over
         // the fallback file rules below.
         //
+        // @@ We could fix this by checking the target type in file_rule,
+        //    similar to build2::file_rule.
+        //
         bs.insert_rule<mtime_target> (perform_install_id,   "install.group", gr);
         bs.insert_rule<mtime_target> (perform_uninstall_id, "install.group", gr);
 
@@ -403,7 +406,7 @@ namespace build2
         // operation, similar to update.
         //
         // @@ Hm, it's a bit fuzzy why we would be updating-for-install
-        //    something outside of any project..?
+        //    something outside of any project?
         //
         scope& gs (rs.global_scope ());
 
diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx
index 097e15a..0d6ec8c 100644
--- a/libbuild2/rule.cxx
+++ b/libbuild2/rule.cxx
@@ -95,6 +95,9 @@ namespace build2
   {
     tracer trace ("file_rule::match");
 
+    if (match_type_ && !t.is_a<mtime_target> ())
+      return false;
+
     // While strictly speaking we should check for the file's existence
     // for every action (because that's the condition for us matching),
     // for some actions this is clearly a waste. Say, perform_clean: we
diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx
index 4f77432..bf6f0a5 100644
--- a/libbuild2/rule.hxx
+++ b/libbuild2/rule.hxx
@@ -126,10 +126,19 @@ namespace build2
     virtual recipe
     apply (action, target&, match_extra&) const override;
 
-    file_rule () {}
+    // While this rule expects an mtime_target-based target, sometimes it's
+    // necessary to register it for something less specific (normally target)
+    // in order to achieve the desired rule matching priority (see the dist
+    // and config modules for an example). For such cases this rule can be
+    // instructed to check the type and only match if it's mtime_target-based.
+    //
+    file_rule (bool match_type = false): match_type_ (match_type) {}
 
-    static const file_rule instance;
+    static const file_rule instance; // Note: does not match the target type.
     static const build2::rule_match rule_match;
+
+  private:
+    bool match_type_;
   };
 
   class LIBBUILD2_SYMEXPORT alias_rule: public simple_rule
diff --git a/libbuild2/test/init.cxx b/libbuild2/test/init.cxx
index b7cf25f..32548f4 100644
--- a/libbuild2/test/init.cxx
+++ b/libbuild2/test/init.cxx
@@ -23,6 +23,8 @@ namespace build2
 {
   namespace test
   {
+    static const file_rule file_rule_ (true /* check_type */);
+
     void
     boot (scope& rs, const location&, module_boot_extra& extra)
     {
@@ -300,18 +302,18 @@ namespace build2
       {
         default_rule& dr (m);
 
-        // Note: register for mtime_target to take priority over the fallback
-        // rule below.
-        //
-        rs.insert_rule<target>       (perform_test_id, "test", dr);
-        rs.insert_rule<mtime_target> (perform_test_id, "test", dr);
-        rs.insert_rule<alias>        (perform_test_id, "test", dr);
+        rs.insert_rule<target> (perform_test_id, "test", dr);
+        rs.insert_rule<alias>  (perform_test_id, "test", dr);
 
         // Register the fallback file rule for the update-for-test operation,
         // similar to update.
         //
-        rs.global_scope ().insert_rule<mtime_target> (
-          perform_test_id, "test.file", file_rule::instance);
+        // Note: use target instead of anything more specific (such as
+        // mtime_target) in order not to take precedence over the "test" rule
+        // above.
+        //
+        rs.global_scope ().insert_rule<target> (
+          perform_test_id, "test.file", file_rule_);
       }
 
       return true;