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

Re: Git autocorrect bug

2014-06-05 Thread Øystein Walle
David Turner dturner at 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


Re: Git autocorrect bug

2014-06-05 Thread Øystein Walle
David Turner dturner at 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-05 Thread Duy Nguyen
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? 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-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 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.  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


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