From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 10 Mar 2017 21:24:20 +0000 (-0800)
Subject: Merge branch 'jc/config-case-cmdline-take-2'
X-Git-Tag: v2.13.0-rc0~151
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/2f54451ff56f22c6da6e0266a3508d54ab34a14f?ds=inline;hp=-c

Merge branch 'jc/config-case-cmdline-take-2'

The code to parse "git -c VAR=VAL cmd" and set configuration
variable for the duration of cmd had two small bugs, which have
been fixed.

* jc/config-case-cmdline-take-2:
config: use git_config_parse_key() in git_config_parse_parameter()
config: move a few helper functions up
---

2f54451ff56f22c6da6e0266a3508d54ab34a14f
diff --combined config.c
index c6b874a7bf,1c1a1520ff..48edc6a384
--- a/config.c
+++ b/config.c
@@@ -66,8 -66,6 +66,8 @@@ static struct key_value_info *current_c
   */
  static enum config_scope current_parsing_scope;
  
 +static int core_compression_seen;
 +static int pack_compression_seen;
  static int zlib_compression_seen;
  
  /*
@@@ -201,11 -199,105 +201,105 @@@ void git_config_push_parameter(const ch
  	strbuf_release(&env);
  }
  
+ static inline int iskeychar(int c)
+ {
+ 	return isalnum(c) || c == '-';
+ }
+ 
+ /*
+  * Auxiliary function to sanity-check and split the key into the section
+  * identifier and variable name.
+  *
+  * Returns 0 on success, -1 when there is an invalid character in the key and
+  * -2 if there is no section name in the key.
+  *
+  * store_key - pointer to char* which will hold a copy of the key with
+  *             lowercase section and variable name
+  * baselen - pointer to int which will hold the length of the
+  *           section + subsection part, can be NULL
+  */
+ static int git_config_parse_key_1(const char *key, char **store_key, int *baselen_, int quiet)
+ {
+ 	int i, dot, baselen;
+ 	const char *last_dot = strrchr(key, '.');
+ 
+ 	/*
+ 	 * Since "key" actually contains the section name and the real
+ 	 * key name separated by a dot, we have to know where the dot is.
+ 	 */
+ 
+ 	if (last_dot == NULL || last_dot == key) {
+ 		if (!quiet)
+ 			error("key does not contain a section: %s", key);
+ 		return -CONFIG_NO_SECTION_OR_NAME;
+ 	}
+ 
+ 	if (!last_dot[1]) {
+ 		if (!quiet)
+ 			error("key does not contain variable name: %s", key);
+ 		return -CONFIG_NO_SECTION_OR_NAME;
+ 	}
+ 
+ 	baselen = last_dot - key;
+ 	if (baselen_)
+ 		*baselen_ = baselen;
+ 
+ 	/*
+ 	 * Validate the key and while at it, lower case it for matching.
+ 	 */
+ 	if (store_key)
+ 		*store_key = xmallocz(strlen(key));
+ 
+ 	dot = 0;
+ 	for (i = 0; key[i]; i++) {
+ 		unsigned char c = key[i];
+ 		if (c == '.')
+ 			dot = 1;
+ 		/* Leave the extended basename untouched.. */
+ 		if (!dot || i > baselen) {
+ 			if (!iskeychar(c) ||
+ 			    (i == baselen + 1 && !isalpha(c))) {
+ 				if (!quiet)
+ 					error("invalid key: %s", key);
+ 				goto out_free_ret_1;
+ 			}
+ 			c = tolower(c);
+ 		} else if (c == '\n') {
+ 			if (!quiet)
+ 				error("invalid key (newline): %s", key);
+ 			goto out_free_ret_1;
+ 		}
+ 		if (store_key)
+ 			(*store_key)[i] = c;
+ 	}
+ 
+ 	return 0;
+ 
+ out_free_ret_1:
+ 	if (store_key) {
+ 		free(*store_key);
+ 		*store_key = NULL;
+ 	}
+ 	return -CONFIG_INVALID_KEY;
+ }
+ 
+ int git_config_parse_key(const char *key, char **store_key, int *baselen)
+ {
+ 	return git_config_parse_key_1(key, store_key, baselen, 0);
+ }
+ 
+ int git_config_key_is_valid(const char *key)
+ {
+ 	return !git_config_parse_key_1(key, NULL, NULL, 1);
+ }
+ 
  int git_config_parse_parameter(const char *text,
  			       config_fn_t fn, void *data)
  {
  	const char *value;
+ 	char *canonical_name;
  	struct strbuf **pair;
+ 	int ret;
  
  	pair = strbuf_split_str(text, '=', 2);
  	if (!pair[0])
@@@ -223,13 -315,15 +317,15 @@@
  		strbuf_list_free(pair);
  		return error("bogus config parameter: %s", text);
  	}
