diff mbox series

[1/6] env--helper: new undocumented builtin wrapping git_env_*()

Message ID 20190619233046.27503-2-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series Change <non-empty?> GIT_TEST_* variables to <boolean> | expand

Commit Message

Ævar Arnfjörð Bjarmason June 19, 2019, 11:30 p.m. UTC 
We have many GIT_TEST_* variables that accept a <boolean> because
they're implemented in C, and then some that take <non-empty?> because
they're implemented at least partially in shellscript.

Add a helper that wraps git_env_bool() and git_env_ulong() as the
first step in fixing this. This isn't being added as a test-tool mode
because some of these are used outside the test suite.

Part of what this tool does can be done via a trick with "git config"
added in 83d842dc8c ("tests: turn on network daemon tests by default",
2014-02-10) for test_tristate(), i.e.:

    git -c magic.variable="$1" config --bool magic.variable 2>/dev/null

But as subsequent changes will show being able to pass along the
default value makes all the difference, and we'll be able to replace
test_tristate() itself with that.

The --mode-bool option will be used by subsequent patches, but not
--mode-ulong. I figured it was easy enough to add it & test for it so
I left it in so we'd have wrappers for both git_env_*() functions.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 .gitignore            |  1 +
 Makefile              |  1 +
 builtin.h             |  1 +
 builtin/env--helper.c | 74 +++++++++++++++++++++++++++++++++++++++++++
 git.c                 |  1 +
 t/t0016-env-helper.sh | 70 ++++++++++++++++++++++++++++++++++++++++
 6 files changed, 148 insertions(+)
 create mode 100644 builtin/env--helper.c
 create mode 100755 t/t0016-env-helper.sh

Comments

Junio C Hamano June 20, 2019, 7:25 p.m. UTC | #1 
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> +	struct option opts[] = {
> +		OPT_CMDMODE(0, "mode-bool", &cmdmode,
> +			    N_("invoke git_env_bool(...)"), ENV_HELPER_BOOL),
> +		OPT_CMDMODE(0, "mode-ulong", &cmdmode,
> +			    N_("invoke git_env_ulong(...)"), ENV_HELPER_ULONG),

It may be a fairly useless nitpick, but is there a reason why we
should not just reuse "--type=<bool|int|...>" that is understood by
"git config"?  In the longer term, it might make sense to expose
this subcommand as "git env" that sits next to "git config", and at
that point we would regret if we miss the opportunity for obvious
parallel between the two.

> +		OPT_STRING(0, "variable", &env_variable, N_("name"),
> +			   N_("which environment variable to ask git_env_*(...) about")),
> +		OPT_STRING(0, "default", &env_default, N_("value"),
> +			   N_("what default value does git_env_*(...) fall back on?")),
> +		OPT_BOOL(0, "exit-code", &exit_code,
> +			 N_("exit code determined by truth of the git_env_*() function")),
> +		OPT_BOOL(0, "quiet", &quiet,
> +			 N_("don't print the git_env_*() return value")),
> +		OPT_END(),
> +	};
> +
> +	if (parse_options(argc, argv, prefix, opts, env__helper_usage, 0))
> +		usage_with_options(env__helper_usage, opts);
> +	if (!env_variable || !env_default ||
> +	    !*env_variable || !*env_default)
> +		usage_with_options(env__helper_usage, opts);

