hook: warn when hook.<friendly-name>.jobs is set

Issue a warning when the user confuses the hook process and event
namespaces by setting hook.<friendly-name>.jobs.

Detect this by checking whether the name carrying .jobs also has
.command, .event, or .parallel configured.  Extract is_friendly_name()
as a helper for this check, to be reused by future per-event config
handling.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/hook.c b/hook.c
index d98b01156366a4b2d7895deacbcfc8dcad3b82e1..0493993bbe6738f40709989eb790661694260d40 100644 (file)
--- a/hook.c
+++ b/hook.c
@@ -279,6 +279,44 @@ void hook_cache_clear(struct strmap *cache)
        strmap_clear(cache, 0);
 }
 
+/*
+ * Return true if `name` is a hook friendly-name, i.e. it has at least one of
+ * .command, .event, or .parallel configured. These are the reliable clues
+ * that distinguish a friendly-name from an event name. Note: .enabled is
+ * deliberately excluded because it can appear under both namespaces.
+ */
+static int is_friendly_name(struct hook_all_config_cb *cb, const char *name)
+{
+       struct hashmap_iter iter;
+       struct strmap_entry *e;
+
+       if (strmap_get(&cb->commands, name) || strmap_get(&cb->parallel_hooks, name))
+               return 1;
+
+       strmap_for_each_entry(&cb->event_hooks, &iter, e) {
+               if (unsorted_string_list_lookup(e->value, name))
+                       return 1;
+       }
+
+       return 0;
+}
+
+/* Warn if any name in event_jobs is also a hook friendly-name. */
+static void warn_jobs_on_friendly_names(struct hook_all_config_cb *cb_data)
+{
+       struct hashmap_iter iter;
+       struct strmap_entry *e;
+
+       strmap_for_each_entry(&cb_data->event_jobs, &iter, e) {
+               if (is_friendly_name(cb_data, e->key))
+                       warning(_("hook.%s.jobs is set but '%s' looks like a "
+                                 "hook friendly-name, not an event name; "
+                                 "hook.<event>.jobs uses the event name "
+                                 "(e.g. hook.post-receive.jobs), so this "
+                                 "setting will be ignored"), e->key, e->key);
+       }
+}
+
 /* Populate `cache` with the complete hook configuration */
 static void build_hook_config_map(struct repository *r, struct strmap *cache)
 {
@@ -295,6 +333,8 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache)
        /* Parse all configs in one run, capturing hook.* including hook.jobs. */
        repo_config(r, hook_config_lookup_all, &cb_data);
 
+       warn_jobs_on_friendly_names(&cb_data);
+
        /* Construct the cache from parsed configs. */
        strmap_for_each_entry(&cb_data.event_hooks, &iter, e) {
                struct string_list *hook_names = e->value;

diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 24a3c92b6deb8043ef913593966b98d54d6cfb97..89fedc48ff497fda94d60507dcfc3a738634327a 100755 (executable)
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -1028,4 +1028,34 @@ test_expect_success 'hook.<event>.jobs still requires hook.<name>.parallel=true'
        test_cmp expect hook.order
 '
 
+test_expect_success 'hook.<friendly-name>.jobs warns when name has .command' '
+       test_config hook.my-hook.command "true" &&
+       test_config hook.my-hook.jobs 2 &&
+       git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err &&
+       test_grep "hook.my-hook.jobs.*friendly-name" err
+'
+
+test_expect_success 'hook.<friendly-name>.jobs warns when name has .event' '
+       test_config hook.my-hook.event test-hook &&
+       test_config hook.my-hook.command "true" &&
+       test_config hook.my-hook.jobs 2 &&
+       git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err &&
+       test_grep "hook.my-hook.jobs.*friendly-name" err
+'
+
+test_expect_success 'hook.<friendly-name>.jobs warns when name has .parallel' '
+       test_config hook.my-hook.event test-hook &&
+       test_config hook.my-hook.command "true" &&
+       test_config hook.my-hook.parallel true &&
+       test_config hook.my-hook.jobs 2 &&
+       git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err &&
+       test_grep "hook.my-hook.jobs.*friendly-name" err
+'
+
+test_expect_success 'hook.<event>.jobs does not warn for a real event name' '
+       test_config hook.test-hook.jobs 2 &&
+       git hook run --allow-unknown-hook-name --ignore-missing test-hook >out 2>err &&
+       test_grep ! "friendly-name" err
+'
+
 test_done