{"thread":{"id":"46887","subject":"Enhancement request: git-push: Allow (configurable) default push-option","startedAt":"2017-10-03T10:15:06Z","lastAt":"2017-10-17T07:16:00Z","messageCount":24,"participants":["Marius Paliga","Stefan Beller","Junio C Hamano","Thais D. Braz","Christian Couder","Thais Diniz","thais braz"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"329547","messageId":"CAK7vU=0ztEwMPsGO4Cd1A5JEnxmwkF57QPKjjvjD8rBUB79dRA@mail.gmail.com","threadId":"46887","inReplyTo":null,"subject":"Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Marius Paliga","fromEmail":"marius.paliga@gmail.com","sentAt":"2017-10-03T10:15:00Z","receivedAt":"2017-10-03T10:15:06Z","isPatch":false,"sender":{"key":"marius.paliga@gmail.com","avatar":"https://avatars.githubusercontent.com/u/54767958?v=4"},"body":"There is a need to pass predefined push-option during \"git push\"\nwithout need to specify it explicitly.\n\nIn another words we need to have a new \"git config\" variable to\nspecify string that will be automatically passed as \"--push-option\"\nwhen pushing to remote.\n\nSomething like the following:\n\ngit config push.optionDefault AllowMultipleCommits\n\nand then command\n git push\nwould silently run\n git push --push-option \"AllowMultipleCommits\"\n"},{"id":"329581","messageId":"CAGZ79kaBJnXW=rSiKuHpds79mXVL0Aoo+PBa0a5V-C_bop=Kbg@mail.gmail.com","threadId":"46887","inReplyTo":"CAK7vU=0ztEwMPsGO4Cd1A5JEnxmwkF57QPKjjvjD8rBUB79dRA@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-03T16:53:11Z","receivedAt":"2017-10-03T16:53:18Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Oct 3, 2017 at 3:15 AM, Marius Paliga wrote:\n> There is a need to pass predefined push-option during \"git push\"\n> without need to specify it explicitly.\n>\n> In another words we need to have a new \"git config\" variable to\n> specify string that will be automatically passed as \"--push-option\"\n> when pushing to remote.\n>\n> Something like the following:\n>\n> git config push.optionDefault AllowMultipleCommits\n>\n> and then command\n> git push\n> would silently run\n> git push --push-option \"AllowMultipleCommits\"\n\nWe would need to\n* design this feature (seems like you already have a good idea what you need)\n* implement it (see builtin/push.c):\n - move \"struct string_list push_options = STRING_LIST_INIT_DUP;\"\n to be a file-static variable, such that we have access to it outside\nof cmd_push.\n - In git_push_config in builtin/push.c that parses the config, we'd\nneed to check\n for \"push.optionDefault\" and add these to the push_options (I assume multiple\n are allowed)\n* document it (Documentation/git-push.txt)\n* add a test for it ? (t/t5545-push-options.sh)\n\nCare to write a patch? Otherwise I'd mark it up as part of\n#leftoverbits for now,\nas it seems like a good starter project.\n\nThanks,\nStefan\n"},{"id":"329690","messageId":"CAK7vU=3whGsx4L4KACSC+XDWQEbUWuZZZqTsW2R=CbF8d7rkuQ@mail.gmail.com","threadId":"46887","inReplyTo":"CAGZ79kaBJnXW=rSiKuHpds79mXVL0Aoo+PBa0a5V-C_bop=Kbg@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Marius Paliga","fromEmail":"marius.paliga@gmail.com","sentAt":"2017-10-04T15:20:10Z","receivedAt":"2017-10-04T15:20:19Z","isPatch":false,"sender":{"key":"marius.paliga@gmail.com","avatar":"https://avatars.githubusercontent.com/u/54767958?v=4"},"body":"Hi Stefan,\n\nI will look at it.\n\nThanks,\nMarius\n\n\n2017-10-03 18:53 GMT+02:00 Stefan Beller :\n> On Tue, Oct 3, 2017 at 3:15 AM, Marius Paliga wrote:\n>> There is a need to pass predefined push-option during \"git push\"\n>> without need to specify it explicitly.\n>>\n>> In another words we need to have a new \"git config\" variable to\n>> specify string that will be automatically passed as \"--push-option\"\n>> when pushing to remote.\n>>\n>> Something like the following:\n>>\n>> git config push.optionDefault AllowMultipleCommits\n>>\n>> and then command\n>> git push\n>> would silently run\n>> git push --push-option \"AllowMultipleCommits\"\n>\n> We would need to\n> * design this feature (seems like you already have a good idea what you need)\n> * implement it (see builtin/push.c):\n> - move \"struct string_list push_options = STRING_LIST_INIT_DUP;\"\n> to be a file-static variable, such that we have access to it outside\n> of cmd_push.\n> - In git_push_config in builtin/push.c that parses the config, we'd\n> need to check\n> for \"push.optionDefault\" and add these to the push_options (I assume multiple\n> are allowed)\n> * document it (Documentation/git-push.txt)\n> * add a test for it ? (t/t5545-push-options.sh)\n>\n> Care to write a patch? Otherwise I'd mark it up as part of\n> #leftoverbits for now,\n> as it seems like a good starter project.\n>\n> Thanks,\n> Stefan\n"},{"id":"330158","messageId":"CAK7vU=22W9mYdSnw_LP2uWYyKZuTzF0JgTVWCX+nMhUnLjQ_Cw@mail.gmail.com","threadId":"46887","inReplyTo":"CAK7vU=3whGsx4L4KACSC+XDWQEbUWuZZZqTsW2R=CbF8d7rkuQ@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Marius Paliga","fromEmail":"marius.paliga@gmail.com","sentAt":"2017-10-11T07:14:14Z","receivedAt":"2017-10-11T07:14:20Z","isPatch":false,"sender":{"key":"marius.paliga@gmail.com","avatar":"https://avatars.githubusercontent.com/u/54767958?v=4"},"body":"Including proposed patch...\n\n\nSigned-off-by: Marius Paliga \n---\n Documentation/git-push.txt | 3 +++\n builtin/push.c | 11 ++++++++++-\n t/t5545-push-options.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 61 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 3e76e99f3..133c42183 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -161,6 +161,9 @@ already exists on the remote side.\n Transmit the given string to the server, which passes them to\n the pre-receive as well as the post-receive hook. The given string\n must not contain a NUL or LF character.\n+ Default push options can also be specified with configuration\n+ variable `push.optiondefault`. String(s) specified here will always\n+ be passed to the server without need to specify it using `--push-option`\n\n --receive-pack=::\n --exec=::\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2ac810422..4dd5d6f0e 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -32,6 +32,8 @@ static const char **refspec;\n static int refspec_nr;\n static int refspec_alloc;\n\n+static struct string_list push_options = STRING_LIST_INIT_DUP;\n+\n static void add_refspec(const char *ref)\n {\n refspec_nr++;\n@@ -467,6 +469,8 @@ static int git_push_config(const char *k, const\nchar *v, void *cb)\n {\n int *flags = cb;\n int status;\n+ const struct string_list *default_push_options;\n+ struct string_list_item *item;\n\n status = git_gpg_config(k, v, NULL);\n if (status)\n@@ -505,6 +509,12 @@ static int git_push_config(const char *k, const\nchar *v, void *cb)\n recurse_submodules = val;\n }\n\n+ default_push_options = git_config_get_value_multi(\"push.optiondefault\");\n+ if (default_push_options)\n+ for_each_string_list_item(item, default_push_options)\n+ if (!string_list_has_string(&push_options, item->string))\n+ string_list_append(&push_options, item->string);\n+\n return git_default_config(k, v, NULL);\n }\n\n@@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const\nchar *prefix)\n int push_cert = -1;\n int rc;\n const char *repo = NULL; /* default repository */\n- struct string_list push_options = STRING_LIST_INIT_DUP;\n const struct string_list_item *item;\n\n struct option options[] = {\ndiff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh\nindex 90a4b0d2f..575f3dc38 100755\n--- a/t/t5545-push-options.sh\n+++ b/t/t5545-push-options.sh\n@@ -140,6 +140,54 @@ test_expect_success 'push options and submodules' '\n test_cmp expect parent_upstream/.git/hooks/post-receive.push_options\n '\n\n+test_expect_success 'default push option' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.optiondefault=default push up master\n+ ) &&\n+ test_refs master master &&\n+ echo \"default\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'two default push options' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.optiondefault=default1 -c\npush.optiondefault=default2 push up master\n+ ) &&\n+ test_refs master master &&\n+ printf \"default1\\ndefault2\\n\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'default and manual push options' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.optiondefault=default push --push-option=manual up master\n+ ) &&\n+ test_refs master master &&\n+ printf \"default\\nmanual\\n\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n\n-- \n2.14.1\n\n\n2017-10-04 17:20 GMT+02:00 Marius Paliga :\n> Hi Stefan,\n>\n> I will look at it.\n>\n> Thanks,\n> Marius\n>\n>\n> 2017-10-03 18:53 GMT+02:00 Stefan Beller :\n>> On Tue, Oct 3, 2017 at 3:15 AM, Marius Paliga wrote:\n>>> There is a need to pass predefined push-option during \"git push\"\n>>> without need to specify it explicitly.\n>>>\n>>> In another words we need to have a new \"git config\" variable to\n>>> specify string that will be automatically passed as \"--push-option\"\n>>> when pushing to remote.\n>>>\n>>> Something like the following:\n>>>\n>>> git config push.optionDefault AllowMultipleCommits\n>>>\n>>> and then command\n>>> git push\n>>> would silently run\n>>> git push --push-option \"AllowMultipleCommits\"\n>>\n>> We would need to\n>> * design this feature (seems like you already have a good idea what you need)\n>> * implement it (see builtin/push.c):\n>> - move \"struct string_list push_options = STRING_LIST_INIT_DUP;\"\n>> to be a file-static variable, such that we have access to it outside\n>> of cmd_push.\n>> - In git_push_config in builtin/push.c that parses the config, we'd\n>> need to check\n>> for \"push.optionDefault\" and add these to the push_options (I assume multiple\n>> are allowed)\n>> * document it (Documentation/git-push.txt)\n>> * add a test for it ? (t/t5545-push-options.sh)\n>>\n>> Care to write a patch? Otherwise I'd mark it up as part of\n>> #leftoverbits for now,\n>> as it seems like a good starter project.\n>>\n>> Thanks,\n>> Stefan\n"},{"id":"330167","messageId":"CAK7vU=0tRu=MAtBWHaQ6pGOGMfYdEorNd_930jq_3n_tLY2hsg@mail.gmail.com","threadId":"46887","inReplyTo":"CAK7vU=22W9mYdSnw_LP2uWYyKZuTzF0JgTVWCX+nMhUnLjQ_Cw@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Marius Paliga","fromEmail":"marius.paliga@gmail.com","sentAt":"2017-10-11T09:18:45Z","receivedAt":"2017-10-11T09:18:53Z","isPatch":false,"sender":{"key":"marius.paliga@gmail.com","avatar":"https://avatars.githubusercontent.com/u/54767958?v=4"},"body":"Found one possible issue when looking for duplicates, we need to use\n \"unsorted_string_list_has_string\" instead of \"string_list_has_string\"\n\n- if (!string_list_has_string(&push_options,\nitem->string))\n+ if\n(!unsorted_string_list_has_string(&push_options, item->string)) {\n\nNew (fixed) patch follows...\n\n\nSigned-off-by: Marius Paliga \n---\n Documentation/git-push.txt | 3 +++\n builtin/push.c | 11 ++++++++++-\n t/t5545-push-options.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 61 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 3e76e99f3..133c42183 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -161,6 +161,9 @@ already exists on the remote side.\n Transmit the given string to the server, which passes them to\n the pre-receive as well as the post-receive hook. The given string\n must not contain a NUL or LF character.\n+ Default push options can also be specified with configuration\n+ variable `push.optiondefault`. String(s) specified here will always\n+ be passed to the server without need to specify it using `--push-option`\n\n --receive-pack=::\n --exec=::\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2ac810422..ab458419a 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -32,6 +32,8 @@ static const char **refspec;\n static int refspec_nr;\n static int refspec_alloc;\n\n+static struct string_list push_options = STRING_LIST_INIT_DUP;\n+\n static void add_refspec(const char *ref)\n {\n refspec_nr++;\n@@ -467,6 +469,8 @@ static int git_push_config(const char *k, const\nchar *v, void *cb)\n {\n int *flags = cb;\n int status;\n+ const struct string_list *default_push_options;\n+ struct string_list_item *item;\n\n status = git_gpg_config(k, v, NULL);\n if (status)\n@@ -505,6 +509,12 @@ static int git_push_config(const char *k, const\nchar *v, void *cb)\n recurse_submodules = val;\n }\n\n+ default_push_options = git_config_get_value_multi(\"push.optiondefault\");\n+ if (default_push_options)\n+ for_each_string_list_item(item, default_push_options)\n+ if (!unsorted_string_list_has_string(&push_options, item->string))\n+ string_list_append(&push_options, item->string);\n+\n return git_default_config(k, v, NULL);\n }\n\n@@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const\nchar *prefix)\n int push_cert = -1;\n int rc;\n const char *repo = NULL; /* default repository */\n- struct string_list push_options = STRING_LIST_INIT_DUP;\n const struct string_list_item *item;\n\n struct option options[] = {\ndiff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh\nindex 90a4b0d2f..575f3dc38 100755\n--- a/t/t5545-push-options.sh\n+++ b/t/t5545-push-options.sh\n@@ -140,6 +140,54 @@ test_expect_success 'push options and submodules' '\n test_cmp expect parent_upstream/.git/hooks/post-receive.push_options\n '\n\n+test_expect_success 'default push option' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.optiondefault=default push up master\n+ ) &&\n+ test_refs master master &&\n+ echo \"default\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'two default push options' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.optiondefault=default1 -c\npush.optiondefault=default2 push up master\n+ ) &&\n+ test_refs master master &&\n+ printf \"default1\\ndefault2\\n\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'default and manual push options' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.optiondefault=default push --push-option=manual up master\n+ ) &&\n+ test_refs master master &&\n+ printf \"default\\nmanual\\n\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n\n-- \n2.14.1\n\n2017-10-11 9:14 GMT+02:00 Marius Paliga :\n> Including proposed patch...\n>\n>\n> Signed-off-by: Marius Paliga \n> ---\n> Documentation/git-push.txt | 3 +++\n> builtin/push.c | 11 ++++++++++-\n> t/t5545-push-options.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 61 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\n> index 3e76e99f3..133c42183 100644\n> --- a/Documentation/git-push.txt\n> +++ b/Documentation/git-push.txt\n> @@ -161,6 +161,9 @@ already exists on the remote side.\n> Transmit the given string to the server, which passes them to\n> the pre-receive as well as the post-receive hook. The given string\n> must not contain a NUL or LF character.\n> + Default push options can also be specified with configuration\n> + variable `push.optiondefault`. String(s) specified here will always\n> + be passed to the server without need to specify it using `--push-option`\n>\n> --receive-pack=::\n> --exec=::\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 2ac810422..4dd5d6f0e 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -32,6 +32,8 @@ static const char **refspec;\n> static int refspec_nr;\n> static int refspec_alloc;\n>\n> +static struct string_list push_options = STRING_LIST_INIT_DUP;\n> +\n> static void add_refspec(const char *ref)\n> {\n> refspec_nr++;\n> @@ -467,6 +469,8 @@ static int git_push_config(const char *k, const\n> char *v, void *cb)\n> {\n> int *flags = cb;\n> int status;\n> + const struct string_list *default_push_options;\n> + struct string_list_item *item;\n>\n> status = git_gpg_config(k, v, NULL);\n> if (status)\n> @@ -505,6 +509,12 @@ static int git_push_config(const char *k, const\n> char *v, void *cb)\n> recurse_submodules = val;\n> }\n>\n> + default_push_options = git_config_get_value_multi(\"push.optiondefault\");\n> + if (default_push_options)\n> + for_each_string_list_item(item, default_push_options)\n> + if (!string_list_has_string(&push_options, item->string))\n> + string_list_append(&push_options, item->string);\n> +\n> return git_default_config(k, v, NULL);\n> }\n>\n> @@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const\n> char *prefix)\n> int push_cert = -1;\n> int rc;\n> const char *repo = NULL; /* default repository */\n> - struct string_list push_options = STRING_LIST_INIT_DUP;\n> const struct string_list_item *item;\n>\n> struct option options[] = {\n> diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh\n> index 90a4b0d2f..575f3dc38 100755\n> --- a/t/t5545-push-options.sh\n> +++ b/t/t5545-push-options.sh\n> @@ -140,6 +140,54 @@ test_expect_success 'push options and submodules' '\n> test_cmp expect parent_upstream/.git/hooks/post-receive.push_options\n> '\n>\n> +test_expect_success 'default push option' '\n> + mk_repo_pair &&\n> + git -C upstream config receive.advertisePushOptions true &&\n> + (\n> + cd workbench &&\n> + test_commit one &&\n> + git push --mirror up &&\n> + test_commit two &&\n> + git -c push.optiondefault=default push up master\n> + ) &&\n> + test_refs master master &&\n> + echo \"default\" >expect &&\n> + test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n> + test_cmp expect upstream/.git/hooks/post-receive.push_options\n> +'\n> +\n> +test_expect_success 'two default push options' '\n> + mk_repo_pair &&\n> + git -C upstream config receive.advertisePushOptions true &&\n> + (\n> + cd workbench &&\n> + test_commit one &&\n> + git push --mirror up &&\n> + test_commit two &&\n> + git -c push.optiondefault=default1 -c\n> push.optiondefault=default2 push up master\n> + ) &&\n> + test_refs master master &&\n> + printf \"default1\\ndefault2\\n\" >expect &&\n> + test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n> + test_cmp expect upstream/.git/hooks/post-receive.push_options\n> +'\n> +\n> +test_expect_success 'default and manual push options' '\n> + mk_repo_pair &&\n> + git -C upstream config receive.advertisePushOptions true &&\n> + (\n> + cd workbench &&\n> + test_commit one &&\n> + git push --mirror up &&\n> + test_commit two &&\n> + git -c push.optiondefault=default push --push-option=manual up master\n> + ) &&\n> + test_refs master master &&\n> + printf \"default\\nmanual\\n\" >expect &&\n> + test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n> + test_cmp expect upstream/.git/hooks/post-receive.push_options\n> +'\n> +\n> . \"$TEST_DIRECTORY\"/lib-httpd.sh\n> start_httpd\n>\n> --\n> 2.14.1\n>\n>\n> 2017-10-04 17:20 GMT+02:00 Marius Paliga :\n>> Hi Stefan,\n>>\n>> I will look at it.\n>>\n>> Thanks,\n>> Marius\n>>\n>>\n>> 2017-10-03 18:53 GMT+02:00 Stefan Beller :\n>>> On Tue, Oct 3, 2017 at 3:15 AM, Marius Paliga wrote:\n>>>> There is a need to pass predefined push-option during \"git push\"\n>>>> without need to specify it explicitly.\n>>>>\n>>>> In another words we need to have a new \"git config\" variable to\n>>>> specify string that will be automatically passed as \"--push-option\"\n>>>> when pushing to remote.\n>>>>\n>>>> Something like the following:\n>>>>\n>>>> git config push.optionDefault AllowMultipleCommits\n>>>>\n>>>> and then command\n>>>> git push\n>>>> would silently run\n>>>> git push --push-option \"AllowMultipleCommits\"\n>>>\n>>> We would need to\n>>> * design this feature (seems like you already have a good idea what you need)\n>>> * implement it (see builtin/push.c):\n>>> - move \"struct string_list push_options = STRING_LIST_INIT_DUP;\"\n>>> to be a file-static variable, such that we have access to it outside\n>>> of cmd_push.\n>>> - In git_push_config in builtin/push.c that parses the config, we'd\n>>> need to check\n>>> for \"push.optionDefault\" and add these to the push_options (I assume multiple\n>>> are allowed)\n>>> * document it (Documentation/git-push.txt)\n>>> * add a test for it ? (t/t5545-push-options.sh)\n>>>\n>>> Care to write a patch? Otherwise I'd mark it up as part of\n>>> #leftoverbits for now,\n>>> as it seems like a good starter project.\n>>>\n>>> Thanks,\n>>> Stefan\n"},{"id":"330170","messageId":"xmqq4lr6lyyb.fsf@gitster.mtv.corp.google.com","threadId":"46887","inReplyTo":"CAK7vU=22W9mYdSnw_LP2uWYyKZuTzF0JgTVWCX+nMhUnLjQ_Cw@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-11T11:13:16Z","receivedAt":"2017-10-11T11:13:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marius Paliga writes:\n\n> + default_push_options = git_config_get_value_multi(\"push.optiondefault\");\n> + if (default_push_options)\n> + for_each_string_list_item(item, default_push_options)\n> + if (!string_list_has_string(&push_options, item->string))\n> + string_list_append(&push_options, item->string);\n\nOne thing that is often overlooked is how to allow users to override\na multi-value configuration variable that gets some values from\nlower priority configuration files (e.g. ~/.gitconfig) with\nrepository specific settings in .git/config, and the way we\ntypically do so is to define \"When a variable definition with an\nempty string is given, it is a signal to clear everything\naccumulated so far.\" E.g. if your ~/.gitconfig has\n\n\t[push]\n\t\tdefaultPushOption = foo\n\t\tdefaultPushOption = bar\n\nand then you write in your .git/config something like\n\n\t[push]\n\t\tdefaultPushOption =\n\t\tdefaultPushOption = baz\n\nThe configuration mechanism reads from lower priority files and then\nproceeds to read higher priority files, so the parser would read them\nin this order:\n\n\tpush.defaultPushOption = foo\n\tpush.defaultPushOption = bar\n\tpush.defaultPushOption = \n\tpush.defaultPushOption = baz\n\nand then it would build a list ('foo'), then ('foo', 'bar'), and\nclears it upon seeing an empty, and compute the final result as\n('baz').\n\nYou may want to do something like that in this code.\n\n \n"},{"id":"330177","messageId":"xmqqh8v5ls8i.fsf@gitster.mtv.corp.google.com","threadId":"46887","inReplyTo":"CAK7vU=22W9mYdSnw_LP2uWYyKZuTzF0JgTVWCX+nMhUnLjQ_Cw@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-11T13:38:21Z","receivedAt":"2017-10-11T13:38:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marius Paliga writes:\n\n> @@ -505,6 +509,12 @@ static int git_push_config(const char *k, const\n> char *v, void *cb)\n> recurse_submodules = val;\n> }\n>\n> + default_push_options = git_config_get_value_multi(\"push.optiondefault\");\n> + if (default_push_options)\n> + for_each_string_list_item(item, default_push_options)\n> + if (!string_list_has_string(&push_options, item->string))\n> + string_list_append(&push_options, item->string);\n> +\n> return git_default_config(k, v, NULL);\n> }\n\nSorry for not catching this earlier, but git_config_get_value* call\ninside git_push_config() is just wrong.\n\nThere are two styles of configuration parsing. The original (and\nstill perfectly valid) way is to call git_config() with a callback\nfunction like git_push_config(). Under this style, the config files\nare read from lower-priority to higher-priority ones, and the\ncallback function is called once for each entry found, with \npair and the callback specific opaque data. One way to add the\nparsing of a new variable like push.optiondefault is to add\n\n\tif (!strcmp(k, \"push.optiondefault\") {\n\t\t... handle one \"[push] optiondefault\" entry here ...\n\t\treturn 0;\n\t}\n\nto the function.\n\nAn alternate way is to use git_config_get_* functions _outside_\ncallback of git_config(). This is a newer invention. Your call to\ngit_config_get_value_multi() will scan all configuration files and\nreturns _all_ entries for the given variable at once.\n\nWhen there is already a callback style parser, in general, it is\ncleaner to simply piggy-back on it, instead of reading variables\nindependently using git_config_get_* functions. When there isn't a\ncallback style parser, using either style is OK. It also is OK to\nswitch to git_config_get_* altogether, rewriting the callback style\nparser, but I do not think it is warranted in this case, which adds\njust one variable.\n\nIn any case, with the above code, you'll end up calling the\ngit_config_get_* function and grabbing all the values for\npush.optiondefault for each and every configuration variable\ndefinition (count \"git config -l | wc -l\" to estimate how many times\nit will be called). Which is probably not what you wanted to do.\n\nAlso, watch out for how a configuration variable defined like below\nis reported to either of the above two styles:\n\n\t[push]\toptiondefault\n\n - To a git_config() callback function like git_push_config(), such\n an entry is called with k==\"push.optiondefault\", v==NULL.\n\n - git_config_get_value_multi() would return a string-list element\n with the string set to NULL to signal that one value is NULL\n (i.e. it is different from \"[push] optiondefault = \").\n\nI suspect that with your code, we'd hit\n\n\tif (strchr(item->string, '\\n'))\n\nand end up dereferencing NULL right there.\n\n> @@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const\n> char *prefix)\n> int push_cert = -1;\n> int rc;\n> const char *repo = NULL; /* default repository */\n> - struct string_list push_options = STRING_LIST_INIT_DUP;\n> const struct string_list_item *item;\n>\n> struct option options[] = {\n\nAlso, I suspect that this code does not allow the command line\noption to override the default set in the configuration file.\nOPT_STRING_LIST() appends to the &push_options string list without\nfirst clearing it, and you are pre-populating the list while reading\nthe configuration, so the values taken from the command line will\nonly add to them.\n\nThe right way to do this would probably be:\n\n - Do not muck with push_options in cmd_push().\n\n - Prepare another string list, push_options_from_config, that is\n file-scope global.\n\n - In git_push_config(), do not call get_multi; instead react to a\n call with k==\"push.optionsdefault\" and\n\n - reject if \"v\" is NULL, with \"return config_error_nonbool(k);\"\n\n - otherwise, append \"v\" to the \"from-config\" string list--do not\n attempt to dedup or sort.\n\n - if \"v\" is an empty string, clear the \"from-config\" list.\n\n - After parse_options() returns to cmd_push(), see if push_options\n is empty. If it is, you did not get any command line option, so\n override it with what you collected in the \"from-config\" string\n list. Otherwise, do not even look at \"from-config\" string list.\n\nBy the way, I really hate \"push.optiondefault\" as the variable\nname. The \"default\" part is obvious and there is no need to say it,\nas the configuration variables are there to give the default to what\nwe would normally give from the command line. Rather, you should\nsay for which option (there are many options \"git push\" takes) this\nvariable gives the default. Perhaps \"push.pushOption\" is a much\nbetter name; I am sure people can come up with even better ones,\nthough ;-)\n\n"},{"id":"330208","messageId":"20171011202505.10113-1-thais.dinizbraz@gmail.com","threadId":"46887","inReplyTo":"CAK7vU=3whGsx4L4KACSC+XDWQEbUWuZZZqTsW2R=CbF8d7rkuQ@mail.gmail.com","subject":"","fromName":"Thais D. Braz","fromEmail":"thais.dinizbraz@gmail.com","sentAt":"2017-10-11T20:25:04Z","receivedAt":"2017-10-11T20:25:23Z","isPatch":false,"sender":{"key":"thais.dinizbraz@gmail.com","avatar":null},"body":"I'll still try to make the test as suggested to complete this demand\n\n"},{"id":"330209","messageId":"20171011202505.10113-2-thais.dinizbraz@gmail.com","threadId":"46887","inReplyTo":"20171011202505.10113-1-thais.dinizbraz@gmail.com","subject":"[PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option","fromName":"Thais D. Braz","fromEmail":"thais.dinizbraz@gmail.com","sentAt":"2017-10-11T20:25:05Z","receivedAt":"2017-10-11T20:25:40Z","isPatch":true,"sender":{"key":"thais.dinizbraz@gmail.com","avatar":null},"body":"---\n Documentation/git-push.txt | 3 +++\n builtin/push.c | 9 ++++++++-\n 2 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 3e76e99f3..e1036feaf 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -161,6 +161,9 @@ already exists on the remote side.\n \tTransmit the given string to the server, which passes them to\n \tthe pre-receive as well as the post-receive hook. The given string\n \tmust not contain a NUL or LF character.\n+\tCan be configured using \"git config push.optionDefault \".\n+\tAfter configured git push will always be executed silently\n+\twith --push-options .\n \n --receive-pack=::\n --exec=::\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2ac810422..ae3efafce 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -467,11 +467,18 @@ static int git_push_config(const char *k, const char *v, void *cb)\n {\n \tint *flags = cb;\n \tint status;\n+\tstruct string_list push_options = STRING_LIST_INIT_DUP;\n \n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n \t\treturn status;\n \n+\tconst struct string_list *optionsDefault = git_config_get_value_multi(\"push.optionDefault\");\n+\tfor (int i = 0; i < optionsDefault->nr; i++) {\n+\t\tstring_list_insert(&push_options, optionsDefault->items[i].string);\n+\t}\n+\n+\n \tif (!strcmp(k, \"push.followtags\")) {\n \t\tif (git_config_bool(k, v))\n \t\t\t*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n@@ -515,7 +522,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \tint push_cert = -1;\n \tint rc;\n \tconst char *repo = NULL;\t/* default repository */\n-\tstruct string_list push_options = STRING_LIST_INIT_DUP;\n+\tstatic struct string_list push_options = STRING_LIST_INIT_DUP;\n \tconst struct string_list_item *item;\n \n \tstruct option options[] = {\n-- \n2.15.0.rc0.39.g2f0e14e.dirty\n\n"},{"id":"330210","messageId":"20171011204019.11093-1-thais.dinizbraz@gmail.com","threadId":"46887","inReplyTo":"CAK7vU=3whGsx4L4KACSC+XDWQEbUWuZZZqTsW2R=CbF8d7rkuQ@mail.gmail.com","subject":"[PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option","fromName":"Thais D. Braz","fromEmail":"thais.dinizbraz@gmail.com","sentAt":"2017-10-11T20:40:19Z","receivedAt":"2017-10-11T20:40:31Z","isPatch":true,"sender":{"key":"thais.dinizbraz@gmail.com","avatar":null},"body":"---\n Documentation/git-push.txt | 3 +++\n builtin/push.c | 9 ++++++++-\n 2 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 3e76e99f3..e1036feaf 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -161,6 +161,9 @@ already exists on the remote side.\n \tTransmit the given string to the server, which passes them to\n \tthe pre-receive as well as the post-receive hook. The given string\n \tmust not contain a NUL or LF character.\n+\tCan be configured using \"git config push.optionDefault \".\n+\tAfter configured git push will always be executed silently\n+\twith --push-options .\n \n --receive-pack=::\n --exec=::\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2ac810422..ae3efafce 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -467,11 +467,18 @@ static int git_push_config(const char *k, const char *v, void *cb)\n {\n \tint *flags = cb;\n \tint status;\n+\tstruct string_list push_options = STRING_LIST_INIT_DUP;\n \n \tstatus = git_gpg_config(k, v, NULL);\n \tif (status)\n \t\treturn status;\n \n+\tconst struct string_list *optionsDefault = git_config_get_value_multi(\"push.optionDefault\");\n+\tfor (int i = 0; i < optionsDefault->nr; i++) {\n+\t\tstring_list_insert(&push_options, optionsDefault->items[i].string);\n+\t}\n+\n+\n \tif (!strcmp(k, \"push.followtags\")) {\n \t\tif (git_config_bool(k, v))\n \t\t\t*flags |= TRANSPORT_PUSH_FOLLOW_TAGS;\n@@ -515,7 +522,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \tint push_cert = -1;\n \tint rc;\n \tconst char *repo = NULL;\t/* default repository */\n-\tstruct string_list push_options = STRING_LIST_INIT_DUP;\n+\tstatic struct string_list push_options = STRING_LIST_INIT_DUP;\n \tconst struct string_list_item *item;\n \n \tstruct option options[] = {\n-- \n2.15.0.rc0.39.g2f0e14e.dirty\n\n"},{"id":"330211","messageId":"CAGZ79kbBY0wAor6wS5gVNw95+ETkhxxeP3UeGoNwbA6g2ZbkkA@mail.gmail.com","threadId":"46887","inReplyTo":"CAK7vU=0tRu=MAtBWHaQ6pGOGMfYdEorNd_930jq_3n_tLY2hsg@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-11T20:52:23Z","receivedAt":"2017-10-11T20:52:29Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Oct 11, 2017 at 2:18 AM, Marius Paliga wrote:\n> Found one possible issue when looking for duplicates, we need to use\n> \"unsorted_string_list_has_string\" instead of \"string_list_has_string\"\n>\n> - if (!string_list_has_string(&push_options,\n> item->string))\n> + if\n> (!unsorted_string_list_has_string(&push_options, item->string)) {\n>\n> New (fixed) patch follows...\n>\n>\n> Signed-off-by: Marius Paliga \n\nYay, thanks for working on this!\n\nJunio gave good advice to the patch itself (the code), another thing\nis the commit message, which follows the formalities with the sign off,\nbut the content is not addressing the target audience.\n\nThe current commit message is written as an improvement compared\nto the previous patch and for readers who are reviewing the patch\nright now.\n\nCommit messages are read by people later in time, also they do not\ncare about the different iterations of the patch, as only the final iteration\nmatters.\n\nI think for the commit message you can borrow from the very first email\nyou sent to the list, maybe something like:\n\n builtin/push.c: add push.pushOption config\n\n Currently push options need to be given explicitly, via\n the command line as \"git push --push-option\".\n\n Some code review systems [which?] need specific push options\n nearly all the time, so the UX of Git would be enhanced if push\n options could be configured instead of given each time on the\\\n command line.\n\n Add the config option push.pushOption, which is a multi\n string option, containing push options that are sent by default.\n\n When push options are set in the system wide config\n (/etc/gitconfig), they can be unset(?) later in the more specific\n repository config by setting the string to the empty string.\n\n Add tests and documentation as well.\n\n Signed-off-by ...\n"},{"id":"330222","messageId":"xmqqa80x2mb4.fsf@gitster.mtv.corp.google.com","threadId":"46887","inReplyTo":"20171011202505.10113-2-thais.dinizbraz@gmail.com","subject":"Re: [PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-12T01:21:19Z","receivedAt":"2017-10-12T01:21:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Thais D. Braz\" writes:\n\n> ---\n> Documentation/git-push.txt | 3 +++\n> builtin/push.c | 9 ++++++++-\n> 2 files changed, 11 insertions(+), 1 deletion(-)\n\nCan somebody explain what is going on? \n\nI am guessing that Thais and marius are different people (judging by\nthe fact that one CC's a message to the other). Are you two\ncollaborating on this change, or something?\n\nIt puzzles me to see almost identical change sent to the list\nwithout much explanation from multiple parties, with no apparent\ninter-developer coordination.\n\nThanks.\n"},{"id":"330227","messageId":"CAP8UFD3VnVod7SPTM11XG=vHBYD4EwA+xZB563iwpL1Ue4pPnA@mail.gmail.com","threadId":"46887","inReplyTo":"xmqqa80x2mb4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2017-10-12T02:41:49Z","receivedAt":"2017-10-12T02:41:59Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Oct 12, 2017 at 3:21 AM, Junio C Hamano wrote:\n> \"Thais D. Braz\" writes:\n>\n>> ---\n>> Documentation/git-push.txt | 3 +++\n>> builtin/push.c | 9 ++++++++-\n>> 2 files changed, 11 insertions(+), 1 deletion(-)\n>\n> Can somebody explain what is going on?\n>\n> I am guessing that Thais and marius are different people (judging by\n> the fact that one CC's a message to the other). Are you two\n> collaborating on this change, or something?\n\nI guess that Thais decided to work on this, because we ask Outreachy\napplicants to search for #leftoverbits mentions in the mailing list\narchive to find small tasks they could work on.\n\nIn this case it looks like Marius sent a patch a few hours before\nThais also sent one.\n\nThais, I am sorry, but as Marius sent a patch first, I think it is\nbetter if you search for another different small task to work on.\nAlso please keep Peff and me in cc.\n\nThanks.\n"},{"id":"330232","messageId":"xmqqmv4x11y9.fsf@gitster.mtv.corp.google.com","threadId":"46887","inReplyTo":"CAP8UFD3VnVod7SPTM11XG=vHBYD4EwA+xZB563iwpL1Ue4pPnA@mail.gmail.com","subject":"Re: [PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-12T03:26:22Z","receivedAt":"2017-10-12T03:30:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>> Can somebody explain what is going on?\n>>\n>> I am guessing that Thais and marius are different people (judging by\n>> the fact that one CC's a message to the other). Are you two\n>> collaborating on this change, or something?\n>\n> I guess that Thais decided to work on this, because we ask Outreachy\n> applicants to search for #leftoverbits mentions in the mailing list\n> archive to find small tasks they could work on.\n>\n> In this case it looks like Marius sent a patch a few hours before\n> Thais also sent one.\n\n... after seeing Marius's already working on it, I think.\n\n> Thais, I am sorry, but as Marius sent a patch first, I think it is\n> better if you search for another different small task to work on.\n\nIn general, I do not mind seeing people working together well, and\nit is one of the more important skills necessary in the open source\ncommunity. I however tend to agree with you that this is a bit too\nsmall a topic for multiple people to be working on.\n\n> Also please keep Peff and me in cc.\n\nYup, that is always a good idea.\n\n"},{"id":"330286","messageId":"CAK7vU=0wbGsFCXmwmCc-XX9K07UF_OZ7tFa4_GVb-H7fxakssg@mail.gmail.com","threadId":"46887","inReplyTo":"xmqqh8v5ls8i.fsf@gitster.mtv.corp.google.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Marius Paliga","fromEmail":"marius.paliga@gmail.com","sentAt":"2017-10-12T14:59:47Z","receivedAt":"2017-10-12T14:59:54Z","isPatch":false,"sender":{"key":"marius.paliga@gmail.com","avatar":"https://avatars.githubusercontent.com/u/54767958?v=4"},"body":"Thank you for your coments and explanation.\n\nJust one thing:\n\n> - After parse_options() returns to cmd_push(), see if push_options\n> is empty. If it is, you did not get any command line option, so\n> override it with what you collected in the \"from-config\" string\n> list. Otherwise, do not even look at \"from-config\" string list.\n\nThe idea is that there are default push options (read from config) that are\nalways sent to the server and you can add (not overwrite) additional by\nspecifying \"--push-option\".\nSo I would rather concatenate both lists - from command line and from-config.\n\n> By the way, I really hate \"push.optiondefault\" as the variable\n> name. The \"default\" part is obvious and there is no need to say it,\n> as the configuration variables are there to give the default to what\n> we would normally give from the command line. Rather, you should\n> say for which option (there are many options \"git push\" takes) this\n> variable gives the default. Perhaps \"push.pushOption\" is a much\n> better name; I am sure people can come up with even better ones,\n> though ;-)\n\nIn the light of the above the \"default\" may be correct, but I don't\nhave a problem\nwith any name.\n\nMarius\n\n\n2017-10-11 15:38 GMT+02:00 Junio C Hamano :\n> Marius Paliga writes:\n>\n>> @@ -505,6 +509,12 @@ static int git_push_config(const char *k, const\n>> char *v, void *cb)\n>> recurse_submodules = val;\n>> }\n>>\n>> + default_push_options = git_config_get_value_multi(\"push.optiondefault\");\n>> + if (default_push_options)\n>> + for_each_string_list_item(item, default_push_options)\n>> + if (!string_list_has_string(&push_options, item->string))\n>> + string_list_append(&push_options, item->string);\n>> +\n>> return git_default_config(k, v, NULL);\n>> }\n>\n> Sorry for not catching this earlier, but git_config_get_value* call\n> inside git_push_config() is just wrong.\n>\n> There are two styles of configuration parsing. The original (and\n> still perfectly valid) way is to call git_config() with a callback\n> function like git_push_config(). Under this style, the config files\n> are read from lower-priority to higher-priority ones, and the\n> callback function is called once for each entry found, with \n> pair and the callback specific opaque data. One way to add the\n> parsing of a new variable like push.optiondefault is to add\n>\n> if (!strcmp(k, \"push.optiondefault\") {\n> ... handle one \"[push] optiondefault\" entry here ...\n> return 0;\n> }\n>\n> to the function.\n>\n> An alternate way is to use git_config_get_* functions _outside_\n> callback of git_config(). This is a newer invention. Your call to\n> git_config_get_value_multi() will scan all configuration files and\n> returns _all_ entries for the given variable at once.\n>\n> When there is already a callback style parser, in general, it is\n> cleaner to simply piggy-back on it, instead of reading variables\n> independently using git_config_get_* functions. When there isn't a\n> callback style parser, using either style is OK. It also is OK to\n> switch to git_config_get_* altogether, rewriting the callback style\n> parser, but I do not think it is warranted in this case, which adds\n> just one variable.\n>\n> In any case, with the above code, you'll end up calling the\n> git_config_get_* function and grabbing all the values for\n> push.optiondefault for each and every configuration variable\n> definition (count \"git config -l | wc -l\" to estimate how many times\n> it will be called). Which is probably not what you wanted to do.\n>\n> Also, watch out for how a configuration variable defined like below\n> is reported to either of the above two styles:\n>\n> [push] optiondefault\n>\n> - To a git_config() callback function like git_push_config(), such\n> an entry is called with k==\"push.optiondefault\", v==NULL.\n>\n> - git_config_get_value_multi() would return a string-list element\n> with the string set to NULL to signal that one value is NULL\n> (i.e. it is different from \"[push] optiondefault = \").\n>\n> I suspect that with your code, we'd hit\n>\n> if (strchr(item->string, '\\n'))\n>\n> and end up dereferencing NULL right there.\n>\n>> @@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const\n>> char *prefix)\n>> int push_cert = -1;\n>> int rc;\n>> const char *repo = NULL; /* default repository */\n>> - struct string_list push_options = STRING_LIST_INIT_DUP;\n>> const struct string_list_item *item;\n>>\n>> struct option options[] = {\n>\n> Also, I suspect that this code does not allow the command line\n> option to override the default set in the configuration file.\n> OPT_STRING_LIST() appends to the &push_options string list without\n> first clearing it, and you are pre-populating the list while reading\n> the configuration, so the values taken from the command line will\n> only add to them.\n>\n> The right way to do this would probably be:\n>\n> - Do not muck with push_options in cmd_push().\n>\n> - Prepare another string list, push_options_from_config, that is\n> file-scope global.\n>\n> - In git_push_config(), do not call get_multi; instead react to a\n> call with k==\"push.optionsdefault\" and\n>\n> - reject if \"v\" is NULL, with \"return config_error_nonbool(k);\"\n>\n> - otherwise, append \"v\" to the \"from-config\" string list--do not\n> attempt to dedup or sort.\n>\n> - if \"v\" is an empty string, clear the \"from-config\" list.\n>\n> - After parse_options() returns to cmd_push(), see if push_options\n> is empty. If it is, you did not get any command line option, so\n> override it with what you collected in the \"from-config\" string\n> list. Otherwise, do not even look at \"from-config\" string list.\n>\n> By the way, I really hate \"push.optiondefault\" as the variable\n> name. The \"default\" part is obvious and there is no need to say it,\n> as the configuration variables are there to give the default to what\n> we would normally give from the command line. Rather, you should\n> say for which option (there are many options \"git push\" takes) this\n> variable gives the default. Perhaps \"push.pushOption\" is a much\n> better name; I am sure people can come up with even better ones,\n> though ;-)\n>\n"},{"id":"330291","messageId":"CAK7vU=0A_nf2bAgV6dQyiesLJ3HHs5guyfNSNTwzYzgS2+YeWg@mail.gmail.com","threadId":"46887","inReplyTo":"CAK7vU=0wbGsFCXmwmCc-XX9K07UF_OZ7tFa4_GVb-H7fxakssg@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Marius Paliga","fromEmail":"marius.paliga@gmail.com","sentAt":"2017-10-12T16:32:09Z","receivedAt":"2017-10-12T16:32:18Z","isPatch":false,"sender":{"key":"marius.paliga@gmail.com","avatar":"https://avatars.githubusercontent.com/u/54767958?v=4"},"body":"Subject: [PATCH] Added support for new configuration parameter push.pushOption\n\nbuiltin/push.c: add push.pushOption config\n\nCurrently push options need to be given explicitly, via\nthe command line as \"git push --push-option\".\n\nThe UX of Git would be enhanced if push options could be\nconfigured instead of given each time on the command line.\n\nAdd the config option push.pushOption, which is a multi\nstring option, containing push options that are sent by default.\n\nWhen push options are set in the system wide config\n(/etc/gitconfig), they can be unset later in the more specific\nrepository config by setting the string to the empty string.\n\nAdd tests and documentation as well.\n\nSigned-off-by: Marius Paliga \n---\n Documentation/git-push.txt | 3 +++\n builtin/push.c | 11 ++++++++++-\n t/t5545-push-options.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 61 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 3e76e99f3..da9b17624 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -161,6 +161,9 @@ already exists on the remote side.\n Transmit the given string to the server, which passes them to\n the pre-receive as well as the post-receive hook. The given string\n must not contain a NUL or LF character.\n+ Default push options can also be specified with configuration\n+ variable `push.pushOption`. String(s) specified here will always\n+ be passed to the server without need to specify it using `--push-option`\n\n --receive-pack=::\n --exec=::\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2ac810422..f761fe4ba 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -32,6 +32,8 @@ static const char **refspec;\n static int refspec_nr;\n static int refspec_alloc;\n\n+static struct string_list push_options = STRING_LIST_INIT_DUP;\n+\n static void add_refspec(const char *ref)\n {\n refspec_nr++;\n@@ -503,6 +505,14 @@ static int git_push_config(const char *k, const\nchar *v, void *cb)\n int val = git_config_bool(k, v) ?\n RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;\n recurse_submodules = val;\n+ } else if (!strcmp(k, \"push.pushoption\")) {\n+ if (v == NULL)\n+ return config_error_nonbool(k);\n+ else\n+ if (v[0] == '\\0')\n+ string_list_clear(&push_options, 0);\n+ else\n+ string_list_append(&push_options, v);\n }\n\n return git_default_config(k, v, NULL);\n@@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const\nchar *prefix)\n int push_cert = -1;\n int rc;\n const char *repo = NULL; /* default repository */\n- struct string_list push_options = STRING_LIST_INIT_DUP;\n const struct string_list_item *item;\n\n struct option options[] = {\ndiff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh\nindex 90a4b0d2f..2cf9f7968 100755\n--- a/t/t5545-push-options.sh\n+++ b/t/t5545-push-options.sh\n@@ -140,6 +140,54 @@ test_expect_success 'push options and submodules' '\n test_cmp expect parent_upstream/.git/hooks/post-receive.push_options\n '\n\n+test_expect_success 'default push option' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.pushOption=default push up master\n+ ) &&\n+ test_refs master master &&\n+ echo \"default\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'two default push options' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.pushOption=default1 -c push.pushOption=default2\npush up master\n+ ) &&\n+ test_refs master master &&\n+ printf \"default1\\ndefault2\\n\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'default and manual push options' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.pushOption=default push --push-option=manual up master\n+ ) &&\n+ test_refs master master &&\n+ printf \"default\\nmanual\\n\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n\n-- \n2.14.1\n\n\n2017-10-12 16:59 GMT+02:00 Marius Paliga :\n> Thank you for your coments and explanation.\n>\n> Just one thing:\n>\n>> - After parse_options() returns to cmd_push(), see if push_options\n>> is empty. If it is, you did not get any command line option, so\n>> override it with what you collected in the \"from-config\" string\n>> list. Otherwise, do not even look at \"from-config\" string list.\n>\n> The idea is that there are default push options (read from config) that are\n> always sent to the server and you can add (not overwrite) additional by\n> specifying \"--push-option\".\n> So I would rather concatenate both lists - from command line and from-config.\n>\n>> By the way, I really hate \"push.optiondefault\" as the variable\n>> name. The \"default\" part is obvious and there is no need to say it,\n>> as the configuration variables are there to give the default to what\n>> we would normally give from the command line. Rather, you should\n>> say for which option (there are many options \"git push\" takes) this\n>> variable gives the default. Perhaps \"push.pushOption\" is a much\n>> better name; I am sure people can come up with even better ones,\n>> though ;-)\n>\n> In the light of the above the \"default\" may be correct, but I don't\n> have a problem\n> with any name.\n>\n> Marius\n>\n>\n> 2017-10-11 15:38 GMT+02:00 Junio C Hamano :\n>> Marius Paliga writes:\n>>\n>>> @@ -505,6 +509,12 @@ static int git_push_config(const char *k, const\n>>> char *v, void *cb)\n>>> recurse_submodules = val;\n>>> }\n>>>\n>>> + default_push_options = git_config_get_value_multi(\"push.optiondefault\");\n>>> + if (default_push_options)\n>>> + for_each_string_list_item(item, default_push_options)\n>>> + if (!string_list_has_string(&push_options, item->string))\n>>> + string_list_append(&push_options, item->string);\n>>> +\n>>> return git_default_config(k, v, NULL);\n>>> }\n>>\n>> Sorry for not catching this earlier, but git_config_get_value* call\n>> inside git_push_config() is just wrong.\n>>\n>> There are two styles of configuration parsing. The original (and\n>> still perfectly valid) way is to call git_config() with a callback\n>> function like git_push_config(). Under this style, the config files\n>> are read from lower-priority to higher-priority ones, and the\n>> callback function is called once for each entry found, with \n>> pair and the callback specific opaque data. One way to add the\n>> parsing of a new variable like push.optiondefault is to add\n>>\n>> if (!strcmp(k, \"push.optiondefault\") {\n>> ... handle one \"[push] optiondefault\" entry here ...\n>> return 0;\n>> }\n>>\n>> to the function.\n>>\n>> An alternate way is to use git_config_get_* functions _outside_\n>> callback of git_config(). This is a newer invention. Your call to\n>> git_config_get_value_multi() will scan all configuration files and\n>> returns _all_ entries for the given variable at once.\n>>\n>> When there is already a callback style parser, in general, it is\n>> cleaner to simply piggy-back on it, instead of reading variables\n>> independently using git_config_get_* functions. When there isn't a\n>> callback style parser, using either style is OK. It also is OK to\n>> switch to git_config_get_* altogether, rewriting the callback style\n>> parser, but I do not think it is warranted in this case, which adds\n>> just one variable.\n>>\n>> In any case, with the above code, you'll end up calling the\n>> git_config_get_* function and grabbing all the values for\n>> push.optiondefault for each and every configuration variable\n>> definition (count \"git config -l | wc -l\" to estimate how many times\n>> it will be called). Which is probably not what you wanted to do.\n>>\n>> Also, watch out for how a configuration variable defined like below\n>> is reported to either of the above two styles:\n>>\n>> [push] optiondefault\n>>\n>> - To a git_config() callback function like git_push_config(), such\n>> an entry is called with k==\"push.optiondefault\", v==NULL.\n>>\n>> - git_config_get_value_multi() would return a string-list element\n>> with the string set to NULL to signal that one value is NULL\n>> (i.e. it is different from \"[push] optiondefault = \").\n>>\n>> I suspect that with your code, we'd hit\n>>\n>> if (strchr(item->string, '\\n'))\n>>\n>> and end up dereferencing NULL right there.\n>>\n>>> @@ -515,7 +525,6 @@ int cmd_push(int argc, const char **argv, const\n>>> char *prefix)\n>>> int push_cert = -1;\n>>> int rc;\n>>> const char *repo = NULL; /* default repository */\n>>> - struct string_list push_options = STRING_LIST_INIT_DUP;\n>>> const struct string_list_item *item;\n>>>\n>>> struct option options[] = {\n>>\n>> Also, I suspect that this code does not allow the command line\n>> option to override the default set in the configuration file.\n>> OPT_STRING_LIST() appends to the &push_options string list without\n>> first clearing it, and you are pre-populating the list while reading\n>> the configuration, so the values taken from the command line will\n>> only add to them.\n>>\n>> The right way to do this would probably be:\n>>\n>> - Do not muck with push_options in cmd_push().\n>>\n>> - Prepare another string list, push_options_from_config, that is\n>> file-scope global.\n>>\n>> - In git_push_config(), do not call get_multi; instead react to a\n>> call with k==\"push.optionsdefault\" and\n>>\n>> - reject if \"v\" is NULL, with \"return config_error_nonbool(k);\"\n>>\n>> - otherwise, append \"v\" to the \"from-config\" string list--do not\n>> attempt to dedup or sort.\n>>\n>> - if \"v\" is an empty string, clear the \"from-config\" list.\n>>\n>> - After parse_options() returns to cmd_push(), see if push_options\n>> is empty. If it is, you did not get any command line option, so\n>> override it with what you collected in the \"from-config\" string\n>> list. Otherwise, do not even look at \"from-config\" string list.\n>>\n>> By the way, I really hate \"push.optiondefault\" as the variable\n>> name. The \"default\" part is obvious and there is no need to say it,\n>> as the configuration variables are there to give the default to what\n>> we would normally give from the command line. Rather, you should\n>> say for which option (there are many options \"git push\" takes) this\n>> variable gives the default. Perhaps \"push.pushOption\" is a much\n>> better name; I am sure people can come up with even better ones,\n>> though ;-)\n>>\n"},{"id":"330293","messageId":"CAGZ79kZEQ9WeJdTu83PxC7KSfvQAE3aX4D0M-vRth8P5O2TrVA@mail.gmail.com","threadId":"46887","inReplyTo":"CAK7vU=0A_nf2bAgV6dQyiesLJ3HHs5guyfNSNTwzYzgS2+YeWg@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-12T16:51:33Z","receivedAt":"2017-10-12T16:51:39Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Oct 12, 2017 at 9:32 AM, Marius Paliga wrote:\n> Subject: [PATCH] Added support for new configuration parameter push.pushOption\n>\n> builtin/push.c: add push.pushOption config\n>\n> Currently push options need to be given explicitly, via\n> the command line as \"git push --push-option\".\n>\n> The UX of Git would be enhanced if push options could be\n> configured instead of given each time on the command line.\n>\n> Add the config option push.pushOption, which is a multi\n> string option, containing push options that are sent by default.\n>\n> When push options are set in the system wide config\n> (/etc/gitconfig), they can be unset later in the more specific\n> repository config by setting the string to the empty string.\n\nNow that I review this patch, this is nice information and can\nremain in the commit message, but it would be more useful\nin the Documentation as that is where the users look.\n(Another thing regarding the documentation: Maybe we want\nto update Documentation/config.txt as well, that contains all\nconfiguration)\n\n> @@ -503,6 +505,14 @@ static int git_push_config(const char *k, const\n> char *v, void *cb)\n> int val = git_config_bool(k, v) ?\n> RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;\n> recurse_submodules = val;\n> + } else if (!strcmp(k, \"push.pushoption\")) {\n> + if (v == NULL)\n> + return config_error_nonbool(k);\n> + else\n> + if (v[0] == '\\0')\n> + string_list_clear(&push_options, 0);\n\nJunio,\ndo we have variables that behave similarly to this?\n\n(I just wondered if the `v == NULL` could be lumped in\nto here, resetting the string list)\n\n>\n> +test_expect_success 'default push option' '\n...\n> +'\n> +\n> +test_expect_success 'two default push options' '\n...\n> +'\n> +\n> +test_expect_success 'default and manual push options' '\n...\n> +'\n\nThanks for adding thorough tests!\nDo we also need tests for the reset of the list?\n\nThanks,\nStefan\n"},{"id":"330325","messageId":"xmqq1sm7yjav.fsf@gitster.mtv.corp.google.com","threadId":"46887","inReplyTo":"CAK7vU=0wbGsFCXmwmCc-XX9K07UF_OZ7tFa4_GVb-H7fxakssg@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-13T00:37:28Z","receivedAt":"2017-10-13T00:37:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marius Paliga writes:\n\n> Thank you for your coments and explanation.\n>\n> Just one thing:\n>\n>> - After parse_options() returns to cmd_push(), see if push_options\n>> is empty. If it is, you did not get any command line option, so\n>> override it with what you collected in the \"from-config\" string\n>> list. Otherwise, do not even look at \"from-config\" string list.\n>\n> The idea is that there are default push options (read from config) that are\n> always sent to the server and you can add (not overwrite) additional by\n> specifying \"--push-option\".\n\nI can imagine that sometimes giving a base from a configuration and\nthen adding more for specific invocation may be useful. \n\nBut I do not think of any --command-line-option and config.variable\npair whose configured value cannot be overriden by the command line\noption; we should strive to avoid making --push-option a special\ncase that the users need to aware of, and more importantly, users\nother than you who expect the more usual \"command line overrides\"\nbehaviour should get that.\n\nYour \"I wanted to accumulate, so I made so and made it impossible to\noverride\" won't fly as a justification. The default should be\n\"command line overrides\", and if you need a way to allow command\nline to add without overiding, that should be added as an optional\nfeature.\n\n\t[alias]\n\t\tmypush = push --push-option=foo --push-option=bar\n\nmay give you a set of push-options that are always in effect (they\nare not even \"by default\") and cannot be overriden.\n\n"},{"id":"330328","messageId":"xmqqh8v3x1uz.fsf@gitster.mtv.corp.google.com","threadId":"46887","inReplyTo":"CAGZ79kZEQ9WeJdTu83PxC7KSfvQAE3aX4D0M-vRth8P5O2TrVA@mail.gmail.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-13T01:39:32Z","receivedAt":"2017-10-13T01:39:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Junio,\n> do we have variables that behave similarly to this?\n\nIf you scan Documentation/config.txt, you'll find http.extraHeader\nas an example' but I do not recall offhand if that was the original\nthat introduced the convention, or it merely followed precedence set\nby other variables.\n\n"},{"id":"330340","messageId":"CAK7vU=2ePR3jQsgu=RxSMrxytAAHqxC0SFrN5YozLzQzP2ZT2A@mail.gmail.com","threadId":"46887","inReplyTo":"xmqq1sm7yjav.fsf@gitster.mtv.corp.google.com","subject":"Re: Enhancement request: git-push: Allow (configurable) default push-option","fromName":"Marius Paliga","fromEmail":"marius.paliga@gmail.com","sentAt":"2017-10-13T08:45:02Z","receivedAt":"2017-10-13T08:45:14Z","isPatch":false,"sender":{"key":"marius.paliga@gmail.com","avatar":"https://avatars.githubusercontent.com/u/54767958?v=4"},"body":"Updated patch and added tests:\n(feel free to modify Documentation)\n\n---\n\nSubject: [PATCH] Added support for new configuration parameter push.pushOption\n\nbuiltin/push.c: add push.pushOption config\n\nCurrently push options need to be given explicitly, via\nthe command line as \"git push --push-option\".\n\nThe UX of Git would be enhanced if push options could be\nconfigured instead of given each time on the command line.\n\nAdd the config option push.pushOption, which is a multi\nstring option, containing push options that are sent by default.\n\nWhen push options are set in the system wide config\n(/etc/gitconfig), they can be unset later in the more specific\nrepository config by setting the string to the empty string.\n\nAdd tests and documentation as well.\n\nSigned-off-by: Marius Paliga \n---\n Documentation/git-push.txt | 3 ++\n builtin/push.c | 12 ++++++++\n t/t5545-push-options.sh | 77 ++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 92 insertions(+)\n\ndiff --git a/Documentation/git-push.txt b/Documentation/git-push.txt\nindex 3e76e99f3..da9b17624 100644\n--- a/Documentation/git-push.txt\n+++ b/Documentation/git-push.txt\n@@ -161,6 +161,9 @@ already exists on the remote side.\n Transmit the given string to the server, which passes them to\n the pre-receive as well as the post-receive hook. The given string\n must not contain a NUL or LF character.\n+ Default push options can also be specified with configuration\n+ variable `push.pushOption`. String(s) specified here will always\n+ be passed to the server without need to specify it using `--push-option`\n\n --receive-pack=::\n --exec=::\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 2ac810422..10e520c8f 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -32,6 +32,8 @@ static const char **refspec;\n static int refspec_nr;\n static int refspec_alloc;\n\n+static struct string_list push_options_from_config = STRING_LIST_INIT_DUP;\n+\n static void add_refspec(const char *ref)\n {\n refspec_nr++;\n@@ -503,6 +505,14 @@ static int git_push_config(const char *k, const\nchar *v, void *cb)\n int val = git_config_bool(k, v) ?\n RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;\n recurse_submodules = val;\n+ } else if (!strcmp(k, \"push.pushoption\")) {\n+ if (v == NULL)\n+ return config_error_nonbool(k);\n+ else\n+ if (v[0] == '\\0')\n+ string_list_clear(&push_options_from_config, 0);\n+ else\n+ string_list_append(&push_options_from_config, v);\n }\n\n return git_default_config(k, v, NULL);\n@@ -562,6 +572,8 @@ int cmd_push(int argc, const char **argv, const\nchar *prefix)\n packet_trace_identity(\"push\");\n git_config(git_push_config, &flags);\n argc = parse_options(argc, argv, prefix, options, push_usage, 0);\n+ if (!push_options.nr)\n+ push_options = push_options_from_config;\n set_push_cert_flags(&flags, push_cert);\n\n if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL |\nTRANSPORT_PUSH_MIRROR))))\ndiff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh\nindex 90a4b0d2f..463783789 100755\n--- a/t/t5545-push-options.sh\n+++ b/t/t5545-push-options.sh\n@@ -140,6 +140,83 @@ test_expect_success 'push options and submodules' '\n test_cmp expect parent_upstream/.git/hooks/post-receive.push_options\n '\n\n+test_expect_success 'default push option' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.pushOption=default push up master\n+ ) &&\n+ test_refs master master &&\n+ echo \"default\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'two default push options' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.pushOption=default1 -c push.pushOption=default2\npush up master\n+ ) &&\n+ test_refs master master &&\n+ printf \"default1\\ndefault2\\n\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'push option from command line overrides\nfrom-config push option' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.pushOption=default push --push-option=manual up master\n+ ) &&\n+ test_refs master master &&\n+ echo \"manual\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'empty value of push.pushOption in config clears\nthe list' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ git -c push.pushOption=default1 -c push.pushOption= -c\npush.pushOption=default2 push up master\n+ ) &&\n+ test_refs master master &&\n+ echo \"default2\" >expect &&\n+ test_cmp expect upstream/.git/hooks/pre-receive.push_options &&\n+ test_cmp expect upstream/.git/hooks/post-receive.push_options\n+'\n+\n+test_expect_success 'invalid push option in config' '\n+ mk_repo_pair &&\n+ git -C upstream config receive.advertisePushOptions true &&\n+ (\n+ cd workbench &&\n+ test_commit one &&\n+ git push --mirror up &&\n+ test_commit two &&\n+ test_must_fail git -c push.pushOption push up master\n+ ) &&\n+ test_refs master HEAD@{1}\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n\n-- \n2.14.1\n\n\n2017-10-13 2:37 GMT+02:00 Junio C Hamano :\n> Marius Paliga writes:\n>\n>> Thank you for your coments and explanation.\n>>\n>> Just one thing:\n>>\n>>> - After parse_options() returns to cmd_push(), see if push_options\n>>> is empty. If it is, you did not get any command line option, so\n>>> override it with what you collected in the \"from-config\" string\n>>> list. Otherwise, do not even look at \"from-config\" string list.\n>>\n>> The idea is that there are default push options (read from config) that are\n>> always sent to the server and you can add (not overwrite) additional by\n>> specifying \"--push-option\".\n>\n> I can imagine that sometimes giving a base from a configuration and\n> then adding more for specific invocation may be useful.\n>\n> But I do not think of any --command-line-option and config.variable\n> pair whose configured value cannot be overriden by the command line\n> option; we should strive to avoid making --push-option a special\n> case that the users need to aware of, and more importantly, users\n> other than you who expect the more usual \"command line overrides\"\n> behaviour should get that.\n>\n> Your \"I wanted to accumulate, so I made so and made it impossible to\n> override\" won't fly as a justification. The default should be\n> \"command line overrides\", and if you need a way to allow command\n> line to add without overiding, that should be added as an optional\n> feature.\n>\n> [alias]\n> mypush = push --push-option=foo --push-option=bar\n>\n> may give you a set of push-options that are always in effect (they\n> are not even \"by default\") and cannot be overriden.\n>\n"},{"id":"330510","messageId":"20171017034743.8418-1-thais.dinizbraz@gmail.com","threadId":"46887","inReplyTo":"xmqqmv4x11y9.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] patch reply","fromName":"Thais Diniz","fromEmail":"thais.dinizbraz@gmail.com","sentAt":"2017-10-17T03:47:43Z","receivedAt":"2017-10-17T03:47:55Z","isPatch":true,"sender":{"key":"thais.dinizbraz@gmail.com","avatar":null},"body":"From: Thais Diniz Braz \n\n---\n emailReply | 4 ++++\n 1 file changed, 4 insertions(+)\n create mode 100644 emailReply\n\ndiff --git a/emailReply b/emailReply\nnew file mode 100644\nindex 000000000..2d591b55b\n--- /dev/null\n+++ b/emailReply\n@@ -0,0 +1,4 @@\n+Just to clarify I did not see Marius patch.\n+Did see Marius' comment saying he would look it in the leftoverbits list,\n+but since i didn't see any patch i thought i could work on it and did so based on Stephan's comment \n+(which i suppose Mario also did and that is why the code resulted to be similar).\n-- \n2.15.0.rc0.39.g2f0e14e.dirty\n\n"},{"id":"330512","messageId":"CAP2EEmdA9txQa9g32ZJmNr3bRVwEkBF=_RrSEJck+Rf87FRZTA@mail.gmail.com","threadId":"46887","inReplyTo":"xmqqmv4x11y9.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option","fromName":"thais braz","fromEmail":"thais.dinizbraz@gmail.com","sentAt":"2017-10-17T03:58:18Z","receivedAt":"2017-10-17T03:58:24Z","isPatch":true,"sender":{"key":"thais.dinizbraz@gmail.com","avatar":null},"body":"Just to clarify I did not see Marius patch.\nDid see Marius' comment saying he would look it in the leftoverbits list,\nbut since i didn't see any patch i thought i could work on it and did\nso based on Stephan's comment\n(which i suppose Mario also did and that is why the code resulted to\nbe similar).\n\nAnd sorry send this email as patch. Didn't know how to use git\nsend-email just as reply\n\nOn Thu, Oct 12, 2017 at 12:26 AM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>>> Can somebody explain what is going on?\n>>>\n>>> I am guessing that Thais and marius are different people (judging by\n>>> the fact that one CC's a message to the other). Are you two\n>>> collaborating on this change, or something?\n>>\n>> I guess that Thais decided to work on this, because we ask Outreachy\n>> applicants to search for #leftoverbits mentions in the mailing list\n>> archive to find small tasks they could work on.\n>>\n>> In this case it looks like Marius sent a patch a few hours before\n>> Thais also sent one.\n>\n> ... after seeing Marius's already working on it, I think.\n>\n>> Thais, I am sorry, but as Marius sent a patch first, I think it is\n>> better if you search for another different small task to work on.\n>\n> In general, I do not mind seeing people working together well, and\n> it is one of the more important skills necessary in the open source\n> community. I however tend to agree with you that this is a bit too\n> small a topic for multiple people to be working on.\n>\n>> Also please keep Peff and me in cc.\n>\n> Yup, that is always a good idea.\n>\n\n\n\n-- \nAtenciosamente Thais Diniz Braz\n"},{"id":"330513","messageId":"xmqqd15mpgmk.fsf@gitster.mtv.corp.google.com","threadId":"46887","inReplyTo":"20171017034743.8418-1-thais.dinizbraz@gmail.com","subject":"Re: [PATCH] patch reply","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-17T04:01:23Z","receivedAt":"2017-10-17T04:01:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thais Diniz writes:\n\n> +Just to clarify I did not see Marius patch.\n> +Did see Marius' comment saying he would look it in the leftoverbits list,\n> +but since i didn't see any patch i thought i could work on it and did so based on Stephan's comment \n> +(which i suppose Mario also did and that is why the code resulted to be similar).\n\nIn any case, both versions share exactly the same \"don't call\nget_multi() to grab the same configuration values from inside the\ncallback of git_config()\" issue, so whoever works on it to complete\nthe topic, it needs further work.\n"},{"id":"330532","messageId":"CAK7vU=2f+EVorUS4Js7WW6GePnbopGi54egviJ_NG9prj6C6OA@mail.gmail.com","threadId":"46887","inReplyTo":"xmqqd15mpgmk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] patch reply","fromName":"Marius Paliga","fromEmail":"marius.paliga@gmail.com","sentAt":"2017-10-17T07:15:54Z","receivedAt":"2017-10-17T07:16:00Z","isPatch":true,"sender":{"key":"marius.paliga@gmail.com","avatar":"https://avatars.githubusercontent.com/u/54767958?v=4"},"body":"I just sent a patch to a new thread which is not what I wanted.\nHowever I already sent the same patch a few days ago:\nhttps://public-inbox.org/git/CAK7vU=2ePR3jQsgu=RxSMrxytAAHqxC0SFrN5YozLzQzP2ZT2A@mail.gmail.com/\n\n\n2017-10-17 6:01 GMT+02:00 Junio C Hamano :\n> Thais Diniz writes:\n>\n>> +Just to clarify I did not see Marius patch.\n>> +Did see Marius' comment saying he would look it in the leftoverbits list,\n>> +but since i didn't see any patch i thought i could work on it and did so based on Stephan's comment\n>> +(which i suppose Mario also did and that is why the code resulted to be similar).\n>\n> In any case, both versions share exactly the same \"don't call\n> get_multi() to grab the same configuration values from inside the\n> callback of git_config()\" issue, so whoever works on it to complete\n> the topic, it needs further work.\n"}]}