- 	strbuf_tolower(pair[0]);
- 	if (fn(pair[0]->buf, value, data) < 0) {
- 		strbuf_list_free(pair);
- 		return -1;
+ 
+ 	if (git_config_parse_key(pair[0]->buf, &canonical_name, NULL)) {
+ 		ret = -1;
+ 	} else {
+ 		ret = (fn(canonical_name, value, data) < 0) ? -1 : 0;
+ 		free(canonical_name);
  	}
  	strbuf_list_free(pair);
- 	return 0;
+ 	return ret;
  }
  
  int git_config_from_parameters(config_fn_t fn, void *data)
@@@ -356,11 -450,6 +452,6 @@@ static char *parse_value(void
  	}
  }
  
- static inline int iskeychar(int c)
- {
- 	return isalnum(c) || c == '-';
- }
- 
  static int get_value(config_fn_t fn, void *data, struct strbuf *name)
  {
  	int c;
@@@ -826,12 -915,7 +917,12 @@@ static int git_default_core_config(cons
  	}
  
  	if (!strcmp(var, "core.logallrefupdates")) {
 -		log_all_ref_updates = git_config_bool(var, value);
 +		if (value && !strcasecmp(value, "always"))
 +			log_all_ref_updates = LOG_REFS_ALWAYS;
 +		else if (git_config_bool(var, value))
 +			log_all_ref_updates = LOG_REFS_NORMAL;
 +		else
 +			log_all_ref_updates = LOG_REFS_NONE;
  		return 0;
  	}
  
@@@ -841,22 -925,13 +932,22 @@@
  	}
  
  	if (!strcmp(var, "core.abbrev")) {
 -		int abbrev = git_config_int(var, value);
 -		if (abbrev < minimum_abbrev || abbrev > 40)
 -			return -1;
 -		default_abbrev = abbrev;
 +		if (!value)
 +			return config_error_nonbool(var);
 +		if (!strcasecmp(value, "auto"))
 +			default_abbrev = -1;
 +		else {
 +			int abbrev = git_config_int(var, value);
 +			if (abbrev < minimum_abbrev || abbrev > 40)
 +				return error("abbrev length out of range: %d", abbrev);
 +			default_abbrev = abbrev;
 +		}
  		return 0;
  	}
  
 +	if (!strcmp(var, "core.disambiguate"))
 +		return set_disambiguate_hint_config(var, value);
 +
  	if (!strcmp(var, "core.loosecompression")) {
  		int level = git_config_int(var, value);
  		if (level == -1)
@@@ -878,8 -953,6 +969,8 @@@
  		core_compression_seen = 1;
  		if (!zlib_compression_seen)
  			zlib_compression_level = level;
 +		if (!pack_compression_seen)
 +			pack_compression_level = level;
  		return 0;
  	}
  
@@@ -945,6 -1018,9 +1036,6 @@@
  		return 0;
  	}
  
 -	if (!strcmp(var, "core.pager"))
 -		return git_config_string(&pager_program, var, value);
 -
  	if (!strcmp(var, "core.editor"))
  		return git_config_string(&editor_program, var, value);
  
@@@ -1140,18 -1216,6 +1231,18 @@@ int git_default_config(const char *var
  		pack_size_limit_cfg = git_config_ulong(var, value);
  		return 0;
  	}
 +
 +	if (!strcmp(var, "pack.compression")) {
 +		int level = git_config_int(var, value);
 +		if (level == -1)
 +			level = Z_DEFAULT_COMPRESSION;
 +		else if (level < 0 || level > Z_BEST_COMPRESSION)
 +			die(_("bad pack compression level %d"), level);
 +		pack_compression_level = level;
 +		pack_compression_seen = 1;
 +		return 0;
 +	}
 +
  	/* Add other config variables here and to Documentation/config.txt. */
  	return 0;
  }
