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 <dturner@xxxxxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html