config: add --literal-value option

As reported [1], 'git maintenance unregister' fails when a repository is located in a directory with regex glob characters.

[1]
https://lore.kernel.org/git/[email protected]/T/#mb96fa4187a0d6aeda097cd95804a8aafc0273022

The discussed solution was to modify 'git config' to specify that the 'value_regex' argument should be treated as an exact string match. This is the primary change in this series, with an additional patch at the end to make 'git maintenance [un]register' use this option, when necessary.

Thanks, -Stolee

Derrick Stolee (7):
t1300: test "set all" mode with value_regex
t1300: add test for --replace-all with value_regex
config: convert multi_replace to flags
config: add --literal-value option, un-implemented
config: plumb --literal-value into config API
config: implement --literal-value with --get*
maintenance: use 'git config --literal-value'

 Documentation/git-config.txt |  20 ++++--
 builtin/branch.c             |   4 +-
 builtin/config.c             |  50 ++++++++++---
 builtin/gc.c                 |   5 +-
 builtin/remote.c             |   8 ++-
 config.c                     |  29 ++++----
 config.h                     |  24 +++++--
 t/t1300-config.sh            | 136 +++++++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh       |  12 ++++
 9 files changed, 248 insertions(+), 40 deletions(-)

base-commit: 0016b618182f642771dc589cf0090289f9fe1b4f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-796%2Fderrickstolee%2Fmaintenance%2Fconfig-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-796/derrickstolee/maintenance/config-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/796

gitgitgadget

From: Derrick Stolee <[email protected]>

Without additional modifiers, 'git config' attempts to set a single value in the .git/config file. When the value_regex parameter is supplied, this command behaves in a non-trivial manner.

Consider 'git config key value value_regex'. The expected behavior is as follows:

1. If there are multiple existing values that match 'value_regex', then the command fails. Users should use --replace-all instead.

2. If there is one existing value that matches 'value_regex', then the new config has one entry where 'key=value'.

3. If there is no existing values match 'value_regex', then the 'key=value' pair is appended, making this 'key' a multi-valued config setting.

Add a test that demonstrates these options.

---
 t/t1300-config.sh | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

index 97ebfe1f9d..ef56b08070 100755
@@ -1914,4 +1914,39 @@ test_expect_success '--replace-all does not invent newlines' '
 	test_cmp expect .git/config
 '
 
+test_expect_success 'set all config with value_regex' '
+	q_to_tab >initial <<-\EOF &&
+	[abc]
+	Qkey = one
+	EOF
+
+	cp initial .git/config &&
+	git config abc.key two a+ &&
+	q_to_tab >expect <<-\EOF &&
+	[abc]
+	Qkey = one
+	Qkey = two
+	EOF
+	test_cmp expect .git/config &&
+
+	test_must_fail git config abc.key three o+ 2>err &&
+	test_i18ngrep "has multiple values" err &&
+	git config abc.key three a+ &&
+	q_to_tab >expect <<-\EOF &&
+	[abc]
+	Qkey = one
+	Qkey = two
+	Qkey = three
+	EOF
+	test_cmp expect .git/config &&
+
+	cp initial .git/config &&
+	git config abc.key three o+ &&
+	q_to_tab >expect <<-\EOF &&
+	[abc]
+	Qkey = three
+	EOF
+	test_cmp expect .git/config
+'
+
 test_done

gitgitgadget

I haven't looked at the code, but if you're just using git config in a test, then overwriting the config file shouldn't be a problem with SHA-256. If you're trying to read or write objects or the index, then that's definitely a problem, and you'll definitely notice exciting failures if you do that.

brian m. carlson (he/him or they/them)
Houston, Texas, US

Yes, that is exactly what worries me.

And I was trolling for ideas from those on cc: list to come up with a better convention to test the behaviour of "git config". I think taking "git config --list [--file FILE]" before and after the ops of interest and basing the good/bad decision on the difference would be a saner approach. It obviously relies on "git config --list" to be working correctly, which the current approach that uses hardcoded "initial" state and expects result that is byte-for-byte identical to a hardcoded file contents after the ops does not have to rely on, but at the same time, the current approach assumes too much on the actual file format (e.g. no user or script would care how "key = val" line is indented, but the current approach insists that they are indented exactly with one tab), so overall it may even be an improvement.

Thanks.

t1300 is full of this kind of junk. Several years ago, while working on some of the repositoryformatversion code, I noticed that we will accept a repository that does not have core.repositoryformatversion set at all, nor even has a .git/config present!

It's easy to fix in the code, but it causes failures all over t1300. So then I started converting t1300 to use "config --file" (which almost certainly didn't exist back when most of those tests were originally written). I don't remember how or why it got hairy, but it was enough that I eventually dropped it (unlike many of my other stale topics, I don't think I've even kept rebasing it forward as a WIP).

Possibly I was concerned that people in the wild were relying on a blank or missing config being the same as repositoryformatversion=0. That will definitely stop working in a sha256 world anyway, though, because they'll need the objectFormat extension.

So that got a bit off-track, but I think:

- t1300 already is very much like this, so it's not a new thing

- but I would be happy not to see it go further in that direction, even if it means inconsistency with the rest of the script

-Peff

Yeah, I especially like the latter half ;-)

Yup. We test this in t3200 as well. I don't love it, but it exists.

Which is exactly why that test in t3200 has a SHA1 prerequisite. I'm sure we'll hear someone complain about the fact that SHA-256 repositories have to have a config file, but I'm fine with us not supporting it.

I should point out that lacking a config file also only works on Unix systems with a POSIX file system (including case-sensitive macOS), since otherwise core.ignorecase (and core.symlinks, if appropriate) aren't set correctly. It also doesn't work for bare repositories on any OS. So hopefully the number of people doing this is quite small.

I agree we shouldn't make things worse.

brian m. carlson (he/him or they/them)
Houston, Texas, US

I started looking at early parts of t1300 and here is how far I managed to get before I can no longer keep staring the existing tests without vomitting.

I am reasonably happy with the "let's keep the vanilla untouched one in .git/config-initial, refrain from using [core] and other sections that MUST be in the initial configuration for testing, and use a wrapper that reads expected addition to the initial one from the standard input for validation" approach I came up with, but I am not happy with the name 'compare_expect'; 'validate_config_result' might be a better name.

In any case, the reason I am sending this out early is if people find this approach to clean things up a sensible one. If we can find concensus, perhaps I (or somebody else---hint, hint) can find time to do the #leftoverbits following the approach after the ds/config-literal-value and ds/maintenance-part-3 topics graduate to 'master'.

 t/t1300-config.sh | 139 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 71 insertions(+), 68 deletions(-)

index df13afaffd..c33520d7fa 100755
@@ -7,80 +7,84 @@ test_description='Test git config in different settings'
 
 . ./test-lib.sh
 
-test_expect_success 'clear default config' '
-	rm -f .git/config
+test_expect_success 'save away default config' '
+	cp .git/config .git/config-initial
 '
 
-cat > expect << EOF
-[core]
-	penguin = little blue
-EOF
-test_expect_success 'initial' '
-	git config core.penguin "little blue" &&
+compare_expect () {
+	{
+		cat .git/config-initial &&
+		sed -e 's/^[|]//'
+	} >expect &&
 	test_cmp expect .git/config
+}
+
+test_expect_success 'initial' '
+	git config configtest.penguin "little blue" &&
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = little blue
+	EOF
 '
 
-cat > expect << EOF
-[core]
-	penguin = little blue
-	Movie = BadPhysics
-EOF
 test_expect_success 'mixed case' '
-	git config Core.Movie BadPhysics &&
-	test_cmp expect .git/config
+	git config ConfigTest.Movie BadPhysics &&
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = little blue
+	|	Movie = BadPhysics
+	EOF
 '
 
-cat > expect << EOF
-[core]
-	penguin = little blue
-	Movie = BadPhysics
-[Cores]
-	WhatEver = Second
-EOF
 test_expect_success 'similar section' '
-	git config Cores.WhatEver Second &&
-	test_cmp expect .git/config
+	git config ConfigTests.WhatEver Second &&
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = little blue
+	|	Movie = BadPhysics
+	[ConfigTests]
+	|	WhatEver = Second
+	EOF
 '
 
-cat > expect << EOF
-[core]
-	penguin = little blue
-	Movie = BadPhysics
-	UPPERCASE = true
-[Cores]
-	WhatEver = Second
-EOF
 test_expect_success 'uppercase section' '
-	git config CORE.UPPERCASE true &&
-	test_cmp expect .git/config
+	git config CONFIGTEST.UPPERCASE true &&
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = little blue
+	|	Movie = BadPhysics
+	|	UPPERCASE = true
+	[ConfigTests]
+	|	WhatEver = Second
+	EOF
 '
 
 test_expect_success 'replace with non-match' '
-	git config core.penguin kingpin !blue
+	git config configtest.penguin kingpin !blue
 '
 
 test_expect_success 'replace with non-match (actually matching)' '
-	git config core.penguin "very blue" !kingpin
+	git config configtest.penguin "very blue" !kingpin
 '
 
-cat > expect << EOF
-[core]
-	penguin = very blue
-	Movie = BadPhysics
-	UPPERCASE = true
-	penguin = kingpin
-[Cores]
-	WhatEver = Second
-EOF
-
-test_expect_success 'non-match result' 'test_cmp expect .git/config'
+test_expect_success 'non-match result' '
+	compare_expect <<-\EOF
+	[configtest]
+	|	penguin = very blue
+	|	Movie = BadPhysics
+	|	UPPERCASE = true
+	|	penguin = kingpin
+	[ConfigTests]
+	|	WhatEver = Second
+	EOF
+'
 
 test_expect_success 'find mixed-case key by canonical name' '
-	test_cmp_config Second cores.whatever
+	test_cmp_config Second configtests.whatever
 '
 
 test_expect_success 'find mixed-case key by non-canonical name' '
-	test_cmp_config Second CoReS.WhAtEvEr
+	test_cmp_config Second CoNfIgTeSts.WhAtEvEr
 '
 
 test_expect_success 'subsections are not canonicalized by git-config' '

@@ -94,28 +98,27 @@ test_expect_success 'subsections are not canonicalized by git-config' '
 	test_cmp_config two section.SubSection.key
 '
 
-cat > .git/config <<\EOF
-[alpha]
-bar = foo
-[beta]
-baz = multiple \
-lines
-foo = bar
-EOF
-
 test_expect_success 'unset with cont. lines' '
-	git config --unset beta.baz
+	{
+		cat .git/config-initial &&
+		cat <<-\EOF
+		[alpha]
+		bar = foo
+		[beta]
+		baz = multiple \
+		lines
+		foo = bar
+		EOF
+	} >.git/config &&
+	git config --unset beta.baz &&
+	compare_expect <<-\EOF
+	[alpha]
+	bar = foo
+	[beta]
+	foo = bar
+	EOF
 '
 
-cat > expect <<\EOF
-[alpha]
-bar = foo
-[beta]
-foo = bar
-EOF
-
-test_expect_success 'unset with cont. lines is correct' 'test_cmp expect .git/config'
-
 cat > .git/config << EOF
 [beta] ; silly comment # another comment
 noIndent= sillyValue ; 'nother silly comment

I think my similar gastric reaction is what caused me to stop looking at it long ago. But it may also have been the test brian mentioned that explicitly checks that this case works (and for which he had to set SHA1 prereq).

IMHO this is worse than just using "config --file" in most of the tests. It's more steps to remember to deal with. And most tests do not care at all what the source file is. There are a few that check the order of lookup with respect to system and user files, but they could probably be run what non-destructive changes.

That said, most of the effort is in the tedium of switching each individual test. I am happy for whoever volunteers to do that work to have the final say in the approach.

-Peff

"Most tests do not care" only indicates the lack of test coverage.

Knowing the implementation, it probably is OK to assume that things would work fine as long as "--file <file>" works correctly, though ;-)

Not having to keep the minimum parts of the real configuration file, and being able to use a throw-away file for each test, certainly makes things cleaner.

Yup.

From: Derrick Stolee <[email protected]>

The --replace-all option was added in 4ddba79d (git-config-set: add more options) but was not tested along with the 'value_regex' parameter. Since we will be updating this option to optionally treat 'value_regex' as a fixed string, let's add a test here that documents the current behavior.

---
 t/t1300-config.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

index ef56b08070..74e0f84c0a 100755
@@ -1949,4 +1949,20 @@ test_expect_success 'set all config with value_regex' '
 	test_cmp expect .git/config
 '
 
+test_expect_success '--replace-all and value_regex' '
+	q_to_tab >.git/config <<-\EOF &&
+	[abc]
+	Qkey = one
+	Qkey = two
+	Qkey = three
+	EOF
+	q_to_tab >expect <<-\EOF &&
+	[abc]
+	Qkey = four
+	Qkey = three
+	EOF
+	git config --replace-all abc.key four "o+" &&
+	test_cmp expect .git/config
+'
+
 test_done

gitgitgadget

From: Derrick Stolee <[email protected]>

We will extend the flexibility of the config API. Before doing so, let's take an existing 'int multi_replace' parameter and replace it with a new 'int flags' parameter that can take multiple options as a bit field.

Update all callers that specified multi_replace to now specify the CONFIG_FLAGS_MULTI_REPLACE flag. To add more clarity, extend the documentation of git_config_set_multivar_in_file() including a clear labeling of its arguments. Other config API methods in config.h do not require a change to their prototypes, but they have an equivalent update to their implementations.

---
 builtin/branch.c |  4 ++--
 builtin/config.c |  6 ++++--
 builtin/remote.c |  8 +++++---
 config.c         | 24 ++++++++++++------------
 config.h         | 17 +++++++++++++----
 5 files changed, 36 insertions(+), 23 deletions(-)

index e82301fb1b..5ce3844d22 100644
@@ -829,10 +829,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die(_("Branch '%s' has no upstream information"), branch->name);
 
 		strbuf_addf(&buf, "branch.%s.remote", branch->name);
-		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "branch.%s.merge", branch->name);
-		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 		strbuf_release(&buf);
 	} else if (argc > 0 && argc <= 2) {
 		if (filter.kind != FILTER_REFS_BRANCHES)

index 5e39f61885..e7c7f3d455 100644
@@ -823,7 +823,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		value = normalize_value(argv[0], argv[1]);
 		UNLEAK(value);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
-							      argv[0], value, argv[2], 1);
+							      argv[0], value, argv[2],
+							      CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_GET) {
 		check_argc(argc, 1, 2);

@@ -859,7 +860,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_write();
 		check_argc(argc, 1, 2);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
-							      argv[0], NULL, argv[1], 1);
+							      argv[0], NULL, argv[1],
+							      CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_RENAME_SECTION) {
 		int ret;

index c8240e9fcd..29b1652975 100644
@@ -708,7 +708,7 @@ static int mv(int argc, const char **argv)
 
 	strbuf_reset(&buf);
 	strbuf_addf(&buf, "remote.%s.fetch", rename.new_name);
-	git_config_set_multivar(buf.buf, NULL, NULL, 1);
+	git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 	strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old_name);
 	for (i = 0; i < oldremote->fetch.raw_nr; i++) {
 		char *ptr;

@@ -1485,7 +1485,8 @@ static int update(int argc, const char **argv)
 
 static int remove_all_fetch_refspecs(const char *key)
 {
-	return git_config_set_multivar_gently(key, NULL, NULL, 1);
+	return git_config_set_multivar_gently(key, NULL, NULL,
+					      CONFIG_FLAGS_MULTI_REPLACE);
 }
 
 static void add_branches(struct remote *remote, const char **branches,

@@ -1674,7 +1675,8 @@ static int set_url(int argc, const char **argv)
 	if (!delete_mode)
 		git_config_set_multivar(name_buf.buf, newurl, oldurl, 0);
 	else
-		git_config_set_multivar(name_buf.buf, NULL, oldurl, 1);
+		git_config_set_multivar(name_buf.buf, NULL, oldurl,
+					CONFIG_FLAGS_MULTI_REPLACE);
 out:
 	strbuf_release(&name_buf);
 	return 0;

index 2b79fe76ad..4841c68a91 100644
@@ -2716,9 +2716,9 @@ void git_config_set(const char *key, const char *value)
  * if value_regex!=NULL, disregard key/value pairs where value does not match.
  * if value_regex==CONFIG_REGEX_NONE, do not match any existing values
  *     (only add a new one)
- * if multi_replace==0, nothing, or only one matching key/value is replaced,
- *     else all matching key/values (regardless how many) are removed,
- *     before the new pair is written.
+ * if (flags & CONFIG_FLAGS_MULTI_REPLACE) == 0, at most one matching
+ *     key/value is replaced, else all matching key/values (regardless
+ *     how many) are removed, before the new pair is written.
  *
  * Returns 0 on success.
  *

@@ -2739,7 +2739,7 @@ void git_config_set(const char *key, const char *value)
 int git_config_set_multivar_in_file_gently(const char *config_filename,
 					   const char *key, const char *value,
 					   const char *value_regex,
-					   int multi_replace)
+					   int flags)
 {
 	int fd = -1, in_fd = -1;
 	int ret;

@@ -2756,7 +2756,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 	if (ret)
 		goto out_free;
 
-	store.multi_replace = multi_replace;
+	store.multi_replace = (flags & CONFIG_FLAGS_MULTI_REPLACE) != 0;
 
 	if (!config_filename)
 		config_filename = filename_buf = git_pathdup("config");

@@ -2845,7 +2845,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 
 		/* if nothing to unset, or too many matches, error out */
 		if ((store.seen_nr == 0 && value == NULL) ||
-		    (store.seen_nr > 1 && multi_replace == 0)) {
+		    (store.seen_nr > 1 && !store.multi_replace)) {
 			ret = CONFIG_NOTHING_SET;
 			goto out_free;
 		}

@@ -2984,10 +2984,10 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 
 void git_config_set_multivar_in_file(const char *config_filename,
 				     const char *key, const char *value,
-				     const char *value_regex, int multi_replace)
+				     const char *value_regex, int flags)
 {
 	if (!git_config_set_multivar_in_file_gently(config_filename, key, value,
-						    value_regex, multi_replace))
+						    value_regex, flags))
 		return;
 	if (value)
 		die(_("could not set '%s' to '%s'"), key, value);

