Re: [PATCH v8 24/37] receive-pack: convert push-to-checkout hook to hook.h

On Fri, Mar 12, 2021 at 02:24:41AM -0800, Junio C Hamano wrote:

Emily Shaffer [off-list ref] writes:

quoted

@@ -1435,12 +1436,19 @@ static const char *push_to_checkout(unsigned char *hash,
 				    struct strvec *env,
 				    const char *work_tree)
 {
+	struct run_hooks_opt opt;
+	run_hooks_opt_init_sync(&opt);
+
 	strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree));
-	if (run_hook_le(env->v, push_to_checkout_hook,
-			hash_to_hex(hash), NULL))
+	strvec_pushv(&opt.env, env->v);
+	strvec_push(&opt.args, hash_to_hex(hash));
+	if (run_hooks(push_to_checkout_hook, &opt)) {
+		run_hooks_opt_clear(&opt);
 		return "push-to-checkout hook declined";
-	else
+	} else {
+		run_hooks_opt_clear(&opt);
 		return NULL;
+	}
 }

OK, we opt_init(), futz with opt, call run_hooks() and opt_clear()
regardless of the outcome from run_hooks().  Narrow-sighted me
wonders if it makes the use of the API easier if run_hooks() did the
opt_clear() before it returns, but I haven't yet seen enough use at
this point to judge.

Hrm, is that idiomatic? I guess it would be convenient, and as long as
it doesn't touch explicitly caller-managed context pointer it should be
safe, but wouldn't it be surprising?

 - Emily