@@@ -1241,10 -1305,10 +1332,10 @@@ int git_config_from_mem(config_fn_t fn
  	return do_config_from(&top, fn, data);
  }
  
 -static int git_config_from_blob_sha1(config_fn_t fn,
 -				     const char *name,
 -				     const unsigned char *sha1,
 -				     void *data)
 +int git_config_from_blob_sha1(config_fn_t fn,
 +			      const char *name,
 +			      const unsigned char *sha1,
 +			      void *data)
  {
  	enum object_type type;
  	char *buf;
@@@ -1316,7 -1380,7 +1407,7 @@@ static int do_git_config_sequence(confi
  	int ret = 0;
  	char *xdg_config = xdg_config_home("config");
  	char *user_config = expand_user_path("~/.gitconfig");
 -	char *repo_config = git_pathdup("config");
 +	char *repo_config = have_git_dir() ? git_pathdup("config") : NULL;
  
  	current_parsing_scope = CONFIG_SCOPE_SYSTEM;
  	if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))
@@@ -1989,93 -2053,6 +2080,6 @@@ void git_config_set(const char *key, co
  	git_config_set_multivar(key, value, NULL, 0);
  }
  
- /*
-  * Auxiliary function to sanity-check and split the key into the section
-  * identifier and variable name.
-  *
-  * Returns 0 on success, -1 when there is an invalid character in the key and
-  * -2 if there is no section name in the key.
-  *
-  * store_key - pointer to char* which will hold a copy of the key with
-  *             lowercase section and variable name
-  * baselen - pointer to int which will hold the length of the
-  *           section + subsection part, can be NULL
-  */
- static int git_config_parse_key_1(const char *key, char **store_key, int *baselen_, int quiet)
- {
- 	int i, dot, baselen;
- 	const char *last_dot = strrchr(key, '.');
- 
- 	/*
- 	 * Since "key" actually contains the section name and the real
- 	 * key name separated by a dot, we have to know where the dot is.
- 	 */
- 
- 	if (last_dot == NULL || last_dot == key) {
- 		if (!quiet)
- 			error("key does not contain a section: %s", key);
- 		return -CONFIG_NO_SECTION_OR_NAME;
- 	}
- 
- 	if (!last_dot[1]) {
- 		if (!quiet)
- 			error("key does not contain variable name: %s", key);
- 		return -CONFIG_NO_SECTION_OR_NAME;
- 	}
- 
- 	baselen = last_dot - key;
- 	if (baselen_)
- 		*baselen_ = baselen;
- 
- 	/*
- 	 * Validate the key and while at it, lower case it for matching.
- 	 */
- 	if (store_key)
- 		*store_key = xmallocz(strlen(key));
- 
- 	dot = 0;
- 	for (i = 0; key[i]; i++) {
- 		unsigned char c = key[i];
- 		if (c == '.')
- 			dot = 1;
- 		/* Leave the extended basename untouched.. */
- 		if (!dot || i > baselen) {
- 			if (!iskeychar(c) ||
- 			    (i == baselen + 1 && !isalpha(c))) {
- 				if (!quiet)
- 					error("invalid key: %s", key);
- 				goto out_free_ret_1;
- 			}
- 			c = tolower(c);
- 		} else if (c == '\n') {
- 			if (!quiet)
- 				error("invalid key (newline): %s", key);
- 			goto out_free_ret_1;
- 		}
- 		if (store_key)
- 			(*store_key)[i] = c;
- 	}
- 
- 	return 0;
- 
- out_free_ret_1:
- 	if (store_key) {
- 		free(*store_key);
- 		*store_key = NULL;
- 	}
- 	return -CONFIG_INVALID_KEY;
- }
- 
- int git_config_parse_key(const char *key, char **store_key, int *baselen)
- {
- 	return git_config_parse_key_1(key, store_key, baselen, 0);
- }
- 
- int git_config_key_is_valid(const char *key)
- {
- 	return !git_config_parse_key_1(key, NULL, NULL, 1);
- }
- 
  /*
   * If value==NULL, unset in (remove from) config,
   * if value_regex!=NULL, disregard key/value pairs where value does not match.
@@@ -2221,12 -2198,7 +2225,12 @@@ int git_config_set_multivar_in_file_gen
  			goto out_free;
  		}
  
 -		fstat(in_fd, &st);
 +		if (fstat(in_fd, &st) == -1) {
 +			error_errno(_("fstat on %s failed"), config_filename);
 +			ret = CONFIG_INVALID_FILE;
 +			goto out_free;
 +		}
 +
  		contents_sz = xsize_t(st.st_size);
  		contents = xmmap_gently(NULL, contents_sz, PROT_READ,
  					MAP_PRIVATE, in_fd, 0);
@@@ -2428,7 -2400,7 +2432,7 @@@ int git_config_rename_section_in_file(c
  
  	if (new_name && !section_name_is_ok(new_name)) {
  		ret = error("invalid section name: %s", new_name);
 -		goto out;
 +		goto out_no_rollback;
  	}
  
  	if (!config_filename)
@@@ -2443,13 -2415,10 +2447,13 @@@
  
  	if (!(config_file = fopen(config_filename, "rb"))) {
  		/* no config file means nothing to rename, no error */
 -		goto unlock_and_out;
 +		goto commit_and_out;
  	}
  
 -	fstat(fileno(config_file), &st);
 +	if (fstat(fileno(config_file), &st) == -1) {
 +		ret = error_errno(_("fstat on %s failed"), config_filename);
 +		goto out;
 +	}
  
  	if (chmod(get_lock_file_path(lock), st.st_mode & 07777) < 0) {
  		ret = error_errno("chmod on %s failed",
@@@ -2505,13 -2474,11 +2509,13 @@@
  		}
  	}
  	fclose(config_file);
 -unlock_and_out:
 +commit_and_out:
  	if (commit_lock_file(lock) < 0)
  		ret = error_errno("could not write config file %s",
  				  config_filename);
  out:
 +	rollback_lock_file(lock);
 +out_no_rollback:
  	free(filename_buf);
  	return ret;
  }
