Re: [PATCH v4 0/9] propose config-based hooks

[Junio C Hamano <gitster@pobox.com>]

Emily Shaffer <emilyshaffer@google.com> writes:

> Since v3, the biggest change is the conversion of commit hooks to use the new
> hook machinery. The first change ("commit: use config-based hooks") is the
> important part; the second change ("run_commit_hook: take strvec instead of varargs")
> is probably subjective, but I thought it was a decent tech debt reduction.
>
> I wanted to send this reroll quickly since I had promised it in standup last
> week, but I've got pretty good progress locally on the patch for configuring
> "hook.runHookDir"; I'm planning to send that soon, probably this week.

I've had the attached merge-fix patch as a way to adjust argv_array
to strvec transition [*1*], but now *most* but not all parts of this
series have been migrated to the strvec API, you should apply some
parts in the merge-fix patch to your copy.  I think the changes in
the old "merge-fix" patch to *.c and *.h are already in your series
that has been rebased on a newer 'master' that has strvec, but
documentation and possibly in-code comments may need to be adjusted.

Another way to sanity check the result would be to run this:

    $ git diff master..es/config-hooks | grep -i argv.array

Thanks.  

[Footnote]

*1* The way I work with a topic that causes conflicts with other
    topics is to merge a new version of topic and letting the rerere
    records I created while resolving the conflicts with the
    previous round.  After textual conflicts are thusly resolved, if
    there are further changes that do not cause textual conflict
    that are necessary, they are written in the form of a
    "merge-fix" patch like the attached.

-- >8 --

 Documentation/technical/api-parse-options.txt  |  4 ++--
 Documentation/technical/config-based-hooks.txt |  4 ++--
 builtin/hook.c                                 | 16 ++++++++--------
 hook.c                                         |  6 +++---
 hook.h                                         |  4 ++--
 parse-options-cb.c                             |  8 ++++----
 parse-options.h                                |  6 +++---
 7 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index b4f1fc4a1a..679bd98629 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -173,9 +173,9 @@ There are some macros to easily define options:
 	The string argument is stored as an element in `string_list`.
 	Use of `--no-option` will clear the list of preceding values.
 
-`OPT_ARGV_ARRAY(short, long, &struct argv_array, arg_str, description)`::
+`OPT_STRVEC(short, long, &struct strvec, arg_str, description)`::
 	Introduce an option with a string argument.
-	The string argument is stored as an element in `argv_array`.
+	The string argument is stored as an element in `strvec`.
 	Use of `--no-option` will clear the list of preceding values.
 
 `OPT_INTEGER(short, long, &int_var, description)`::