@@ -2996,17 +2996,17 @@ void git_config_set_multivar_in_file(const char *config_filename,
 }
 
 int git_config_set_multivar_gently(const char *key, const char *value,
-				   const char *value_regex, int multi_replace)
+				   const char *value_regex, int flags)
 {
 	return git_config_set_multivar_in_file_gently(NULL, key, value, value_regex,
-						      multi_replace);
+						      flags);
 }
 
 void git_config_set_multivar(const char *key, const char *value,
-			     const char *value_regex, int multi_replace)
+			     const char *value_regex, int flags)
 {
 	git_config_set_multivar_in_file(NULL, key, value, value_regex,
-					multi_replace);
+					flags);
 }
 
 static int section_name_match (const char *buf, const char *name)

index 060874488f..266eb22e46 100644
@@ -70,6 +70,13 @@ enum config_event_t {
 	CONFIG_EVENT_ERROR
 };
 
+/*
+ * When CONFIG_FLAGS_MULTI_REPLACE is specified, all matching key/values
+ * are removed before a new pair is written. If the flag is not present,
+ * then set operations replace only the first match.
+ */
+#define CONFIG_FLAGS_MULTI_REPLACE (1 << 0)
+
 /*
  * The parser event function (if not NULL) is called with the event type and
  * the begin/end offsets of the parsed elements.

@@ -276,13 +283,15 @@ int git_config_set_multivar_in_file_gently(const char *, const char *, const cha
  * - the value regex, as a string. It will disregard key/value pairs where value
  *   does not match.
  *
- * - a multi_replace value, as an int. If value is equal to zero, nothing or only
- *   one matching key/value is replaced, else all matching key/values (regardless
- *   how many) are removed, before the new pair is written.
+ * - a flags value with bits corresponding to the CONFIG_FLAG_* macros.
  *
  * It returns 0 on success.
  */
-void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
+void git_config_set_multivar_in_file(const char *config_filename,
+				     const char *key,
+				     const char *value,
+				     const char *value_regex,
+				     int flags);
 
 /**
  * rename or remove sections in the config file

gitgitgadget

Nice. I was afraid we may have to _add_ a new parameter, but here, we can turn an existing bool into a flags word, which is great. But please use "unsigned" for such flags, just out of inertia/convention.

From: Derrick Stolee <[email protected]>

The 'git config' builtin takes a 'value_regex' parameter for several actions. This can cause confusion when expecting exact value matches instead of regex matches, especially when the input string contains glob characters. While callers can escape the patterns themselves, it would be more friendly to allow an argument to disable the pattern matching in favor of an exact string match.

Add a new '--literal-value' option that does not currently change the behavior. The implementation will follow for each appropriate action. For now, check and test that --literal-value will abort the command when included with an incompatible action.

The name '--literal-value' was chosen over something simpler like '--literal' because some commands allow regular expressions on the key in addition to the value.

---
 Documentation/git-config.txt | 20 +++++++++++++-------
 builtin/config.c             | 17 +++++++++++++++++
 t/t1300-config.sh            | 13 +++++++++++++
 3 files changed, 43 insertions(+), 7 deletions(-)

index 7573160f21..e244dd73f0 100644
@@ -9,15 +9,15 @@ git-config - Get and set repository or global options
 SYNOPSIS
 --------
 [verse]
-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] name [value [value_regex]]
+'git config' [<file-option>] [--type=<type>] [--literal-value] [--show-origin] [--show-scope] [-z|--null] name [value [value_regex]]
 'git config' [<file-option>] [--type=<type>] --add name value
-'git config' [<file-option>] [--type=<type>] --replace-all name value [value_regex]
-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get name [value_regex]
-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get-all name [value_regex]
-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]
+'git config' [<file-option>] [--type=<type>] [--literal-value] --replace-all name value [value_regex]
+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--literal-value] --get name [value_regex]
+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--literal-value] --get-all name [value_regex]
+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--literal-value] [--name-only] --get-regexp name_regex [value_regex]
 'git config' [<file-option>] [--type=<type>] [-z|--null] --get-urlmatch name URL
-'git config' [<file-option>] --unset name [value_regex]
-'git config' [<file-option>] --unset-all name [value_regex]
+'git config' [<file-option>] [--literal-value] --unset name [value_regex]
+'git config' [<file-option>] [--literal-value] --unset-all name [value_regex]
 'git config' [<file-option>] --rename-section old_name new_name
 'git config' [<file-option>] --remove-section name
 'git config' [<file-option>] [--show-origin] [--show-scope] [-z|--null] [--name-only] -l | --list

@@ -165,6 +165,12 @@ See also <<FILES>>.
 --list::
 	List all variables set in config file, along with their values.
 
+--literal-value::
+	When used with the `value_regex` argument, treat `value_regex` as
+	an exact string instead of a regular expression. This will restrict
+	the name/value pairs that are matched to only those where the value
+	is exactly equal to the `value_regex`.
+
 --type <type>::
   'git config' will ensure that any input or output is valid under the given
   type constraint(s), and will canonicalize outgoing values in `<type>`'s

index e7c7f3d455..ad6c695737 100644
@@ -34,6 +34,7 @@ static int respect_includes_opt = -1;
 static struct config_options config_options;
 static int show_origin;
 static int show_scope;
+static int literal;
 
 #define ACTION_GET (1<<0)
 #define ACTION_GET_ALL (1<<1)

@@ -52,6 +53,16 @@ static int show_scope;
 #define ACTION_GET_COLORBOOL (1<<14)
 #define ACTION_GET_URLMATCH (1<<15)
 
+#define ACTION_LITERAL_ALLOWED (\
+	ACTION_GET |\
+	ACTION_GET_ALL |\
+	ACTION_GET_REGEXP |\
+	ACTION_REPLACE_ALL |\
+	ACTION_UNSET |\
+	ACTION_UNSET_ALL |\
+	ACTION_SET_ALL\
+)
+
 /*
  * The actions "ACTION_LIST | ACTION_GET_*" which may produce more than
  * one line of output and which should therefore be paged.

@@ -141,6 +152,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "rename-section", &actions, N_("rename section: old-name new-name"), ACTION_RENAME_SECTION),
 	OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"), ACTION_REMOVE_SECTION),
 	OPT_BIT('l', "list", &actions, N_("list all"), ACTION_LIST),
+	OPT_BOOL(0, "literal-value", &literal, N_("use literal equality when matching values")),
 	OPT_BIT('e', "edit", &actions, N_("open an editor"), ACTION_EDIT),
 	OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR),
 	OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL),

@@ -745,6 +757,11 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		usage_builtin_config();
 	}
 
+	if (literal && !(actions & ACTION_LITERAL_ALLOWED)) {
+		error(_("--literal only applies with 'value_regex'"));
+		usage_builtin_config();
+	}
+
 	if (actions & PAGING_ACTIONS)
 		setup_auto_pager("config", 1);
 

index 74e0f84c0a..73f5ca4361 100755
@@ -1965,4 +1965,17 @@ test_expect_success '--replace-all and value_regex' '
 	test_cmp expect .git/config
 '
 
+test_expect_success 'refuse --literal-value for incompatible actions' '
+	git config dev.null bogus &&
+	test_must_fail git config --literal-value --add dev.null bogus &&
+	test_must_fail git config --literal-value --get-urlmatch dev.null bogus &&
+	test_must_fail git config --literal-value --get-urlmatch dev.null bogus &&
+	test_must_fail git config --literal-value --rename-section dev null &&
+	test_must_fail git config --literal-value --remove-section dev &&
+	test_must_fail git config --literal-value --list &&
+	test_must_fail git config --literal-value --get-color dev.null &&
+	test_must_fail git config --literal-value --get-colorbool dev.null &&
+	test_must_fail git config --literal-value --edit
+'
+
 test_done

gitgitgadget

From: Derrick Stolee <[email protected]>

The git_config_set_multivar_in_file_gently() and related methods now take a 'flags' bitfield, so add a new bit representing the --literal-value option from 'git config'. This alters the purpose of the value_regex parameter to be an exact string match. This requires some initialization changes in git_config_set_multivar_in_file_gently() and a new strcmp() call in the matches() method.

The new CONFIG_FLAGS_LITERAL_VALUE flag is initialized in builtin/config.c based on the --literal-value option, and that needs to be updated in several callers.

This patch only affects some of the modes of 'git config', and the rest will be completed in the next change.

---
 builtin/config.c  | 16 +++++++++++-----
 config.c          |  5 +++++
 config.h          |  7 +++++++
 t/t1300-config.sh | 48 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 5 deletions(-)

index ad6c695737..6ab5a0d68f 100644
@@ -626,6 +626,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 {
 	int nongit = !startup_info->have_repository;
 	char *value;
+	int flags = 0;
 
 	given_config_source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT));
 

@@ -761,6 +762,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		error(_("--literal only applies with 'value_regex'"));
 		usage_builtin_config();
 	}
+	if (literal)
+		flags = CONFIG_FLAGS_LITERAL_VALUE;
 
 	if (actions & PAGING_ACTIONS)
 		setup_auto_pager("config", 1);

@@ -823,7 +826,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		value = normalize_value(argv[0], argv[1]);
 		UNLEAK(value);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
-							      argv[0], value, argv[2], 0);
+							      argv[0], value, argv[2],
+							      flags);
 	}
 	else if (actions == ACTION_ADD) {
 		check_write();

@@ -832,7 +836,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		UNLEAK(value);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
 							      argv[0], value,
-							      CONFIG_REGEX_NONE, 0);
+							      CONFIG_REGEX_NONE,
+							      flags);
 	}
 	else if (actions == ACTION_REPLACE_ALL) {
 		check_write();

@@ -841,7 +846,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		UNLEAK(value);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
 							      argv[0], value, argv[2],
-							      CONFIG_FLAGS_MULTI_REPLACE);
+							      flags | CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_GET) {
 		check_argc(argc, 1, 2);

@@ -868,7 +873,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 1, 2);
 		if (argc == 2)
 			return git_config_set_multivar_in_file_gently(given_config_source.file,
-								      argv[0], NULL, argv[1], 0);
+								      argv[0], NULL, argv[1],
+								      flags);
 		else
 			return git_config_set_in_file_gently(given_config_source.file,
 							     argv[0], NULL);

@@ -878,7 +884,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 1, 2);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
 							      argv[0], NULL, argv[1],
-							      CONFIG_FLAGS_MULTI_REPLACE);
+							      flags | CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_RENAME_SECTION) {
 		int ret;

index 4841c68a91..5028c1e736 100644
@@ -2402,6 +2402,7 @@ struct config_store_data {
 	size_t baselen;
 	char *key;
 	int do_not_match;
+	const char *literal_value;
 	regex_t *value_regex;
 	int multi_replace;
 	struct {

@@ -2431,6 +2432,8 @@ static int matches(const char *key, const char *value,
 {
 	if (strcmp(key, store->key))
 		return 0; /* not ours */
+	if (store->literal_value)
+		return !strcmp(store->literal_value, value);
 	if (!store->value_regex)
 		return 1; /* always matches */
 	if (store->value_regex == CONFIG_REGEX_NONE)

@@ -2803,6 +2806,8 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 			store.value_regex = NULL;
 		else if (value_regex == CONFIG_REGEX_NONE)
 			store.value_regex = CONFIG_REGEX_NONE;
