From 77bef9b64857b1d2ae96dafc2f531cadb374f561 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 20 Sep 2023 09:03:44 +0200 Subject: 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. --- libbuild2/config/init.cxx | 22 +++++++++++++--------- libbuild2/dist/init.cxx | 9 +++++++-- libbuild2/install/init.cxx | 5 ++++- libbuild2/rule.cxx | 3 +++ libbuild2/rule.hxx | 13 +++++++++++-- libbuild2/test/init.cxx | 18 ++++++++++-------- 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 (configure_id, 0, "config.alias", alias_rule::instance); + + // This allows a custom configure rule while doing nothing by default. + // + rs.insert_rule (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 ( - configure_id, 0, "config.file", file_rule::instance); - - rs.insert_rule (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 (configure_id, 0, "config.noop", noop_rule::instance); - rs.insert_rule (configure_id, 0, "config.noop", noop_rule::instance); + rs.global_scope ().insert_rule ( + 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 ( - dist_id, 0, "dist.file", file_rule::instance); + rs.global_scope ().insert_rule ( + 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 (perform_install_id, "install.group", gr); bs.insert_rule (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 ()) + 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 (perform_test_id, "test", dr); - rs.insert_rule (perform_test_id, "test", dr); - rs.insert_rule (perform_test_id, "test", dr); + rs.insert_rule (perform_test_id, "test", dr); + rs.insert_rule (perform_test_id, "test", dr); // Register the fallback file rule for the update-for-test operation, // similar to update. // - rs.global_scope ().insert_rule ( - 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 ( + perform_test_id, "test.file", file_rule_); } return true; -- cgit v1.1