Re: [PATCH 1/4] push options: {pre,post}-receive hook learns about push options
From: Junio C Hamano <hidden>
Date: 2016-07-07 20:21:04
Stefan Beller [off-list ref] writes:
As we first want to focus on getting simple strings to work reliably, we go with the last option for now.
OK.
quoted hunk
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index d82e912..c875cde 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt@@ -247,6 +247,9 @@ Both standard output and standard error output are forwarded to 'git send-pack' on the other end, so you can simply `echo` messages for the user. +The number of push options are available in the variable GIT_PUSH_OPTION_COUNT +and the options itself are in GIT_PUSH_OPTION_{0,1,...}. + [[update]] update ~~~~~~@@ -322,6 +325,9 @@ a sample script `post-receive-email` provided in the `contrib/hooks` directory in Git distribution, which implements sending commit emails. +The number of push options are available in the variable GIT_PUSH_OPTION_COUNT +and the options itself are in GIT_PUSH_OPTION_{0,1,...}. +
The mention of "push options" in these two entries sounded a bit too
abrupt, at least to me. Perhaps add a short phrase before the
sentence, e.g.
When 'git push --push-option=...' is used, the number of push
options are avaiable ...
or
The number of push options given on the command line of "git
push --push-option=..." can be read from the environment
variable GIT_PUSH_OPTION_COUNT, and the options themselves are
found in GIT_PUSH_OPTION_0, GIT_PUSH_OPTION_1,...
We can read any of the above variants in two ways to describe what
should happen when "git push" is run without "--push-option=...".
Is GIT_PUSH_OPTION_COUNT unset, or set to 0? Either way, it may be
better to be a bit more explicit.
The hook driver code does not explicitly clear these environment
variables; it is safe to assume that these won't seep in from the
environment, I think, but I am not sure.
quoted hunk
@@ -1756,6 +1771,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix) if ((commands = read_head_info(&shallow)) != NULL) { const char *unpack_status = NULL; + struct string_list *push_options = NULL; prepare_shallow_info(&si, &shallow); if (!si.nr_ours && !si.nr_theirs)@@ -1764,13 +1780,19 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix) unpack_status = unpack_with_sideband(&si); update_shallow_info(commands, &si, &ref); } - execute_commands(commands, unpack_status, &si); + execute_commands(commands, unpack_status, &si, + push_options); if (pack_lockfile) unlink_or_warn(pack_lockfile); if (report_status) report(commands, unpack_status); - run_receive_hook(commands, "post-receive", 1); + run_receive_hook(commands, "post-receive", 1, + push_options); run_update_post_hook(commands); + if (push_options) { + string_list_clear(push_options, 0); + free(push_options); + } if (auto_gc) { const char *argv_gc_auto[] = { "gc", "--auto", "--quiet", NULL,diff --git a/templates/hooks--pre-receive.sample b/templates/hooks--pre-receive.sample new file mode 100644 index 0000000..e4d3edc --- /dev/null +++ b/templates/hooks--pre-receive.sample@@ -0,0 +1,22 @@ +#!/bin/sh +# +# An example hook script to make use of push options. +# The example simply echoes all push options that start with 'echoback=' +# and rejects all pushes when the "reject" push option is used. +# +# To enable this hook, rename this file to "pre-receive". + +if test -n "$GIT_PUSH_OPTION_COUNT"; then + i=0 + while test "$i" -lt "$GIT_PUSH_OPTION_COUNT"; do
Style:
if test -n "$GIT_PUSH_OPTION_COUNT"
then
...
while test ...
do
...
+ eval "value=\$GIT_PUSH_OPTION_$i"
+ case "$value" in
+ echoback=*)
+ echo "echo from the pre-receive-hook ${value#*=}" >&2
+ ;;
+ reject)
+ exit 1
+ esac
+ i=$((i + 1))
+ done
+fiWhat is suboptimal about the structure of the series is that we won't bisect down to any of the potential bugs in the above code even if we ever see any bug in the future. It also does not hint where push_options is expected to be read in the code in the subsequent patches in the series. If I were doing this series, I would probably have done 2/4 first without plumbing it through (i.e. it is sent and accumulated in a string list at the receiver, and then cleared and freed without being used), and then added the processing (i.e. this step) as the second patch.