threads / patch / 55520

patch, 2 partsgit: support separate arg for `--config-env`'s value

Subject: [PATCH 0/2] git: support separate arg for `--config-env`'s value

## tl;dr

11 messages between Apr 19, 2021 and Apr 30, 2021. Diffs are folded; open one to read it.

replies: 10people: 4as markdown or json

Patrick Steinhardt· Apr 19, 2021, 15:56 UTC · lore
Hi,

As pointed out by Ævar, the synopsis in the git(1) manpage was misdocumenting the syntax of the `--config-env` option. This patch series fixes it and also addresses the shortcoming that while most of the other options support both `--git-dir=foo` and `--git-dir foo`, `--config-env` only supported the former format.

I'm happy to drop the latter patch if it's generally seen as something that's not needed, but Ævar and Peff got a point in that it feels inconsistent with the other options.

Patrick
Patrick Steinhardt (2):
  git.txt: fix synopsis of `--config-env` missing the equals sign
  git: support separate arg for `--config-env`'s value
 Documentation/git.txt |  2 +-
 git.c                 |  8 ++++++++
 t/t1300-config.sh     | 15 ++++++++++++++-
 3 files changed, 23 insertions(+), 2 deletions(-)
-- 
2.31.1
Patrick Steinhardt· Apr 19, 2021, 15:56 UTC · re: Patrick Steinhardt · lore

[PATCH 1/2] git.txt: fix synopsis of `--config-env` missing the equals sign

When executing `git -h`, then the `--config-env` documentation rightly lists the option as requiring an equals between the option and its argument: this is the only currently supported format. But the git(1) manpage incorrectly lists the option as taking a space in between.

