Re: Git autocorrect bug

2014-06-06 Thread Duy Nguyen
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  
> > 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("can

Re: Git autocorrect bug

2014-06-05 Thread David Turner
On Thu, 2014-06-05 at 13:29 +0700, Duy Nguyen wrote:
> On Thu, Jun 5, 2014 at 10:49 AM, David Turner  
> 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.  That is, I generally expect autocorrect to work without internal
errors.  This one was a genuine typo (I typed git, got distracted, and
typed it again).  

(I actually think "git git" should just be "git", and I know some people
have a command for that; I should set that up)

--
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


Re: Git autocorrect bug

2014-06-04 Thread Duy Nguyen
On Thu, Jun 5, 2014 at 10:49 AM, David Turner  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? 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?
-- 
Duy
--
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


Re: Git autocorrect bug

2014-06-04 Thread Øystein Walle
David Turner  twopensource.com> writes:

> 
> (I am extremely unlikely to fix this bug myself, since it only arises in
> very rare circumstances).
>

I see now that `git init foo` and `git git foo` (with git corrected
to init) behave differently. Is this the bug you're referring to? 


--
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


Re: Git autocorrect bug

2014-06-04 Thread Øystein Walle
David Turner  twopensource.com> writes:

> 
> $ cd [some existing git repo]
> $ git git foo
> WARNING: You called a Git command named 'git', which does not exist.
> Continuing under the assumption that you meant 'init'
> in 0.1 seconds automatically...
> fatal: internal error: work tree has already been set
> Current worktree: /home/dturner/git
> New worktree: /home/dturner/git/foo
> 
> (I am extremely unlikely to fix this bug myself, since it only arises in
> very rare circumstances).
> 
> 

You have help.autocorrect enabled. Then this is expected behaviour (it
doesn't seem out of place to me at least)

Running `git config --unset help.autocorrect` should disable it. It
may be that you enabled it for only one repo by accident.

Øsse


--
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


Git autocorrect bug

2014-06-04 Thread David Turner
$ cd [some existing git repo]
$ git git foo
WARNING: You called a Git command named 'git', which does not exist.
Continuing under the assumption that you meant 'init'
in 0.1 seconds automatically...
fatal: internal error: work tree has already been set
Current worktree: /home/dturner/git
New worktree: /home/dturner/git/foo

(I am extremely unlikely to fix this bug myself, since it only arises in
very rare circumstances).


--
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