[PATCH] Added support for new configuration parameter push.pushOption

[PATCH] Added support for new configuration parameter push.pushOption

[Marius Paliga]

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@gmail.com>
---
 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}

Re: [PATCH] Added support for new configuration parameter push.pushOption

[Junio C Hamano]

Marius Paliga <marius.paliga@gmail.com> writes:

> builtin/push.c: add push.pushOption config

This is a good title for this change; it would be perfect if it were
on the Subject: header ;-)

> 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.

s/Currently p/P/ would be sufficient; we describe the status quo in
present tense, and give an order to the codebase to "become like so"
(or command a patch monkey to "make the codebase like so") by using
imperative mood.

I think something shorter like this would be sufficient:

    Push options need to be given explicitly, via the command line as "git
    push --push-option <option>".  Add the config option push.pushOption,
    which is a multi-valued option, containing push options that are sent
    by default.

    When push options are set in the lower-priority configulation file
    (e.g. /etc/gitconfig, or $HOME/.gitconfig), they can be unset later in
    the more specific repository config by the empty string.

> 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`

"will always be passed" is not a "Default".  

If we really need to say it, say something like

	When no `--push-option <option>` is given from the command
	line, the values of this configuration variable are used
	instead.

But that is what "Default" means, so dropping the second sentence
altogether would be more concise and better.

> 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')

Make this "else if (!*v)" on a single line and dedent the remainder.

> +                string_list_clear(&push_options_from_config, 0);
> +            else
> +                string_list_append(&push_options_from_config, v);
>      }

> @@ -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;

We encourage our developers to think twice before doing a structure
assignment.

I think this is a bad idea, primarily because at the point where
string_list_clear() is later called on &push_options, it is unclear
how to release the resources held by push_options_from_config().  It
also is bad to depend on the implementation detail that overwriting
the structure do not leak push_options.item when push_options.nr is
zero, but it is conceivable that STRING_LIST_INIT_* could later be
modified to pre-allocate a small array, at which point this will
become a memory leak.

> 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' '
> ...
> +        git -c push.pushOption=default1 -c push.pushOption=default2
> push up master
> ...
> +test_expect_success 'push option from command line overrides
> from-config push option' '

It appears that your MUA is expanding tabs to spaces and wrapping
long lines in your patch?  Please double check and make sure that
will not happen.

I couldn't use your patch (because it was broken by your MUA) as a
base to show an incremental improvement, but here is how I would do
the "code" part.

 builtin/push.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 03846e8379..89ef029c67 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_config = STRING_LIST_INIT_DUP;
+
 static void add_refspec(const char *ref)
 {
 	refspec_nr++;
@@ -503,6 +505,13 @@ 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)
+			return config_error_nonbool(k);
+		else if (!*v)
+			string_list_clear(&push_options_config, 0);
+		else
+			string_list_append(&push_options_config, v);
 	}
 
 	return git_default_config(k, v, NULL);
@@ -515,7 +524,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int push_cert = -1;
 	int rc;
 	const char *repo = NULL;	/* default repository */
-	struct string_list push_options = STRING_LIST_INIT_DUP;
+	struct string_list push_options_cmdline = STRING_LIST_INIT_DUP;
+	struct string_list *push_options;
 	const struct string_list_item *item;
 
 	struct option options[] = {
@@ -562,6 +572,9 @@ 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);
+	push_options = (push_options_cmdline.nr
+			? &push_options_cmdline
+			: &push_options_config);
 	set_push_cert_flags(&flags, push_cert);
 
 	if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
@@ -584,12 +597,13 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		set_refspecs(argv + 1, argc - 1, repo);
 	}
 
-	for_each_string_list_item(item, &push_options)
+	for_each_string_list_item(item, push_options)
 		if (strchr(item->string, '\n'))
 			die(_("push options must not have new line characters"));
 
-	rc = do_push(repo, flags, &push_options);
-	string_list_clear(&push_options, 0);
+	rc = do_push(repo, flags, push_options);
+	string_list_clear(&push_options_cmdline, 0);
+	string_list_clear(&push_options_config, 0);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else

Re: [PATCH] Added support for new configuration parameter push.pushOption

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> base to show an incremental improvement, but here is how I would do
> the "code" part.
>
>  builtin/push.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 03846e8379..89ef029c67 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_config = STRING_LIST_INIT_DUP;
> +
>  static void add_refspec(const char *ref)
>  {
>  	refspec_nr++;
> @@ -503,6 +505,13 @@ 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)
> +			return config_error_nonbool(k);
> +		else if (!*v)
> +			string_list_clear(&push_options_config, 0);
> +		else
> +			string_list_append(&push_options_config, v);

Here needs to be

		return 0;

as there is no point in falling through to call
git_default_config().

>  	}
>  
>  	return git_default_config(k, v, NULL);

[PATCH] builtin/push.c: add push.pushOption config

[Marius Paliga]

Push options need to be given explicitly, via the command line as "git
push --push-option <option>".  Add the config option push.pushOption,
which is a multi-valued option, containing push options that are sent
by default.

When push options are set in the lower-priority configulation file
(e.g. /etc/gitconfig, or $HOME/.gitconfig), they can be unset later in
the more specific repository config by the empty string.

Add tests and update documentation as well.

Signed-off-by: Marius Paliga <marius.paliga@gmail.com>
---
 Documentation/git-push.txt |  3 ++
 builtin/push.c             | 26 +++++++++++++---
 t/t5545-push-options.sh    | 77 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 3e76e99f3..aa78057dc 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.
+	When no `--push-option <option>` is given from the command
+	line, the values of configuration variable `push.pushOption`
+	are used instead.
 
 --receive-pack=<git-receive-pack>::
 --exec=<git-receive-pack>::
diff --git a/builtin/push.c b/builtin/push.c
index 2ac810422..1c28427d8 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_config = STRING_LIST_INIT_DUP;
+
 static void add_refspec(const char *ref)
 {
 	refspec_nr++;
@@ -503,6 +505,15 @@ 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)
+			return config_error_nonbool(k);
+		else
+			if (!*v)
+				string_list_clear(&push_options_config, 0);
+			else
+				string_list_append(&push_options_config, v);
+		return 0;
 	}
 
 	return git_default_config(k, v, NULL);
@@ -515,7 +526,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int push_cert = -1;
 	int rc;
 	const char *repo = NULL;	/* default repository */
-	struct string_list push_options = STRING_LIST_INIT_DUP;
+	struct string_list push_options_cmdline = STRING_LIST_INIT_DUP;
+	struct string_list *push_options;
 	const struct string_list_item *item;
 
 	struct option options[] = {
@@ -551,7 +563,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		  0, "signed", &push_cert, "yes|no|if-asked", N_("GPG sign the push"),
 		  PARSE_OPT_OPTARG, option_parse_push_signed },
 		OPT_BIT(0, "atomic", &flags, N_("request atomic transaction on remote side"), TRANSPORT_PUSH_ATOMIC),
-		OPT_STRING_LIST('o', "push-option", &push_options, N_("server-specific"), N_("option to transmit")),
+		OPT_STRING_LIST('o', "push-option", &push_options_cmdline, N_("server-specific"), N_("option to transmit")),
 		OPT_SET_INT('4', "ipv4", &family, N_("use IPv4 addresses only"),
 				TRANSPORT_FAMILY_IPV4),
 		OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
@@ -562,6 +574,9 @@ 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);
+	push_options = (push_options_cmdline.nr
+		? &push_options_cmdline
+		: &push_options_config);
 	set_push_cert_flags(&flags, push_cert);
 
 	if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
@@ -584,12 +599,13 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		set_refspecs(argv + 1, argc - 1, repo);
 	}
 
-	for_each_string_list_item(item, &push_options)
+	for_each_string_list_item(item, push_options)
 		if (strchr(item->string, '\n'))
 			die(_("push options must not have new line characters"));
 
-	rc = do_push(repo, flags, &push_options);
-	string_list_clear(&push_options, 0);
+	rc = do_push(repo, flags, push_options);
+	string_list_clear(&push_options_cmdline, 0);
+	string_list_clear(&push_options_config, 0);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
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

Re: [PATCH] builtin/push.c: add push.pushOption config

[Stefan Beller]

On Thu, Oct 19, 2017 at 10:47 AM, Marius Paliga <marius.paliga@gmail.com> wrote:
> Push options need to be given explicitly, via the command line as "git
> push --push-option <option>".  Add the config option push.pushOption,
> which is a multi-valued option, containing push options that are sent
> by default.
>
> When push options are set in the lower-priority configulation file
> (e.g. /etc/gitconfig, or $HOME/.gitconfig), they can be unset later in
> the more specific repository config by the empty string.
>
> Add tests and update documentation as well.

Thanks for keeping working on this feature!


> @@ -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.
> +       When no `--push-option <option>` is given from the command
> +       line, the values of configuration variable `push.pushOption`
> +       are used instead.

We'd also want to document how push.pushOption works in
Documentation/config.txt (that contains all the configs)

So in the config, we have to explicitly give an empty option to
clear the previous options, but on the command line we do not need
that, but instead we'd have to repeat any push options that we desire
that were configured?

Example:

  /etc/gitconfig
  push.pushoption = a
  push.pushoption = b

  ~/.gitconfig
  push.pushoption = c

  repo/.git/config
  push.pushoption =
  push.pushoption = b

will result in only b as a and c are
cleared.

If I were to run
  git -c push.pushOption=d push ... (in repo)
it would be b and d, but
  git push --push-option=d
would be d only?

As a user I might have expected it to be the same,
expecting
  git push --push-option='' --push-option=d
to suppress b.


> @@ -584,12 +599,13 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>                 set_refspecs(argv + 1, argc - 1, repo);
>         }
>
> -       for_each_string_list_item(item, &push_options)
> +       for_each_string_list_item(item, push_options)

We have to do the same for _cmdline here, too?

The tests look good!

Thanks,
Stefan

Re: [PATCH] builtin/push.c: add push.pushOption config

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

>> @@ -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.
>> +       When no `--push-option <option>` is given from the command
>> +       line, the values of configuration variable `push.pushOption`
>> +       are used instead.
>
> We'd also want to document how push.pushOption works in
> Documentation/config.txt (that contains all the configs)

Perhaps.

> So in the config, we have to explicitly give an empty option to
> clear the previous options, but on the command line we do not need
> that, but instead we'd have to repeat any push options that we desire
> that were configured?

It is not wrong per-se to phrase it like so, but I think that is
making it unnecessarily confusing by conflating two things.  (1)
configured values are overridden from the command line, just like
any other --option/config.variable pair and (2) unlike usual single
value variables where "last one wins" rule is simple enough to
explain,, multi-value variables need a way to "forget everything we
said so far and start from scratch" syntax, especially when multiple
input files are involved.

> Example:
>
>   /etc/gitconfig
>   push.pushoption = a
>   push.pushoption = b
>
>   ~/.gitconfig
>   push.pushoption = c
>
>   repo/.git/config
>   push.pushoption =
>   push.pushoption = b
>
> will result in only b as a and c are
> cleared.

The above is correct, and it might be worth giving it as an example
in the doc, because not just "give an empty entry to clear what has
been accumulated so far" but a multi-valued option in general is a
rather rare thing.

> If I were to run
>   git -c push.pushOption=d push ... (in repo)
> it would be b and d, but
>   git push --push-option=d
> would be d only?

>> @@ -584,12 +599,13 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>>                 set_refspecs(argv + 1, argc - 1, repo);
>>         }
>>
>> -       for_each_string_list_item(item, &push_options)
>> +       for_each_string_list_item(item, push_options)
>
> We have to do the same for _cmdline here, too?

I do not think so.  The point of these lines that appear before this
loop:

 	git_config(git_push_config, &flags);
 	argc = parse_options(argc, argv, prefix, options, push_usage, 0);
+	push_options = (push_options_cmdline.nr
+		? &push_options_cmdline
+		: &push_options_config);

is that the command line overrides configured values, just like any
other configuration.  Adding _cmdline variant here is doubly wrong
when command line options are given in that it (1) duplicates what
was obtained from the command line, and (2) does not clear the
configured values.

Re: [PATCH] builtin/push.c: add push.pushOption config

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> We'd also want to document how push.pushOption works in
>> Documentation/config.txt (that contains all the configs)
>
> Perhaps.

Here is my attempt.  I have a feeling that the way http.extraheaders
is described may be much easier to read and we may want to mimick
its style.  I dunno.

 Documentation/config.txt | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1ac0ae6adb..631ed1172e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2621,6 +2621,16 @@ push.gpgSign::
 	override a value from a lower-priority config file. An explicit
 	command-line flag always overrides this config option.
 
+push.pushOption::
+	When no `--push-option=<option>` argument is given from the
+	command line, `git push` behaves as if each <value> of
+	this variable is given as `--push-option=<value>`.
++
+This is a multi-valued variable, and an empty value can be used in a
+higher priority cofiguration file (e.g. `.git/config` in a
+repository) to clear the values inherited from a lower priority
+configuration files (e.g. `$HOME/.gitconfig`).
+
 push.recurseSubmodules::
 	Make sure all submodule commits used by the revisions to be pushed
 	are available on a remote-tracking branch. If the value is 'check'

Re: [PATCH] builtin/push.c: add push.pushOption config

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>>> We'd also want to document how push.pushOption works in
>>> Documentation/config.txt (that contains all the configs)
>>
>> Perhaps.
>
> Here is my attempt.

Another thing I noticed while we are around the area is that unlike
all other options in git-push.txt page, this one forgets to say it
always takes mandatory string.  Here is a possible fix.

 Documentation/git-push.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index aa78057dc5..a8504e0ae3 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -156,12 +156,12 @@ already exists on the remote side.
 	Either all refs are updated, or on error, no refs are updated.
 	If the server does not support atomic pushes the push will fail.
 
--o::
---push-option::
+-o <option>::
+--push-option=<option>::
 	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.
-	When no `--push-option <option>` is given from the command
+	When no `--push-option=<option>` is given from the command
 	line, the values of configuration variable `push.pushOption`
 	are used instead.
 

Re: [PATCH] builtin/push.c: add push.pushOption config

[Marius Paliga]

> --o::
> ---push-option::
> +-o <option>::
> +--push-option=<option>::
>         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.
> -       When no `--push-option <option>` is given from the command
> +       When no `--push-option=<option>` is given from the command
>         line, the values of configuration variable `push.pushOption`
>         are used instead.

Should we also mention that this option can take multiple values from
the command line?

-o <option>::
--push-option=<option>::
        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.
+       Multiple `--push-option=<option>` are allowed on the command line.
       When no `--push-option=<option>` is given from the command
        line, the values of configuration variable `push.pushOption`
        are used instead.



2017-10-20 8:18 GMT+02:00 Junio C Hamano <gitster@pobox.com>:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>>> We'd also want to document how push.pushOption works in
>>>> Documentation/config.txt (that contains all the configs)
>>>
>>> Perhaps.
>>
>> Here is my attempt.
>
> Another thing I noticed while we are around the area is that unlike
> all other options in git-push.txt page, this one forgets to say it
> always takes mandatory string.  Here is a possible fix.
>
>  Documentation/git-push.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index aa78057dc5..a8504e0ae3 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -156,12 +156,12 @@ already exists on the remote side.
>         Either all refs are updated, or on error, no refs are updated.
>         If the server does not support atomic pushes the push will fail.
>
> --o::
> ---push-option::
> +-o <option>::
> +--push-option=<option>::
>         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.
> -       When no `--push-option <option>` is given from the command
> +       When no `--push-option=<option>` is given from the command
>         line, the values of configuration variable `push.pushOption`
>         are used instead.
>

Re: [PATCH] builtin/push.c: add push.pushOption config

[Stefan Beller]

On Thu, Oct 19, 2017 at 7:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>>> @@ -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.
>>> +       When no `--push-option <option>` is given from the command
>>> +       line, the values of configuration variable `push.pushOption`
>>> +       are used instead.
>>
>> We'd also want to document how push.pushOption works in
>> Documentation/config.txt (that contains all the configs)
>
> Perhaps.
>
>> So in the config, we have to explicitly give an empty option to
>> clear the previous options, but on the command line we do not need
>> that, but instead we'd have to repeat any push options that we desire
>> that were configured?
>
> It is not wrong per-se to phrase it like so, but I think that is
> making it unnecessarily confusing by conflating two things.  (1)
> configured values are overridden from the command line, just like
> any other --option/config.variable pair

because they are single value options (usually).

> and (2) unlike usual single
> value variables where "last one wins" rule is simple enough to
> explain,, multi-value variables need a way to "forget everything we
> said so far and start from scratch" syntax, especially when multiple
> input files are involved.

ok, my view of how that should be done is clashing once again
with the projects established standards. Sorry for the noise.

>> Example:
>>
>>   /etc/gitconfig
>>   push.pushoption = a
>>   push.pushoption = b
>>
>>   ~/.gitconfig
>>   push.pushoption = c
>>
>>   repo/.git/config
>>   push.pushoption =
>>   push.pushoption = b
>>
>> will result in only b as a and c are
>> cleared.
>
> The above is correct, and it might be worth giving it as an example
> in the doc, because not just "give an empty entry to clear what has
> been accumulated so far" but a multi-valued option in general is a
> rather rare thing.
>
>> If I were to run
>>   git -c push.pushOption=d push ... (in repo)
>> it would be b and d, but
>>   git push --push-option=d
>> would be d only?
>
>>> @@ -584,12 +599,13 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>>>                 set_refspecs(argv + 1, argc - 1, repo);
>>>         }
>>>
>>> -       for_each_string_list_item(item, &push_options)
>>> +       for_each_string_list_item(item, push_options)
>>
>> We have to do the same for _cmdline here, too?
>
> I do not think so.  The point of these lines that appear before this
> loop:
>
>         git_config(git_push_config, &flags);
>         argc = parse_options(argc, argv, prefix, options, push_usage, 0);
> +       push_options = (push_options_cmdline.nr
> +               ? &push_options_cmdline
> +               : &push_options_config);
>
> is that the command line overrides configured values, just like any
> other configuration.  Adding _cmdline variant here is doubly wrong
> when command line options are given in that it (1) duplicates what
> was obtained from the command line, and (2) does not clear the
> configured values.

Oh, ok. Sorry for the noise once again.

Thanks,
Stefan

Re: [PATCH] builtin/push.c: add push.pushOption config

[Junio C Hamano]

Marius Paliga <marius.paliga@gmail.com> writes:

> Should we also mention that this option can take multiple values from
> the command line?
>
> -o <option>::
> --push-option=<option>::
>         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.
> +       Multiple `--push-option=<option>` are allowed on the command line.
>        When no `--push-option=<option>` is given from the command
>         line, the values of configuration variable `push.pushOption`
>         are used instead.

My first reaction was "Do we do that for other options that can be
given multiple times?  If not, perhaps this is not needed."

Then my second reaction was "Do we have that many options that can
be given multiple times in the first place?  If not, perhaps a
change like this to highlight these oddball options may not be a bad
idea."

And my third reaction was "Well, even if we have many such options,
and even if most of them are not explicitly marked as usable
multiple times in the current documentation, that's not a reason to
keep it that way---the readers ought to be able to find out which
ones can be used multiple times and how these multiple instances
interact with each other, because the usual rule for single-instance
options is 'the last one wins' (e.g. 'git status -uall -uno' should
be the same as 'git status -uno') but to these multi-value options
that rule does not hold".

So, yes, I think it is a good idea.

But it opens a tangent #leftoverbits.  Among the most commonly used
commands listed in "git --help", there indeed are some commands that
take multiple options of the same kind (even if we do not count an
obvious exception "--verbose", which everybody understands as the
number of times it is given indicates the verbosity level).
Somebody needs to go over their documentation and add "this can be
given multiple times from the command line, and here is what happens
to them".

In your suggestion for "push -o <value1> -o <value2>", "here is what
happens" is missing.  Perhaps

	When multiple `--push-option=<option>` are given, they are
	all sent to the other side in the order listed on the
	command line.

or something like that.

Thanks.

Re: [PATCH] builtin/push.c: add push.pushOption config

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

>>> So in the config, we have to explicitly give an empty option to
>>> clear the previous options, but on the command line we do not need
>>> that, but instead we'd have to repeat any push options that we desire
>>> that were configured?
>>
>> It is not wrong per-se to phrase it like so, but I think that is
>> making it unnecessarily confusing by conflating two things.  (1)
>> configured values are overridden from the command line, just like
>> any other --option/config.variable pair
>
> because they are single value options (usually).
>
>> and (2) unlike usual single
>> value variables where "last one wins" rule is simple enough to
>> explain,, multi-value variables need a way to "forget everything we
>> said so far and start from scratch" syntax, especially when multiple
>> input files are involved.
>
> ok, my view of how that should be done is clashing once again
> with the projects established standards. Sorry for the noise.

I do not think it is a noise.  I was primarily focusing on "have to
explicitly" part, making it sound as if it was a flaw.  I do think
it is a good idea to explain a variable and/or an option is
multi-valued and how multiple instances of them interact with each
other.  I think the status quo is:

	Both configuration and command line, these multi-valued
	things accumulate.  The "command line can be used to
	override things from the configuration" rule applies as any
	other config/option pair.

	In addition, in the configuration, there is a mechanism to
	clear the values read so far with the "an empty value
	clears" rule, because you may not have control over, or it
	may be cumbersome to modify, lower-priority configuration
	files.  There is no such thing for command line, as the
	entirety of the command line for each invocation is under
	your control.

If somebody has

	[alias] mypush = push -ofoo

then the command line argument for one invocation of "git mypush"
may *not* be under your control and it might be also convenient if
there were a mechanism to clear what has been accumulated from the
command line.  It may be worth considering, but if we were going in
that direction, I suspect that it is probably a good idea to first
step back a bit and introduce a mechanism to easily define pairs of
option/config in a more sysmtematic way [*1*].  Once we have such a
mechanism, the "clear the previous" syntax for the command line
would be implemented uniformly in there.


[Footnote]

*1* E.g. right now, the fact that "push --push-option" and
    "push.pushOption" are related, or that "status -u<mode>" and
    "status.showUntrackedFiles" are related, is only known to the
    code and the documentation; I am not sure if it is even a good
    idea to add config to each and every option that exists, but for
    the ones that do exist, it would be nicer if there were a more
    uniform and systematic way for parse-options.c and config.c APIs
    to help our code, instead of writing git_config() callback and
    options[] array to give to parse_options(), making sure they
    refer to the same internal variable, and the latter overrides
    the former.

[PATCH] builtin/push.c: add push.pushOption config

[Marius Paliga]

Push options need to be given explicitly, via the command line as "git
push --push-option <option>".  Add the config option push.pushOption,
which is a multi-valued option, containing push options that are sent
by default.

When push options are set in the lower-priority configulation file
(e.g. /etc/gitconfig, or $HOME/.gitconfig), they can be unset later in
the more specific repository config by the empty string.

Add tests and update documentation as well.

Signed-off-by: Marius Paliga <marius.paliga@gmail.com>
---
 Documentation/config.txt   | 29 +++++++++++++++++
 Documentation/git-push.txt | 10 ++++--
 builtin/push.c             | 26 +++++++++++++---
 t/t5545-push-options.sh    | 77 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 135 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1ac0ae6ad..0bd46a6ab 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2621,6 +2621,35 @@ push.gpgSign::
 	override a value from a lower-priority config file. An explicit
 	command-line flag always overrides this config option.
 
+push.pushOption::
+	When no `--push-option=<option>` argument is given from the
+	command line, `git push` behaves as if each <value> of
+	this variable is given as `--push-option=<value>`.
++
+This is a multi-valued variable, and an empty value can be used in a
+higher priority cofiguration file (e.g. `.git/config` in a
+repository) to clear the values inherited from a lower priority
+configuration files (e.g. `$HOME/.gitconfig`).
++
+--
+
+Example:
+
+/etc/gitconfig
+  push.pushoption = a
+  push.pushoption = b
+
+~/.gitconfig
+  push.pushoption = c
+
+repo/.git/config
+  push.pushoption =
+  push.pushoption = b
+
+This will result in only b (a and c are cleared).
+
+--
+
 push.recurseSubmodules::
 	Make sure all submodule commits used by the revisions to be pushed
 	are available on a remote-tracking branch. If the value is 'check'
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 3e76e99f3..5b08302fc 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -156,11 +156,17 @@ already exists on the remote side.
 	Either all refs are updated, or on error, no refs are updated.
 	If the server does not support atomic pushes the push will fail.
 
--o::
---push-option::
+-o <option>::
+--push-option=<option>::
 	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.
+	When multiple `--push-option=<option>` are given, they are
+	all sent to the other side in the order listed on the
+	command line.
+	When no `--push-option=<option>` is given from the command
+	line, the values of configuration variable `push.pushOption`
+	are used instead.
 
 --receive-pack=<git-receive-pack>::
 --exec=<git-receive-pack>::
diff --git a/builtin/push.c b/builtin/push.c
index 2ac810422..1c28427d8 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_config = STRING_LIST_INIT_DUP;
+
 static void add_refspec(const char *ref)
 {
 	refspec_nr++;
@@ -503,6 +505,15 @@ 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)
+			return config_error_nonbool(k);
+		else
+			if (!*v)
+				string_list_clear(&push_options_config, 0);
+			else
+				string_list_append(&push_options_config, v);
+		return 0;
 	}
 
 	return git_default_config(k, v, NULL);
@@ -515,7 +526,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 	int push_cert = -1;
 	int rc;
 	const char *repo = NULL;	/* default repository */
-	struct string_list push_options = STRING_LIST_INIT_DUP;
+	struct string_list push_options_cmdline = STRING_LIST_INIT_DUP;
+	struct string_list *push_options;
 	const struct string_list_item *item;
 
 	struct option options[] = {
@@ -551,7 +563,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		  0, "signed", &push_cert, "yes|no|if-asked", N_("GPG sign the push"),
 		  PARSE_OPT_OPTARG, option_parse_push_signed },
 		OPT_BIT(0, "atomic", &flags, N_("request atomic transaction on remote side"), TRANSPORT_PUSH_ATOMIC),
-		OPT_STRING_LIST('o', "push-option", &push_options, N_("server-specific"), N_("option to transmit")),
+		OPT_STRING_LIST('o', "push-option", &push_options_cmdline, N_("server-specific"), N_("option to transmit")),
 		OPT_SET_INT('4', "ipv4", &family, N_("use IPv4 addresses only"),
 				TRANSPORT_FAMILY_IPV4),
 		OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
@@ -562,6 +574,9 @@ 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);
+	push_options = (push_options_cmdline.nr
+		? &push_options_cmdline
+		: &push_options_config);
 	set_push_cert_flags(&flags, push_cert);
 
 	if (deleterefs && (tags || (flags & (TRANSPORT_PUSH_ALL | TRANSPORT_PUSH_MIRROR))))
@@ -584,12 +599,13 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		set_refspecs(argv + 1, argc - 1, repo);
 	}
 
-	for_each_string_list_item(item, &push_options)
+	for_each_string_list_item(item, push_options)
 		if (strchr(item->string, '\n'))
 			die(_("push options must not have new line characters"));
 
-	rc = do_push(repo, flags, &push_options);
-	string_list_clear(&push_options, 0);
+	rc = do_push(repo, flags, push_options);
+	string_list_clear(&push_options_cmdline, 0);
+	string_list_clear(&push_options_config, 0);
 	if (rc == -1)
 		usage_with_options(push_usage, options);
 	else
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

Re: [PATCH] builtin/push.c: add push.pushOption config

[Junio C Hamano]

Marius Paliga <marius.paliga@gmail.com> writes:

> Push options need to be given explicitly, via the command line as "git
> push --push-option <option>".  Add the config option push.pushOption,
> which is a multi-valued option, containing push options that are sent
> by default.
>
> When push options are set in the lower-priority configulation file
> (e.g. /etc/gitconfig, or $HOME/.gitconfig), they can be unset later in
> the more specific repository config by the empty string.
>
> Add tests and update documentation as well.
>
> Signed-off-by: Marius Paliga <marius.paliga@gmail.com>
> ---

Looks good.

> +This is a multi-valued variable, and an empty value can be used in a
> +higher priority cofiguration file (e.g. `.git/config` in a

s/cofig/config/; I think you inherited from me, sorry.

I'll tweak it while queuing; there is not need to resend only to fix
this.

Thanks.