Merge branch 'js/alias-early-config'
authorJunio C Hamano <gitster@pobox.com>
Sat, 24 Jun 2017 21:28:39 +0000 (14:28 -0700)
committerJunio C Hamano <gitster@pobox.com>
Sat, 24 Jun 2017 21:28:40 +0000 (14:28 -0700)
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

alias.c
config.c
git.c
help.c
setup.c
t/t1300-repo-config.sh
t/t1308-config-set.sh
t/t7006-pager.sh
diff --git a/alias.c b/alias.c
index 3b90397a99d9f7ed4a0c1c5a83f5e69c879e752f..05263046614e10945cb8d81ef3e42e0d5f2c9a2e 100644 (file)
--- a/alias.c
+++ b/alias.c
@@ -1,14 +1,28 @@
 #include "cache.h"
 
+struct config_alias_data {
+       const char *alias;
+       char *v;
+};
+
+static int config_alias_cb(const char *key, const char *value, void *d)
+{
+       struct config_alias_data *data = d;
+       const char *p;
+
+       if (skip_prefix(key, "alias.", &p) && !strcmp(p, data->alias))
+               return git_config_string((const char **)&data->v, key, value);
+
+       return 0;
+}
+
 char *alias_lookup(const char *alias)
 {
-       char *v = NULL;
-       struct strbuf key = STRBUF_INIT;
-       strbuf_addf(&key, "alias.%s", alias);
-       if (git_config_key_is_valid(key.buf))
-               git_config_get_string(key.buf, &v);
-       strbuf_release(&key);
-       return v;
+       struct config_alias_data data = { alias, NULL };
+
+       read_early_config(config_alias_cb, &data);
+
+       return data.v;
 }
 
 #define SPLIT_CMDLINE_BAD_ENDING 1
index 34a139c40bd57ff2dd23318beca53fd2e2948000..547daf87d40ebfe67272e913635826441dd97320 100644 (file)
--- a/config.c
+++ b/config.c
@@ -604,7 +604,8 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name)
         */
        cf->linenr--;
        ret = fn(name->buf, value, data);
-       cf->linenr++;
+       if (ret >= 0)
+               cf->linenr++;
        return ret;
 }
 
diff --git a/git.c b/git.c
index 1b8b7f51a65fa2a8e62d11d7ad99f19e2926977f..6150d6d281e5f804816bbbbd7d17d1df93aefc7d 100644 (file)
--- a/git.c
+++ b/git.c
@@ -16,53 +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:
@@ -255,19 +211,18 @@ static int handle_alias(int *argcp, const char ***argv)
        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 +268,6 @@ static int handle_alias(int *argcp, const char ***argv)
                ret = 1;
        }
 
-       restore_env(0);
-
        errno = saved_errno;
 
        return ret;
diff --git a/help.c b/help.c
index f637fc80062094784d5734df91523642933ddcfc..5cdfac7dc20d23089a1230c885dc1701a2caf1ac 100644 (file)
--- a/help.c
+++ b/help.c
@@ -290,7 +290,7 @@ const char *help_unknown_cmd(const char *cmd)
        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);
 
diff --git a/setup.c b/setup.c
index 751d02b9be627176009cdda4f1b20a3674b0dc95..ae3e0ddda22d9b517485a5c91b8d191c3e9ef92e 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -1004,6 +1004,7 @@ const char *discover_git_directory(struct strbuf *gitdir)
                warning("ignoring git dir '%s': %s",
                        gitdir->buf + gitdir_offset, err.buf);
                strbuf_release(&err);
+               strbuf_setlen(gitdir, gitdir_offset);
                return NULL;
        }
 
index 13b7851f7c2feb87f63449d917380452e85c1f84..a37ef0422212eafdae4b4c0fae9e28d4a183117f 100755 (executable)
@@ -703,6 +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
index e495a616161660bcade644dea42b5c8e53cc64b3..bafed5c9b88481992ca6cf4cd6c13596c7baf16b 100755 (executable)
@@ -226,7 +226,9 @@ test_expect_success 'check line errors for malformed values' '
                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' '
index 4f3794d415e95b0b65ca29b829ce278b45ea5dbd..20b4d83c281e2fde8812f309bdebef012c6ecb74 100755 (executable)
@@ -391,6 +391,17 @@ test_expect_success TTY 'core.pager in repo config works and retains cwd' '
        )
 '
 
+test_expect_success TTY 'core.pager is found via alias in subdirectory' '
+       sane_unset GIT_PAGER &&
+       test_config core.pager "cat >via-alias" &&
+       (
+               cd sub &&
+               rm -f via-alias &&
+               test_terminal git -c alias.r="-p rev-parse" r HEAD &&
+               test_path_is_file via-alias
+       )
+'
+
 test_doesnt_paginate      expect_failure test_must_fail 'git -p nonsense'
 
 test_pager_choices                       'git shortlog'