The default must be supplied?  That makes it smell like it should
not be a command line "option", as it is not optional at all.  Off
the top of my head without the benefit of insight you gained by
working on the remainder of the series (read: this is merely my knee
jerk reaction, and it is very probable that there is sound rationale
why your design is not like what I'll outline), I would imagine an
interface that look more like:

 $ git env --type=bool --default=yes GIT_TEST_FOO 

    Says "true"/"false" on the standard output and exits with
    success status if $GIT_TEST_FOO is set to the usual
    truth/falsehood values, and gives "true" on the exits with
    success status if $GIT_TEST_FOO is unset.

 $ git env --type=bool GIT_TEST_FOO

    The same as above when $GIT_TEST_FOO is set; exits with failure
    when GIT_TEST_FOO is not exported.

 $ git env --type=bool [--default=yes] --exit-code GIT_TEST_FOO

    Similar to the above two, but the standard output is silent, and
    true/false/failure are given via the exit status (perhaps 0, 1
    and 125 or something like that).

> +	switch (cmdmode) {
> +	case ENV_HELPER_BOOL:
> +		tmp_int = strtol(env_default, (char **)&env_default, 10);
> +		if (*env_default) {
> +			error(_("option `--default' expects a numerical value with `--mode-bool`"));

No kidding.  "--mode-bool" does not like "--default=false"?
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index 4470d7cfc0..1f7a83fb3c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -58,6 +58,7 @@ 
 /git-difftool
 /git-difftool--helper
 /git-describe
+/git-env--helper
 /git-fast-export
 /git-fast-import
 /git-fetch
diff --git a/Makefile b/Makefile
index f58bf14c7b..f2cfc8d812 100644
--- a/Makefile
+++ b/Makefile
@@ -1059,6 +1059,7 @@  BUILTIN_OBJS += builtin/diff-index.o
 BUILTIN_OBJS += builtin/diff-tree.o
 BUILTIN_OBJS += builtin/diff.o
 BUILTIN_OBJS += builtin/difftool.o
+BUILTIN_OBJS += builtin/env--helper.o
 BUILTIN_OBJS += builtin/fast-export.o
 BUILTIN_OBJS += builtin/fetch-pack.o
 BUILTIN_OBJS += builtin/fetch.o
diff --git a/builtin.h b/builtin.h
index ec7e0954c4..93bd49fe4f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -160,6 +160,7 @@  int cmd_diff_index(int argc, const char **argv, const char *prefix);
 int cmd_diff(int argc, const char **argv, const char *prefix);
 int cmd_diff_tree(int argc, const char **argv, const char *prefix);
 int cmd_difftool(int argc, const char **argv, const char *prefix);
+int cmd_env__helper(int argc, const char **argv, const char *prefix);
 int cmd_fast_export(int argc, const char **argv, const char *prefix);
 int cmd_fetch(int argc, const char **argv, const char *prefix);
 int cmd_fetch_pack(int argc, const char **argv, const char *prefix);
diff --git a/builtin/env--helper.c b/builtin/env--helper.c
new file mode 100644
index 0000000000..2bb65ecf3f
--- /dev/null
+++ b/builtin/env--helper.c
@@ -0,0 +1,74 @@ 
+#include "builtin.h"
+#include "config.h"
+#include "parse-options.h"
+
+static char const * const env__helper_usage[] = {
+	N_("git env--helper [--mode-bool | --mode-ulong] --env-variable=<VAR> --env-default=<DEF> [<options>]"),
+	NULL
+};
+
+int cmd_env__helper(int argc, const char **argv, const char *prefix)
+{
+	enum {
+		ENV_HELPER_BOOL = 1,
+		ENV_HELPER_ULONG,
+	} cmdmode = 0;
+	int exit_code = 0;
+	int quiet = 0;
+	const char *env_variable = NULL;
+	const char *env_default = NULL;
+	int ret;
+	int ret_int, tmp_int;
+	unsigned long ret_ulong, tmp_ulong;
+	struct option opts[] = {
+		OPT_CMDMODE(0, "mode-bool", &cmdmode,
+			    N_("invoke git_env_bool(...)"), ENV_HELPER_BOOL),
+		OPT_CMDMODE(0, "mode-ulong", &cmdmode,
+			    N_("invoke git_env_ulong(...)"), ENV_HELPER_ULONG),
+		OPT_STRING(0, "variable", &env_variable, N_("name"),
+			   N_("which environment variable to ask git_env_*(...) about")),
+		OPT_STRING(0, "default", &env_default, N_("value"),
+			   N_("what default value does git_env_*(...) fall back on?")),
+		OPT_BOOL(0, "exit-code", &exit_code,
+			 N_("exit code determined by truth of the git_env_*() function")),
+		OPT_BOOL(0, "quiet", &quiet,
+			 N_("don't print the git_env_*() return value")),
+		OPT_END(),
+	};
+
+	if (parse_options(argc, argv, prefix, opts, env__helper_usage, 0))
+		usage_with_options(env__helper_usage, opts);
+	if (!env_variable || !env_default ||
+	    !*env_variable || !*env_default)
+		usage_with_options(env__helper_usage, opts);
+
+	switch (cmdmode) {
+	case ENV_HELPER_BOOL:
+		tmp_int = strtol(env_default, (char **)&env_default, 10);
+		if (*env_default) {
+			error(_("option `--default' expects a numerical value with `--mode-bool`"));
+			usage_with_options(env__helper_usage, opts);
+		}
+		ret_int = git_env_bool(env_variable, tmp_int);
+		if (!quiet)
+			printf("%d\n", ret_int);
+		ret = ret_int;
+		break;
+	case ENV_HELPER_ULONG:
+		tmp_ulong = strtoll(env_default, (char **)&env_default, 10);
+		if (*env_default) {
+			error(_("option `--default' expects a numerical value with `--mode-ulong`"));
+			usage_with_options(env__helper_usage, opts);
+		}
+		ret_ulong = git_env_ulong(env_variable, tmp_ulong);
+		if (!quiet)
+			printf("%lu\n", ret_ulong);
+		ret = ret_ulong;
+		break;
+	}
+
+	if (exit_code)
+		return !ret;
+
+	return 0;
+}
diff --git a/git.c b/git.c
index c2eec470c9..a43e1dd98e 100644
--- a/git.c
+++ b/git.c
@@ -500,6 +500,7 @@  static struct cmd_struct commands[] = {
 	{ "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },
 	{ "diff-tree", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },
 	{ "difftool", cmd_difftool, RUN_SETUP_GENTLY },
+	{ "env--helper", cmd_env__helper },
 	{ "fast-export", cmd_fast_export, RUN_SETUP },
 	{ "fetch", cmd_fetch, RUN_SETUP },
 	{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
diff --git a/t/t0016-env-helper.sh b/t/t0016-env-helper.sh
new file mode 100755
index 0000000000..4dc4ab35e5
--- /dev/null
+++ b/t/t0016-env-helper.sh
@@ -0,0 +1,70 @@ 
+#!/bin/sh
+
+test_description='test env--helper'
+
+. ./test-lib.sh
+
+
+test_expect_success 'env--helper usage' '
+	test_must_fail git env--helper &&
+	test_must_fail git env--helper --mode-bool &&
+	test_must_fail git env--helper --mode-ulong &&
+	test_must_fail git env--helper --mode-bool --variable &&
+	test_must_fail git env--helper --mode-bool --variable --default &&
+	test_must_fail git env--helper --mode-bool --variable= --default=
+'
+
+test_expect_success 'env--helper bad default values' '
+	test_must_fail git env--helper --mode-bool --variable=MISSING --default=1xyz &&
+	test_must_fail git env--helper --mode-ulong --variable=MISSING --default=1xyz
+'
+
+test_expect_success 'env--helper --mode-bool' '
+	echo 1 >expected &&
+	git env--helper --mode-bool --variable=MISSING --default=1 --exit-code >actual &&
+	test_cmp expected actual &&
+
+	echo 0 >expected &&
+	test_must_fail git env--helper --mode-bool --variable=MISSING --default=0 --exit-code >actual &&
+	test_cmp expected actual &&
+
+	git env--helper --mode-bool --variable=MISSING --default=0 >actual &&
+	test_cmp expected actual &&
+
+	>expected &&
+	git env--helper --mode-bool --variable=MISSING --default=1 --exit-code --quiet >actual &&
+	test_cmp expected actual &&
+
+	EXISTS=true git env--helper --mode-bool --variable=EXISTS --default=0 --exit-code --quiet >actual &&
+	test_cmp expected actual &&
+
+	echo 1 >expected &&
+	EXISTS=true git env--helper --mode-bool --variable=EXISTS --default=0 --exit-code >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'env--helper --mode-ulong' '
+	echo 1234567890 >expected &&
+	git env--helper --mode-ulong --variable=MISSING --default=1234567890 --exit-code >actual &&
+	test_cmp expected actual &&
+
+	echo 0 >expected &&
+	test_must_fail git env--helper --mode-ulong --variable=MISSING --default=0 --exit-code >actual &&
+	test_cmp expected actual &&
+
+	git env--helper --mode-ulong --variable=MISSING --default=0 >actual &&
+	test_cmp expected actual &&
+
+	>expected &&
+	git env--helper --mode-ulong --variable=MISSING --default=1234567890 --exit-code --quiet >actual &&
+	test_cmp expected actual &&
+
+	EXISTS=1234567890 git env--helper --mode-ulong --variable=EXISTS --default=0 --exit-code --quiet >actual &&
+	test_cmp expected actual &&
+
+	echo 1234567890 >expected &&
+	EXISTS=1234567890 git env--helper --mode-ulong --variable=EXISTS --default=0 --exit-code >actual &&
+	test_cmp expected actual
+'
+
+test_done