On Thu, Jun 05, 2014 at 04:28:23AM -0400, David Turner wrote:
> On Thu, 2014-06-05 at 13:29 +0700, Duy Nguyen wrote:
> > On Thu, Jun 5, 2014 at 10:49 AM, David Turner <dtur...@twopensource.com> 
> > wrote:
> > > fatal: internal error: work tree has already been set
> > > Current worktree: /home/dturner/git
> > > New worktree: /home/dturner/git/foo
> > 
> > This is the part you complain about, right? 
> 
> Yes.
> 
> > I think I might know
> > what's going on here. But do you expect "git git foo" to turn to "git
> > init foo" in the first place?
> 
> Yes.

I was hoping you would say no so I could get away without doing
anything :) The problem is "setup pollution". When somebody looks up
an alias (autocorrect does), $GIT_DIR must be searched because
$GIT_DIR/config may have repo-local aliases. But 'git init' (and
clone) expects a clean no-setup state.

This is a known issue. You can reproduce by aliasing init to
something, then init a new repo using that alias. In fact Jonathan
wrote a few test to catch this. The solution is we start out fresh in
a new process. The fork/exec overhead should not matter because this
is interactive session.

I'm just wondering if I should remove the "only applicable to init and
clone" check in the patch because there's another companion problem:
if we find $GIT_DIR automatically, then $GIT_DIR/config points out
that work-tree must be moved, it'll get nasty because we already set
everything up for the auto-found worktree. But maybe I already solved
that, not sure..

-- 8< --
diff --git a/compat/mingw.c b/compat/mingw.c
index e9892f8..34722fe 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1080,19 +1080,6 @@ int mingw_kill(pid_t pid, int sig)
        return -1;
 }
 
-static char **copy_environ(void)
-{
-       char **env;
-       int i = 0;
-       while (environ[i])
-               i++;
-       env = xmalloc((i+1)*sizeof(*env));
-       for (i = 0; environ[i]; i++)
-               env[i] = xstrdup(environ[i]);
-       env[i] = NULL;
-       return env;
-}
-
 void free_environ(char **env)
 {
        int i;
diff --git a/git-compat-util.h b/git-compat-util.h
index 76910e6..1db4dec 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -732,4 +732,6 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);
 #define gmtime_r git_gmtime_r
 #endif
 
+char **copy_environ(void);
+
 #endif
diff --git a/git.c b/git.c
index 7780572..77d9204 100644
--- a/git.c
+++ b/git.c
@@ -20,6 +20,22 @@ const char git_more_info_string[] =
 
 static struct startup_info git_startup_info;
 static int use_pager = -1;
+static char orig_cwd[PATH_MAX];
+static char **orig_environ;
+
+static void save_env(void)
+{
+       getcwd(orig_cwd, sizeof(orig_cwd));
+       orig_environ = copy_environ();
+}
+
+static void restore_env(void)
+{
+       if (*orig_cwd && chdir(orig_cwd))
+               die_errno("could not move to %s", orig_cwd);
+       if (orig_environ)
+               environ = orig_environ;
+}
 
 static void commit_pager_choice(void) {
        switch (use_pager) {
@@ -459,7 +475,7 @@ int is_builtin(const char *s)
        return 0;
 }
 
-static void handle_builtin(int argc, const char **argv)
+static void handle_builtin(int argc, const char **argv, int preprocessed)
 {
        const char *cmd = argv[0];
        int i;
@@ -484,6 +500,11 @@ static void handle_builtin(int argc, const char **argv)
                struct cmd_struct *p = commands+i;
                if (strcmp(p->cmd, cmd))
                        continue;
+               if (preprocessed &&
+                   (p->fn == cmd_init_db || p->fn == cmd_clone)) {
+                       restore_env();
+                       break;
+               }
                exit(run_builtin(p, argc, argv));
        }
 }
@@ -524,13 +545,13 @@ static void execv_dashed_external(const char **argv)
        strbuf_release(&cmd);
 }
 
-static int run_argv(int *argcp, const char ***argv)
+static int run_argv(int *argcp, const char ***argv, int done_help)
 {
        int done_alias = 0;
 
        while (1) {
                /* See if it's a builtin */
-               handle_builtin(*argcp, *argv);
+               handle_builtin(*argcp, *argv, done_help || done_alias);
 
                /* .. then try the external ones */
                execv_dashed_external(*argv);
@@ -539,7 +560,10 @@ static int run_argv(int *argcp, const char ***argv)
                 * of overriding "git log" with "git show" by having
                 * alias.log = show
                 */
-               if (done_alias || !handle_alias(argcp, argv))
+               if (done_alias)
+                       break;
+               save_env();
+               if (!handle_alias(argcp, argv))
                        break;
                done_alias = 1;
        }
@@ -581,7 +605,7 @@ int main(int argc, char **av)
        if (starts_with(cmd, "git-")) {
                cmd += 4;
                argv[0] = cmd;
-               handle_builtin(argc, argv);
+               handle_builtin(argc, argv, 0);
                die("cannot handle %s as a builtin", cmd);
        }
 
@@ -613,7 +637,7 @@ int main(int argc, char **av)
        while (1) {
                static int done_help = 0;
                static int was_alias = 0;
-               was_alias = run_argv(&argc, &argv);
+               was_alias = run_argv(&argc, &argv, done_help);
                if (errno != ENOENT)
                        break;
                if (was_alias) {
@@ -623,6 +647,7 @@ int main(int argc, char **av)
                        exit(1);
                }
                if (!done_help) {
+                       save_env();
                        cmd = argv[0] = help_unknown_cmd(cmd);
                        done_help = 1;
                } else
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 2f30203..e62c0ff 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -56,7 +56,7 @@ test_expect_success 'plain through aliased command, outside 
any git repo' '
        check_config plain-aliased/.git false unset
 '
 
-test_expect_failure 'plain nested through aliased command' '
+test_expect_success 'plain nested through aliased command' '
        (
                git init plain-ancestor-aliased &&
                cd plain-ancestor-aliased &&
@@ -68,7 +68,7 @@ test_expect_failure 'plain nested through aliased command' '
        check_config plain-ancestor-aliased/plain-nested/.git false unset
 '
 
-test_expect_failure 'plain nested in bare through aliased command' '
+test_expect_success 'plain nested in bare through aliased command' '
        (
                git init --bare bare-ancestor-aliased.git &&
                cd bare-ancestor-aliased.git &&
diff --git a/wrapper.c b/wrapper.c
index bc1bfb8..8a82097 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -493,3 +493,16 @@ struct passwd *xgetpwuid_self(void)
                    errno ? strerror(errno) : _("no such user"));
        return pw;
 }
+
+char **copy_environ(void)
+{
+       char **env;
+       int i = 0;
+       while (environ[i])
+               i++;
+       env = xmalloc((i+1)*sizeof(*env));
+       for (i = 0; environ[i]; i++)
+               env[i] = xstrdup(environ[i]);
+       env[i] = NULL;
+       return env;
+}
-- 8< --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to