+		else if (flags & CONFIG_FLAGS_LITERAL_VALUE)
+			store.literal_value = value_regex;
 		else {
 			if (value_regex[0] == '!') {
 				store.do_not_match = 1;

index 266eb22e46..ddb81e1e32 100644
@@ -77,6 +77,13 @@ enum config_event_t {
  */
 #define CONFIG_FLAGS_MULTI_REPLACE (1 << 0)
 
+/*
+ * When CONFIG_FLAGS_LITERAL_VALUE is specified, match key/value pairs
+ * by string comparison (not regex match) to the provided value_regex
+ * parameter.
+ */
+#define CONFIG_FLAGS_LITERAL_VALUE (1 << 1)
+
 /*
  * The parser event function (if not NULL) is called with the event type and
  * the begin/end offsets of the parsed elements.

gitgitgadget

From: Derrick Stolee <[email protected]>

The config builtin does its own regex matching of values for the --get, --get-all, and --get-regexp modes. Plumb the existing 'flags' parameter to the get_value() method so we can initialize the value_regex argument as a fixed string instead of a regex pattern.

---
 builtin/config.c  | 15 ++++++++++-----
 t/t1300-config.sh | 24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 5 deletions(-)

index 6ab5a0d68f..1d5cfab8f1 100644
@@ -14,6 +14,7 @@ static const char *const builtin_config_usage[] = {
 
 static char *key;
 static regex_t *key_regexp;
+static const char *value_regex;
 static regex_t *regexp;
 static int show_keys;
 static int omit_values;

@@ -298,6 +299,8 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 		return 0;
 	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
 		return 0;
+	if (literal && strcmp(value_regex, (value_?value_:"")))
+		return 0;
 	if (regexp != NULL &&
 	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
 		return 0;

@@ -308,7 +311,7 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 	return format_config(&values->items[values->nr++], key_, value_);
 }
 
-static int get_value(const char *key_, const char *regex_)
+static int get_value(const char *key_, const char *regex_, int flags)
 {
 	int ret = CONFIG_GENERIC_ERROR;
 	struct strbuf_list values = {NULL};

@@ -345,7 +348,9 @@ static int get_value(const char *key_, const char *regex_)
 		}
 	}
 
-	if (regex_) {
+	if (regex_ && (flags & CONFIG_FLAGS_LITERAL_VALUE))
+		value_regex = regex_;
+	else if (regex_) {
 		if (regex_[0] == '!') {
 			do_not_match = 1;
 			regex_++;

@@ -850,19 +855,19 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 	else if (actions == ACTION_GET) {
 		check_argc(argc, 1, 2);
-		return get_value(argv[0], argv[1]);
+		return get_value(argv[0], argv[1], flags);
 	}
 	else if (actions == ACTION_GET_ALL) {
 		do_all = 1;
 		check_argc(argc, 1, 2);
-		return get_value(argv[0], argv[1]);
+		return get_value(argv[0], argv[1], flags);
 	}
 	else if (actions == ACTION_GET_REGEXP) {
 		show_keys = 1;
 		use_key_regexp = 1;
 		do_all = 1;
 		check_argc(argc, 1, 2);
-		return get_value(argv[0], argv[1]);
+		return get_value(argv[0], argv[1], flags);
 	}
 	else if (actions == ACTION_GET_URLMATCH) {
 		check_argc(argc, 2, 2);

index ed12b4f7a0..8531d01c7c 100755
@@ -2026,4 +2026,28 @@ test_expect_success '--literal-value uses exact string matching' '
 	test_cmp_config bogus literal.test
 '
 
+test_expect_success '--get and --get-all with --literal-value' '
+	GLOB="a+b*c?d[e]f.g" &&
+	q_to_tab >.git/config <<-EOF &&
+	[literal]
+	Qtest = bogus
+	Qtest = $GLOB
+	EOF
+
+	git config --get literal.test bogus &&
+	test_must_fail git config --get literal.test "$GLOB" &&
+	git config --get --literal-value literal.test "$GLOB" &&
+	test_must_fail git config --get --literal-value literal.test non-existent &&
+
+	git config --get-all literal.test bogus &&
+	test_must_fail git config --get-all literal.test "$GLOB" &&
+	git config --get-all --literal-value literal.test "$GLOB" &&
+	test_must_fail git config --get-all --literal-value literal.test non-existent &&
+
+	git config --get-regexp literal+ bogus &&
+	test_must_fail git config  --get-regexp literal+ "$GLOB" &&
+	git config --get-regexp --literal-value literal+ "$GLOB" &&
+	test_must_fail git config --get-regexp --literal-value literal+ non-existent
+'
+
 test_done

gitgitgadget

From: Derrick Stolee <[email protected]>

When a repository's leading directories contain regex glob characters, the config calls for 'git maintenance register' and 'git maintenance unregister' are not careful enough. Use the new --literal-value option to direct the config machinery to use exact string matches. This is a more robust option than excaping these arguments in a piecemeal fashion.

---
 builtin/gc.c           |  5 +++--
 t/t7900-maintenance.sh | 12 ++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

index e3098ef6a1..0784bbdc6a 100644
@@ -1452,7 +1452,8 @@ static int maintenance_register(void)
 		git_config_set("maintenance.strategy", "incremental");
 
 	config_get.git_cmd = 1;
-	strvec_pushl(&config_get.args, "config", "--global", "--get", "maintenance.repo",
+	strvec_pushl(&config_get.args, "config", "--global", "--get",
+		     "--literal-value", "maintenance.repo",
 		     the_repository->worktree ? the_repository->worktree
 					      : the_repository->gitdir,
 			 NULL);

@@ -1483,7 +1484,7 @@ static int maintenance_unregister(void)
 
 	config_unset.git_cmd = 1;
 	strvec_pushl(&config_unset.args, "config", "--global", "--unset",
-		     "maintenance.repo",
+		     "--literal-value", "maintenance.repo",
 		     the_repository->worktree ? the_repository->worktree
 					      : the_repository->gitdir,
 		     NULL);

index 20184e96e1..2ee5512158 100755
@@ -367,6 +367,18 @@ test_expect_success 'register and unregister' '
 	test_cmp before actual
 '
 
+test_expect_success 'register and unregister with glob characters' '
+	GLOB="a+b*c" &&
+	git init "$GLOB" &&
+	git -C "$GLOB" maintenance register &&
+	git config --get-all --show-origin maintenance.repo &&
+	git config --get-all --global --literal-value \
+		maintenance.repo "$(pwd)/$GLOB" &&
+	git -C "$GLOB" maintenance unregister &&
+	test_must_fail git config --get-all --global --literal-value \
+		maintenance.repo "$(pwd)/$GLOB"
+'
+
 test_expect_success 'start from empty cron table' '
 	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
 

gitgitgadget

Just as bikeshedding on the name: Did you consider something thematically similar to the corresponding git-grep option, i.e. --fixed-string[s]. I see -F is also free in git-config(1).

I definitely wanted to be specific about "value" in the name, since some options include regexes on the key as well. I'm open to new ideas, and combining your idea with mine would introduce "--fixed-value". Thoughts?

Thanks,
-Stolee

I very much appreciate "value" is in the name, with the current semantics that this only controls how the pattern matching is done on the value side and not on the key side. When making an obvious addition of a separate option to control how the pattern matching is done on keys in the future, we would regret if we called this option "--fixed-strings" today. And no, I do not think it is an acceptable option to introduce "--fixed-strings" that only affects value side and then later change its behaviour to affect also on the key side.

Side note. It _is_ possible to ship such a "--fixed-strings" option that does not work on the key side and document it as a known bug, later to be fixed. I am not sure if I like it.

But stepping back a bit, is the extra flexibility that allows us to control the matching on keys and values separately with such a scheme really worth the complexity (at the end-user facing interface level, not the implementation level)?

So an alternative may be to use a single option, whose name would probably be one of "--(literal|fixed)-(match|strings)", but extend the implementation in this series to make the single option affect both the value and key matching the same way.

That would however be more work in the shorter term. Offhand, I am not sure if I like it (i.e. spending time and effort that is more than the absolute minimum necessary to fix a breakage. And the end result of doing so is less powerful/flexible, even though it may be easier to explain to users simply because the feature is less powerful than it could be). It would be easier, if I can convince myself that the extra flexibility is not worth it, to just declare that simpler is better here, but I am not quite ready to do so yet.

As to "-F", I do not think it is a good idea; in some context "-F" means work on a <file> given via that option, i.e. "-F <file>".

Thanks.

I had not thought about making this option be related to the key names at all. In particular, we already have the --get-regexp option:

--get-regexp::
Like --get-all, but interprets the name as a regular expression and writes out the key names. Regular expression matching is currently case-sensitive and done against a canonicalized version of the key in which section and variable names are lowercased, but subsection names are not.

I suppose that there could be reason to create a similar --unset-regexp that is the same equivalent of --unset-all. Perhaps the --replace-all mode could benefit, too? These are also more dangerous to use with regex matches since they also _change_ the config, not just query it.

Since there is already asymmetry between the key and value (only one mode, --get-regexp, has a "name_regex"), I would prefer to treat the value_regex in isolation here.

Thanks,
-Stolee

Ah, I forgot about --get-regexp vs --get; so the ship has already sailed---I agree with you that --(fixed|literal)-value is all we need.

Thanks.

Hi. I see you've already sent a v2 with that. Looks good to me. FWIW I didn't feel strongly one way or the other about it, either would be a fine choice by someone who's eyeballing this more than my cursory glance.

I just wanted to point out the grep option in case you'd missed it, since we've got a bazillion CLI options all over the place, and sometimes there's an existing option that's thematically similar to a proposed new one that's easy to miss. So we can re-use (or partially re-use) the name, behavior, or not.

As reported [1], 'git maintenance unregister' fails when a repository is located in a directory with regex glob characters.

[1]
https://lore.kernel.org/git/[email protected]/T/#mb96fa4187a0d6aeda097cd95804a8aafc0273022

The discussed solution was to modify 'git config' to specify that the 'value_regex' argument should be treated as an exact string match. This is the primary change in this series, with an additional patch at the end to make 'git maintenance [un]register' use this option, when necessary.

Updates in V2
=============

* Updated the test structure for the new tests in t1300-config.sh

* Renamed option from '--literal-value' to '--fixed-value'

* Use 'unsigned flags' instead of 'int flags'. This changes more prototypes than before.

* Move flag definitions closer to method declarations.

* Test the uses of --fixed-value to reject when value_regex is missing.

Thanks, -Stolee

Derrick Stolee (7):
t1300: test "set all" mode with value_regex
t1300: add test for --replace-all with value_regex
config: convert multi_replace to flags
config: add --fixed-value option, un-implemented
config: plumb --fixed-value into config API
config: implement --fixed-value with --get*
maintenance: use 'git config --fixed-value'

 Documentation/git-config.txt |  20 +++--
 builtin/branch.c             |   4 +-
 builtin/config.c             |  59 +++++++++++---
 builtin/gc.c                 |   5 +-
 builtin/remote.c             |   8 +-
 config.c                     |  29 ++++---
 config.h                     |  36 +++++++--
 t/t1300-config.sh            | 148 +++++++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh       |  12 +++
 9 files changed, 278 insertions(+), 43 deletions(-)

base-commit: 0016b618182f642771dc589cf0090289f9fe1b4f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-796%2Fderrickstolee%2Fmaintenance%2Fconfig-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-796/derrickstolee/maintenance/config-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/796

Range-diff vs v1:

1: 2da2131114 ! 1: ea3099719c t1300: test "set all" mode with value_regex

     @@ Commit message
             'key=value' pair is appended, making this 'key' a multi-valued
             config setting.
      
     -    Add a test that demonstrates these options.
     +    Add a test that demonstrates these options. Break from the existing
     +    pattern in t1300-config.sh to use 'git config --file=<file>' instead of
     +    modifying .git/config directly. Also use 'git config --file=<file>
     +    --list' for config state comparison instead of the config file format.
      
          Signed-off-by: Derrick Stolee <[email protected]>
      
     @@ t/t1300-config.sh: test_expect_success '--replace-all does not invent newlines'
       '
       
      +test_expect_success 'set all config with value_regex' '
     -+	q_to_tab >initial <<-\EOF &&
     -+	[abc]
     -+	Qkey = one
     -+	EOF
     ++	git config --file=initial abc.key one &&
      +
     -+	cp initial .git/config &&
     -+	git config abc.key two a+ &&
     -+	q_to_tab >expect <<-\EOF &&
     -+	[abc]
     -+	Qkey = one
     -+	Qkey = two
     ++	cp initial config &&
     ++	git config --file=config abc.key two a+ &&
     ++	git config --file=config --list >actual &&
     ++	cat >expect <<-\EOF &&
     ++	abc.key=one
     ++	abc.key=two
      +	EOF
     -+	test_cmp expect .git/config &&
     ++	test_cmp expect actual &&
      +
     -+	test_must_fail git config abc.key three o+ 2>err &&
     ++	test_must_fail git config --file=config abc.key three o+ 2>err &&
      +	test_i18ngrep "has multiple values" err &&
     -+	git config abc.key three a+ &&
     -+	q_to_tab >expect <<-\EOF &&
     -+	[abc]
     -+	Qkey = one
     -+	Qkey = two
     -+	Qkey = three
     ++	git config --file=config abc.key three a+ &&
     ++	git config --file=config --list >actual &&
     ++	cat >expect <<-\EOF &&
     ++	abc.key=one
     ++	abc.key=two
     ++	abc.key=three
      +	EOF
     -+	test_cmp expect .git/config &&
     ++	test_cmp expect actual &&
      +
     -+	cp initial .git/config &&
     -+	git config abc.key three o+ &&
     -+	q_to_tab >expect <<-\EOF &&
     -+	[abc]
     -+	Qkey = three
     ++	cp initial config &&
     ++	git config --file=config abc.key three o+ &&
     ++	git config --file=config --list >actual &&
     ++	cat >expect <<-\EOF &&
     ++	abc.key=three
      +	EOF
     -+	test_cmp expect .git/config
     ++	test_cmp expect actual
      +'
      +
       test_done
 2:  1237289706 ! 2:  829d0ccd8c t1300: add test for --replace-all with value_regex
     @@ Commit message
      
       ## t/t1300-config.sh ##
      @@ t/t1300-config.sh: test_expect_success 'set all config with value_regex' '
     - 	test_cmp expect .git/config
     + 	test_cmp expect actual
       '
       
      +test_expect_success '--replace-all and value_regex' '
     -+	q_to_tab >.git/config <<-\EOF &&
     -+	[abc]
     -+	Qkey = one
     -+	Qkey = two
     -+	Qkey = three
     ++	rm -f config &&
     ++	git config --file=config --add abc.key one &&
     ++	git config --file=config --add abc.key two &&
     ++	git config --file=config --add abc.key three &&
     ++	git config --file=config --replace-all abc.key four "o+" &&
     ++	git config --file=config --list >actual &&
     ++	cat >expect <<-\EOF &&
     ++	abc.key=four
     ++	abc.key=three
      +	EOF
     -+	q_to_tab >expect <<-\EOF &&
     -+	[abc]
     -+	Qkey = four
     -+	Qkey = three
     -+	EOF
     -+	git config --replace-all abc.key four "o+" &&
     -+	test_cmp expect .git/config
     ++	test_cmp expect actual
      +'
      +
       test_done

3: c9caed3854 ! 3: 0c152faa00 config: convert multi_replace to flags
@@ Commit message

We will extend the flexibility of the config API. Before doing so, let's take an existing 'int multi_replace' parameter and replace it with a new
- 'int flags' parameter that can take multiple options as a bit field.
+ 'unsigned flags' parameter that can take multiple options as a bit field.

Update all callers that specified multi_replace to now specify the CONFIG_FLAGS_MULTI_REPLACE flag. To add more clarity, extend the documentation of git_config_set_multivar_in_file() including a clear
- labeling of its arguments. Other config API methods in config.h do not
- require a change to their prototypes, but they have an equivalent update
- to their implementations.
+ labeling of its arguments. Other config API methods in config.h require
+ only a change of the final parameter from 'int' to 'unsigned'.

@@ config.c: void git_config_set(const char *key, const char *value)
const char *key, const char *value, const char *value_regex,
- int multi_replace)
-+ int flags)
++ unsigned flags)
{
int fd = -1, in_fd = -1;
int ret;
@@ config.c: int git_config_set_multivar_in_file_gently(const char *config_filename
void git_config_set_multivar_in_file(const char *config_filename,
const char *key, const char *value,
- const char *value_regex, int multi_replace)
-+ const char *value_regex, int flags)
++ const char *value_regex, unsigned flags)
{
if (!git_config_set_multivar_in_file_gently(config_filename, key, value,
- value_regex, multi_replace))
@@ config.c: void git_config_set_multivar_in_file(const char *config_filename,

int git_config_set_multivar_gently(const char *key, const char *value,
- const char *value_regex, int multi_replace)
-+ const char *value_regex, int flags)
++ const char *value_regex, unsigned flags)
{
return git_config_set_multivar_in_file_gently(NULL, key, value, value_regex,
- multi_replace);
@@ config.c: void git_config_set_multivar_in_file(const char *config_filename,

void git_config_set_multivar(const char *key, const char *value,
- const char *value_regex, int multi_replace)
-+ const char *value_regex, int flags)
++ const char *value_regex, unsigned flags)
{
git_config_set_multivar_in_file(NULL, key, value, value_regex,
- multi_replace);
@@ config.c: void git_config_set_multivar_in_file(const char *config_filename,
static int section_name_match (const char *buf, const char *name)

## config.h ##
-@@ config.h: enum config_event_t {
- CONFIG_EVENT_ERROR
- };
+@@ config.h: void git_config_set(const char *, const char *);

+ int git_config_parse_key(const char *, char **, size_t *);
+ int git_config_key_is_valid(const char *key);
+-int git_config_set_multivar_gently(const char *, const char *, const char *, int);
+-void git_config_set_multivar(const char *, const char *, const char *, int);
+-int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, int); ++
++/*
++ * The following macros specify flag bits that alter the behavior
++ * of the git_config_set_multivar*() methods.
++ */
++
+/*
+ * When CONFIG_FLAGS_MULTI_REPLACE is specified, all matching key/values
+ * are removed before a new pair is written. If the flag is not present,
@@ config.h: enum config_event_t {
+ */
+#define CONFIG_FLAGS_MULTI_REPLACE (1 << 0)
+
- /*
- * The parser event function (if not NULL) is called with the event type and
- * the begin/end offsets of the parsed elements.
++int git_config_set_multivar_gently(const char *, const char *, const char *, unsigned); ++void git_config_set_multivar(const char *, const char *, const char *, unsigned);
++int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, unsigned); +
+ /**
+ * takes four parameters:
@@ config.h: int git_config_set_multivar_in_file_gently(const char *, const char *, const cha
* - the value regex, as a string. It will disregard key/value pairs where value
* does not match.
@@ config.h: int git_config_set_multivar_in_file_gently(const char *, const char *,
+ const char *key,
+ const char *value,
+ const char *value_regex,
-+ int flags);
++ unsigned flags);

/**
* rename or remove sections in the config file
4: 28493184b6 ! 4: 0e6a7371ed config: add --literal-value option, un-implemented

     @@ Metadata
      Author: Derrick Stolee <[email protected]>
      
       ## Commit message ##
     -    config: add --literal-value option, un-implemented
     +    config: add --fixed-value option, un-implemented
      
          The 'git config' builtin takes a 'value_regex' parameter for several
          actions. This can cause confusion when expecting exact value matches
     @@ Commit message
          be more friendly to allow an argument to disable the pattern matching in
          favor of an exact string match.
      
     -    Add a new '--literal-value' option that does not currently change the
     +    Add a new '--fixed-value' option that does not currently change the
          behavior. The implementation will follow for each appropriate action.
     -    For now, check and test that --literal-value will abort the command when
     -    included with an incompatible action.
     +    For now, check and test that --fixed-value will abort the command when
     +    included with an incompatible action or without a 'value_regex'
     +    argument.
      
     -    The name '--literal-value' was chosen over something simpler like
     -    '--literal' because some commands allow regular expressions on the
     +    The name '--fixed-value' was chosen over something simpler like
     +    '--fixed' because some commands allow regular expressions on the
          key in addition to the value.
      
          Signed-off-by: Derrick Stolee <[email protected]>
     @@ Documentation/git-config.txt: git-config - Get and set repository or global opti
       --------
       [verse]
      -'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] name [value [value_regex]]
     -+'git config' [<file-option>] [--type=<type>] [--literal-value] [--show-origin] [--show-scope] [-z|--null] name [value [value_regex]]
     ++'git config' [<file-option>] [--type=<type>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] name [value [value_regex]]
       'git config' [<file-option>] [--type=<type>] --add name value
      -'git config' [<file-option>] [--type=<type>] --replace-all name value [value_regex]
      -'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get name [value_regex]
      -'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get-all name [value_regex]
      -'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]
     -+'git config' [<file-option>] [--type=<type>] [--literal-value] --replace-all name value [value_regex]
     -+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--literal-value] --get name [value_regex]
     -+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--literal-value] --get-all name [value_regex]
     -+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--literal-value] [--name-only] --get-regexp name_regex [value_regex]
     ++'git config' [<file-option>] [--type=<type>] [--fixed-value] --replace-all name value [value_regex]
     ++'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get name [value_regex]
     ++'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get-all name [value_regex]
     ++'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] [--name-only] --get-regexp name_regex [value_regex]
       'git config' [<file-option>] [--type=<type>] [-z|--null] --get-urlmatch name URL
      -'git config' [<file-option>] --unset name [value_regex]
      -'git config' [<file-option>] --unset-all name [value_regex]
     -+'git config' [<file-option>] [--literal-value] --unset name [value_regex]
     -+'git config' [<file-option>] [--literal-value] --unset-all name [value_regex]
     ++'git config' [<file-option>] [--fixed-value] --unset name [value_regex]
     ++'git config' [<file-option>] [--fixed-value] --unset-all name [value_regex]
       'git config' [<file-option>] --rename-section old_name new_name
       'git config' [<file-option>] --remove-section name
       'git config' [<file-option>] [--show-origin] [--show-scope] [-z|--null] [--name-only] -l | --list
     @@ Documentation/git-config.txt: See also <<FILES>>.
       --list::
       	List all variables set in config file, along with their values.
       
     -+--literal-value::
     ++--fixed-value::
      +	When used with the `value_regex` argument, treat `value_regex` as
      +	an exact string instead of a regular expression. This will restrict
      +	the name/value pairs that are matched to only those where the value
     @@ builtin/config.c: static int respect_includes_opt = -1;
       static struct config_options config_options;
       static int show_origin;
       static int show_scope;
     -+static int literal;
     ++static int fixed_value;
       
       #define ACTION_GET (1<<0)
       #define ACTION_GET_ALL (1<<1)
     -@@ builtin/config.c: static int show_scope;
     - #define ACTION_GET_COLORBOOL (1<<14)
     - #define ACTION_GET_URLMATCH (1<<15)
     - 
     -+#define ACTION_LITERAL_ALLOWED (\
     -+	ACTION_GET |\
     -+	ACTION_GET_ALL |\
     -+	ACTION_GET_REGEXP |\
     -+	ACTION_REPLACE_ALL |\
     -+	ACTION_UNSET |\
     -+	ACTION_UNSET_ALL |\
     -+	ACTION_SET_ALL\
     -+)
     -+
     - /*
     -  * The actions "ACTION_LIST | ACTION_GET_*" which may produce more than
     -  * one line of output and which should therefore be paged.
      @@ builtin/config.c: static struct option builtin_config_options[] = {
       	OPT_BIT(0, "rename-section", &actions, N_("rename section: old-name new-name"), ACTION_RENAME_SECTION),
       	OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"), ACTION_REMOVE_SECTION),
       	OPT_BIT('l', "list", &actions, N_("list all"), ACTION_LIST),
     -+	OPT_BOOL(0, "literal-value", &literal, N_("use literal equality when matching values")),
     ++	OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when matching values")),
       	OPT_BIT('e', "edit", &actions, N_("open an editor"), ACTION_EDIT),
       	OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR),
       	OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL),
     @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix
       		usage_builtin_config();
       	}
       
     -+	if (literal && !(actions & ACTION_LITERAL_ALLOWED)) {
     -+		error(_("--literal only applies with 'value_regex'"));
     -+		usage_builtin_config();
     ++	if (fixed_value) {
     ++		int allowed_usage = 0;
     ++
     ++		switch (actions) {
     ++		case ACTION_GET:
     ++		case ACTION_GET_ALL:
     ++		case ACTION_GET_REGEXP:
     ++		case ACTION_UNSET:
     ++		case ACTION_UNSET_ALL:
     ++			allowed_usage = argc > 1 && !!argv[1];
     ++			break;
     ++
     ++		case ACTION_SET_ALL:
     ++		case ACTION_REPLACE_ALL:
     ++			allowed_usage = argc > 2 && !!argv[2];
     ++			break;
     ++		}
     ++
     ++		if (!allowed_usage) {
     ++			error(_("--fixed-value only applies with 'value_regex'"));
     ++			usage_builtin_config();
     ++		}
      +	}
      +
       	if (actions & PAGING_ACTIONS)
     @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix
      
       ## t/t1300-config.sh ##
      @@ t/t1300-config.sh: test_expect_success '--replace-all and value_regex' '
     - 	test_cmp expect .git/config
     + 	test_cmp expect actual
       '
       
     -+test_expect_success 'refuse --literal-value for incompatible actions' '
     -+	git config dev.null bogus &&
     -+	test_must_fail git config --literal-value --add dev.null bogus &&
     -+	test_must_fail git config --literal-value --get-urlmatch dev.null bogus &&
     -+	test_must_fail git config --literal-value --get-urlmatch dev.null bogus &&
     -+	test_must_fail git config --literal-value --rename-section dev null &&
     -+	test_must_fail git config --literal-value --remove-section dev &&
     -+	test_must_fail git config --literal-value --list &&
     -+	test_must_fail git config --literal-value --get-color dev.null &&
     -+	test_must_fail git config --literal-value --get-colorbool dev.null &&
     -+	test_must_fail git config --literal-value --edit
     ++test_expect_success 'refuse --fixed-value for incompatible actions' '
     ++	git config --file=config dev.null bogus &&
     ++
     ++	# These modes do not allow --fixed-value at all
     ++	test_must_fail git config --file=config --fixed-value --add dev.null bogus &&
     ++	test_must_fail git config --file=config --fixed-value --get-urlmatch dev.null bogus &&
     ++	test_must_fail git config --file=config --fixed-value --get-urlmatch dev.null bogus &&
     ++	test_must_fail git config --file=config --fixed-value --rename-section dev null &&
     ++	test_must_fail git config --file=config --fixed-value --remove-section dev &&
     ++	test_must_fail git config --file=config --fixed-value --list &&
     ++	test_must_fail git config --file=config --fixed-value --get-color dev.null &&
     ++	test_must_fail git config --file=config --fixed-value --get-colorbool dev.null &&
     ++
     ++	# These modes complain when --fixed-value has no value_regex
     ++	test_must_fail git config --file=config --fixed-value dev.null bogus &&
     ++	test_must_fail git config --file=config --fixed-value --replace-all dev.null bogus &&
     ++	test_must_fail git config --file=config --fixed-value --get dev.null &&
     ++	test_must_fail git config --file=config --fixed-value --get-all dev.null &&
     ++	test_must_fail git config --file=config --fixed-value --get-regexp "dev.*" &&
     ++	test_must_fail git config --file=config --fixed-value --unset dev.null &&
     ++	test_must_fail git config --file=config --fixed-value --unset-all dev.null
      +'
      +
       test_done
 5:  5881b2d987 ! 5:  39718048cd config: plumb --literal-value into config API
     @@ Metadata
      Author: Derrick Stolee <[email protected]>
      
       ## Commit message ##
     -    config: plumb --literal-value into config API
     +    config: plumb --fixed-value into config API
      
          The git_config_set_multivar_in_file_gently() and related methods now
     -    take a 'flags' bitfield, so add a new bit representing the
     -    --literal-value option from 'git config'. This alters the purpose of the
     -    value_regex parameter to be an exact string match. This requires some
     -    initialization changes in git_config_set_multivar_in_file_gently() and a
     -    new strcmp() call in the matches() method.
     +    take a 'flags' bitfield, so add a new bit representing the --fixed-value
     +    option from 'git config'. This alters the purpose of the value_regex
     +    parameter to be an exact string match. This requires some initialization
     +    changes in git_config_set_multivar_in_file_gently() and a new strcmp()
     +    call in the matches() method.
      
     -    The new CONFIG_FLAGS_LITERAL_VALUE flag is initialized in
     -    builtin/config.c based on the --literal-value option, and that needs to
     -    be updated in several callers.
     +    The new CONFIG_FLAGS_FIXED_VALUE flag is initialized in builtin/config.c
     +    based on the --fixed-value option, and that needs to be updated in
     +    several callers.
      
          This patch only affects some of the modes of 'git config', and the rest
          will be completed in the next change.
     @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix
       	given_config_source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT));
       
      @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)
     - 		error(_("--literal only applies with 'value_regex'"));
     - 		usage_builtin_config();
     + 			error(_("--fixed-value only applies with 'value_regex'"));
     + 			usage_builtin_config();
     + 		}
     ++
     ++		flags = CONFIG_FLAGS_FIXED_VALUE;
       	}
     -+	if (literal)
     -+		flags = CONFIG_FLAGS_LITERAL_VALUE;
       
       	if (actions & PAGING_ACTIONS)
     - 		setup_auto_pager("config", 1);
      @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)
       		value = normalize_value(argv[0], argv[1]);
       		UNLEAK(value);
     @@ config.c: int git_config_set_multivar_in_file_gently(const char *config_filename
       			store.value_regex = NULL;
       		else if (value_regex == CONFIG_REGEX_NONE)
       			store.value_regex = CONFIG_REGEX_NONE;
     -+		else if (flags & CONFIG_FLAGS_LITERAL_VALUE)
     ++		else if (flags & CONFIG_FLAGS_FIXED_VALUE)
      +			store.literal_value = value_regex;
       		else {
       			if (value_regex[0] == '!') {
       				store.do_not_match = 1;
      
       ## config.h ##
     -@@ config.h: enum config_event_t {
     +@@ config.h: int git_config_key_is_valid(const char *key);
        */
       #define CONFIG_FLAGS_MULTI_REPLACE (1 << 0)
       
      +/*
     -+ * When CONFIG_FLAGS_LITERAL_VALUE is specified, match key/value pairs
     ++ * When CONFIG_FLAGS_FIXED_VALUE is specified, match key/value pairs
      + * by string comparison (not regex match) to the provided value_regex
      + * parameter.
      + */
     -+#define CONFIG_FLAGS_LITERAL_VALUE (1 << 1)
     ++#define CONFIG_FLAGS_FIXED_VALUE (1 << 1)
      +
     - /*
     -  * The parser event function (if not NULL) is called with the event type and
     -  * the begin/end offsets of the parsed elements.
     + int git_config_set_multivar_gently(const char *, const char *, const char *, unsigned);
     + void git_config_set_multivar(const char *, const char *, const char *, unsigned);
     + int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, unsigned);
      
       ## t/t1300-config.sh ##
     -@@ t/t1300-config.sh: test_expect_success 'refuse --literal-value for incompatible actions' '
     - 	test_must_fail git config --literal-value --edit
     +@@ t/t1300-config.sh: test_expect_success 'refuse --fixed-value for incompatible actions' '
     + 	test_must_fail git config --file=config --fixed-value --unset-all dev.null
       '
       
     -+test_expect_success '--literal-value uses exact string matching' '
     ++test_expect_success '--fixed-value uses exact string matching' '
      +	GLOB="a+b*c?d[e]f.g" &&
     -+	q_to_tab >initial <<-EOF &&
     -+	[literal]
     -+	Qtest = $GLOB
     -+	EOF
     ++	rm -f initial &&
     ++	git config --file=initial fixed.test "$GLOB" &&
      +
     -+	cp initial .git/config &&
     -+	git config literal.test bogus "$GLOB" &&
     -+	q_to_tab >expect <<-EOF &&
     -+	[literal]
     -+	Qtest = $GLOB
     -+	Qtest = bogus
     ++	cp initial config &&
     ++	git config --file=config fixed.test bogus "$GLOB" &&
     ++	git config --file=config --list >actual &&
     ++	cat >expect <<-EOF &&
     ++	fixed.test=$GLOB
     ++	fixed.test=bogus
      +	EOF
     -+	test_cmp expect .git/config &&
     -+	cp initial .git/config &&
     -+	git config --literal-value literal.test bogus "$GLOB" &&
     -+	q_to_tab >expect <<-EOF &&
     -+	[literal]
     -+	Qtest = bogus
     -+	EOF
     -+	test_cmp expect .git/config &&
     ++	test_cmp expect actual &&
     ++
     ++	cp initial config &&
     ++	git config --file=config --fixed-value fixed.test bogus "$GLOB" &&
     ++	git config --file=config --list >actual &&
     ++	printf "fixed.test=bogus\n" >expect &&
     ++	test_cmp expect actual &&
      +
     -+	cp initial .git/config &&
     -+	test_must_fail git config --unset literal.test "$GLOB" &&
     -+	git config --literal-value --unset literal.test "$GLOB" &&
     -+	test_must_fail git config literal.test &&
     ++	cp initial config &&
     ++	test_must_fail git config --file=config --unset fixed.test "$GLOB" &&
     ++	git config --file=config --fixed-value --unset fixed.test "$GLOB" &&
     ++	test_must_fail git config --file=config fixed.test &&
      +
     -+	cp initial .git/config &&
     -+	test_must_fail git config --unset-all literal.test "$GLOB" &&
     -+	git config --literal-value --unset-all literal.test "$GLOB" &&
     -+	test_must_fail git config literal.test &&
     ++	cp initial config &&
     ++	test_must_fail git config --file=config --unset-all fixed.test "$GLOB" &&
     ++	git config --file=config --fixed-value --unset-all fixed.test "$GLOB" &&
     ++	test_must_fail git config --file=config fixed.test &&
      +
     -+	cp initial .git/config &&
     -+	git config --replace-all literal.test bogus "$GLOB" &&
     -+	q_to_tab >expect <<-EOF &&
     -+	[literal]
     -+	Qtest = $GLOB
     -+	Qtest = bogus
     ++	cp initial config &&
     ++	git config --file=config --replace-all fixed.test bogus "$GLOB" &&
     ++	git config --file=config --list >actual &&
     ++	cat >expect <<-EOF &&
     ++	fixed.test=$GLOB
     ++	fixed.test=bogus
      +	EOF
     -+	test_cmp expect .git/config &&
     ++	test_cmp expect actual &&
      +
     -+	cp initial .git/config &&
     -+	git config --replace-all literal.test bogus "$GLOB" &&
     -+	git config --literal-value --replace-all literal.test bogus "$GLOB" &&
     -+	test_cmp_config bogus literal.test
     ++	cp initial config &&
     ++	git config --file=config --replace-all fixed.test bogus "$GLOB" &&
     ++	git config --file=config --list >actual &&
     ++	cat >expect <<-EOF &&
     ++	fixed.test=$GLOB
     ++	fixed.test=bogus
     ++	EOF
     ++	test_cmp expect actual &&
     ++
     ++	git config --file=config --fixed-value --replace-all fixed.test bogus "$GLOB" &&
     ++	git config --file=config --list >actual &&
     ++	cat >expect <<-EOF &&
     ++	fixed.test=bogus
     ++	fixed.test=bogus
     ++	EOF
     ++	test_cmp expect actual
      +'
      +
       test_done
 6:  7c8e3769c7 ! 6:  8e0111c7b4 config: implement --literal-value with --get*
     @@ Metadata
      Author: Derrick Stolee <[email protected]>
      
       ## Commit message ##
     -    config: implement --literal-value with --get*
     +    config: implement --fixed-value with --get*
      
          The config builtin does its own regex matching of values for the --get,
          --get-all, and --get-regexp modes. Plumb the existing 'flags' parameter
     @@ builtin/config.c: static int collect_config(const char *key_, const char *value_
       		return 0;
       	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
       		return 0;
     -+	if (literal && strcmp(value_regex, (value_?value_:"")))
     ++	if (fixed_value && strcmp(value_regex, (value_?value_:"")))
      +		return 0;
       	if (regexp != NULL &&
       	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
     @@ builtin/config.c: static int get_value(const char *key_, const char *regex_)
       	}
       
      -	if (regex_) {
     -+	if (regex_ && (flags & CONFIG_FLAGS_LITERAL_VALUE))
     ++	if (regex_ && (flags & CONFIG_FLAGS_FIXED_VALUE))
      +		value_regex = regex_;
      +	else if (regex_) {
       		if (regex_[0] == '!') {
     @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix
       		check_argc(argc, 2, 2);
      
       ## t/t1300-config.sh ##
     -@@ t/t1300-config.sh: test_expect_success '--literal-value uses exact string matching' '
     - 	test_cmp_config bogus literal.test
     +@@ t/t1300-config.sh: test_expect_success '--fixed-value uses exact string matching' '
     + 	test_cmp expect actual
       '
       
     -+test_expect_success '--get and --get-all with --literal-value' '
     ++test_expect_success '--get and --get-all with --fixed-value' '
      +	GLOB="a+b*c?d[e]f.g" &&
     -+	q_to_tab >.git/config <<-EOF &&
     -+	[literal]
     -+	Qtest = bogus
     -+	Qtest = $GLOB
     -+	EOF
     ++	rm -f config &&
     ++	git config --file=config fixed.test bogus &&
     ++	git config --file=config --add fixed.test "$GLOB" &&
      +
     -+	git config --get literal.test bogus &&
     -+	test_must_fail git config --get literal.test "$GLOB" &&
     -+	git config --get --literal-value literal.test "$GLOB" &&
     -+	test_must_fail git config --get --literal-value literal.test non-existent &&
     ++	git config --file=config --get fixed.test bogus &&
     ++	test_must_fail git config --file=config --get fixed.test "$GLOB" &&
     ++	git config --file=config --get --fixed-value fixed.test "$GLOB" &&
     ++	test_must_fail git config --file=config --get --fixed-value fixed.test non-existent &&
      +
     -+	git config --get-all literal.test bogus &&
     -+	test_must_fail git config --get-all literal.test "$GLOB" &&
     -+	git config --get-all --literal-value literal.test "$GLOB" &&
     -+	test_must_fail git config --get-all --literal-value literal.test non-existent &&
     ++	git config --file=config --get-all fixed.test bogus &&
     ++	test_must_fail git config --file=config --get-all fixed.test "$GLOB" &&
     ++	git config --file=config --get-all --fixed-value fixed.test "$GLOB" &&
     ++	test_must_fail git config --file=config --get-all --fixed-value fixed.test non-existent &&
      +
     -+	git config --get-regexp literal+ bogus &&
     -+	test_must_fail git config  --get-regexp literal+ "$GLOB" &&
     -+	git config --get-regexp --literal-value literal+ "$GLOB" &&
     -+	test_must_fail git config --get-regexp --literal-value literal+ non-existent
     ++	git config --file=config --get-regexp fixed+ bogus &&
     ++	test_must_fail git config --file=config --get-regexp fixed+ "$GLOB" &&
     ++	git config --file=config --get-regexp --fixed-value fixed+ "$GLOB" &&
     ++	test_must_fail git config --file=config --get-regexp --fixed-value fixed+ non-existent
      +'
      +
       test_done
 7:  1d1c2fd23e ! 7:  5a3acf8119 maintenance: use 'git config --literal-value'
     @@ Metadata
      Author: Derrick Stolee <[email protected]>
      
       ## Commit message ##
     -    maintenance: use 'git config --literal-value'
     +    maintenance: use 'git config --fixed-value'
      
          When a repository's leading directories contain regex glob characters,
          the config calls for 'git maintenance register' and 'git maintenance
     -    unregister' are not careful enough. Use the new --literal-value option
     +    unregister' are not careful enough. Use the new --fixed-value option
          to direct the config machinery to use exact string matches. This is a
          more robust option than excaping these arguments in a piecemeal fashion.
      
     +    For the test, require that we are not running on Windows since the '+'
     +    character is not allowed on that filesystem.
     +
          Reported-by: Emily Shaffer <[email protected]>
          Reported-by: Jonathan Nieder <[email protected]>
          Signed-off-by: Derrick Stolee <[email protected]>
     @@ builtin/gc.c: static int maintenance_register(void)
       	config_get.git_cmd = 1;
      -	strvec_pushl(&config_get.args, "config", "--global", "--get", "maintenance.repo",
      +	strvec_pushl(&config_get.args, "config", "--global", "--get",
     -+		     "--literal-value", "maintenance.repo",
     ++		     "--fixed-value", "maintenance.repo",
       		     the_repository->worktree ? the_repository->worktree
       					      : the_repository->gitdir,
       			 NULL);
     @@ builtin/gc.c: static int maintenance_unregister(void)
       	config_unset.git_cmd = 1;
       	strvec_pushl(&config_unset.args, "config", "--global", "--unset",
      -		     "maintenance.repo",
     -+		     "--literal-value", "maintenance.repo",
     ++		     "--fixed-value", "maintenance.repo",
       		     the_repository->worktree ? the_repository->worktree
       					      : the_repository->gitdir,
       		     NULL);
     @@ t/t7900-maintenance.sh: test_expect_success 'register and unregister' '
       	test_cmp before actual
       '
       
     -+test_expect_success 'register and unregister with glob characters' '
     ++test_expect_success !MINGW 'register and unregister with glob characters' '
      +	GLOB="a+b*c" &&
      +	git init "$GLOB" &&
      +	git -C "$GLOB" maintenance register &&
      +	git config --get-all --show-origin maintenance.repo &&
     -+	git config --get-all --global --literal-value \
     ++	git config --get-all --global --fixed-value \
      +		maintenance.repo "$(pwd)/$GLOB" &&
      +	git -C "$GLOB" maintenance unregister &&
     -+	test_must_fail git config --get-all --global --literal-value \
     ++	test_must_fail git config --get-all --global --fixed-value \
      +		maintenance.repo "$(pwd)/$GLOB"
      +'
      +

gitgitgadget

From: Derrick Stolee <[email protected]>

Without additional modifiers, 'git config' attempts to set a single value in the .git/config file. When the value_regex parameter is supplied, this command behaves in a non-trivial manner.

Consider 'git config key value value_regex'. The expected behavior is as follows:

1. If there are multiple existing values that match 'value_regex', then the command fails. Users should use --replace-all instead.

2. If there is one existing value that matches 'value_regex', then the new config has one entry where 'key=value'.

3. If there is no existing values match 'value_regex', then the 'key=value' pair is appended, making this 'key' a multi-valued config setting.

Add a test that demonstrates these options. Break from the existing pattern in t1300-config.sh to use 'git config --file=<file>' instead of modifying .git/config directly. Also use 'git config --file=<file> --list' for config state comparison instead of the config file format.

---
 t/t1300-config.sh | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

index 97ebfe1f9d..19836ec83b 100755
@@ -1914,4 +1914,36 @@ test_expect_success '--replace-all does not invent newlines' '
 	test_cmp expect .git/config
 '
 
+test_expect_success 'set all config with value_regex' '
+	git config --file=initial abc.key one &&
+
+	cp initial config &&
+	git config --file=config abc.key two a+ &&
+	git config --file=config --list >actual &&
+	cat >expect <<-\EOF &&
+	abc.key=one
+	abc.key=two
+	EOF
+	test_cmp expect actual &&
+
+	test_must_fail git config --file=config abc.key three o+ 2>err &&
+	test_i18ngrep "has multiple values" err &&
+	git config --file=config abc.key three a+ &&
+	git config --file=config --list >actual &&
+	cat >expect <<-\EOF &&
+	abc.key=one
+	abc.key=two
+	abc.key=three
+	EOF
+	test_cmp expect actual &&
+
+	cp initial config &&
+	git config --file=config abc.key three o+ &&
+	git config --file=config --list >actual &&
+	cat >expect <<-\EOF &&
+	abc.key=three
+	EOF
+	test_cmp expect actual
+'
+
 test_done

gitgitgadget

I'd normally say "but *why* do you want to do that?" but I think it's actually pretty clear in the diff based on the naming of the config files (and clever - I like it). I do think pointing out that you're breaking from existing style is useful since the existing style isn't contained in the diff alone.

From: Derrick Stolee <[email protected]>

The --replace-all option was added in 4ddba79d (git-config-set: add more options) but was not tested along with the 'value_regex' parameter. Since we will be updating this option to optionally treat 'value_regex' as a fixed string, let's add a test here that documents the current behavior.

---
 t/t1300-config.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

index 19836ec83b..8783767d4f 100755
@@ -1946,4 +1946,18 @@ test_expect_success 'set all config with value_regex' '
 	test_cmp expect actual
 '
 
+test_expect_success '--replace-all and value_regex' '
+	rm -f config &&
+	git config --file=config --add abc.key one &&
+	git config --file=config --add abc.key two &&
+	git config --file=config --add abc.key three &&
+	git config --file=config --replace-all abc.key four "o+" &&
+	git config --file=config --list >actual &&
+	cat >expect <<-\EOF &&
+	abc.key=four
+	abc.key=three
+	EOF
+	test_cmp expect actual
+'
+
 test_done

gitgitgadget

I like the commit message and the diff a lot. Concise and complete, explains when the problem came from. I don't think it even needs the rationale about adding more options later - there's missing test coverage and you're adding it. But it's useful to reviewers and a handy step on the journey of this topic.

From: Derrick Stolee <[email protected]>

We will extend the flexibility of the config API. Before doing so, let's take an existing 'int multi_replace' parameter and replace it with a new 'unsigned flags' parameter that can take multiple options as a bit field.

Update all callers that specified multi_replace to now specify the CONFIG_FLAGS_MULTI_REPLACE flag. To add more clarity, extend the documentation of git_config_set_multivar_in_file() including a clear labeling of its arguments. Other config API methods in config.h require only a change of the final parameter from 'int' to 'unsigned'.

---
 builtin/branch.c |  4 ++--
 builtin/config.c |  6 ++++--
 builtin/remote.c |  8 +++++---
 config.c         | 24 ++++++++++++------------
 config.h         | 29 ++++++++++++++++++++++-------
 5 files changed, 45 insertions(+), 26 deletions(-)

index e82301fb1b..5ce3844d22 100644
@@ -829,10 +829,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die(_("Branch '%s' has no upstream information"), branch->name);
 
 		strbuf_addf(&buf, "branch.%s.remote", branch->name);
-		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "branch.%s.merge", branch->name);
-		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 		strbuf_release(&buf);
 	} else if (argc > 0 && argc <= 2) {
 		if (filter.kind != FILTER_REFS_BRANCHES)

index 5e39f61885..e7c7f3d455 100644
@@ -823,7 +823,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		value = normalize_value(argv[0], argv[1]);
 		UNLEAK(value);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
-							      argv[0], value, argv[2], 1);
+							      argv[0], value, argv[2],
+							      CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_GET) {
 		check_argc(argc, 1, 2);

@@ -859,7 +860,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_write();
 		check_argc(argc, 1, 2);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
-							      argv[0], NULL, argv[1], 1);
+							      argv[0], NULL, argv[1],
+							      CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_RENAME_SECTION) {
 		int ret;

index c8240e9fcd..29b1652975 100644
@@ -708,7 +708,7 @@ static int mv(int argc, const char **argv)
 
 	strbuf_reset(&buf);
 	strbuf_addf(&buf, "remote.%s.fetch", rename.new_name);
-	git_config_set_multivar(buf.buf, NULL, NULL, 1);
+	git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 	strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old_name);
 	for (i = 0; i < oldremote->fetch.raw_nr; i++) {
 		char *ptr;

@@ -1485,7 +1485,8 @@ static int update(int argc, const char **argv)
 
 static int remove_all_fetch_refspecs(const char *key)
 {
-	return git_config_set_multivar_gently(key, NULL, NULL, 1);
+	return git_config_set_multivar_gently(key, NULL, NULL,
+					      CONFIG_FLAGS_MULTI_REPLACE);
 }
 
 static void add_branches(struct remote *remote, const char **branches,

@@ -1674,7 +1675,8 @@ static int set_url(int argc, const char **argv)
 	if (!delete_mode)
 		git_config_set_multivar(name_buf.buf, newurl, oldurl, 0);
 	else
-		git_config_set_multivar(name_buf.buf, NULL, oldurl, 1);
+		git_config_set_multivar(name_buf.buf, NULL, oldurl,
+					CONFIG_FLAGS_MULTI_REPLACE);
 out:
 	strbuf_release(&name_buf);
 	return 0;

index 2b79fe76ad..096f2eae0d 100644
@@ -2716,9 +2716,9 @@ void git_config_set(const char *key, const char *value)
  * if value_regex!=NULL, disregard key/value pairs where value does not match.
  * if value_regex==CONFIG_REGEX_NONE, do not match any existing values
  *     (only add a new one)
- * if multi_replace==0, nothing, or only one matching key/value is replaced,
- *     else all matching key/values (regardless how many) are removed,
- *     before the new pair is written.
+ * if (flags & CONFIG_FLAGS_MULTI_REPLACE) == 0, at most one matching
+ *     key/value is replaced, else all matching key/values (regardless
+ *     how many) are removed, before the new pair is written.
  *
  * Returns 0 on success.
  *

@@ -2739,7 +2739,7 @@ void git_config_set(const char *key, const char *value)
 int git_config_set_multivar_in_file_gently(const char *config_filename,
 					   const char *key, const char *value,
 					   const char *value_regex,
-					   int multi_replace)
+					   unsigned flags)
 {
 	int fd = -1, in_fd = -1;
 	int ret;

@@ -2756,7 +2756,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 	if (ret)
 		goto out_free;
 
-	store.multi_replace = multi_replace;
+	store.multi_replace = (flags & CONFIG_FLAGS_MULTI_REPLACE) != 0;
 
 	if (!config_filename)
 		config_filename = filename_buf = git_pathdup("config");

@@ -2845,7 +2845,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 
 		/* if nothing to unset, or too many matches, error out */
 		if ((store.seen_nr == 0 && value == NULL) ||
-		    (store.seen_nr > 1 && multi_replace == 0)) {
+		    (store.seen_nr > 1 && !store.multi_replace)) {
 			ret = CONFIG_NOTHING_SET;
 			goto out_free;
 		}

@@ -2984,10 +2984,10 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 
 void git_config_set_multivar_in_file(const char *config_filename,
 				     const char *key, const char *value,
-				     const char *value_regex, int multi_replace)
+				     const char *value_regex, unsigned flags)
 {
 	if (!git_config_set_multivar_in_file_gently(config_filename, key, value,
-						    value_regex, multi_replace))
+						    value_regex, flags))
 		return;
 	if (value)
 		die(_("could not set '%s' to '%s'"), key, value);

@@ -2996,17 +2996,17 @@ void git_config_set_multivar_in_file(const char *config_filename,
 }
 
 int git_config_set_multivar_gently(const char *key, const char *value,
-				   const char *value_regex, int multi_replace)
+				   const char *value_regex, unsigned flags)
 {
 	return git_config_set_multivar_in_file_gently(NULL, key, value, value_regex,
-						      multi_replace);
+						      flags);
 }
 
 void git_config_set_multivar(const char *key, const char *value,
-			     const char *value_regex, int multi_replace)
+			     const char *value_regex, unsigned flags)
 {
 	git_config_set_multivar_in_file(NULL, key, value, value_regex,
-					multi_replace);
+					flags);
 }
 
 static int section_name_match (const char *buf, const char *name)