diff --git a/Documentation/technical/config-based-hooks.txt b/Documentation/technical/config-based-hooks.txt
index c6e762b192..4443f70ded 100644
--- a/Documentation/technical/config-based-hooks.txt
+++ b/Documentation/technical/config-based-hooks.txt
@@ -106,10 +106,10 @@ a concise config afterwards. It may take a form similar to `git rebase
 `hook.c` and `hook.h` are responsible for interacting with the config files. In
 the case when the code generating a hook event doesn't have special concerns
 about how to run the hooks, the hook library will provide a basic API to call
-all hooks in config order with an `argv_array` provided by the code which
+all hooks in config order with an `strvec` provided by the code which
 generates the hook event:
 
-*`int run_hooks(const char *hookname, struct argv_array *args)`*
+*`int run_hooks(const char *hookname, struct strvec *args)`*
 
 This call includes the hook command provided by `run-command.h:find_hook()`;
 eventually, this legacy hook will be gated by a config `hook.runHookDir`. The
diff --git a/builtin/hook.c b/builtin/hook.c
index cd61fad5fb..debcb5a77a 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -5,7 +5,7 @@
 #include "hook.h"
 #include "parse-options.h"
 #include "strbuf.h"
-#include "argv-array.h"
+#include "strvec.h"
 
 static const char * const builtin_hook_usage[] = {
 	N_("git hook list <hookname>"),
@@ -67,14 +67,14 @@ static int list(int argc, const char **argv, const char *prefix)
 static int run(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf hookname = STRBUF_INIT;
-	struct argv_array env_argv = ARGV_ARRAY_INIT;
-	struct argv_array arg_argv = ARGV_ARRAY_INIT;
+	struct strvec env_argv = STRVEC_INIT;
+	struct strvec arg_argv = STRVEC_INIT;
 
 	struct option run_options[] = {
-		OPT_ARGV_ARRAY('e', "env", &env_argv, N_("var"),
-			       N_("environment variables for hook to use")),
-		OPT_ARGV_ARRAY('a', "arg", &arg_argv, N_("args"),
-			       N_("argument to pass to hook")),
+		OPT_STRVEC('e', "env", &env_argv, N_("var"),
+			   N_("environment variables for hook to use")),
+		OPT_STRVEC('a', "arg", &arg_argv, N_("args"),
+			   N_("argument to pass to hook")),
 		OPT_END(),
 	};
 
@@ -87,7 +87,7 @@ static int run(int argc, const char **argv, const char *prefix)
 
 	strbuf_addstr(&hookname, argv[0]);
 
-	return run_hooks(env_argv.argv, &hookname, &arg_argv);
+	return run_hooks(env_argv.v, &hookname, &arg_argv);
 }
 
 int cmd_hook(int argc, const char **argv, const char *prefix)
diff --git a/hook.c b/hook.c
index 902e213173..40d319adb1 100644
--- a/hook.c
+++ b/hook.c
@@ -98,7 +98,7 @@ struct list_head* hook_list(const struct strbuf* hookname)
 }
 
 int run_hooks(const char *const *env, const struct strbuf *hookname,
-	      const struct argv_array *args)
+	      const struct strvec *args)
 {
 	struct list_head *to_run, *pos = NULL, *tmp = NULL;
 	int rc = 0;
@@ -110,14 +110,14 @@ int run_hooks(const char *const *env, const struct strbuf *hookname,
 		struct hook *hook = list_entry(pos, struct hook, list);
 
 		/* add command */
-		argv_array_push(&hook_proc.args, hook->command.buf);
+		strvec_push(&hook_proc.args, hook->command.buf);
 
 		/*
 		 * add passed-in argv, without expanding - let the user get back
 		 * exactly what they put in
 		 */
 		if (args)
-			argv_array_pushv(&hook_proc.args, args->argv);
+			strvec_pushv(&hook_proc.args, args->v);
 
 		hook_proc.env = env;
 		hook_proc.no_stdin = 1;
diff --git a/hook.h b/hook.h
index cf598d6ccb..d020788a6b 100644
--- a/hook.h
+++ b/hook.h
@@ -1,7 +1,7 @@
 #include "config.h"
 #include "list.h"
 #include "strbuf.h"
-#include "argv-array.h"
+#include "strvec.h"
 
 struct hook
 {
@@ -12,7 +12,7 @@ struct hook
 
 struct list_head* hook_list(const struct strbuf *hookname);
 int run_hooks(const char *const *env, const struct strbuf *hookname,
-	      const struct argv_array *args);
+	      const struct strvec *args);
 
 void free_hook(struct hook *ptr);
 void clear_hook_list(void);
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 4f993cd734..d2b8b7b98a 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -205,19 +205,19 @@ int parse_opt_string_list(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
-int parse_opt_argv_array(const struct option *opt, const char *arg, int unset)
+int parse_opt_strvec(const struct option *opt, const char *arg, int unset)
 {
-	struct argv_array *v = opt->value;
+	struct strvec *v = opt->value;
 
 	if (unset) {
-		argv_array_clear(v);
+		strvec_clear(v);
 		return 0;
 	}
 
 	if (!arg)
 		return -1;
 
-	argv_array_push(v, arg);
+	strvec_push(v, arg);
 	return 0;
 }
 
diff --git a/parse-options.h b/parse-options.h
index e2e2de75c8..177259488b 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -177,9 +177,9 @@ struct option {
 #define OPT_STRING_LIST(s, l, v, a, h) \
 				    { OPTION_CALLBACK, (s), (l), (v), (a), \
 				      (h), 0, &parse_opt_string_list }
-#define OPT_ARGV_ARRAY(s, l, v, a, h) \
+#define OPT_STRVEC(s, l, v, a, h) \
 				    { OPTION_CALLBACK, (s), (l), (v), (a), \
-				      (h), 0, &parse_opt_argv_array }
+				      (h), 0, &parse_opt_strvec }
 #define OPT_UYN(s, l, v, h)         { OPTION_CALLBACK, (s), (l), (v), NULL, \
 				      (h), PARSE_OPT_NOARG, &parse_opt_tertiary }
 #define OPT_EXPIRY_DATE(s, l, v, h) \
@@ -299,7 +299,7 @@ int parse_opt_commits(const struct option *, const char *, int);
 int parse_opt_commit(const struct option *, const char *, int);
 int parse_opt_tertiary(const struct option *, const char *, int);
 int parse_opt_string_list(const struct option *, const char *, int);
-int parse_opt_argv_array(const struct option *, const char *, int);
+int parse_opt_strvec(const struct option *, const char *, int);
 int parse_opt_noop_cb(const struct option *, const char *, int);
 enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
 					   const struct option *,
-- 
2.28.0-558-g7a0184fd7b