Fix the issue by adding the missing space.
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-of-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/git.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to Documentation/git.txt +1 −1
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 3a9c44987f..ba5c8e9d98 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -13,7 +13,7 @@ SYNOPSIS
     [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
     [-p|--paginate|-P|--no-pager] [--no-replace-objects] [--bare]
     [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
-    [--super-prefix=<path>] [--config-env <name>=<envvar>]
+    [--super-prefix=<path>] [--config-env=<name>=<envvar>]
     <command> [<args>]
 
 DESCRIPTION
-- 
2.31.1
Patrick Steinhardt· Apr 19, 2021, 15:56 UTC · re: Patrick Steinhardt · lore

[PATCH 2/2] git: support separate arg for `--config-env`'s value

While not documented as such, many of the top-level options like `--git-dir` and `--work-tree` support two syntaxes: they accept both an equals sign between option and its value, and they do support option and value as two separate arguments. The recently added `--config-env` option only supports the syntax with an equals sign.

Mitigate this inconsistency by accepting both syntaxes and add tests to verify both work.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 git.c             |  8 ++++++++
 t/t1300-config.sh | 15 ++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)
Show changes to 2 files +22 −1

git.c, t/t1300-config.sh

diff --git a/git.c b/git.c
index b53e665671..ad365c05c7 100644
--- a/git.c
+++ b/git.c
@@ -255,6 +255,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			git_config_push_parameter((*argv)[1]);
 			(*argv)++;
 			(*argc)--;
+		} else if (!strcmp(cmd, "--config-env")) {
+			if (*argc < 2) {
+				fprintf(stderr, _("no config key given for --config-env\n" ));
+				usage(git_usage_string);
+			}
+			git_config_push_env((*argv)[1]);
+			(*argv)++;
+			(*argc)--;
 		} else if (skip_prefix(cmd, "--config-env=", &cmd)) {
 			git_config_push_env(cmd);
 		} else if (!strcmp(cmd, "--literal-pathspecs")) {
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index e0dd5d65ce..18803f9953 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1374,16 +1374,29 @@ test_expect_success 'git --config-env=key=envvar support' '
 	cat >expect <<-\EOF &&
 	value
 	value
+	value
+	value
+	false
 	false
 	EOF
 	{
 		ENVVAR=value git --config-env=core.name=ENVVAR config core.name &&
+		ENVVAR=value git --config-env core.name=ENVVAR config core.name &&
 		ENVVAR=value git --config-env=foo.CamelCase=ENVVAR config foo.camelcase &&
-		ENVVAR= git --config-env=foo.flag=ENVVAR config --bool foo.flag
+		ENVVAR=value git --config-env foo.CamelCase=ENVVAR config foo.camelcase &&
+		ENVVAR= git --config-env=foo.flag=ENVVAR config --bool foo.flag &&
+		ENVVAR= git --config-env foo.flag=ENVVAR config --bool foo.flag
 	} >actual &&
 	test_cmp expect actual
 '
 
+test_expect_success 'git --config-env with missing value' '
+	test_must_fail env ENVVAR=value git --config-env 2>error &&
+	test_i18ngrep "no config key given for --config-env" error &&
+	test_must_fail env ENVVAR=value git --config-env config core.name 2>error &&
+	test_i18ngrep "invalid config format: config" error
+'
+
 test_expect_success 'git --config-env fails with invalid parameters' '
 	test_must_fail git --config-env=foo.flag config --bool foo.flag 2>error &&
 	test_i18ngrep "invalid config format: foo.flag" error &&
-- 
2.31.1
Ævar Arnfjörð Bjarmason· Apr 20, 2021, 10:52 UTC · re: Patrick Steinhardt · lore

Re: [PATCH 2/2] git: support separate arg for `--config-env`'s value

On Mon, Apr 19 2021, Patrick Steinhardt wrote:
Show 10 quoted lines
> While not documented as such, many of the top-level options like
> `--git-dir` and `--work-tree` support two syntaxes: they accept both an
> equals sign between option and its value, and they do support option and
> value as two separate arguments. The recently added `--config-env`
> option only supports the syntax with an equals sign.
>
> Mitigate this inconsistency by accepting both syntaxes and add tests to
> verify both work.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
This whole series LGTM.
Show 15 quoted lines
>  git.c             |  8 ++++++++
>  t/t1300-config.sh | 15 ++++++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/git.c b/git.c
> index b53e665671..ad365c05c7 100644
> --- a/git.c
> +++ b/git.c
> @@ -255,6 +255,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>  			git_config_push_parameter((*argv)[1]);
>  			(*argv)++;
>  			(*argc)--;
> +		} else if (!strcmp(cmd, "--config-env")) {
> +			if (*argc < 2) {
> +				fprintf(stderr, _("no config key given for --config-env\n" ));

I found this use of fprintf() slightly odd, why not error(), but then went back and read the function and saw that it has N number of "fprintf(stderr, [...])" already.

That could probably all be converted to error(), but better to be consistent for now.

Show 43 quoted lines
> +				usage(git_usage_string);
> +			}
> +			git_config_push_env((*argv)[1]);
> +			(*argv)++;
> +			(*argc)--;
>  		} else if (skip_prefix(cmd, "--config-env=", &cmd)) {
>  			git_config_push_env(cmd);
>  		} else if (!strcmp(cmd, "--literal-pathspecs")) {
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index e0dd5d65ce..18803f9953 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -1374,16 +1374,29 @@ test_expect_success 'git --config-env=key=envvar support' '
>  	cat >expect <<-\EOF &&
>  	value
>  	value
> +	value
> +	value
> +	false
>  	false
>  	EOF
>  	{
>  		ENVVAR=value git --config-env=core.name=ENVVAR config core.name &&
> +		ENVVAR=value git --config-env core.name=ENVVAR config core.name &&
>  		ENVVAR=value git --config-env=foo.CamelCase=ENVVAR config foo.camelcase &&
> -		ENVVAR= git --config-env=foo.flag=ENVVAR config --bool foo.flag
> +		ENVVAR=value git --config-env foo.CamelCase=ENVVAR config foo.camelcase &&
> +		ENVVAR= git --config-env=foo.flag=ENVVAR config --bool foo.flag &&
> +		ENVVAR= git --config-env foo.flag=ENVVAR config --bool foo.flag
>  	} >actual &&
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'git --config-env with missing value' '
> +	test_must_fail env ENVVAR=value git --config-env 2>error &&
> +	test_i18ngrep "no config key given for --config-env" error &&
> +	test_must_fail env ENVVAR=value git --config-env config core.name 2>error &&
> +	test_i18ngrep "invalid config format: config" error
> +'
> +
>  test_expect_success 'git --config-env fails with invalid parameters' '
>  	test_must_fail git --config-env=foo.flag config --bool foo.flag 2>error &&
>  	test_i18ngrep "invalid config format: foo.flag" error &&
Jeff King· Apr 23, 2021, 10:07 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 2/2] git: support separate arg for `--config-env`'s value