diff --combined t/t1300-repo-config.sh
index 052f120216,ea371020fa..afcca0d52c
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@@ -1097,6 -1097,68 +1097,68 @@@ test_expect_success 'multiple git -c ap
  	test_cmp expect actual
  '
  
+ test_expect_success 'last one wins: two level vars' '
+ 
+ 	# sec.var and sec.VAR are the same variable, as the first
+ 	# and the last level of a configuration variable name is
+ 	# case insensitive.
+ 
+ 	echo VAL >expect &&
+ 
+ 	git -c sec.var=val -c sec.VAR=VAL config --get sec.var >actual &&
+ 	test_cmp expect actual &&
+ 	git -c SEC.var=val -c sec.var=VAL config --get sec.var >actual &&
+ 	test_cmp expect actual &&
+ 
+ 	git -c sec.var=val -c sec.VAR=VAL config --get SEC.var >actual &&
+ 	test_cmp expect actual &&
+ 	git -c SEC.var=val -c sec.var=VAL config --get sec.VAR >actual &&
+ 	test_cmp expect actual
+ '
+ 
+ test_expect_success 'last one wins: three level vars' '
+ 
+ 	# v.a.r and v.A.r are not the same variable, as the middle
+ 	# level of a three-level configuration variable name is
+ 	# case sensitive.
+ 
+ 	echo val >expect &&
+ 	git -c v.a.r=val -c v.A.r=VAL config --get v.a.r >actual &&
+ 	test_cmp expect actual &&
+ 	git -c v.a.r=val -c v.A.r=VAL config --get V.a.R >actual &&
+ 	test_cmp expect actual &&
+ 
+ 	# v.a.r and V.a.R are the same variable, as the first
+ 	# and the last level of a configuration variable name is
+ 	# case insensitive.
+ 
+ 	echo VAL >expect &&
+ 	git -c v.a.r=val -c v.a.R=VAL config --get v.a.r >actual &&
+ 	test_cmp expect actual &&
+ 	git -c v.a.r=val -c V.a.r=VAL config --get v.a.r >actual &&
+ 	test_cmp expect actual &&
+ 	git -c v.a.r=val -c v.a.R=VAL config --get V.a.R >actual &&
+ 	test_cmp expect actual &&
+ 	git -c v.a.r=val -c V.a.r=VAL config --get V.a.R >actual &&
+ 	test_cmp expect actual
+ '
+ 
+ for VAR in a .a a. a.0b a."b c". a."b c".0d
+ do
+ 	test_expect_success "git -c $VAR=VAL rejects invalid '$VAR'" '
+ 		test_must_fail git -c "$VAR=VAL" config -l
+ 	'
+ done
+ 
+ for VAR in a.b a."b c".d
+ do
+ 	test_expect_success "git -c $VAR=VAL works with valid '$VAR'" '
+ 		echo VAL >expect &&
+ 		git -c "$VAR=VAL" config --get "$VAR" >actual &&
+ 		test_cmp expect actual
+ 	'
+ done
+ 
  test_expect_success 'git -c is not confused by empty environment' '
  	GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list
  '
