@matheusbsbs / matheus-bagliano / commits / 86d26f240f

setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..

Commit d95138e [1] attempted to fix a .git file problem by setting GIT_WORK_TREE whenever GIT_DIR is set. It sounded harmless because we handle GIT_DIR and GIT_WORK_TREE side by side for most commands, with two exceptions: git-init and git-clone. "git clone" is not happy with d95138e. This command ignores GIT_DIR but respects GIT_WORK_TREE [2] [3] which means it used to run fine from a hook, where GIT_DIR was set but GIT_WORK_TREE was not (*). With d95138e, GIT_WORK_TREE is set all the time and git-clone interprets that as "I give you order to put the worktree here", usually against the user's intention. The solution in d95138e is reverted earlier, and instead we reuse the solution from c056261 [4]. It fixed another setup-messed- up-by-alias by saving and restoring env and spawning a new process, but for git-clone and git-init only. Now we conclude that setup-messed-up-by-alias is always evil. So the env restoration is done for _all_ commands, including external ones, whenever aliases are involved. It fixes what d95138e tried to fix, without upsetting git-clone-inside-hooks. The test from d95138e remains to verify it's not broken by this. A new test is added to make sure git-clone-inside-hooks remains happy. (*) GIT_WORK_TREE was not set _most of the time_. In some cases GIT_WORK_TREE is set and git-clone will behave differently. The use of GIT_WORK_TREE to direct git-clone to put work tree elsewhere looks like a mistake because it causes surprises this way. But that's a separate story. [1] d95138e (setup: set env $GIT_WORK_TREE when work tree is set, like $GIT_DIR - 2015-06-26) [2] 2beebd2 (clone: create intermediate directories of destination repo - 2008-06-25) [3] 20ccef4 (make git-clone GIT_WORK_TREE aware - 2007-07-06) [4] c056261 (git potty: restore environments after alias expansion - 2014-06-08) Reported-by: Anthony Sottile <asottile@umich.edu> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Dec 20, 2015 at 14:50 UTC 86d26f240fcb4f287258ad459efc2b5e30e60cfd

3 files changed +36 -12

git.c

+12 -11

@@ -226,7 +226,6 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)

226 static int handle_alias(int *argcp, const char ***argv)

227 {

228 int envchanged = 0, ret = 0, saved_errno = errno;

229 - const char *subdir;

229 int count, option_count;

230 const char **new_argv;

231 const char *alias_command;

@@ -234,7 +233,7 @@ static int handle_alias(int *argcp, const char ***argv)

233 int unused_nongit;

234

235 save_env_before_alias();

237 - subdir = setup_git_directory_gently(&unused_nongit);

236 + setup_git_directory_gently(&unused_nongit);

237

238 alias_command = (*argv)[0];

239 alias_string = alias_lookup(alias_command);

@@ -292,8 +291,7 @@ static int handle_alias(int *argcp, const char ***argv)

291 ret = 1;

292 }

293

295 - if (subdir && chdir(subdir))

296 - die_errno("Cannot change to '%s'", subdir);

294 + restore_env();

295

296 errno = saved_errno;

297

@@ -308,7 +306,6 @@ static int handle_alias(int *argcp, const char ***argv)

306 * RUN_SETUP for reading from the configuration file.

307 */

308 #define NEED_WORK_TREE (1<<3)

311 -#define NO_SETUP (1<<4)

309

310 struct cmd_struct {

311 const char *cmd;

@@ -389,7 +386,7 @@ static struct cmd_struct commands[] = {

386 { "cherry", cmd_cherry, RUN_SETUP },

387 { "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },

388 { "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },

392 - { "clone", cmd_clone, NO_SETUP },

389 + { "clone", cmd_clone },

390 { "column", cmd_column, RUN_SETUP_GENTLY },

391 { "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },

392 { "commit-tree", cmd_commit_tree, RUN_SETUP },

@@ -415,8 +412,8 @@ static struct cmd_struct commands[] = {

412 { "hash-object", cmd_hash_object },

413 { "help", cmd_help },

414 { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },

418 - { "init", cmd_init_db, NO_SETUP },

419 - { "init-db", cmd_init_db, NO_SETUP },

415 + { "init", cmd_init_db },

416 + { "init-db", cmd_init_db },

417 { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP },

418 { "log", cmd_log, RUN_SETUP },

419 { "ls-files", cmd_ls_files, RUN_SETUP },

@@ -528,9 +525,13 @@ static void handle_builtin(int argc, const char **argv)

525

526 builtin = get_builtin(cmd);

527 if (builtin) {

531 - if (saved_env_before_alias && (builtin->option & NO_SETUP))

532 - restore_env();

533 - else

528 + /*

529 + * XXX: if we can figure out cases where it is _safe_

530 + * to do, we can avoid spawning a new process when

531 + * saved_env_before_alias is true

532 + * (i.e. setup_git_dir* has been run once)

533 + */

534 + if (!saved_env_before_alias)

535 exit(run_builtin(builtin, argc, argv));

536 }

537 }

t/t0002-gitfile.sh

+1 -1

@@ -99,7 +99,7 @@ test_expect_success 'check rev-list' '

99 test "$SHA" = "$(git rev-list HEAD)"

100 '

101

102 -test_expect_failure 'setup_git_dir twice in subdir' '

102 +test_expect_success 'setup_git_dir twice in subdir' '

103 git init sgd &&

104 (

105 cd sgd &&

t/t5601-clone.sh

+23

@@ -65,6 +65,29 @@ test_expect_success 'clone respects GIT_WORK_TREE' '

66 '

68 +test_expect_success 'clone from hooks' '

69 +

70 + test_create_repo r0 &&

71 + cd r0 &&

72 + test_commit initial &&

73 + cd .. &&

74 + git init r1 &&

75 + cd r1 &&

76 + cat >.git/hooks/pre-commit <<-\EOF &&

77 + #!/bin/sh

78 + git clone ../r0 ../r2

79 + exit 1

80 + EOF

81 + chmod u+x .git/hooks/pre-commit &&

82 + : >file &&

83 + git add file &&

84 + test_must_fail git commit -m invoke-hook &&

85 + cd .. &&

86 + test_cmp r0/.git/HEAD r2/.git/HEAD &&

87 + test_cmp r0/initial.t r2/initial.t

88 +

89 +'

90 +

91 test_expect_success 'clone creates intermediate directories' '

93 git clone src long/path/to/dst &&