index 060874488f..80844604ab 100644
@@ -256,9 +256,22 @@ void git_config_set(const char *, const char *);
 
 int git_config_parse_key(const char *, char **, size_t *);
 int git_config_key_is_valid(const char *key);
-int git_config_set_multivar_gently(const char *, const char *, const char *, int);
-void git_config_set_multivar(const char *, const char *, const char *, int);
-int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, int);
+
+/*
+ * The following macros specify flag bits that alter the behavior
+ * of the git_config_set_multivar*() methods.
+ */
+
+/*
+ * When CONFIG_FLAGS_MULTI_REPLACE is specified, all matching key/values
+ * are removed before a new pair is written. If the flag is not present,
+ * then set operations replace only the first match.
+ */
+#define CONFIG_FLAGS_MULTI_REPLACE (1 << 0)
+
+int git_config_set_multivar_gently(const char *, const char *, const char *, unsigned);
+void git_config_set_multivar(const char *, const char *, const char *, unsigned);
+int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, unsigned);
 
 /**
  * takes four parameters:

@@ -276,13 +289,15 @@ int git_config_set_multivar_in_file_gently(const char *, const char *, const cha
  * - the value regex, as a string. It will disregard key/value pairs where value
  *   does not match.
  *
- * - a multi_replace value, as an int. If value is equal to zero, nothing or only
- *   one matching key/value is replaced, else all matching key/values (regardless
- *   how many) are removed, before the new pair is written.
+ * - a flags value with bits corresponding to the CONFIG_FLAG_* macros.
  *
  * It returns 0 on success.
  */