@@@ -1177,111 -1239,6 +1239,111 @@@ test_expect_success 'urlmatch' 
  	test_cmp expect actual
  '
  
 +test_expect_success 'urlmatch favors more specific URLs' '
 +	cat >.git/config <<-\EOF &&
 +	[http "https://example.com/"]
 +		cookieFile = /tmp/root.txt
 +	[http "https://example.com/subdirectory"]
 +		cookieFile = /tmp/subdirectory.txt
 +	[http "https://user@example.com/"]
 +		cookieFile = /tmp/user.txt
 +	[http "https://averylonguser@example.com/"]
 +		cookieFile = /tmp/averylonguser.txt
 +	[http "https://preceding.example.com"]
 +		cookieFile = /tmp/preceding.txt
 +	[http "https://*.example.com"]
 +		cookieFile = /tmp/wildcard.txt
 +	[http "https://*.example.com/wildcardwithsubdomain"]
 +		cookieFile = /tmp/wildcardwithsubdomain.txt
 +	[http "https://trailing.example.com"]
 +		cookieFile = /tmp/trailing.txt
 +	[http "https://user@*.example.com/"]
 +		cookieFile = /tmp/wildcardwithuser.txt
 +	[http "https://sub.example.com/"]
 +		cookieFile = /tmp/sub.txt
 +	EOF
 +
 +	echo http.cookiefile /tmp/root.txt >expect &&
 +	git config --get-urlmatch HTTP https://example.com >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.cookiefile /tmp/subdirectory.txt >expect &&
 +	git config --get-urlmatch HTTP https://example.com/subdirectory >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.cookiefile /tmp/subdirectory.txt >expect &&
 +	git config --get-urlmatch HTTP https://example.com/subdirectory/nested >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.cookiefile /tmp/user.txt >expect &&
 +	git config --get-urlmatch HTTP https://user@example.com/ >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.cookiefile /tmp/subdirectory.txt >expect &&
 +	git config --get-urlmatch HTTP https://averylonguser@example.com/subdirectory >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.cookiefile /tmp/preceding.txt >expect &&
 +	git config --get-urlmatch HTTP https://preceding.example.com >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.cookiefile /tmp/wildcard.txt >expect &&
 +	git config --get-urlmatch HTTP https://wildcard.example.com >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.cookiefile /tmp/sub.txt >expect &&
 +	git config --get-urlmatch HTTP https://sub.example.com/wildcardwithsubdomain >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.cookiefile /tmp/trailing.txt >expect &&
 +	git config --get-urlmatch HTTP https://trailing.example.com >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.cookiefile /tmp/sub.txt >expect &&
 +	git config --get-urlmatch HTTP https://user@sub.example.com >actual &&
 +	test_cmp expect actual
 +'
 +
 +test_expect_success 'urlmatch with wildcard' '
 +	cat >.git/config <<-\EOF &&
 +	[http]
 +		sslVerify
 +	[http "https://*.example.com"]
 +		sslVerify = false
 +		cookieFile = /tmp/cookie.txt
 +	EOF
 +
 +	test_expect_code 1 git config --bool --get-urlmatch doesnt.exist https://good.example.com >actual &&
 +	test_must_be_empty actual &&
 +
 +	echo true >expect &&
 +	git config --bool --get-urlmatch http.SSLverify https://example.com >actual &&
 +	test_cmp expect actual &&
 +
 +	echo true >expect &&
 +	git config --bool --get-urlmatch http.SSLverify https://good-example.com >actual &&
 +	test_cmp expect actual &&
 +
 +	echo true >expect &&
 +	git config --bool --get-urlmatch http.sslverify https://deep.nested.example.com >actual &&
 +	test_cmp expect actual &&
 +
 +	echo false >expect &&
 +	git config --bool --get-urlmatch http.sslverify https://good.example.com >actual &&
 +	test_cmp expect actual &&
 +
 +	{
 +		echo http.cookiefile /tmp/cookie.txt &&
 +		echo http.sslverify false
 +	} >expect &&
 +	git config --get-urlmatch HTTP https://good.example.com >actual &&
 +	test_cmp expect actual &&
 +
 +	echo http.sslverify >expect &&
 +	git config --get-urlmatch HTTP https://more.example.com.au >actual &&
 +	test_cmp expect actual
 +'
 +
  # good section hygiene
  test_expect_failure 'unsetting the last key in a section removes header' '
  	cat >.git/config <<-\EOF &&