Updated patch and added tests: (feel free to modify Documentation) --- Subject: [PATCH] Added support for new configuration parameter push.pushOption builtin/push.c: add push.pushOption config Currently push options need to be given explicitly, via the command line as "git push --push-option". The UX of Git would be enhanced if push options could be configured instead of given each time on the command line. Add the config option push.pushOption, which is a multi string option, containing push options that are sent by default. When push options are set in the system wide config (/etc/gitconfig), they can be unset later in the more specific repository config by setting the string to the empty string. Add tests and documentation as well. Signed-off-by: Marius Paliga <marius.paliga@xxxxxxxxx> --- Documentation/git-push.txt | 3 ++ builtin/push.c | 12 ++++++++ t/t5545-push-options.sh | 77 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 3e76e99f3..da9b17624 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -161,6 +161,9 @@ already exists on the remote side. Transmit the given string to the server, which passes them to the pre-receive as well as the post-receive hook. The given string must not contain a NUL or LF character. + Default push options can also be specified with configuration + variable `push.pushOption`. String(s) specified here will always + be passed to the server without need to specify it using `--push-option` --receive-pack=<git-receive-pack>:: --exec=<git-receive-pack>:: diff --git a/builtin/push.c b/builtin/push.c index 2ac810422..10e520c8f 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -32,6 +32,8 @@ static const char **refspec; static int refspec_nr; static int refspec_alloc; +static struct string_list push_options_from_config = STRING_LIST_INIT_DUP; + static void add_refspec(const char *ref) { refspec_nr++; @@ -503,6 +505,14 @@ static int git_push_config(const char *k, const char *v, void *cb) int val = git_config_bool(k, v) ? RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF; recurse_submodules = val; + } else if (!strcmp(k, "push.pushoption")) { + if (v == NULL) + return config_error_nonbool(k); + else + if (v[0] == '\0') + string_list_clear(&push_options_from_config, 0); + else + string_list_append(&push_options_from_config, v); } return git_default_config(k, v, NULL); @@ -562,6 +572,8 @@ int cmd_push(int argc, const char **argv, const char *prefix) packet_trace_identity("push"); git_config(git_push_config, &flags); argc = parse_options(argc, argv, prefix, options, push_usage, 0); + if (!push_options.nr) + push_options = push_options_from_config; set_push_cert_flags(&flags, push_cert); if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR)))) diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh index 90a4b0d2f..463783789 100755 --- a/t/t5545-push-options.sh +++ b/t/t5545-push-options.sh @@ -140,6 +140,83 @@ test_expect_success 'push options and submodules' ' test_cmp expect parent_upstream/.git/hooks/post-receive.push_options ' +test_expect_success 'default push option' ' + mk_repo_pair && + git -C upstream config receive.advertisePushOptions true && + ( + cd workbench && + test_commit one && + git push --mirror up && + test_commit two && + git -c push.pushOption=default push up master + ) && + test_refs master master && + echo "default" >expect && + test_cmp expect upstream/.git/hooks/pre-receive.push_options && + test_cmp expect upstream/.git/hooks/post-receive.push_options +' + +test_expect_success 'two default push options' ' + mk_repo_pair && + git -C upstream config receive.advertisePushOptions true && + ( + cd workbench && + test_commit one && + git push --mirror up && + test_commit two && + git -c push.pushOption=default1 -c push.pushOption=default2 push up master + ) && + test_refs master master && + printf "default1\ndefault2\n" >expect && + test_cmp expect upstream/.git/hooks/pre-receive.push_options && + test_cmp expect upstream/.git/hooks/post-receive.push_options +' + +test_expect_success 'push option from command line overrides from-config push option' ' + mk_repo_pair && + git -C upstream config receive.advertisePushOptions true && + ( + cd workbench && + test_commit one && + git push --mirror up && + test_commit two && + git -c push.pushOption=default push --push-option=manual up master + ) && + test_refs master master && + echo "manual" >expect && + test_cmp expect upstream/.git/hooks/pre-receive.push_options && + test_cmp expect upstream/.git/hooks/post-receive.push_options +' + +test_expect_success 'empty value of push.pushOption in config clears the list' ' + mk_repo_pair && + git -C upstream config receive.advertisePushOptions true && + ( + cd workbench && + test_commit one && + git push --mirror up && + test_commit two && + git -c push.pushOption=default1 -c push.pushOption= -c push.pushOption=default2 push up master + ) && + test_refs master master && + echo "default2" >expect && + test_cmp expect upstream/.git/hooks/pre-receive.push_options && + test_cmp expect upstream/.git/hooks/post-receive.push_options +' + +test_expect_success 'invalid push option in config' ' + mk_repo_pair && + git -C upstream config receive.advertisePushOptions true && + ( + cd workbench && + test_commit one && + git push --mirror up && + test_commit two && + test_must_fail git -c push.pushOption push up master + ) && + test_refs master HEAD@{1} +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd -- 2.14.1 2017-10-13 2:37 GMT+02:00 Junio C Hamano <gitster@xxxxxxxxx>: > Marius Paliga <marius.paliga@xxxxxxxxx> writes: > >> Thank you for your coments and explanation. >> >> Just one thing: >> >>> - After parse_options() returns to cmd_push(), see if push_options >>> is empty. If it is, you did not get any command line option, so >>> override it with what you collected in the "from-config" string >>> list. Otherwise, do not even look at "from-config" string list. >> >> The idea is that there are default push options (read from config) that are >> always sent to the server and you can add (not overwrite) additional by >> specifying "--push-option". > > I can imagine that sometimes giving a base from a configuration and > then adding more for specific invocation may be useful. > > But I do not think of any --command-line-option and config.variable > pair whose configured value cannot be overriden by the command line > option; we should strive to avoid making --push-option a special > case that the users need to aware of, and more importantly, users > other than you who expect the more usual "command line overrides" > behaviour should get that. > > Your "I wanted to accumulate, so I made so and made it impossible to > override" won't fly as a justification. The default should be > "command line overrides", and if you need a way to allow command > line to add without overiding, that should be added as an optional > feature. > > [alias] > mypush = push --push-option=foo --push-option=bar > > may give you a set of push-options that are always in effect (they > are not even "by default") and cannot be overriden. >