Brandon Williams wrote:
> --- a/builtin/send-pack.c
> +++ b/builtin/send-pack.c
> @@ -152,6 +152,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
> int progress = -1;
> int from_stdin = 0;
> struct push_cas_option cas = {0};
> + struct string_list push_options = STRING_LIST_INIT_DUP;
It's safe for this to be NODUP, since the strings added to it live in
argv.
>
> struct option options[] = {
> OPT__VERBOSITY(&verbose),
> @@ -171,6 +172,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
> OPT_BOOL(0, "stateless-rpc", &stateless_rpc, N_("use stateless RPC protocol")),
> OPT_BOOL(0, "stdin", &from_stdin, N_("read refs from stdin")),
> OPT_BOOL(0, "helper-status", &helper_status, N_("print status from remote helper")),
> + OPT_STRING_LIST('o', "push-option", &push_options,
> + N_("server-specific"),
> + N_("option to transmit")),
Does this need the short-and-sweet option name 'o'? For this command,
I think I'd prefer giving it no short name.
Should this option be documented in the manpage, too?
[...]
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -22,6 +22,7 @@ struct options {
> unsigned long depth;
> char *deepen_since;
> struct string_list deepen_not;
> + struct string_list push_options;
> unsigned progress : 1,
> check_self_contained_and_connected : 1,
> cloning : 1,
> @@ -139,6 +140,9 @@ static int set_option(const char *name, const char *value)
> else
> return -1;
> return 0;
> + } else if (!strcmp(name, "push-option")) {
> + string_list_append(&options.push_options, value);
> + return 0;
push_options has strdup_strings enabled so this takes ownership of a
copy of value. Good.
[...]
> --- a/t/t5545-push-options.sh
> +++ b/t/t5545-push-options.sh
> @@ -102,7 +102,9 @@ test_expect_success 'two push options work' '
> test_cmp expect upstream/.git/hooks/post-receive.push_options
> '
>
> -test_expect_success 'push option denied properly by http remote helper' '\
> +test_expect_success 'push option denied properly by http server' '
Should this test use test_i18ngrep to check that the error message
diagnoses the problem correctly instead of hitting an unrelated error
condition?
[...]
> @@ -113,6 +115,32 @@ test_expect_success 'push option denied properly by http remote helper' '\
> git -C test_http_clone push origin master
> '
>
> +test_expect_success 'push options work properly across http' '
Nice.
Tested-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
With whatever subset of the following changes seems sensible to you
squashed in,
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks.
diff --git i/Documentation/git-send-pack.txt w/Documentation/git-send-pack.txt
index a831dd0288..966abb0df8 100644
--- i/Documentation/git-send-pack.txt
+++ w/Documentation/git-send-pack.txt
@@ -81,6 +81,12 @@ be in a separate packet, and the list must end with a flush packet.
will also fail if the actual call to `gpg --sign` fails. See
linkgit:git-receive-pack[1] for the details on the receiving end.
+--push-option=<string>::
+ Pass the specified string as a push option for consumption by
+ hooks on the server side. If the server doesn't support push
+ options, error out. See linkgit:git-push[1] and
+ linkgit:githooks[5] for details.
+
<host>::
A remote host to house the repository. When this
part is specified, 'git-receive-pack' is invoked via
diff --git i/builtin/send-pack.c w/builtin/send-pack.c
index 6796f33687..832fd7ed0a 100644
--- i/builtin/send-pack.c
+++ w/builtin/send-pack.c
@@ -144,6 +144,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
unsigned force_update = 0;
unsigned quiet = 0;
int push_cert = 0;
+ struct string_list push_options = STRING_LIST_INIT_NODUP;
unsigned use_thin_pack = 0;
unsigned atomic = 0;
unsigned stateless_rpc = 0;
@@ -152,7 +153,6 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
int progress = -1;
int from_stdin = 0;
struct push_cas_option cas = {0};
- struct string_list push_options = STRING_LIST_INIT_DUP;
struct option options[] = {
OPT__VERBOSITY(&verbose),
@@ -166,15 +166,15 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
{ OPTION_CALLBACK,
0, "signed", &push_cert, "yes|no|if-asked", N_("GPG sign the push"),
PARSE_OPT_OPTARG, option_parse_push_signed },
+ OPT_STRING_LIST(0, "push-option", &push_options,
+ N_("server-specific"),
+ N_("option to transmit")),
OPT_BOOL(0, "progress", &progress, N_("force progress reporting")),
OPT_BOOL(0, "thin", &use_thin_pack, N_("use thin pack")),
OPT_BOOL(0, "atomic", &atomic, N_("request atomic transaction on remote side")),
OPT_BOOL(0, "stateless-rpc", &stateless_rpc, N_("use stateless RPC protocol")),
OPT_BOOL(0, "stdin", &from_stdin, N_("read refs from stdin")),
OPT_BOOL(0, "helper-status", &helper_status, N_("print status from remote helper")),
- OPT_STRING_LIST('o', "push-option", &push_options,
- N_("server-specific"),
- N_("option to transmit")),
{ OPTION_CALLBACK,
0, CAS_OPT_NAME, &cas, N_("refname>:<expect"),
N_("require old value of ref to be at this value"),