Re: [PATCH v7 2/8] config: add new way to pass config via `--config-env`
From: Junio C Hamano <hidden>
Date: 2021-01-11 22:35:51
Patrick Steinhardt [off-list ref] writes:
quoted hunk
+void git_config_push_env(const char *spec) +{ + struct strbuf buf = STRBUF_INIT; + const char *env_name; + const char *env_value; + + env_name = strrchr(spec, '='); + if (!env_name) + die(_("invalid config format: %s"), spec); + env_name++; + if (!*env_name) + die(_("missing value for --config-env"));
If reporting the name of the configuration variable, for which we
checked an environment variable, is worth doing in the !env_value
case below, shouldn't we be doing the same here, too? I.e.
die(_("missing environment variable name in %s", spec));;
or something to complain against "git --config-env foo="?
quoted hunk
+ env_value = getenv(env_name); + if (!env_value) + die(_("missing environment variable '%s' for configuration '%.*s'"), + env_name, (int)(env_name - spec - 1), spec);
quoted hunk
+test_expect_success 'git --config-env=key=envvar support' ' + cat >expect <<-\EOF && + value + value + false + EOF + { + env ENVVAR=value git --config-env=core.name=ENVVAR config core.name && + env ENVVAR=value git --config-env=foo.CamelCase=ENVVAR config foo.camelcase && + env ENVVAR= git --config-env=foo.flag=ENVVAR config --bool foo.flag
These "env " prefixes are not wrong per-se but are unnecessary. The same for the rest of this patch.
quoted hunk
+ } >actual && + test_cmp expect actual +' + +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" error && + test_must_fail git --config-env=foo.flag= config --bool foo.flag 2>error && + test_i18ngrep "missing value for --config-env" error && + test_must_fail git --config-env=foo.flag=NONEXISTENT config --bool foo.flag 2>error &&
How are we making sure $ NONEXISTENT=True make test is not what the end-user is running? sane_unset X && test_must_fail git --config-env foo.flag=X config --bool foo.flag or something along that line, perhaps?