-void git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
+void git_config_set_multivar_in_file(const char *config_filename,
+				     const char *key,
+				     const char *value,
+				     const char *value_regex,
+				     unsigned flags);
 
 /**
  * rename or remove sections in the config file

gitgitgadget

From: Derrick Stolee <[email protected]>

The 'git config' builtin takes a 'value_regex' parameter for several actions. This can cause confusion when expecting exact value matches instead of regex matches, especially when the input string contains glob characters. While callers can escape the patterns themselves, it would be more friendly to allow an argument to disable the pattern matching in favor of an exact string match.

Add a new '--fixed-value' option that does not currently change the behavior. The implementation will follow for each appropriate action. For now, check and test that --fixed-value will abort the command when included with an incompatible action or without a 'value_regex' argument.

The name '--fixed-value' was chosen over something simpler like '--fixed' because some commands allow regular expressions on the key in addition to the value.

---
 Documentation/git-config.txt | 20 +++++++++++++-------
 builtin/config.c             | 26 ++++++++++++++++++++++++++
 t/t1300-config.sh            | 23 +++++++++++++++++++++++
 3 files changed, 62 insertions(+), 7 deletions(-)

index 7573160f21..d4bb928ea7 100644
@@ -9,15 +9,15 @@ git-config - Get and set repository or global options
 SYNOPSIS
 --------
 [verse]
-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] name [value [value_regex]]
+'git config' [<file-option>] [--type=<type>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] name [value [value_regex]]
 'git config' [<file-option>] [--type=<type>] --add name value
-'git config' [<file-option>] [--type=<type>] --replace-all name value [value_regex]
-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get name [value_regex]
-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get-all name [value_regex]
-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]
+'git config' [<file-option>] [--type=<type>] [--fixed-value] --replace-all name value [value_regex]
+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get name [value_regex]
+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get-all name [value_regex]
+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] [--name-only] --get-regexp name_regex [value_regex]
 'git config' [<file-option>] [--type=<type>] [-z|--null] --get-urlmatch name URL
-'git config' [<file-option>] --unset name [value_regex]
-'git config' [<file-option>] --unset-all name [value_regex]
+'git config' [<file-option>] [--fixed-value] --unset name [value_regex]
+'git config' [<file-option>] [--fixed-value] --unset-all name [value_regex]
 'git config' [<file-option>] --rename-section old_name new_name
 'git config' [<file-option>] --remove-section name
 'git config' [<file-option>] [--show-origin] [--show-scope] [-z|--null] [--name-only] -l | --list

@@ -165,6 +165,12 @@ See also <<FILES>>.
 --list::
 	List all variables set in config file, along with their values.
 
+--fixed-value::
+	When used with the `value_regex` argument, treat `value_regex` as
+	an exact string instead of a regular expression. This will restrict
+	the name/value pairs that are matched to only those where the value
+	is exactly equal to the `value_regex`.
+
 --type <type>::
   'git config' will ensure that any input or output is valid under the given
   type constraint(s), and will canonicalize outgoing values in `<type>`'s

index e7c7f3d455..bfb55a96df 100644
@@ -34,6 +34,7 @@ static int respect_includes_opt = -1;
 static struct config_options config_options;
 static int show_origin;
 static int show_scope;
+static int fixed_value;
 
 #define ACTION_GET (1<<0)
 #define ACTION_GET_ALL (1<<1)

@@ -141,6 +142,7 @@ static struct option builtin_config_options[] = {
 	OPT_BIT(0, "rename-section", &actions, N_("rename section: old-name new-name"), ACTION_RENAME_SECTION),
 	OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"), ACTION_REMOVE_SECTION),
 	OPT_BIT('l', "list", &actions, N_("list all"), ACTION_LIST),
+	OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when matching values")),
 	OPT_BIT('e', "edit", &actions, N_("open an editor"), ACTION_EDIT),
 	OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR),
 	OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL),

@@ -745,6 +747,30 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		usage_builtin_config();
 	}
 
+	if (fixed_value) {
+		int allowed_usage = 0;
+
+		switch (actions) {
+		case ACTION_GET:
+		case ACTION_GET_ALL:
+		case ACTION_GET_REGEXP:
+		case ACTION_UNSET:
+		case ACTION_UNSET_ALL:
+			allowed_usage = argc > 1 && !!argv[1];
+			break;
+
+		case ACTION_SET_ALL:
+		case ACTION_REPLACE_ALL:
+			allowed_usage = argc > 2 && !!argv[2];
+			break;
+		}
+
+		if (!allowed_usage) {
+			error(_("--fixed-value only applies with 'value_regex'"));
+			usage_builtin_config();
+		}
+	}
+
 	if (actions & PAGING_ACTIONS)
 		setup_auto_pager("config", 1);
 

index 8783767d4f..6dc8117241 100755
@@ -1960,4 +1960,27 @@ test_expect_success '--replace-all and value_regex' '
 	test_cmp expect actual
 '
 
+test_expect_success 'refuse --fixed-value for incompatible actions' '
+	git config --file=config dev.null bogus &&
+
+	# These modes do not allow --fixed-value at all
+	test_must_fail git config --file=config --fixed-value --add dev.null bogus &&
+	test_must_fail git config --file=config --fixed-value --get-urlmatch dev.null bogus &&
+	test_must_fail git config --file=config --fixed-value --get-urlmatch dev.null bogus &&
+	test_must_fail git config --file=config --fixed-value --rename-section dev null &&
+	test_must_fail git config --file=config --fixed-value --remove-section dev &&
+	test_must_fail git config --file=config --fixed-value --list &&
+	test_must_fail git config --file=config --fixed-value --get-color dev.null &&
+	test_must_fail git config --file=config --fixed-value --get-colorbool dev.null &&
+
+	# These modes complain when --fixed-value has no value_regex
+	test_must_fail git config --file=config --fixed-value dev.null bogus &&
+	test_must_fail git config --file=config --fixed-value --replace-all dev.null bogus &&
+	test_must_fail git config --file=config --fixed-value --get dev.null &&
+	test_must_fail git config --file=config --fixed-value --get-all dev.null &&
+	test_must_fail git config --file=config --fixed-value --get-regexp "dev.*" &&
+	test_must_fail git config --file=config --fixed-value --unset dev.null &&
+	test_must_fail git config --file=config --fixed-value --unset-all dev.null
+'
+
 test_done

gitgitgadget

Update the document and help text with s/value_regex/value_pattern/ and say "use value_pattern as a fixed string, not an extended regexp", perhaps?

Nice.

If I go about changing all documentation and error messages to say "value_pattern" instead of "value_regex", should I also update the uses in the *.po translation files? Or, should I leave them unmodified to trigger manual intervention by the translators?

Thanks,
-Stolee

I have discovered that changing these causes conflicts with automated line number updates. I'll leave these unchanged with a note for translators.

Thanks,
-Stolee

A minor request: If you are going to put in the work to make that substitution, perhaps change it to "value-pattern" rather than "value_pattern" since a hyphen is more common in documentation for this sort of thing than underscore.

Yup, I misspoke. s/value_regex/value-regex/ unless it is a variable name where '-' cannot appear.

If you do, you do not have to worry. The i18n/l10n coordinator will update the po/git.pot file when we near the code freeze using an automated tool that extracts strings from the sources, and the po/xx.po files for languages are updated from the updated po/git.pot mechanically with another tool, reusing unmodified entries, adding new ones, and marking near-hit ones to help avoid unnecessary work by translators.

I earlier thought that the "when matching values" phrase you chose in this round, without such a clean-up, would be OK in the context of this topic (which is depended on by a bugfix topic), but after seeing how we need to clarify the way '!' negation prefix works in the documentation that has deeply ingrained assumption that value is matched using ERE, it may be necessary to bite the bullet and do the regex->pattern now.

Thanks.

From: Derrick Stolee <[email protected]>

The git_config_set_multivar_in_file_gently() and related methods now take a 'flags' bitfield, so add a new bit representing the --fixed-value option from 'git config'. This alters the purpose of the value_regex parameter to be an exact string match. This requires some initialization changes in git_config_set_multivar_in_file_gently() and a new strcmp() call in the matches() method.

The new CONFIG_FLAGS_FIXED_VALUE flag is initialized in builtin/config.c based on the --fixed-value option, and that needs to be updated in several callers.

This patch only affects some of the modes of 'git config', and the rest will be completed in the next change.

---
 builtin/config.c  | 16 ++++++++-----
 config.c          |  5 +++++
 config.h          |  7 ++++++
 t/t1300-config.sh | 57 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 80 insertions(+), 5 deletions(-)

index bfb55a96df..3e49e04411 100644
@@ -616,6 +616,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 {
 	int nongit = !startup_info->have_repository;
 	char *value;
+	int flags = 0;
 
 	given_config_source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT));
 

@@ -769,6 +770,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			error(_("--fixed-value only applies with 'value_regex'"));
 			usage_builtin_config();
 		}
+
+		flags = CONFIG_FLAGS_FIXED_VALUE;
 	}
 
 	if (actions & PAGING_ACTIONS)

@@ -832,7 +835,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		value = normalize_value(argv[0], argv[1]);
 		UNLEAK(value);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
-							      argv[0], value, argv[2], 0);
+							      argv[0], value, argv[2],
+							      flags);
 	}
 	else if (actions == ACTION_ADD) {
 		check_write();

@@ -841,7 +845,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		UNLEAK(value);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
 							      argv[0], value,
-							      CONFIG_REGEX_NONE, 0);
+							      CONFIG_REGEX_NONE,
+							      flags);
 	}
 	else if (actions == ACTION_REPLACE_ALL) {
 		check_write();

@@ -850,7 +855,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		UNLEAK(value);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
 							      argv[0], value, argv[2],
-							      CONFIG_FLAGS_MULTI_REPLACE);
+							      flags | CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_GET) {
 		check_argc(argc, 1, 2);

@@ -877,7 +882,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 1, 2);
 		if (argc == 2)
 			return git_config_set_multivar_in_file_gently(given_config_source.file,
-								      argv[0], NULL, argv[1], 0);
+								      argv[0], NULL, argv[1],
+								      flags);
 		else
 			return git_config_set_in_file_gently(given_config_source.file,
 							     argv[0], NULL);

@@ -887,7 +893,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_argc(argc, 1, 2);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
 							      argv[0], NULL, argv[1],
-							      CONFIG_FLAGS_MULTI_REPLACE);
+							      flags | CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_RENAME_SECTION) {
 		int ret;

index 096f2eae0d..64bab2b08e 100644
@@ -2402,6 +2402,7 @@ struct config_store_data {
 	size_t baselen;
 	char *key;
 	int do_not_match;
+	const char *literal_value;
 	regex_t *value_regex;
 	int multi_replace;
 	struct {

@@ -2431,6 +2432,8 @@ static int matches(const char *key, const char *value,
 {
 	if (strcmp(key, store->key))
 		return 0; /* not ours */