On Tue, Apr 20, 2021 at 12:52:43PM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 6 quoted lines
> > Mitigate this inconsistency by accepting both syntaxes and add tests to
> > verify both work.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> 
> This whole series LGTM.
Me too. Thanks for finding it (and Patrick for fixing it).
Show 11 quoted lines
> > +		} else if (!strcmp(cmd, "--config-env")) {
> > +			if (*argc < 2) {
> > +				fprintf(stderr, _("no config key given for --config-env\n" ));
> 
> 
> I found this use of fprintf() slightly odd, why not error(), but then
> went back and read the function and saw that it has N number of
> "fprintf(stderr, [...])" already.
> 
> That could probably all be converted to error(), but better to be
> consistent for now.
Yep. Likewise the weird extra space at the end of the string.
-Peff
Ævar Arnfjörð Bjarmason· Apr 20, 2021, 11:24 UTC · re: Patrick Steinhardt · lore

Re: [PATCH 2/2] git: support separate arg for `--config-env`'s value

On Mon, Apr 19 2021, Patrick Steinhardt wrote:
Show 7 quoted lines
> +test_expect_success 'git --config-env with missing value' '
> +	test_must_fail env ENVVAR=value git --config-env 2>error &&
> +	test_i18ngrep "no config key given for --config-env" error &&
> +	test_must_fail env ENVVAR=value git --config-env config core.name 2>error &&
> +	test_i18ngrep "invalid config format: config" error
> +'
> +
Nit: (not spotted on the first reading): s/test_i18ngrep/grep/.
>  test_expect_success 'git --config-env fails with invalid parameters' '
>  	test_must_fail git --config-env=foo.flag config --bool foo.flag 2>error &&
>  	test_i18ngrep "invalid config format: foo.flag" error &&
Patrick Steinhardt· Apr 29, 2021, 12:55 UTC · re: Patrick Steinhardt · lore

[PATCH v2 0/2] git: support separate arg for `--config-env`'s value

Hi,

this is the second version of this series fixing inconsistencies with the `--config-env` parameter. It's only got a single change compared to v1, which is to replace `test_i18ngrep` with `grep` as pointed out by Ævar.

Patrick
Patrick Steinhardt (2):
  git.txt: fix synopsis of `--config-env` missing the equals sign
  git: support separate arg for `--config-env`'s value
 Documentation/git.txt |  2 +-
 git.c                 |  8 ++++++++
 t/t1300-config.sh     | 15 ++++++++++++++-
 3 files changed, 23 insertions(+), 2 deletions(-)
Range-diff against v1:
1:  08049f3b10 = 1:  08049f3b10 git.txt: fix synopsis of `--config-env` missing the equals sign
2:  d52db89bc2 ! 2:  5264fb6fa7 git: support separate arg for `--config-env`'s value
    @@ t/t1300-config.sh: test_expect_success 'git --config-env=key=envvar support' '
      
     +test_expect_success 'git --config-env with missing value' '
     +	test_must_fail env ENVVAR=value git --config-env 2>error &&
    -+	test_i18ngrep "no config key given for --config-env" error &&
    ++	grep "no config key given for --config-env" error &&
     +	test_must_fail env ENVVAR=value git --config-env config core.name 2>error &&
    -+	test_i18ngrep "invalid config format: config" error
    ++	grep "invalid config format: config" error
     +'
     +
      test_expect_success 'git --config-env fails with invalid parameters' '
-- 
2.31.1
Patrick Steinhardt· Apr 29, 2021, 12:55 UTC · re: Patrick Steinhardt · lore

[PATCH v2 1/2] git.txt: fix synopsis of `--config-env` missing the equals sign

When executing `git -h`, then the `--config-env` documentation rightly lists the option as requiring an equals between the option and its argument: this is the only currently supported format. But the git(1) manpage incorrectly lists the option as taking a space in between.

Fix the issue by adding the missing space.
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-of-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/git.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to Documentation/git.txt +1 −1
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 3a9c44987f..ba5c8e9d98 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -13,7 +13,7 @@ SYNOPSIS
     [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
     [-p|--paginate|-P|--no-pager] [--no-replace-objects] [--bare]
     [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
-    [--super-prefix=<path>] [--config-env <name>=<envvar>]
+    [--super-prefix=<path>] [--config-env=<name>=<envvar>]
     <command> [<args>]
 
 DESCRIPTION
-- 
2.31.1
Patrick Steinhardt· Apr 29, 2021, 12:55 UTC · re: Patrick Steinhardt · lore

[PATCH v2 2/2] git: support separate arg for `--config-env`'s value

While not documented as such, many of the top-level options like `--git-dir` and `--work-tree` support two syntaxes: they accept both an equals sign between option and its value, and they do support option and value as two separate arguments. The recently added `--config-env` option only supports the syntax with an equals sign.

Mitigate this inconsistency by accepting both syntaxes and add tests to verify both work.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 git.c             |  8 ++++++++
 t/t1300-config.sh | 15 ++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)
Show changes to 2 files +22 −1

git.c, t/t1300-config.sh

diff --git a/git.c b/git.c
index b53e665671..ad365c05c7 100644
--- a/git.c
+++ b/git.c
@@ -255,6 +255,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			git_config_push_parameter((*argv)[1]);
 			(*argv)++;
 			(*argc)--;
+		} else if (!strcmp(cmd, "--config-env")) {
+			if (*argc < 2) {
+				fprintf(stderr, _("no config key given for --config-env\n" ));
+				usage(git_usage_string);
+			}
+			git_config_push_env((*argv)[1]);
+			(*argv)++;
+			(*argc)--;
 		} else if (skip_prefix(cmd, "--config-env=", &cmd)) {
 			git_config_push_env(cmd);
 		} else if (!strcmp(cmd, "--literal-pathspecs")) {
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index e0dd5d65ce..ad4e6d0cfc 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1374,16 +1374,29 @@ test_expect_success 'git --config-env=key=envvar support' '
 	cat >expect <<-\EOF &&
 	value
 	value
+	value
+	value
+	false
 	false
 	EOF
 	{
 		ENVVAR=value git --config-env=core.name=ENVVAR config core.name &&
+		ENVVAR=value git --config-env core.name=ENVVAR config core.name &&
 		ENVVAR=value git --config-env=foo.CamelCase=ENVVAR config foo.camelcase &&
-		ENVVAR= git --config-env=foo.flag=ENVVAR config --bool foo.flag
+		ENVVAR=value git --config-env foo.CamelCase=ENVVAR config foo.camelcase &&
+		ENVVAR= git --config-env=foo.flag=ENVVAR config --bool foo.flag &&
+		ENVVAR= git --config-env foo.flag=ENVVAR config --bool foo.flag
 	} >actual &&
 	test_cmp expect actual
 '
 
+test_expect_success 'git --config-env with missing value' '
+	test_must_fail env ENVVAR=value git --config-env 2>error &&
+	grep "no config key given for --config-env" error &&
+	test_must_fail env ENVVAR=value git --config-env config core.name 2>error &&
+	grep "invalid config format: config" error
+'
+
 test_expect_success 'git --config-env fails with invalid parameters' '
 	test_must_fail git --config-env=foo.flag config --bool foo.flag 2>error &&
 	test_i18ngrep "invalid config format: foo.flag" error &&
-- 
2.31.1
Jeff King· Apr 29, 2021, 20:04 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v2 0/2] git: support separate arg for `--config-env`'s value

On Thu, Apr 29, 2021 at 02:55:25PM +0200, Patrick Steinhardt wrote:
> this is the second version of this series fixing inconsistencies with
> the `--config-env` parameter. It's only got a single change compared to
> v1, which is to replace `test_i18ngrep` with `grep` as pointed out by
> Ævar.
Thanks, this version looks good to me.
-Peff
Junio C Hamano· Apr 30, 2021, 00:47 UTC · re: Jeff King · lore

Re: [PATCH v2 0/2] git: support separate arg for `--config-env`'s value

Jeff King <peff@peff.net> writes:
Show 10 quoted lines
> On Thu, Apr 29, 2021 at 02:55:25PM +0200, Patrick Steinhardt wrote:
>
>> this is the second version of this series fixing inconsistencies with
>> the `--config-env` parameter. It's only got a single change compared to
>> v1, which is to replace `test_i18ngrep` with `grep` as pointed out by
>> Ævar.
>
> Thanks, this version looks good to me.
>
> -Peff
Thanks, will replace.

← back to recent threads