From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 24 Jun 2017 21:28:39 +0000 (-0700)
Subject: Merge branch 'js/alias-early-config'
X-Git-Tag: v2.14.0-rc0~61
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/1c3d87cf5594cc7e57d71c7b2d99dd4982285951?hp=-c

Merge branch 'js/alias-early-config'

The code to pick up and execute command alias definition from the
configuration used to switch to the top of the working tree and
then come back when the expanded alias was executed, which was
unnecessarilyl complex.  Attempt to simplify the logic by using the
early-config mechanism that does not chdir around.

* js/alias-early-config:
alias: use the early config machinery to expand aliases
t7006: demonstrate a problem with aliases in subdirectories
t1308: relax the test verifying that empty alias values are disallowed
help: use early config when autocorrecting aliases
config: report correct line number upon error
discover_git_directory(): avoid setting invalid git_dir
---

1c3d87cf5594cc7e57d71c7b2d99dd4982285951
diff --combined config.c
index 34a139c40b,3df7515db2..547daf87d4
--- a/config.c
+++ b/config.c
@@@ -214,7 -214,6 +214,7 @@@ static int include_by_gitdir(const stru
  	struct strbuf pattern = STRBUF_INIT;
  	int ret = 0, prefix;
  	const char *git_dir;
 +	int already_tried_absolute = 0;
  
  	if (opts->git_dir)
  		git_dir = opts->git_dir;
@@@ -227,7 -226,6 +227,7 @@@
  	strbuf_add(&pattern, cond, cond_len);
  	prefix = prepare_include_condition_pattern(&pattern);
  
 +again:
  	if (prefix < 0)
  		goto done;
  
@@@ -247,20 -245,6 +247,20 @@@
  	ret = !wildmatch(pattern.buf + prefix, text.buf + prefix,
  			 icase ? WM_CASEFOLD : 0, NULL);
  
 +	if (!ret && !already_tried_absolute) {
 +		/*
 +		 * We've tried e.g. matching gitdir:~/work, but if
 +		 * ~/work is a symlink to /mnt/storage/work
 +		 * strbuf_realpath() will expand it, so the rule won't
 +		 * match. Let's match against a
 +		 * strbuf_add_absolute_path() version of the path,
 +		 * which'll do the right thing
 +		 */
 +		strbuf_reset(&text);
 +		strbuf_add_absolute_path(&text, git_dir);
 +		already_tried_absolute = 1;
 +		goto again;
 +	}
  done:
  	strbuf_release(&pattern);
  	strbuf_release(&text);
@@@ -604,7 -588,8 +604,8 @@@ static int get_value(config_fn_t fn, vo
  	 */
  	cf->linenr--;
  	ret = fn(name->buf, value, data);
- 	cf->linenr++;
+ 	if (ret >= 0)
+ 		cf->linenr++;
  	return ret;
  }
  
@@@ -1438,7 -1423,7 +1439,7 @@@ int git_config_from_file(config_fn_t fn
  	int ret = -1;
  	FILE *f;
  
 -	f = fopen(filename, "r");
 +	f = fopen_or_warn(filename, "r");
  	if (f) {
  		flockfile(f);
  		ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename, filename, f, data);
@@@ -1981,7 -1966,7 +1982,7 @@@ int git_config_get_expiry(const char *k
  	if (ret)
  		return ret;
  	if (strcmp(*output, "now")) {
 -		unsigned long now = approxidate("now");
 +		timestamp_t now = approxidate("now");
  		if (approxidate(*output) >= now)
  			git_die_config(key, _("Invalid %s: '%s'"), key, *output);
  	}
@@@ -2637,7 -2622,7 +2638,7 @@@ int git_config_rename_section_in_file(c
  	struct lock_file *lock;
  	int out_fd;
  	char buf[1024];
 -	FILE *config_file;
 +	FILE *config_file = NULL;
  	struct stat st;
  
  	if (new_name && !section_name_is_ok(new_name)) {
@@@ -2656,9 -2641,6 +2657,9 @@@
  	}
  
  	if (!(config_file = fopen(config_filename, "rb"))) {
 +		ret = warn_on_fopen_errors(config_filename);
 +		if (ret)
 +			goto out;
  		/* no config file means nothing to rename, no error */
  		goto commit_and_out;
  	}
@@@ -2722,14 -2704,11 +2723,14 @@@
  		}
  	}
  	fclose(config_file);
 +	config_file = NULL;
  commit_and_out:
  	if (commit_lock_file(lock) < 0)
  		ret = error_errno("could not write config file %s",
  				  config_filename);
  out:
 +	if (config_file)
 +		fclose(config_file);
  	rollback_lock_file(lock);
  out_no_rollback:
  	free(filename_buf);
diff --combined git.c
index 1b8b7f51a6,58ef570294..6150d6d281
--- a/git.c
+++ b/git.c
@@@ -16,53 -16,7 +16,9 @@@ const char git_more_info_string[] 
  	   "to read about a specific subcommand or concept.");
  
  static int use_pager = -1;
- static char *orig_cwd;
- static const char *env_names[] = {
- 	GIT_DIR_ENVIRONMENT,
- 	GIT_WORK_TREE_ENVIRONMENT,
- 	GIT_IMPLICIT_WORK_TREE_ENVIRONMENT,
- 	GIT_PREFIX_ENVIRONMENT
- };
- static char *orig_env[4];
- static int save_restore_env_balance;
  
 +static void list_builtins(void);
 +
- static void save_env_before_alias(void)
- {
- 	int i;
- 
- 	assert(save_restore_env_balance == 0);
- 	save_restore_env_balance = 1;
- 	orig_cwd = xgetcwd();
- 	for (i = 0; i < ARRAY_SIZE(env_names); i++) {
- 		orig_env[i] = getenv(env_names[i]);
- 		orig_env[i] = xstrdup_or_null(orig_env[i]);
- 	}
- }
- 
- static void restore_env(int external_alias)
- {
- 	int i;
- 
- 	assert(save_restore_env_balance == 1);
- 	save_restore_env_balance = 0;
- 	if (!external_alias && orig_cwd && chdir(orig_cwd))
- 		die_errno("could not move to %s", orig_cwd);
- 	free(orig_cwd);
- 	for (i = 0; i < ARRAY_SIZE(env_names); i++) {
- 		if (external_alias &&
- 		    !strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))
- 			continue;
- 		if (orig_env[i]) {
- 			setenv(env_names[i], orig_env[i], 1);
- 			free(orig_env[i]);
- 		} else {
- 			unsetenv(env_names[i]);
- 		}
- 	}
- }
- 
  static void commit_pager_choice(void) {
  	switch (use_pager) {
  	case 0:
@@@ -234,9 -188,6 +190,9 @@@ static int handle_options(const char **
  			}
  			(*argv)++;
  			(*argc)--;
 +		} else if (!strcmp(cmd, "--list-builtins")) {
 +			list_builtins();
 +			exit(0);
  		} else {
  			fprintf(stderr, "Unknown option: %s\n", cmd);
  			usage(git_usage_string);
@@@ -255,19 -206,18 +211,18 @@@ static int handle_alias(int *argcp, con
  	const char **new_argv;
  	const char *alias_command;
  	char *alias_string;
- 	int unused_nongit;
- 
- 	save_env_before_alias();
- 	setup_git_directory_gently(&unused_nongit);
  
  	alias_command = (*argv)[0];
  	alias_string = alias_lookup(alias_command);
  	if (alias_string) {
  		if (alias_string[0] == '!') {
  			struct child_process child = CHILD_PROCESS_INIT;
+ 			int nongit_ok;
+ 
+ 			/* Aliases expect GIT_PREFIX, GIT_DIR etc to be set */
+ 			setup_git_directory_gently(&nongit_ok);
  
  			commit_pager_choice();
- 			restore_env(1);
  
  			child.use_shell = 1;
  			argv_array_push(&child.args, alias_string + 1);
@@@ -313,8 -263,6 +268,6 @@@
  		ret = 1;
  	}
  
- 	restore_env(0);
- 
  	errno = saved_errno;
  
  	return ret;
@@@ -534,13 -482,6 +487,13 @@@ int is_builtin(const char *s
  	return !!get_builtin(s);
  }
  
 +static void list_builtins(void)
 +{
 +	int i;
 +	for (i = 0; i < ARRAY_SIZE(commands); i++)
 +		printf("%s\n", commands[i].cmd);
 +}
 +
  #ifdef STRIP_EXTENSION
  static void strip_extension(const char **argv)
  {
diff --combined help.c
index f637fc8006,91ec8ab36f..5cdfac7dc2
--- a/help.c
+++ b/help.c
@@@ -1,7 -1,6 +1,7 @@@
  #include "cache.h"
  #include "builtin.h"
  #include "exec_cmd.h"
 +#include "run-command.h"
  #include "levenshtein.h"
  #include "help.h"
  #include "common-cmds.h"
@@@ -9,7 -8,6 +9,7 @@@
  #include "column.h"
  #include "version.h"
  #include "refs.h"
 +#include "parse-options.h"
  
  void add_cmdname(struct cmdnames *cmds, const char *name, int len)
  {
@@@ -98,6 -96,48 +98,6 @@@ static void pretty_print_cmdnames(struc
  	string_list_clear(&list, 0);
  }
  
 -static int is_executable(const char *name)
 -{
 -	struct stat st;
 -
 -	if (stat(name, &st) || /* stat, not lstat */
 -	    !S_ISREG(st.st_mode))
 -		return 0;
 -
 -#if defined(GIT_WINDOWS_NATIVE)
 -	/*
 -	 * On Windows there is no executable bit. The file extension
 -	 * indicates whether it can be run as an executable, and Git
 -	 * has special-handling to detect scripts and launch them
 -	 * through the indicated script interpreter. We test for the
 -	 * file extension first because virus scanners may make
 -	 * it quite expensive to open many files.
 -	 */
 -	if (ends_with(name, ".exe"))
 -		return S_IXUSR;
 -
 -{
 -	/*
 -	 * Now that we know it does not have an executable extension,
 -	 * peek into the file instead.
 -	 */
 -	char buf[3] = { 0 };
 -	int n;
 -	int fd = open(name, O_RDONLY);
 -	st.st_mode &= ~S_IXUSR;
 -	if (fd >= 0) {
 -		n = read(fd, buf, 2);
 -		if (n == 2)
 -			/* look for a she-bang */
 -			if (!strcmp(buf, "#!"))
 -				st.st_mode |= S_IXUSR;
 -		close(fd);
 -	}
 -}
 -#endif
 -	return st.st_mode & S_IXUSR;
 -}
 -
  static void list_commands_in_dir(struct cmdnames *cmds,
  					 const char *path,
  					 const char *prefix)
@@@ -290,7 -330,7 +290,7 @@@ const char *help_unknown_cmd(const cha
  	memset(&other_cmds, 0, sizeof(other_cmds));
  	memset(&aliases, 0, sizeof(aliases));
  
- 	git_config(git_unknown_cmd_config, NULL);
+ 	read_early_config(git_unknown_cmd_config, NULL);
  
  	load_command_list("git-", &main_cmds, &other_cmds);
  
@@@ -371,8 -411,8 +371,8 @@@
  
  	if (SIMILAR_ENOUGH(best_similarity)) {
  		fprintf_ln(stderr,
 -			   Q_("\nDid you mean this?",
 -			      "\nDid you mean one of these?",
 +			   Q_("\nThe most similar command is",
 +			      "\nThe most similar commands are",
  			   n));
  
  		for (i = 0; i < n; i++)
@@@ -384,30 -424,16 +384,30 @@@
  
  int cmd_version(int argc, const char **argv, const char *prefix)
  {
 +	int build_options = 0;
 +	const char * const usage[] = {
 +		N_("git version [<options>]"),
 +		NULL
 +	};
 +	struct option options[] = {
 +		OPT_BOOL(0, "build-options", &build_options,
 +			 "also print build options"),
 +		OPT_END()
 +	};
 +
 +	argc = parse_options(argc, argv, prefix, options, usage, 0);
 +
  	/*
  	 * The format of this string should be kept stable for compatibility
  	 * with external projects that rely on the output of "git version".
 +	 *
 +	 * Always show the version, even if other options are given.
  	 */
  	printf("git version %s\n", git_version_string);
 -	while (*++argv) {
 -		if (!strcmp(*argv, "--build-options")) {
 -			printf("sizeof-long: %d\n", (int)sizeof(long));
 -			/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
 -		}
 +
 +	if (build_options) {
 +		printf("sizeof-long: %d\n", (int)sizeof(long));
 +		/* NEEDSWORK: also save and output GIT-BUILD_OPTIONS? */
  	}
  	return 0;
  }
diff --combined setup.c
index 751d02b9be,ba3241bf02..ae3e0ddda2
--- a/setup.c
+++ b/setup.c
@@@ -134,27 -134,23 +134,27 @@@ int path_inside_repo(const char *prefix
  
  int check_filename(const char *prefix, const char *arg)
  {
 -	const char *name;
  	char *to_free = NULL;
  	struct stat st;
  
 -	if (starts_with(arg, ":/")) {
 -		if (arg[2] == '\0') /* ":/" is root dir, always exists */
 +	if (skip_prefix(arg, ":/", &arg)) {
 +		if (!*arg) /* ":/" is root dir, always exists */
  			return 1;
 -		name = arg + 2;
 -	} else if (prefix)
 -		name = to_free = prefix_filename(prefix, arg);
 -	else
 -		name = arg;
 -	if (!lstat(name, &st)) {
 +		prefix = NULL;
 +	} else if (skip_prefix(arg, ":!", &arg) ||
 +		   skip_prefix(arg, ":^", &arg)) {
 +		if (!*arg) /* excluding everything is silly, but allowed */
 +			return 1;
 +	}
 +
 +	if (prefix)
 +		arg = to_free = prefix_filename(prefix, arg);
 +
 +	if (!lstat(arg, &st)) {
  		free(to_free);
  		return 1; /* file exists */
  	}
 -	if (errno == ENOENT || errno == ENOTDIR) {
 +	if (is_missing_file_error(errno)) {
  		free(to_free);
  		return 0; /* file does not exist */
  	}
@@@ -185,24 -181,6 +185,24 @@@ static void NORETURN die_verify_filenam
  
  }
  
 +/*
 + * Check for arguments that don't resolve as actual files,
 + * but which look sufficiently like pathspecs that we'll consider
 + * them such for the purposes of rev/pathspec DWIM parsing.
 + */
 +static int looks_like_pathspec(const char *arg)
 +{
 +	/* anything with a wildcard character */
 +	if (!no_wildcard(arg))
 +		return 1;
 +
 +	/* long-form pathspec magic */
 +	if (starts_with(arg, ":("))
 +		return 1;
 +
 +	return 0;
 +}
 +
  /*
   * Verify a filename that we got as an argument for a pathspec
   * entry. Note that a filename that begins with "-" never verifies
@@@ -229,7 -207,7 +229,7 @@@ void verify_filename(const char *prefix
  {
  	if (*arg == '-')
  		die("bad flag '%s' used after filename", arg);
 -	if (check_filename(prefix, arg) || !no_wildcard(arg))
 +	if (looks_like_pathspec(arg) || check_filename(prefix, arg))
  		return;
  	die_verify_filename(prefix, arg, diagnose_misspelt_rev);
  }
@@@ -725,16 -703,11 +725,16 @@@ static const char *setup_discovered_git
  
  	/* --work-tree is set without --git-dir; use discovered one */
  	if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
 +		char *to_free = NULL;
 +		const char *ret;
 +
  		if (offset != cwd->len && !is_absolute_path(gitdir))
 -			gitdir = real_pathdup(gitdir, 1);
 +			gitdir = to_free = real_pathdup(gitdir, 1);
  		if (chdir(cwd->buf))
  			die_errno("Could not come back to cwd");
 -		return setup_explicit_git_dir(gitdir, cwd, nongit_ok);
 +		ret = setup_explicit_git_dir(gitdir, cwd, nongit_ok);
 +		free(to_free);
 +		return ret;
  	}
  
  	/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
@@@ -775,7 -748,7 +775,7 @@@ static const char *setup_bare_git_dir(s
  
  	/* --work-tree is set without --git-dir; use discovered one */
  	if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
 -		const char *gitdir;
 +		static const char *gitdir;
  
  		gitdir = offset == cwd->len ? "." : xmemdupz(cwd->buf, offset);
  		if (chdir(cwd->buf))
@@@ -1004,6 -977,7 +1004,7 @@@ const char *discover_git_directory(stru
  		warning("ignoring git dir '%s': %s",
  			gitdir->buf + gitdir_offset, err.buf);
  		strbuf_release(&err);
+ 		strbuf_setlen(gitdir, gitdir_offset);
  		return NULL;
  	}
  
diff --combined t/t1300-repo-config.sh
index 13b7851f7c,f664bfc671..a37ef04222
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@@ -703,6 -703,12 +703,12 @@@ test_expect_success 'invalid unit' 
  	test_i18ngrep "bad numeric config value .1auto. for .aninvalid.unit. in file .git/config: invalid unit" actual
  '
  
+ test_expect_success 'line number is reported correctly' '
+ 	printf "[bool]\n\tvar\n" >invalid &&
+ 	test_must_fail git config -f invalid --path bool.var 2>actual &&
+ 	test_i18ngrep "line 2" actual
+ '
+ 
  test_expect_success 'invalid stdin config' '
  	echo "[broken" | test_must_fail git config --list --file - >output 2>&1 &&
  	test_i18ngrep "bad config line 1 in standard input" output
@@@ -1539,10 -1545,4 +1545,10 @@@ test_expect_success !MINGW '--show-orig
  	test_cmp expect output
  '
  
 +test_expect_success '--local requires a repo' '
 +	# we expect 128 to ensure that we do not simply
 +	# fail to find anything and return code "1"
 +	test_expect_code 128 nongit git config --local foo.bar
 +'
 +
  test_done
diff --combined t/t1308-config-set.sh
index e495a61616,69a0aa56d6..bafed5c9b8
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@@ -183,22 -183,11 +183,22 @@@ test_expect_success 'proper error on no
  	test_cmp expect actual
  '
  
 +test_expect_success 'proper error on directory "files"' '
 +	echo "Error (-1) reading configuration file a-directory." >expect &&
 +	mkdir a-directory &&
 +	test_expect_code 2 test-config configset_get_value foo.bar a-directory 2>output &&
 +	grep "^warning:" output &&
 +	grep "^Error" output >actual &&
 +	test_cmp expect actual
 +'
 +
  test_expect_success POSIXPERM,SANITY 'proper error on non-accessible files' '
  	chmod -r .git/config &&
  	test_when_finished "chmod +r .git/config" &&
  	echo "Error (-1) reading configuration file .git/config." >expect &&
 -	test_expect_code 2 test-config configset_get_value foo.bar .git/config 2>actual &&
 +	test_expect_code 2 test-config configset_get_value foo.bar .git/config 2>output &&
 +	grep "^warning:" output &&
 +	grep "^Error" output >actual &&
  	test_cmp expect actual
  '
  
@@@ -226,7 -215,9 +226,9 @@@ test_expect_success 'check line errors 
  		br
  	EOF
  	test_expect_code 128 git br 2>result &&
- 	test_i18ngrep "fatal: .*alias\.br.*\.git/config.*line 2" result
+ 	test_i18ngrep "missing value for .alias\.br" result &&
+ 	test_i18ngrep "fatal: .*\.git/config" result &&
+ 	test_i18ngrep "fatal: .*line 2" result
  '
  
  test_expect_success 'error on modifying repo config without repo' '