+	if (store->literal_value)
+		return !strcmp(store->literal_value, value);
 	if (!store->value_regex)
 		return 1; /* always matches */
 	if (store->value_regex == CONFIG_REGEX_NONE)

@@ -2803,6 +2806,8 @@ int git_config_set_multivar_in_file_gently(const char *config_filename,
 			store.value_regex = NULL;
 		else if (value_regex == CONFIG_REGEX_NONE)
 			store.value_regex = CONFIG_REGEX_NONE;
+		else if (flags & CONFIG_FLAGS_FIXED_VALUE)
+			store.literal_value = value_regex;
 		else {
 			if (value_regex[0] == '!') {
 				store.do_not_match = 1;

index 80844604ab..977e690be8 100644
@@ -269,6 +269,13 @@ int git_config_key_is_valid(const char *key);
  */
 #define CONFIG_FLAGS_MULTI_REPLACE (1 << 0)
 
+/*
+ * When CONFIG_FLAGS_FIXED_VALUE is specified, match key/value pairs
+ * by string comparison (not regex match) to the provided value_regex
+ * parameter.
+ */
+#define CONFIG_FLAGS_FIXED_VALUE (1 << 1)
+
 int git_config_set_multivar_gently(const char *, const char *, const char *, unsigned);
 void git_config_set_multivar(const char *, const char *, const char *, unsigned);
 int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, unsigned);

gitgitgadget

[snip]

  $ git config --file=test foo.test aaa
  $ git config --file=test --add foo.test bbb
  $ git config --list --file=test
  foo.test=aaa
  foo.test=bbb
  $ git config --file=test --replace-all foo.test bbb a+
  $ git config --list --file=test
  foo.test=bbb
  foo.test=bbb

For creating a one-line file, the verbosity of a here-doc may be overkill. At any rate, echo would be more idiomatic than printf:

    echo "fixed.test=bogus" >expect &&

(Though, this alone is likely not worth a re-roll.)

Good catch.

Reminds me to call this 'fixed_value'.

Good idea. Will do.

I can use a heredoc just to be consistent.

(To also respond to Eric's message, I tend to use printf instead of echo because echo starts a process while printf does not.)

If no value is provided, then 'git config <name>' _is_ a query. It's not a failed set. 'git config <name> ""' would be the way to try and set the value to an empty string.

This is specifically demonstrating the difference that --fixed-value provides. The tests here show "the match doesn't work by default, but then works with --fixed-value". I'll make this clearer in the commit message.

Yes, accidentally over-zealous copy/paste.

I'm less in favor of splitting the tests, since they rely on a shared initial config. Any failure in one part of this test is likely to also fail the rest of the commands, so grouping them by this toggle makes sense to me.

Thanks,
-Stolee

echo is a builtin[1] in many or most shells (just as printf also often is), thus is unlikely to start a process.

[1] Aside: You can use type <cmd> or command -V <cmd> to see if <cmd> is a builtin in your shell.

From: Derrick Stolee <[email protected]>

The config builtin does its own regex matching of values for the --get, --get-all, and --get-regexp modes. Plumb the existing 'flags' parameter to the get_value() method so we can initialize the value_regex argument as a fixed string instead of a regex pattern.

---
 builtin/config.c  | 15 ++++++++++-----
 t/t1300-config.sh | 22 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 5 deletions(-)

index 3e49e04411..d3772b5efe 100644
@@ -14,6 +14,7 @@ static const char *const builtin_config_usage[] = {
 
 static char *key;
 static regex_t *key_regexp;
+static const char *value_regex;
 static regex_t *regexp;
 static int show_keys;
 static int omit_values;

@@ -288,6 +289,8 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 		return 0;
 	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
 		return 0;
+	if (fixed_value && strcmp(value_regex, (value_?value_:"")))
+		return 0;
 	if (regexp != NULL &&
 	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
 		return 0;

@@ -298,7 +301,7 @@ static int collect_config(const char *key_, const char *value_, void *cb)
 	return format_config(&values->items[values->nr++], key_, value_);
 }
 
-static int get_value(const char *key_, const char *regex_)
+static int get_value(const char *key_, const char *regex_, int flags)
 {
 	int ret = CONFIG_GENERIC_ERROR;
 	struct strbuf_list values = {NULL};

@@ -335,7 +338,9 @@ static int get_value(const char *key_, const char *regex_)
 		}
 	}
 
-	if (regex_) {
+	if (regex_ && (flags & CONFIG_FLAGS_FIXED_VALUE))
+		value_regex = regex_;
+	else if (regex_) {
 		if (regex_[0] == '!') {
 			do_not_match = 1;
 			regex_++;

@@ -859,19 +864,19 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 	else if (actions == ACTION_GET) {
 		check_argc(argc, 1, 2);
-		return get_value(argv[0], argv[1]);
+		return get_value(argv[0], argv[1], flags);
 	}
 	else if (actions == ACTION_GET_ALL) {
 		do_all = 1;
 		check_argc(argc, 1, 2);
-		return get_value(argv[0], argv[1]);
+		return get_value(argv[0], argv[1], flags);
 	}
 	else if (actions == ACTION_GET_REGEXP) {
 		show_keys = 1;
 		use_key_regexp = 1;
 		do_all = 1;
 		check_argc(argc, 1, 2);
-		return get_value(argv[0], argv[1]);
+		return get_value(argv[0], argv[1], flags);
 	}
 	else if (actions == ACTION_GET_URLMATCH) {
 		check_argc(argc, 2, 2);

index 30e80ae9cb..13211b6bf4 100755
@@ -2040,4 +2040,26 @@ test_expect_success '--fixed-value uses exact string matching' '
 	test_cmp expect actual
 '
 
+test_expect_success '--get and --get-all with --fixed-value' '
+	GLOB="a+b*c?d[e]f.g" &&
+	rm -f config &&
+	git config --file=config fixed.test bogus &&
+	git config --file=config --add fixed.test "$GLOB" &&
+
+	git config --file=config --get fixed.test bogus &&
+	test_must_fail git config --file=config --get fixed.test "$GLOB" &&
+	git config --file=config --get --fixed-value fixed.test "$GLOB" &&
+	test_must_fail git config --file=config --get --fixed-value fixed.test non-existent &&
+
+	git config --file=config --get-all fixed.test bogus &&
+	test_must_fail git config --file=config --get-all fixed.test "$GLOB" &&
+	git config --file=config --get-all --fixed-value fixed.test "$GLOB" &&
+	test_must_fail git config --file=config --get-all --fixed-value fixed.test non-existent &&
+
+	git config --file=config --get-regexp fixed+ bogus &&
+	test_must_fail git config --file=config --get-regexp fixed+ "$GLOB" &&
+	git config --file=config --get-regexp --fixed-value fixed+ "$GLOB" &&
+	test_must_fail git config --file=config --get-regexp --fixed-value fixed+ non-existent
+'
+
 test_done

gitgitgadget

Allowing myself a sensible chuckle at the commit subject using glob syntax on a series about regex matching. ;)

From: Derrick Stolee <[email protected]>

When a repository's leading directories contain regex glob characters, the config calls for 'git maintenance register' and 'git maintenance unregister' are not careful enough. Use the new --fixed-value option to direct the config machinery to use exact string matches. This is a more robust option than excaping these arguments in a piecemeal fashion.

For the test, require that we are not running on Windows since the '+' character is not allowed on that filesystem.

---
 builtin/gc.c           |  5 +++--
 t/t7900-maintenance.sh | 12 ++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

index e3098ef6a1..6dde3ce1bb 100644
@@ -1452,7 +1452,8 @@ static int maintenance_register(void)
 		git_config_set("maintenance.strategy", "incremental");
 
 	config_get.git_cmd = 1;
-	strvec_pushl(&config_get.args, "config", "--global", "--get", "maintenance.repo",
+	strvec_pushl(&config_get.args, "config", "--global", "--get",
+		     "--fixed-value", "maintenance.repo",
 		     the_repository->worktree ? the_repository->worktree
 					      : the_repository->gitdir,
 			 NULL);

@@ -1483,7 +1484,7 @@ static int maintenance_unregister(void)
 
 	config_unset.git_cmd = 1;
 	strvec_pushl(&config_unset.args, "config", "--global", "--unset",
-		     "maintenance.repo",
+		     "--fixed-value", "maintenance.repo",
 		     the_repository->worktree ? the_repository->worktree
 					      : the_repository->gitdir,
 		     NULL);

index 20184e96e1..c4e5564c31 100755
@@ -367,6 +367,18 @@ test_expect_success 'register and unregister' '
 	test_cmp before actual
 '
 
+test_expect_success !MINGW 'register and unregister with glob characters' '
+	GLOB="a+b*c" &&
+	git init "$GLOB" &&
+	git -C "$GLOB" maintenance register &&
+	git config --get-all --show-origin maintenance.repo &&
+	git config --get-all --global --fixed-value \
+		maintenance.repo "$(pwd)/$GLOB" &&
+	git -C "$GLOB" maintenance unregister &&
+	test_must_fail git config --get-all --global --fixed-value \
+		maintenance.repo "$(pwd)/$GLOB"
+'
+
 test_expect_success 'start from empty cron table' '
 	GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
 

gitgitgadget

Not a huge deal, but we do not have to rely on '+', though. Using "a*b*c" instead of "a+b*c" would perfectly be fine. It would only match "a*b*c" literally and not as ERE.

Minor "well, actually" - I'm not sure 'glob' is the right word to use here.

Typo on 'escaping'.

A careful reader ;-) "metacharacters" is probably what we want.

Patches [1/7] and [2/7] got much less scary with this change.

Nice.

Thanks, will replace (but with other topics depend on this series, it may take a bit more time than usual).

As reported [1], 'git maintenance unregister' fails when a repository is located in a directory with regex glob characters.

[1]
https://lore.kernel.org/git/[email protected]/T/#mb96fa4187a0d6aeda097cd95804a8aafc0273022

The discussed solution was to modify 'git config' to specify that the 'value_regex' argument should be treated as an exact string match. This is the primary change in this series, with an additional patch at the end to make 'git maintenance [un]register' use this option, when necessary.

While we're here, let's rename 'value_regex' to 'value_pattern' to make things a bit clearer.

Updates in V3
=============

* Renamed 'value_regex' to 'value_pattern' in code and 'value-pattern' in docs (except po/)

* Reordered commits slightly to help with that rename.

* Updated tests to use 'test_when_finished rm -f ...'

* Changed all references to "glob" characters to "meta" characters.

* Several other test modifications. Thanks, Emily, for the review!

Thanks, -Stolee

P.S. Happy Thanksgiving to those celebrating!

Derrick Stolee (8):
config: convert multi_replace to flags
config: replace 'value_regex' with 'value_pattern'
t1300: test "set all" mode with value-pattern
t1300: add test for --replace-all with value-pattern
config: add --fixed-value option, un-implemented
config: plumb --fixed-value into config API
config: implement --fixed-value with --get*
maintenance: use 'git config --fixed-value'

 Documentation/git-config.txt |  26 +++---
 builtin/branch.c             |   4 +-
 builtin/config.c             |  81 +++++++++++++++----
 builtin/gc.c                 |   5 +-
 builtin/remote.c             |   8 +-
 config.c                     |  75 ++++++++++--------
 config.h                     |  36 +++++++--
 t/t1300-config.sh            | 149 +++++++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh       |  12 +++
 9 files changed, 321 insertions(+), 75 deletions(-)

base-commit: 0016b618182f642771dc589cf0090289f9fe1b4f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-796%2Fderrickstolee%2Fmaintenance%2Fconfig-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-796/derrickstolee/maintenance/config-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/796

Range-diff vs v2:

3: 0c152faa00 ! 1: f0ed492096 config: convert multi_replace to flags
@@ config.c: void git_config_set(const char *key, const char *value)
- * if multi_replace==0, nothing, or only one matching key/value is replaced,
- * else all matching key/values (regardless how many) are removed,
- * before the new pair is written.
-+ * if (flags & CONFIG_FLAGS_MULTI_REPLACE) == 0, at most one matching
-+ * key/value is replaced, else all matching key/values (regardless
-+ * how many) are removed, before the new pair is written.
++ * if flags contains the CONFIG_FLAGS_MULTI_REPLACE flag, all matching
++ * key/values are removed before a single new pair is written. If the
++ * flag is not present, then replace only the first match.
*
* Returns 0 on success.
*
@@ config.h: void git_config_set(const char *, const char *);
+
+/*
+ * When CONFIG_FLAGS_MULTI_REPLACE is specified, all matching key/values
-+ * are removed before a new pair is written. If the flag is not present, -+ * then set operations replace only the first match.
++ * are removed before a single new pair is written. If the flag is not ++ * present, then set operations replace only the first match.
+ */
+#define CONFIG_FLAGS_MULTI_REPLACE (1 << 0)
+

 -:  ---------- > 2:  f135b001ad config: replace 'value_regex' with 'value_pattern'
 1:  ea3099719c ! 3:  3b72082326 t1300: test "set all" mode with value_regex

@@ Metadata
Author: Derrick Stolee <[email protected]>

## Commit message ##
- t1300: test "set all" mode with value_regex
+ t1300: test "set all" mode with value-pattern

- Without additional modifiers, 'git config' attempts to set a single
- value in the .git/config file. When the value_regex parameter is
- supplied, this command behaves in a non-trivial manner.
+ Without additional modifiers, 'git config <key> <value>' attempts
+ to set a single value in the .git/config file. When the
+ value-pattern parameter is supplied, this command behaves in a
+ non-trivial manner.

- Consider 'git config key value value_regex'. The expected behavior
- is as follows:
+ Consider 'git config <key> <value> <value-pattern>'. The expected
+ behavior is as follows:

- 1. If there are multiple existing values that match 'value_regex',
+ 1. If there are multiple existing values that match 'value-pattern',
then the command fails. Users should use --replace-all instead.

- 2. If there is one existing value that matches 'value_regex', then
- the new config has one entry where 'key=value'.
-
- 3. If there is no existing values match 'value_regex', then the
+ 2. If there is no existing values match 'value-pattern', then the
'key=value' pair is appended, making this 'key' a multi-valued config setting.

+ 3. If there is one existing value that matches 'value-pattern', then
+ the new config has one entry where 'key=value'.
+
Add a test that demonstrates these options. Break from the existing pattern in t1300-config.sh to use 'git config --file=<file>' instead of
- modifying .git/config directly. Also use 'git config --file=<file>
- --list' for config state comparison instead of the config file format.
+ modifying .git/config directly to prevent possibly incompatible repo
+ states. Also use 'git config --file=<file> --list' for config state
+ comparison instead of the config file format. This makes the tests
+ more readable.

@@ t/t1300-config.sh: test_expect_success '--replace-all does not invent newlines'
test_cmp expect .git/config
'

-+test_expect_success 'set all config with value_regex' ' ++test_expect_success 'set all config with value-pattern' '
++ test_when_finished rm -f config initial &&
+ git config --file=initial abc.key one &&
+
++ # no match => add new entry
+ cp initial config &&
+ git config --file=config abc.key two a+ &&
+ git config --file=config --list >actual &&
@@ t/t1300-config.sh: test_expect_success '--replace-all does not invent newlines'
+ EOF
+ test_cmp expect actual &&
+
++ # multiple matches => failure
+ test_must_fail git config --file=config abc.key three o+ 2>err &&
+ test_i18ngrep "has multiple values" err &&
++
++ # multiple values, no match => add
+ git config --file=config abc.key three a+ &&
+ git config --file=config --list >actual &&
+ cat >expect <<-\EOF &&
@@ t/t1300-config.sh: test_expect_success '--replace-all does not invent newlines'
+ EOF
+ test_cmp expect actual &&
+
-+ cp initial config &&
-+ git config --file=config abc.key three o+ &&
++ # single match => replace
++ git config --file=config abc.key four h+ &&
+ git config --file=config --list >actual &&
+ cat >expect <<-\EOF &&
-+ abc.key=three
++ abc.key=one
++ abc.key=two
++ abc.key=four
+ EOF
+ test_cmp expect actual
+'
2: 829d0ccd8c ! 4: 75fb74da83 t1300: add test for --replace-all with value_regex
@@ Metadata
Author: Derrick Stolee <[email protected]>

## Commit message ##
- t1300: add test for --replace-all with value_regex
+ t1300: add test for --replace-all with value-pattern

The --replace-all option was added in 4ddba79d (git-config-set: add more
- options) but was not tested along with the 'value_regex' parameter.
- Since we will be updating this option to optionally treat 'value_regex'
+ options) but was not tested along with the 'value-pattern' parameter.
+ Since we will be updating this option to optionally treat 'value-pattern'
as a fixed string, let's add a test here that documents the current behavior.

## t/t1300-config.sh ##
-@@ t/t1300-config.sh: test_expect_success 'set all config with value_regex' ' +@@ t/t1300-config.sh: test_expect_success 'set all config with value-pattern' '
test_cmp expect actual
'

-+test_expect_success '--replace-all and value_regex' '
-+ rm -f config &&
++test_expect_success '--replace-all and value-pattern' '
++ test_when_finished rm -f config &&
+ git config --file=config --add abc.key one &&
+ git config --file=config --add abc.key two &&
+ git config --file=config --add abc.key three &&
4: 0e6a7371ed ! 5: 0c276ffcee config: add --fixed-value option, un-implemented

     @@ Metadata
       ## Commit message ##
          config: add --fixed-value option, un-implemented
      
     -    The 'git config' builtin takes a 'value_regex' parameter for several
     +    The 'git config' builtin takes a 'value-pattern' parameter for several
          actions. This can cause confusion when expecting exact value matches
     -    instead of regex matches, especially when the input string contains glob
     -    characters. While callers can escape the patterns themselves, it would
     -    be more friendly to allow an argument to disable the pattern matching in
     -    favor of an exact string match.
     +    instead of regex matches, especially when the input string contains
     +    metacharacters. While callers can escape the patterns themselves, it
     +    would be more friendly to allow an argument to disable the pattern
     +    matching in favor of an exact string match.
      
          Add a new '--fixed-value' option that does not currently change the
     -    behavior. The implementation will follow for each appropriate action.
     -    For now, check and test that --fixed-value will abort the command when
     -    included with an incompatible action or without a 'value_regex'
     -    argument.
     +    behavior. The implementation will be filled in by later changes for
     +    each appropriate action. For now, check and test that --fixed-value
     +    will abort the command when included with an incompatible action or
     +    without a 'value-pattern' argument.
      
          The name '--fixed-value' was chosen over something simpler like
          '--fixed' because some commands allow regular expressions on the
     @@ Documentation/git-config.txt: git-config - Get and set repository or global opti
       SYNOPSIS
       --------
       [verse]
     --'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] name [value [value_regex]]
     -+'git config' [<file-option>] [--type=<type>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] name [value [value_regex]]
     +-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] name [value [value-pattern]]
     ++'git config' [<file-option>] [--type=<type>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] name [value [value-pattern]]
       'git config' [<file-option>] [--type=<type>] --add name value
     --'git config' [<file-option>] [--type=<type>] --replace-all name value [value_regex]
     --'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get name [value_regex]
     --'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get-all name [value_regex]
     --'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]
     -+'git config' [<file-option>] [--type=<type>] [--fixed-value] --replace-all name value [value_regex]
     -+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get name [value_regex]
     -+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get-all name [value_regex]
     -+'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] [--name-only] --get-regexp name_regex [value_regex]
     +-'git config' [<file-option>] [--type=<type>] --replace-all name value [value-pattern]
     +-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get name [value-pattern]
     +-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] --get-all name [value-pattern]
     +-'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--name-only] --get-regexp name_regex [value-pattern]
     ++'git config' [<file-option>] [--type=<type>] [--fixed-value] --replace-all name value [value-pattern]
     ++'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get name [value-pattern]
     ++'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get-all name [value-pattern]
     ++'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] [--name-only] --get-regexp name_regex [value-pattern]
       'git config' [<file-option>] [--type=<type>] [-z|--null] --get-urlmatch name URL
     --'git config' [<file-option>] --unset name [value_regex]
     --'git config' [<file-option>] --unset-all name [value_regex]
     -+'git config' [<file-option>] [--fixed-value] --unset name [value_regex]
     -+'git config' [<file-option>] [--fixed-value] --unset-all name [value_regex]
     +-'git config' [<file-option>] --unset name [value-pattern]
     +-'git config' [<file-option>] --unset-all name [value-pattern]
     ++'git config' [<file-option>] [--fixed-value] --unset name [value-pattern]
     ++'git config' [<file-option>] [--fixed-value] --unset-all name [value-pattern]
       'git config' [<file-option>] --rename-section old_name new_name
       'git config' [<file-option>] --remove-section name
       'git config' [<file-option>] [--show-origin] [--show-scope] [-z|--null] [--name-only] -l | --list
     @@ Documentation/git-config.txt: See also <<FILES>>.
       	List all variables set in config file, along with their values.
       
      +--fixed-value::
     -+	When used with the `value_regex` argument, treat `value_regex` as
     ++	When used with the `value-pattern` argument, treat `value-pattern` as
      +	an exact string instead of a regular expression. This will restrict
      +	the name/value pairs that are matched to only those where the value
     -+	is exactly equal to the `value_regex`.
     ++	is exactly equal to the `value-pattern`.
      +
       --type <type>::
         'git config' will ensure that any input or output is valid under the given
     @@ builtin/config.c: static struct option builtin_config_options[] = {
       	OPT_BIT(0, "rename-section", &actions, N_("rename section: old-name new-name"), ACTION_RENAME_SECTION),
       	OPT_BIT(0, "remove-section", &actions, N_("remove a section: name"), ACTION_REMOVE_SECTION),
       	OPT_BIT('l', "list", &actions, N_("list all"), ACTION_LIST),
     -+	OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when matching values")),
     ++	OPT_BOOL(0, "fixed-value", &fixed_value, N_("use string equality when comparing values to 'value-pattern'")),
       	OPT_BIT('e', "edit", &actions, N_("open an editor"), ACTION_EDIT),
       	OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR),
       	OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL),
     @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix
       		usage_builtin_config();
       	}
       
     ++	/* check usage of --fixed-value */
      +	if (fixed_value) {
      +		int allowed_usage = 0;
      +
      +		switch (actions) {
     ++		/* git config --get <name> <value-pattern> */
      +		case ACTION_GET:
     ++		/* git config --get-all <name> <value-pattern> */
      +		case ACTION_GET_ALL:
     ++		/* git config --get-regexp <name-pattern> <value-pattern> */
      +		case ACTION_GET_REGEXP:
     ++		/* git config --unset <name> <value-pattern> */
      +		case ACTION_UNSET:
     ++		/* git config --unset-all <name> <value-pattern> */
      +		case ACTION_UNSET_ALL:
      +			allowed_usage = argc > 1 && !!argv[1];
      +			break;
      +
     ++		/* git config <name> <value> <value-pattern> */
      +		case ACTION_SET_ALL:
     ++		/* git config --replace-all <name> <value> <value-pattern> */
      +		case ACTION_REPLACE_ALL:
      +			allowed_usage = argc > 2 && !!argv[2];
      +			break;
     ++
     ++		/* other options don't allow --fixed-value */
      +		}
      +
      +		if (!allowed_usage) {
     -+			error(_("--fixed-value only applies with 'value_regex'"));
     ++			error(_("--fixed-value only applies with 'value-pattern'"));
      +			usage_builtin_config();
      +		}
      +	}
     @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix
       
      
       ## t/t1300-config.sh ##
     -@@ t/t1300-config.sh: test_expect_success '--replace-all and value_regex' '
     +@@ t/t1300-config.sh: test_expect_success '--replace-all and value-pattern' '
       	test_cmp expect actual
       '
       
      +test_expect_success 'refuse --fixed-value for incompatible actions' '
     ++	test_when_finished rm -f config &&
      +	git config --file=config dev.null bogus &&
      +
      +	# These modes do not allow --fixed-value at all
     @@ t/t1300-config.sh: test_expect_success '--replace-all and value_regex' '
      +	test_must_fail git config --file=config --fixed-value --get-color dev.null &&
      +	test_must_fail git config --file=config --fixed-value --get-colorbool dev.null &&
      +
     -+	# These modes complain when --fixed-value has no value_regex
     ++	# These modes complain when --fixed-value has no value-pattern
      +	test_must_fail git config --file=config --fixed-value dev.null bogus &&
      +	test_must_fail git config --file=config --fixed-value --replace-all dev.null bogus &&
      +	test_must_fail git config --file=config --fixed-value --get dev.null &&
 5:  39718048cd ! 6:  763401da97 config: plumb --fixed-value into config API
     @@ Commit message
      
          The git_config_set_multivar_in_file_gently() and related methods now
          take a 'flags' bitfield, so add a new bit representing the --fixed-value
     -    option from 'git config'. This alters the purpose of the value_regex
     +    option from 'git config'. This alters the purpose of the value_pattern
          parameter to be an exact string match. This requires some initialization
          changes in git_config_set_multivar_in_file_gently() and a new strcmp()
          call in the matches() method.
     @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix
       	given_config_source.file = xstrdup_or_null(getenv(CONFIG_ENVIRONMENT));
       
      @@ builtin/config.c: int cmd_config(int argc, const char **argv, const char *prefix)
     - 			error(_("--fixed-value only applies with 'value_regex'"));
     + 			error(_("--fixed-value only applies with 'value-pattern'"));
       			usage_builtin_config();
       		}
      +
     -+		flags = CONFIG_FLAGS_FIXED_VALUE;
     ++		flags |= CONFIG_FLAGS_FIXED_VALUE;
       	}
       
       	if (actions & PAGING_ACTIONS)
     @@ config.c: struct config_store_data {
       	size_t baselen;
       	char *key;
       	int do_not_match;
     -+	const char *literal_value;
     - 	regex_t *value_regex;
     ++	const char *fixed_value;
     + 	regex_t *value_pattern;
       	int multi_replace;
       	struct {
      @@ config.c: static int matches(const char *key, const char *value,
       {
       	if (strcmp(key, store->key))
       		return 0; /* not ours */
     -+	if (store->literal_value)
     -+		return !strcmp(store->literal_value, value);
     - 	if (!store->value_regex)
     ++	if (store->fixed_value)
     ++		return !strcmp(store->fixed_value, value);
     + 	if (!store->value_pattern)
       		return 1; /* always matches */
     - 	if (store->value_regex == CONFIG_REGEX_NONE)
     + 	if (store->value_pattern == CONFIG_REGEX_NONE)
      @@ config.c: int git_config_set_multivar_in_file_gently(const char *config_filename,
     - 			store.value_regex = NULL;
     - 		else if (value_regex == CONFIG_REGEX_NONE)
     - 			store.value_regex = CONFIG_REGEX_NONE;
     + 			store.value_pattern = NULL;
     + 		else if (value_pattern == CONFIG_REGEX_NONE)
     + 			store.value_pattern = CONFIG_REGEX_NONE;
      +		else if (flags & CONFIG_FLAGS_FIXED_VALUE)
     -+			store.literal_value = value_regex;
     ++			store.fixed_value = value_pattern;
       		else {
     - 			if (value_regex[0] == '!') {
     + 			if (value_pattern[0] == '!') {
       				store.do_not_match = 1;
      
       ## config.h ##
     @@ config.h: int git_config_key_is_valid(const char *key);
       
      +/*
      + * When CONFIG_FLAGS_FIXED_VALUE is specified, match key/value pairs
     -+ * by string comparison (not regex match) to the provided value_regex
     ++ * by string comparison (not regex match) to the provided value_pattern
      + * parameter.
      + */
      +#define CONFIG_FLAGS_FIXED_VALUE (1 << 1)
     @@ t/t1300-config.sh: test_expect_success 'refuse --fixed-value for incompatible ac
       '
       
      +test_expect_success '--fixed-value uses exact string matching' '
     -+	GLOB="a+b*c?d[e]f.g" &&
     -+	rm -f initial &&
     -+	git config --file=initial fixed.test "$GLOB" &&
     ++	test_when_finished rm -f config initial &&
     ++	META="a+b*c?d[e]f.g" &&
     ++	git config --file=initial fixed.test "$META" &&
      +
      +	cp initial config &&
     -+	git config --file=config fixed.test bogus "$GLOB" &&
     ++	git config --file=config fixed.test bogus "$META" &&
      +	git config --file=config --list >actual &&
      +	cat >expect <<-EOF &&
     -+	fixed.test=$GLOB
     ++	fixed.test=$META
      +	fixed.test=bogus
      +	EOF
      +	test_cmp expect actual &&
      +
      +	cp initial config &&
     -+	git config --file=config --fixed-value fixed.test bogus "$GLOB" &&
     ++	git config --file=config --fixed-value fixed.test bogus "$META" &&
      +	git config --file=config --list >actual &&
     -+	printf "fixed.test=bogus\n" >expect &&
     ++	cat >expect <<-\EOF &&
     ++	fixed.test=bogus
     ++	EOF
      +	test_cmp expect actual &&
      +
      +	cp initial config &&
     -+	test_must_fail git config --file=config --unset fixed.test "$GLOB" &&
     -+	git config --file=config --fixed-value --unset fixed.test "$GLOB" &&
     ++	test_must_fail git config --file=config --unset fixed.test "$META" &&
     ++	git config --file=config --fixed-value --unset fixed.test "$META" &&
      +	test_must_fail git config --file=config fixed.test &&
      +
      +	cp initial config &&
     -+	test_must_fail git config --file=config --unset-all fixed.test "$GLOB" &&
     -+	git config --file=config --fixed-value --unset-all fixed.test "$GLOB" &&
     ++	test_must_fail git config --file=config --unset-all fixed.test "$META" &&
     ++	git config --file=config --fixed-value --unset-all fixed.test "$META" &&
      +	test_must_fail git config --file=config fixed.test &&
      +
      +	cp initial config &&
     -+	git config --file=config --replace-all fixed.test bogus "$GLOB" &&
     -+	git config --file=config --list >actual &&
     -+	cat >expect <<-EOF &&
     -+	fixed.test=$GLOB
     -+	fixed.test=bogus
     -+	EOF
     -+	test_cmp expect actual &&
     -+
     -+	cp initial config &&
     -+	git config --file=config --replace-all fixed.test bogus "$GLOB" &&
     ++	git config --file=config --replace-all fixed.test bogus "$META" &&
      +	git config --file=config --list >actual &&
      +	cat >expect <<-EOF &&
     -+	fixed.test=$GLOB
     ++	fixed.test=$META
      +	fixed.test=bogus
      +	EOF
      +	test_cmp expect actual &&
      +
     -+	git config --file=config --fixed-value --replace-all fixed.test bogus "$GLOB" &&
     ++	git config --file=config --fixed-value --replace-all fixed.test bogus "$META" &&
      +	git config --file=config --list >actual &&
      +	cat >expect <<-EOF &&
      +	fixed.test=bogus
 6:  8e0111c7b4 ! 7:  d813c84275 config: implement --fixed-value with --get*
     @@ Commit message
      
          The config builtin does its own regex matching of values for the --get,
          --get-all, and --get-regexp modes. Plumb the existing 'flags' parameter
     -    to the get_value() method so we can initialize the value_regex argument
     +    to the get_value() method so we can initialize the value-pattern argument
          as a fixed string instead of a regex pattern.
      
          Signed-off-by: Derrick Stolee <[email protected]>
     @@ builtin/config.c: static const char *const builtin_config_usage[] = {
       
       static char *key;
       static regex_t *key_regexp;
     -+static const char *value_regex;
     ++static const char *value_pattern;
       static regex_t *regexp;
       static int show_keys;
       static int omit_values;
     @@ builtin/config.c: static int collect_config(const char *key_, const char *value_
       		return 0;
       	if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0))
       		return 0;
     -+	if (fixed_value && strcmp(value_regex, (value_?value_:"")))
     ++	if (fixed_value && strcmp(value_pattern, (value_?value_:"")))
      +		return 0;
       	if (regexp != NULL &&
       	    (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
     @@ builtin/config.c: static int collect_config(const char *key_, const char *value_
       }
       
      -static int get_value(const char *key_, const char *regex_)
     -+static int get_value(const char *key_, const char *regex_, int flags)
     ++static int get_value(const char *key_, const char *regex_, unsigned flags)
       {
       	int ret = CONFIG_GENERIC_ERROR;
       	struct strbuf_list values = {NULL};
     @@ builtin/config.c: static int get_value(const char *key_, const char *regex_)
       
      -	if (regex_) {
      +	if (regex_ && (flags & CONFIG_FLAGS_FIXED_VALUE))
     -+		value_regex = regex_;
     ++		value_pattern = regex_;
      +	else if (regex_) {
       		if (regex_[0] == '!') {
       			do_not_match = 1;
     @@ t/t1300-config.sh: test_expect_success '--fixed-value uses exact string matching
       '
       
      +test_expect_success '--get and --get-all with --fixed-value' '
     -+	GLOB="a+b*c?d[e]f.g" &&
     -+	rm -f config &&
     ++	test_when_finished rm -f config &&
     ++	META="a+b*c?d[e]f.g" &&
      +	git config --file=config fixed.test bogus &&
     -+	git config --file=config --add fixed.test "$GLOB" &&
     ++	git config --file=config --add fixed.test "$META" &&
      +
      +	git config --file=config --get fixed.test bogus &&
     -+	test_must_fail git config --file=config --get fixed.test "$GLOB" &&
     -+	git config --file=config --get --fixed-value fixed.test "$GLOB" &&
     ++	test_must_fail git config --file=config --get fixed.test "$META" &&
     ++	git config --file=config --get --fixed-value fixed.test "$META" &&
      +	test_must_fail git config --file=config --get --fixed-value fixed.test non-existent &&
      +
      +	git config --file=config --get-all fixed.test bogus &&
     -+	test_must_fail git config --file=config --get-all fixed.test "$GLOB" &&
     -+	git config --file=config --get-all --fixed-value fixed.test "$GLOB" &&
     ++	test_must_fail git config --file=config --get-all fixed.test "$META" &&
     ++	git config --file=config --get-all --fixed-value fixed.test "$META" &&
      +	test_must_fail git config --file=config --get-all --fixed-value fixed.test non-existent &&
      +
      +	git config --file=config --get-regexp fixed+ bogus &&
     -+	test_must_fail git config --file=config --get-regexp fixed+ "$GLOB" &&
     -+	git config --file=config --get-regexp --fixed-value fixed+ "$GLOB" &&
     ++	test_must_fail git config --file=config --get-regexp fixed+ "$META" &&
     ++	git config --file=config --get-regexp --fixed-value fixed+ "$META" &&
      +	test_must_fail git config --file=config --get-regexp --fixed-value fixed+ non-existent
      +'
      +
 7:  5a3acf8119 ! 8:  558775f83d maintenance: use 'git config --fixed-value'
     @@ Metadata
       ## Commit message ##
          maintenance: use 'git config --fixed-value'
      
     -    When a repository's leading directories contain regex glob characters,
     +    When a repository's leading directories contain regex metacharacters,
          the config calls for 'git maintenance register' and 'git maintenance
          unregister' are not careful enough. Use the new --fixed-value option
          to direct the config machinery to use exact string matches. This is a
     -    more robust option than excaping these arguments in a piecemeal fashion.
     +    more robust option than escaping these arguments in a piecemeal fashion.
      
          For the test, require that we are not running on Windows since the '+'
     -    character is not allowed on that filesystem.
     +    and '*' characters are not allowed on that filesystem.
      
          Reported-by: Emily Shaffer <[email protected]>
          Reported-by: Jonathan Nieder <[email protected]>
     @@ t/t7900-maintenance.sh: test_expect_success 'register and unregister' '
       	test_cmp before actual
       '
       
     -+test_expect_success !MINGW 'register and unregister with glob characters' '
     -+	GLOB="a+b*c" &&
     -+	git init "$GLOB" &&
     -+	git -C "$GLOB" maintenance register &&
     ++test_expect_success !MINGW 'register and unregister with regex metacharacters' '
     ++	META="a+b*c" &&
     ++	git init "$META" &&
     ++	git -C "$META" maintenance register &&
      +	git config --get-all --show-origin maintenance.repo &&
      +	git config --get-all --global --fixed-value \
     -+		maintenance.repo "$(pwd)/$GLOB" &&
     -+	git -C "$GLOB" maintenance unregister &&
     ++		maintenance.repo "$(pwd)/$META" &&
     ++	git -C "$META" maintenance unregister &&
      +	test_must_fail git config --get-all --global --fixed-value \
     -+		maintenance.repo "$(pwd)/$GLOB"
     ++		maintenance.repo "$(pwd)/$META"
      +'
      +
       test_expect_success 'start from empty cron table' '

gitgitgadget

From: Derrick Stolee <[email protected]>

We will extend the flexibility of the config API. Before doing so, let's take an existing 'int multi_replace' parameter and replace it with a new 'unsigned flags' parameter that can take multiple options as a bit field.

Update all callers that specified multi_replace to now specify the CONFIG_FLAGS_MULTI_REPLACE flag. To add more clarity, extend the documentation of git_config_set_multivar_in_file() including a clear labeling of its arguments. Other config API methods in config.h require only a change of the final parameter from 'int' to 'unsigned'.

---
 builtin/branch.c |  4 ++--
 builtin/config.c |  6 ++++--
 builtin/remote.c |  8 +++++---
 config.c         | 24 ++++++++++++------------
 config.h         | 29 ++++++++++++++++++++++-------
 5 files changed, 45 insertions(+), 26 deletions(-)

index e82301fb1b..5ce3844d22 100644
@@ -829,10 +829,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die(_("Branch '%s' has no upstream information"), branch->name);
 
 		strbuf_addf(&buf, "branch.%s.remote", branch->name);
-		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "branch.%s.merge", branch->name);
-		git_config_set_multivar(buf.buf, NULL, NULL, 1);
+		git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 		strbuf_release(&buf);
 	} else if (argc > 0 && argc <= 2) {
 		if (filter.kind != FILTER_REFS_BRANCHES)

index 5e39f61885..e7c7f3d455 100644
@@ -823,7 +823,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		value = normalize_value(argv[0], argv[1]);
 		UNLEAK(value);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
-							      argv[0], value, argv[2], 1);
+							      argv[0], value, argv[2],
+							      CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_GET) {
 		check_argc(argc, 1, 2);

@@ -859,7 +860,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		check_write();
 		check_argc(argc, 1, 2);
 		return git_config_set_multivar_in_file_gently(given_config_source.file,
-							      argv[0], NULL, argv[1], 1);
+							      argv[0], NULL, argv[1],
+							      CONFIG_FLAGS_MULTI_REPLACE);
 	}
 	else if (actions == ACTION_RENAME_SECTION) {
 		int ret;

index c8240e9fcd..29b1652975 100644
@@ -708,7 +708,7 @@ static int mv(int argc, const char **argv)
 
 	strbuf_reset(&buf);
 	strbuf_addf(&buf, "remote.%s.fetch", rename.new_name);
-	git_config_set_multivar(buf.buf, NULL, NULL, 1);
+	git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 	strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old_name);
 	for (i = 0; i < oldremote->fetch.raw_nr; i++) {
 		char *ptr;