{"thread":{"id":"43910","subject":"Bug with git worktrees and git init","startedAt":"2016-08-23T12:36:42Z","lastAt":"2016-09-25T03:16:42Z","messageCount":34,"participants":["Max Nordlund","Michael J Gruber","Duy Nguyen","Nguyễn Thái Ngọc Duy","Junio C Hamano","Jeff King","Jacob Keller"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"299925","messageId":"CALqjkKZO_y0DNcRJjooyZ7Eso7yBMGhvZ6fE92oO4Su7JeCeng@mail.gmail.com","threadId":"43910","inReplyTo":null,"subject":"Bug with git worktrees and git init","fromName":"Max Nordlund","fromEmail":"max.nordlund@sqore.com","sentAt":"2016-08-23T12:35:31Z","receivedAt":"2016-08-23T12:36:42Z","isPatch":false,"sender":{"key":"max.nordlund@sqore.com","avatar":null},"body":"Hi,\n\nI've been using multiple worktrees for months without issue (it's a\ngreat feature, thanks), until recently when I wanted to add hooks to\nthem. So, when I added a template for the hooks, everything was fine\nuntil I did a git reset --hard in the original repo which both applied\nthose changes to the other worktrees' working tree (the files on\ndisk), and made my master branch kinda lose it's connection to the\nremote/think it was a kinda bare repo.\n\nTo reproduce this:\n\n```\nmkdir source-repo\ncd source-repo\ngit init\ntouch foo\ngit add foo\ngit commit -m 'Add foo'\ngit worktree add ../worktree # which also creates a new branch 'worktree'\ntouch bar\ngit add bar\ngit commit -m 'Add bar'\ncd ../worktree\ngit init\ncd ../source-repo\ngit reset --hard master\ncd ../worktree\ngit status # Suddenly `bar` has appear the working tree and is not tracked\n```\n\nI don't really now what is up with this, but I did notice that it is\nthe last worktree in which git init has been run that is affected. I\nonly ran git init to copy the hooks from the template, but if that is\nnot something you should do in a worktree then a check would have been\nnice.\n\nThanks for this awesome tool, and I hope this helps\nMax Nordlund\n"},{"id":"299933","messageId":"f55e70fa-5244-66a0-d3a7-c6479528039d@drmicha.warpmail.net","threadId":"43910","inReplyTo":"CALqjkKZO_y0DNcRJjooyZ7Eso7yBMGhvZ6fE92oO4Su7JeCeng@mail.gmail.com","subject":"Re: Bug with git worktrees and git init","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-08-23T15:21:57Z","receivedAt":"2016-08-23T15:22:05Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Max Nordlund venit, vidit, dixit 23.08.2016 14:35:\n> Hi,\n> \n> I've been using multiple worktrees for months without issue (it's a\n> great feature, thanks), until recently when I wanted to add hooks to\n> them. So, when I added a template for the hooks, everything was fine\n> until I did a git reset --hard in the original repo which both applied\n> those changes to the other worktrees' working tree (the files on\n> disk), and made my master branch kinda lose it's connection to the\n> remote/think it was a kinda bare repo.\n> \n> To reproduce this:\n> \n> ```\n> mkdir source-repo\n> cd source-repo\n> git init\n> touch foo\n> git add foo\n> git commit -m 'Add foo'\n> git worktree add ../worktree # which also creates a new branch 'worktree'\n> touch bar\n> git add bar\n> git commit -m 'Add bar'\n> cd ../worktree\n> git init\n\nThis is where the problem is created already: git init does not quite\nnotice that it is in a linked worktree. It treats\n\nsource-repo/.git/worktrees/worktree\n\nas a proper gitdir and creates the full gitdir structure in there\n(branches, hooks, info) which is usually shared among worktrees; also,\nit adds the config setting\n\ncore.worktree = /worktree\n\nto the main config (source-repo/.git/config) which is why \"git status\"\nthinks that bar is missing - it indeed is in that worktree.\n\nI've cc'ed the master of worktrees.\n\n> cd ../source-repo\n> git reset --hard master\n> cd ../worktree\n> git status # Suddenly `bar` has appear the working tree and is not tracked\n> ```\n> \n> I don't really now what is up with this, but I did notice that it is\n> the last worktree in which git init has been run that is affected. I\n> only ran git init to copy the hooks from the template, but if that is\n> not something you should do in a worktree then a check would have been\n> nice.\n> \n> Thanks for this awesome tool, and I hope this helps\n> Max Nordlund\n> \n\n"},{"id":"300035","messageId":"CACsJy8CZf0O+uyQaeJ4gcx4XN8ivfFyni+3586WX_R2QM4XgVw@mail.gmail.com","threadId":"43910","inReplyTo":"f55e70fa-5244-66a0-d3a7-c6479528039d@drmicha.warpmail.net","subject":"Re: Bug with git worktrees and git init","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-08-24T09:35:00Z","receivedAt":"2016-08-24T09:35:43Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Aug 23, 2016 at 10:21 PM, Michael J Gruber\n wrote:\n> I've cc'ed the master of worktrees.\n\nThanks for the analysis. I'll provide some patch as soon as possible.\nThis git-init may also be a good place to repair broken worktrees too\n(e.g. because you moved a worktree manually)..\n\nPS. Master? Wheeee I'm a master!! :D\n-- \nDuy\n"},{"id":"301401","messageId":"20160908134719.27955-1-pclouds@gmail.com","threadId":"43910","inReplyTo":"CACsJy8CZf0O+uyQaeJ4gcx4XN8ivfFyni+3586WX_R2QM4XgVw@mail.gmail.com","subject":"[PATCH 0/3] Fix git-init in linked worktrees","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-08T13:47:16Z","receivedAt":"2016-09-08T13:47:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"My ASAP is not so ASAP. Sorry about that but I think I have fixed it.\n\nSide note about 2/3. I've known this problem (in general) for years\n(accidentally reading .git config file before .git is searched) and\ncould not do anything about it. And because test_expect_failure should\nonly be there if someone will eventually fix it, and I don't see\nmyself doing it, and I don't see it super important that other people\nwould bother, so I decided to simply sweep it under the rug. I could\nmake a test_expect_failure if people think otherwise.\n\nSide note about 3/3. Yes there's a FIXME in there. It will take a lot\nmore time to remove that FIXME (because setup_git_directory is not as\nsimple). For now it should be ok to leave it there. When we find a use\nfor get_first_git_dir outside git-init, we can fix it then.\n\nNguyễn Thái Ngọc Duy (3):\n init: correct re-initialization from a linked worktree\n t0001: work around the bug that reads config file before repo setup\n init: do not set core.worktree more often than necessary\n\n builtin/init-db.c | 4 ++--\n cache.h | 1 +\n environment.c | 16 +++++++++++++++-\n t/t0001-init.sh | 19 +++++++++++++++++++\n 4 files changed, 37 insertions(+), 3 deletions(-)\n\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"301402","messageId":"20160908134719.27955-2-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160908134719.27955-1-pclouds@gmail.com","subject":"[PATCH 1/3] init: correct re-initialization from a linked worktree","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-08T13:47:17Z","receivedAt":"2016-09-08T13:47:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When 'git init' is called from a linked worktree, '.git' dir as the main\n'.git' (i.e. $GIT_COMMON_DIR) and populate the whole repository skeleton\nin there. It does not harm anything (*) but it is still wrong.\n\nSince 'git init' calls set_git_dir() at preparation time, which\nindirectly calls get_common_dir() and correctly detects multiple\nworktree setup, all git_path_buf() calls in create_default_files() will\nreturn correct paths in both single and multiple worktree setups. The\nonly thing left is copy_templates(), which targets $GIT_DIR, not\n$GIT_COMMON_DIR.\n\nFix that with get_git_common_dir(). This function will return $GIT_DIR\nin single-worktree setup, so we don't have to make a special case for\nmultiple-worktree here.\n\n(*) It does in fact, thanks to another bug. More on that later.\n\nNoticed-by: Max Nordlund \nHelped-by: Michael J Gruber \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/init-db.c | 2 +-\n t/t0001-init.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 3a45f0b..6d9552e 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -138,7 +138,7 @@ static void copy_templates(const char *template_dir)\n \t\tgoto close_free_return;\n \t}\n \n-\tstrbuf_addstr(&path, get_git_dir());\n+\tstrbuf_addstr(&path, get_git_common_dir());\n \tstrbuf_complete(&path, '/');\n \tcopy_templates_1(&path, &template_path, dir);\n close_free_return:\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex a6fdd5e..d64e5e3 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -384,4 +384,19 @@ test_expect_success MINGW 'bare git dir not hidden' '\n \t! is_hidden newdir\n '\n \n+test_expect_success 're-init from a linked worktree' '\n+\tgit init main-worktree &&\n+\t(\n+\t\tcd main-worktree &&\n+\t\ttest_commit first &&\n+\t\tgit worktree add ../linked-worktree &&\n+\t\tmv .git/info/exclude expected-exclude &&\n+\t\tfind .git/worktrees -print | sort >expected &&\n+\t\tgit -C ../linked-worktree init &&\n+\t\ttest_cmp expected-exclude .git/info/exclude &&\n+\t\tfind .git/worktrees -print | sort >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n test_done\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"301403","messageId":"20160908134719.27955-3-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160908134719.27955-1-pclouds@gmail.com","subject":"[PATCH 2/3] t0001: work around the bug that reads config file before repo setup","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-08T13:47:18Z","receivedAt":"2016-09-08T13:47:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"git-init somehow reads '.git/config' at current directory and sets\nlog_all_ref_updates based on this file. Because log_all_ref_updates is\nnot unspecified (-1) any more. It will not be written to the new repo's\nconfig file (see create_default_files() function).\n\nThis will affect our tests in the next patch as we will compare the\nconfig file and expect that core.logallrefupdates is already set to true\nby \"git init main-worktree\".\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n t/t0001-init.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex d64e5e3..393c940 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -385,7 +385,9 @@ test_expect_success MINGW 'bare git dir not hidden' '\n '\n \n test_expect_success 're-init from a linked worktree' '\n+\tmv .git/config work-around-init-reading-wrong-file &&\n \tgit init main-worktree &&\n+\tmv work-around-init-reading-wrong-file .git/config &&\n \t(\n \t\tcd main-worktree &&\n \t\ttest_commit first &&\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"301404","messageId":"20160908134719.27955-4-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160908134719.27955-1-pclouds@gmail.com","subject":"[PATCH 3/3] init: do not set core.worktree more often than necessary","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-08T13:47:19Z","receivedAt":"2016-09-08T13:47:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When \"git init\" is called with GIT_WORK_TREE environment set, we want to\nkeep this worktree's location in core.worktree so the user does not have\nto set the environment again and again. See ef6f0af (git-init: set\ncore.worktree if GIT_WORK_TREE is specified - 2007-07-04)\n\nWe detect that by this logic (in needs_work_tree_config): normally\nworktree's top dir would contains \".git\" directory, if this is not true,\nworktree is probably set to elsewhere by the user.\n\nUnfortunately when it calls get_git_dir() it does not take \".git\" files\ninto account. When we find a .git file, we immediately follow the file\nuntil we find the real \".git\" directory. The location of this first\n\".git\" file is lost.\n\nThe .git file would satisfy the logic above and not create\ncore.worktree (correct). But because the final .git's location is used,\nneeds_work_tree_config() is misled and creates core.worktree anyway.\n\nThis would not be a huge deal normally. But if this happens in a\nmultiple worktree setup it becomes a real problem because up until now,\ncore.worktree will be applied to the main worktree only. If you\naccidentally do \"git init\" from a linked worktree, you set\ncore.worktree (for the main repo) pointing to the _linked_ worktree.\nAfter that point, may you live in interesting times.\n\nRecord the .git file location and use it here.\n\nNoticed-by: Max Nordlund \nHelped-by: Michael J Gruber \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/init-db.c | 2 +-\n cache.h | 1 +\n environment.c | 16 +++++++++++++++-\n t/t0001-init.sh | 2 ++\n 4 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 6d9552e..36255f2 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -254,7 +254,7 @@ static int create_default_files(const char *template_path)\n \t\t/* allow template config file to override the default */\n \t\tif (log_all_ref_updates == -1)\n \t\t\tgit_config_set(\"core.logallrefupdates\", \"true\");\n-\t\tif (needs_work_tree_config(get_git_dir(), work_tree))\n+\t\tif (needs_work_tree_config(get_first_git_dir(), work_tree))\n \t\t\tgit_config_set(\"core.worktree\", work_tree);\n \t}\n \ndiff --git a/cache.h b/cache.h\nindex b780a91..e6c05f8 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -460,6 +460,7 @@ extern char *git_work_tree_cfg;\n extern int is_inside_work_tree(void);\n extern const char *get_git_dir(void);\n extern const char *get_git_common_dir(void);\n+extern const char *get_first_git_dir(void);\n extern char *get_object_directory(void);\n extern char *get_index_file(void);\n extern char *get_graft_file(void);\ndiff --git a/environment.c b/environment.c\nindex ca72464..8cfb8f3 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -100,7 +100,7 @@ static char *work_tree;\n static const char *namespace;\n static size_t namespace_len;\n \n-static const char *git_dir, *git_common_dir;\n+static const char *git_dir, *git_common_dir, *first_git_dir;\n static char *git_object_dir, *git_index_file, *git_graft_file;\n int git_db_env, git_index_env, git_graft_env, git_common_dir_env;\n \n@@ -168,6 +168,8 @@ static void setup_git_env(void)\n \tif (!git_dir)\n \t\tgit_dir = DEFAULT_GIT_DIR_ENVIRONMENT;\n \tgitfile = read_gitfile(git_dir);\n+\tif (gitfile && !first_git_dir)\n+\t\tfirst_git_dir = xstrdup(git_dir);\n \tgit_dir = xstrdup(gitfile ? gitfile : git_dir);\n \tif (get_common_dir(&sb, git_dir))\n \t\tgit_common_dir_env = 1;\n@@ -203,6 +205,18 @@ const char *get_git_dir(void)\n \treturn git_dir;\n }\n \n+/*\n+ * Return the first \".git\" that we have encountered.\n+ * FIXME this function for not entirely correct because\n+ * setup_git_directory() and enter_repo() do not update first_git_dir\n+ * when they follow .git files. The function in its current state is\n+ * only suitable for \"git init\".\n+ */\n+const char *get_first_git_dir(void)\n+{\n+\treturn first_git_dir ? first_git_dir : git_dir;\n+}\n+\n const char *get_git_common_dir(void)\n {\n \treturn git_common_dir;\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 393c940..d59669a 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -393,9 +393,11 @@ test_expect_success 're-init from a linked worktree' '\n \t\ttest_commit first &&\n \t\tgit worktree add ../linked-worktree &&\n \t\tmv .git/info/exclude expected-exclude &&\n+\t\tcp .git/config expected-config &&\n \t\tfind .git/worktrees -print | sort >expected &&\n \t\tgit -C ../linked-worktree init &&\n \t\ttest_cmp expected-exclude .git/info/exclude &&\n+\t\ttest_cmp expected-config .git/config &&\n \t\tfind .git/worktrees -print | sort >actual &&\n \t\ttest_cmp expected actual\n \t)\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"301434","messageId":"xmqqk2emgp2m.fsf@gitster.mtv.corp.google.com","threadId":"43910","inReplyTo":"20160908134719.27955-2-pclouds@gmail.com","subject":"Re: [PATCH 1/3] init: correct re-initialization from a linked worktree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-08T19:37:53Z","receivedAt":"2016-09-08T19:38:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> When 'git init' is called from a linked worktree, '.git' dir as the main\n> '.git' (i.e. $GIT_COMMON_DIR) and populate the whole repository skeleton\n> in there. It does not harm anything (*) but it is still wrong.\n\n-ECANNOTPARSE. Did you mean \"... worktree, we treat '.git' dir as\nif it is the main '.git' ...\" or something entirely different?\n\n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index 3a45f0b..6d9552e 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -138,7 +138,7 @@ static void copy_templates(const char *template_dir)\n> \t\tgoto close_free_return;\n> \t}\n> \n> -\tstrbuf_addstr(&path, get_git_dir());\n> +\tstrbuf_addstr(&path, get_git_common_dir());\n> \tstrbuf_complete(&path, '/');\n> \tcopy_templates_1(&path, &template_path, dir);\n> close_free_return:\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index a6fdd5e..d64e5e3 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -384,4 +384,19 @@ test_expect_success MINGW 'bare git dir not hidden' '\n> \t! is_hidden newdir\n> '\n> \n> +test_expect_success 're-init from a linked worktree' '\n> +\tgit init main-worktree &&\n> +\t(\n> +\t\tcd main-worktree &&\n> +\t\ttest_commit first &&\n> +\t\tgit worktree add ../linked-worktree &&\n> +\t\tmv .git/info/exclude expected-exclude &&\n> +\t\tfind .git/worktrees -print | sort >expected &&\n> +\t\tgit -C ../linked-worktree init &&\n> +\t\ttest_cmp expected-exclude .git/info/exclude &&\n> +\t\tfind .git/worktrees -print | sort >actual &&\n> +\t\ttest_cmp expected actual\n> +\t)\n> +'\n> +\n> test_done\n"},{"id":"301435","messageId":"xmqqfupagorl.fsf@gitster.mtv.corp.google.com","threadId":"43910","inReplyTo":"20160908134719.27955-3-pclouds@gmail.com","subject":"Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-08T19:44:30Z","receivedAt":"2016-09-08T19:44:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> git-init somehow reads '.git/config' at current directory and sets\n> log_all_ref_updates based on this file. Because log_all_ref_updates is\n> not unspecified (-1) any more. It will not be written to the new repo's\n> config file (see create_default_files() function).\n\nI vaguely recall this was discussed recently somewhere. Is this\nrelated to <20160902091130.jxckdeomw35eewky@sigill.intra.peff.net>?\n\n\n> This will affect our tests in the next patch as we will compare the\n> config file and expect that core.logallrefupdates is already set to true\n> by \"git init main-worktree\".\n\nUgh. The \"mv\" workaround is fine to demonstrate that you have fixed\none of two problems, but as you said, it would be nice to have another\nthat expects failure until both of them gets fixed.\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> t/t0001-init.sh | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index d64e5e3..393c940 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -385,7 +385,9 @@ test_expect_success MINGW 'bare git dir not hidden' '\n> '\n> \n> test_expect_success 're-init from a linked worktree' '\n> +\tmv .git/config work-around-init-reading-wrong-file &&\n> \tgit init main-worktree &&\n> +\tmv work-around-init-reading-wrong-file .git/config &&\n> \t(\n> \t\tcd main-worktree &&\n> \t\ttest_commit first &&\n"},{"id":"301437","messageId":"xmqqbmzygoat.fsf@gitster.mtv.corp.google.com","threadId":"43910","inReplyTo":"20160908134719.27955-4-pclouds@gmail.com","subject":"Re: [PATCH 3/3] init: do not set core.worktree more often than necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-08T19:54:34Z","receivedAt":"2016-09-08T19:54:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> +/*\n> + * Return the first \".git\" that we have encountered.\n> + * FIXME this function for not entirely correct because\n> + * setup_git_directory() and enter_repo() do not update first_git_dir\n> + * when they follow .git files. The function in its current state is\n> + * only suitable for \"git init\".\n> + */\n\nWould it be possible to move this to \"init-db.c\" then?\n\nThe very first thing cmd_init_db() does to what is in the\nenvironment.c is to call set_git_dir() via set_git_dir_init() to\ntell it where the \".git\" thing is, no? Can't that code remember the\nlocation itself, instead of adding code that is known not to be\nusable by other callers? That would help avoiding the future\nconfusion.\n\n> +const char *get_first_git_dir(void)\n> +{\n> +\treturn first_git_dir ? first_git_dir : git_dir;\n> +}\n> +\n> const char *get_git_common_dir(void)\n> {\n> \treturn git_common_dir;\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index 393c940..d59669a 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -393,9 +393,11 @@ test_expect_success 're-init from a linked worktree' '\n> \t\ttest_commit first &&\n> \t\tgit worktree add ../linked-worktree &&\n> \t\tmv .git/info/exclude expected-exclude &&\n> +\t\tcp .git/config expected-config &&\n> \t\tfind .git/worktrees -print | sort >expected &&\n> \t\tgit -C ../linked-worktree init &&\n> \t\ttest_cmp expected-exclude .git/info/exclude &&\n> +\t\ttest_cmp expected-config .git/config &&\n> \t\tfind .git/worktrees -print | sort >actual &&\n> \t\ttest_cmp expected actual\n> \t)\n"},{"id":"301438","messageId":"20160908200202.gmvpqrwwjavxmojb@sigill.intra.peff.net","threadId":"43910","inReplyTo":"20160908134719.27955-3-pclouds@gmail.com","subject":"Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-08T20:02:02Z","receivedAt":"2016-09-08T20:02:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 08, 2016 at 08:47:18PM +0700, Nguyễn Thái Ngọc Duy wrote:\n\n> git-init somehow reads '.git/config' at current directory and sets\n> log_all_ref_updates based on this file. Because log_all_ref_updates is\n> not unspecified (-1) any more. It will not be written to the new repo's\n> config file (see create_default_files() function).\n> \n> This will affect our tests in the next patch as we will compare the\n> config file and expect that core.logallrefupdates is already set to true\n> by \"git init main-worktree\".\n\nThis is a bug for more than worktrees, and is something I'm working on\nfixing (what I'd like to do is kill off the lazy fallback to \".git/\" as\nthe repo name, which is almost always the wrong thing to do).\n\nI'm not opposed to your workaround, but just FYI.\n\n-Peff\n"},{"id":"301477","messageId":"CACsJy8DmbdGODY+qT38OSkaegSpdO7yAt6e67sHB=_BT8Y7=_Q@mail.gmail.com","threadId":"43910","inReplyTo":"20160908200202.gmvpqrwwjavxmojb@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-09T10:32:21Z","receivedAt":"2016-09-09T10:33:06Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Sep 9, 2016 at 3:02 AM, Jeff King wrote:\n> On Thu, Sep 08, 2016 at 08:47:18PM +0700, Nguyễn Thái Ngọc Duy wrote:\n>\n>> git-init somehow reads '.git/config' at current directory and sets\n>> log_all_ref_updates based on this file. Because log_all_ref_updates is\n>> not unspecified (-1) any more. It will not be written to the new repo's\n>> config file (see create_default_files() function).\n>>\n>> This will affect our tests in the next patch as we will compare the\n>> config file and expect that core.logallrefupdates is already set to true\n>> by \"git init main-worktree\".\n>\n> This is a bug for more than worktrees, and is something I'm working on\n> fixing\n\nGreat! test_expect_failure it is. But I'll make a separate patch,\nindependent from this series though.\n-- \nDuy\n"},{"id":"301478","messageId":"CACsJy8D3ek5KRN_L2Kc-s=eQkeu8L2Nsd6F9+R9jY1BFRLQr3w@mail.gmail.com","threadId":"43910","inReplyTo":"xmqqbmzygoat.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] init: do not set core.worktree more often than necessary","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-09T10:33:45Z","receivedAt":"2016-09-09T10:34:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Sep 9, 2016 at 2:54 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> +/*\n>> + * Return the first \".git\" that we have encountered.\n>> + * FIXME this function for not entirely correct because\n>> + * setup_git_directory() and enter_repo() do not update first_git_dir\n>> + * when they follow .git files. The function in its current state is\n>> + * only suitable for \"git init\".\n>> + */\n>\n> Would it be possible to move this to \"init-db.c\" then?\n>\n> The very first thing cmd_init_db() does to what is in the\n> environment.c is to call set_git_dir() via set_git_dir_init() to\n> tell it where the \".git\" thing is, no? Can't that code remember the\n> location itself, instead of adding code that is known not to be\n> usable by other callers? That would help avoiding the future\n> confusion.\n\nGood idea. I was fixated on read_gitfile()m it didn't occur to me.\n-- \nDuy\n"},{"id":"301479","messageId":"CACsJy8B7-K+WryPcY3u0qeza_mNOgjN0R9NNEvUpbo4YTch3Qg@mail.gmail.com","threadId":"43910","inReplyTo":"xmqqk2emgp2m.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] init: correct re-initialization from a linked worktree","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-09T10:36:39Z","receivedAt":"2016-09-09T10:37:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Sep 9, 2016 at 2:37 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> When 'git init' is called from a linked worktree, '.git' dir as the main\n>> '.git' (i.e. $GIT_COMMON_DIR) and populate the whole repository skeleton\n>> in there. It does not harm anything (*) but it is still wrong.\n>\n> -ECANNOTPARSE. Did you mean \"... worktree, we treat '.git' dir as\n> if it is the main '.git' ...\"\n\nYep I accidentally a couple words there. Will add them back.\n\n> or something entirely different?\n-- \nDuy\n"},{"id":"301485","messageId":"20160909112217.gfalcbtiy2tzzhm5@sigill.intra.peff.net","threadId":"43910","inReplyTo":"CACsJy8DmbdGODY+qT38OSkaegSpdO7yAt6e67sHB=_BT8Y7=_Q@mail.gmail.com","subject":"Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-09T11:22:18Z","receivedAt":"2016-09-09T11:22:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 09, 2016 at 05:32:21PM +0700, Duy Nguyen wrote:\n\n> On Fri, Sep 9, 2016 at 3:02 AM, Jeff King wrote:\n> > On Thu, Sep 08, 2016 at 08:47:18PM +0700, Nguyễn Thái Ngọc Duy wrote:\n> >\n> >> git-init somehow reads '.git/config' at current directory and sets\n> >> log_all_ref_updates based on this file. Because log_all_ref_updates is\n> >> not unspecified (-1) any more. It will not be written to the new repo's\n> >> config file (see create_default_files() function).\n> >>\n> >> This will affect our tests in the next patch as we will compare the\n> >> config file and expect that core.logallrefupdates is already set to true\n> >> by \"git init main-worktree\".\n> >\n> > This is a bug for more than worktrees, and is something I'm working on\n> > fixing\n> \n> Great! test_expect_failure it is. But I'll make a separate patch,\n> independent from this series though.\n\nIf you're curious what the fix looks like, it's in:\n\n https://github.com/peff/git jk/config-repo-setup\n\nThe actual fix is in the final patch, but it needed a lot of preparatory\nwork to avoid breaking various programs that made bad assumptions (and\nin the process, I uncovered a ton of other minor bugs).\n\nThis is just a preview in case you're interested, for two reasons:\n\n 1. I literally _just_ put the finishing touches on it, and it's\n extensive and tricky enough that I really should give it one more\n proofread.\n\n 2. There may be other related fallouts from the bug related to running\n \"git init /path/to/foo\" when \"/path/to/foo\" already exists (and in\n that case we _do_ want to read its config, but not the config from\n an existing repository). This may all just work fine, but I need to\n think about some tests.\n\n-Peff\n"},{"id":"301532","messageId":"CA+P7+xqDGjmSNdRjsMtBdZrXLeA2OfntgEELxgG5eSd6G2uwJA@mail.gmail.com","threadId":"43910","inReplyTo":"20160909112217.gfalcbtiy2tzzhm5@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] t0001: work around the bug that reads config file before repo setup","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-09-09T17:45:55Z","receivedAt":"2016-09-09T17:46:21Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Sep 9, 2016 at 4:22 AM, Jeff King wrote:\n> If you're curious what the fix looks like, it's in:\n>\n> https://github.com/peff/git jk/config-repo-setup\n>\n> The actual fix is in the final patch, but it needed a lot of preparatory\n> work to avoid breaking various programs that made bad assumptions (and\n> in the process, I uncovered a ton of other minor bugs).\n>\n> This is just a preview in case you're interested, for two reasons:\n>\n> 1. I literally _just_ put the finishing touches on it, and it's\n> extensive and tricky enough that I really should give it one more\n> proofread.\n>\n> 2. There may be other related fallouts from the bug related to running\n> \"git init /path/to/foo\" when \"/path/to/foo\" already exists (and in\n> that case we _do_ want to read its config, but not the config from\n> an existing repository). This may all just work fine, but I need to\n> think about some tests.\n>\n> -Peff\n\nI looked over the series and it all seems like solid improvements to\nme, and I agree with the reasoning and logic. It's nice to also read\nvery descriptive commit messages that clearly explain each\nimprovement.\n\nRegards,\nJake\n"},{"id":"302259","messageId":"20160921112939.3444-1-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160908134719.27955-1-pclouds@gmail.com","subject":"[PATCH v2 0/3] Fix git-init in linked worktrees","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-21T11:29:36Z","receivedAt":"2016-09-21T11:30:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"v2 requires jk/setup-sequence-update so I could kill my workaround\npatch and avoid conflicts in t0001. And:\n\n - 1/3 has a few missing words back in its commit message\n - 2/3, which was 3/3 in v1, no longer has the ugly hacky\n get_first_git_dir()\n - 3/3 is a new tiny code improvement after the new 2/3\n\nNguyễn Thái Ngọc Duy (3):\n init: correct re-initialization from a linked worktree\n init: do not set core.worktree more often than necessary\n init: reuse original_git_dir in set_git_dir_init()\n\n builtin/init-db.c | 11 +++++++----\n t/t0001-init.sh | 17 +++++++++++++++++\n 2 files changed, 24 insertions(+), 4 deletions(-)\n\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302260","messageId":"20160921112939.3444-2-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160921112939.3444-1-pclouds@gmail.com","subject":"[PATCH v2 1/3] init: correct re-initialization from a linked worktree","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-21T11:29:37Z","receivedAt":"2016-09-21T11:30:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When 'git init' is called from a linked worktree, we treat '.git'\ndir (which is $GIT_COMMON_DIR/worktrees/something) as the main\n'.git' (i.e. $GIT_COMMON_DIR) and populate the whole repository skeleton\nin there. It does not harm anything (*) but it is still wrong.\n\nSince 'git init' calls set_git_dir() at preparation time, which\nindirectly calls get_common_dir() and correctly detects multiple\nworktree setup, all git_path_buf() calls in create_default_files() will\nreturn correct paths in both single and multiple worktree setups. The\nonly thing left is copy_templates(), which targets $GIT_DIR, not\n$GIT_COMMON_DIR.\n\nFix that with get_git_common_dir(). This function will return $GIT_DIR\nin single-worktree setup, so we don't have to make a special case for\nmultiple-worktree here.\n\n(*) It does in fact, thanks to another bug. More on that later.\n\nNoticed-by: Max Nordlund \nHelped-by: Michael J Gruber \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/init-db.c | 2 +-\n t/t0001-init.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex cc09fca..d5d7558 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -138,7 +138,7 @@ static void copy_templates(const char *template_dir)\n \t\tgoto close_free_return;\n \t}\n \n-\tstrbuf_addstr(&path, get_git_dir());\n+\tstrbuf_addstr(&path, get_git_common_dir());\n \tstrbuf_complete(&path, '/');\n \tcopy_templates_1(&path, &template_path, dir);\n close_free_return:\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 8ffbbea..488564b 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -393,4 +393,19 @@ test_expect_success 'remote init from does not use config from cwd' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 're-init from a linked worktree' '\n+\tgit init main-worktree &&\n+\t(\n+\t\tcd main-worktree &&\n+\t\ttest_commit first &&\n+\t\tgit worktree add ../linked-worktree &&\n+\t\tmv .git/info/exclude expected-exclude &&\n+\t\tfind .git/worktrees -print | sort >expected &&\n+\t\tgit -C ../linked-worktree init &&\n+\t\ttest_cmp expected-exclude .git/info/exclude &&\n+\t\tfind .git/worktrees -print | sort >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n test_done\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302261","messageId":"20160921112939.3444-3-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160921112939.3444-1-pclouds@gmail.com","subject":"[PATCH v2 2/3] init: do not set core.worktree more often than necessary","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-21T11:29:38Z","receivedAt":"2016-09-21T11:30:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When \"git init\" is called with GIT_WORK_TREE environment set, we want to\nkeep this worktree's location in core.worktree so the user does not have\nto set the environment again and again. See ef6f0af (git-init: set\ncore.worktree if GIT_WORK_TREE is specified - 2007-07-04)\n\nWe detect that by this logic (in needs_work_tree_config): normally\nworktree's top dir would contains \".git\" directory, if this is not true,\nworktree is probably set to elsewhere by the user.\n\nUnfortunately when it calls get_git_dir() it does not take \".git\" files\ninto account. When we find a .git file, we immediately follow the file\nuntil we find the real \".git\" directory. The location of this first\n\".git\" file is lost.\n\nThe .git file would satisfy the logic above and not create\ncore.worktree (correct). But because the final .git's location is used,\nneeds_work_tree_config() is misled and creates core.worktree anyway.\n\nThis would not be a huge deal normally. But if this happens in a\nmultiple worktree setup it becomes a real problem because up until now,\ncore.worktree will be applied to the main worktree only. If you\naccidentally do \"git init\" from a linked worktree, you set\ncore.worktree (for the main repo) pointing to the _linked_ worktree.\nAfter that point, may you live in interesting times.\n\nRecord the .git file location and use it here.\n\nPS. real_path() resolves symlinks So original_git_dir is not truly\noriginal if '.git' is a symlink. Hopefully it's not longer used in favor\nof .git files.\n\nNoticed-by: Max Nordlund \nHelped-by: Michael J Gruber \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/init-db.c | 5 ++++-\n t/t0001-init.sh | 2 ++\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex d5d7558..0d5cc76 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -23,6 +23,7 @@ static int init_is_bare_repository = 0;\n static int init_shared_repository = -1;\n static const char *init_db_template_dir;\n static const char *git_link;\n+static const char *original_git_dir;\n \n static void copy_templates_1(struct strbuf *path, struct strbuf *template,\n \t\t\t DIR *dir)\n@@ -263,7 +264,7 @@ static int create_default_files(const char *template_path)\n \t\t/* allow template config file to override the default */\n \t\tif (log_all_ref_updates == -1)\n \t\t\tgit_config_set(\"core.logallrefupdates\", \"true\");\n-\t\tif (needs_work_tree_config(get_git_dir(), work_tree))\n+\t\tif (needs_work_tree_config(original_git_dir, work_tree))\n \t\t\tgit_config_set(\"core.worktree\", work_tree);\n \t}\n \n@@ -314,6 +315,8 @@ static void create_object_directory(void)\n int set_git_dir_init(const char *git_dir, const char *real_git_dir,\n \t\t int exist_ok)\n {\n+\toriginal_git_dir = xstrdup(real_path(git_dir));\n+\n \tif (real_git_dir) {\n \t\tstruct stat st;\n \ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 488564b..b8fc588 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -400,9 +400,11 @@ test_expect_success 're-init from a linked worktree' '\n \t\ttest_commit first &&\n \t\tgit worktree add ../linked-worktree &&\n \t\tmv .git/info/exclude expected-exclude &&\n+\t\tcp .git/config expected-config &&\n \t\tfind .git/worktrees -print | sort >expected &&\n \t\tgit -C ../linked-worktree init &&\n \t\ttest_cmp expected-exclude .git/info/exclude &&\n+\t\ttest_cmp expected-config .git/config &&\n \t\tfind .git/worktrees -print | sort >actual &&\n \t\ttest_cmp expected actual\n \t)\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302262","messageId":"20160921112939.3444-4-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160921112939.3444-1-pclouds@gmail.com","subject":"[PATCH v2 3/3] init: reuse original_git_dir in set_git_dir_init()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-21T11:29:39Z","receivedAt":"2016-09-21T11:30:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Since original_git_dir is a copy of real_path(git_dir), let's reuse it\nand avoid calling real_path() more than necessary.\n\nThe xstrdup() is removed too because original_git_dir is already a copy,\nand we're not going to free git_link in this code probably forever.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/init-db.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 0d5cc76..d70fc45 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -330,11 +330,11 @@ int set_git_dir_init(const char *git_dir, const char *real_git_dir,\n \t\t * make sure symlinks are resolved because we'll be\n \t\t * moving the target repo later on in separate_git_dir()\n \t\t */\n-\t\tgit_link = xstrdup(real_path(git_dir));\n+\t\tgit_link = original_git_dir;\n \t\tset_git_dir(real_path(real_git_dir));\n \t}\n \telse {\n-\t\tset_git_dir(real_path(git_dir));\n+\t\tset_git_dir(original_git_dir);\n \t\tgit_link = NULL;\n \t}\n \tstartup_info->have_repository = 1;\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302311","messageId":"xmqqh99986c8.fsf@gitster.mtv.corp.google.com","threadId":"43910","inReplyTo":"20160921112939.3444-1-pclouds@gmail.com","subject":"Re: [PATCH v2 0/3] Fix git-init in linked worktrees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-21T18:18:15Z","receivedAt":"2016-09-21T18:18:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> v2 requires jk/setup-sequence-update so I could kill my workaround\n> patch and avoid conflicts in t0001. And:\n>\n> - 1/3 has a few missing words back in its commit message\n> - 2/3, which was 3/3 in v1, no longer has the ugly hacky\n> get_first_git_dir()\n> - 3/3 is a new tiny code improvement after the new 2/3\n>\n> Nguyễn Thái Ngọc Duy (3):\n> init: correct re-initialization from a linked worktree\n> init: do not set core.worktree more often than necessary\n> init: reuse original_git_dir in set_git_dir_init()\n\nThanks. Will take a look.\n"},{"id":"302319","messageId":"xmqqd1jx854z.fsf@gitster.mtv.corp.google.com","threadId":"43910","inReplyTo":"20160921112939.3444-3-pclouds@gmail.com","subject":"Re: [PATCH v2 2/3] init: do not set core.worktree more often than necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-21T18:44:12Z","receivedAt":"2016-09-21T18:44:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> When \"git init\" is called with GIT_WORK_TREE environment set, we want to\n> keep this worktree's location in core.worktree so the user does not have\n> to set the environment again and again. See ef6f0af (git-init: set\n> core.worktree if GIT_WORK_TREE is specified - 2007-07-04)\n>\n> We detect that by this logic (in needs_work_tree_config): normally\n> worktree's top dir would contains \".git\" directory, if this is not true,\n\ns/contains/contain/;\n\n> worktree is probably set to elsewhere by the user.\n>\n> Unfortunately when it calls get_git_dir() it does not take \".git\" files\n> into account. When we find a .git file, we immediately follow the file\n> until we find the real \".git\" directory. The location of this first\n> \".git\" file is lost.\n>\n> The .git file would satisfy the logic above and not create\n> core.worktree (correct). But because the final .git's location is used,\n> needs_work_tree_config() is misled and creates core.worktree anyway.\n\nThe above explanation makes it sound like the correct fix belongs to\nneeds_work_tree_config(), though.\n\nI am starting to wonder if what ef6f0af did was misguided and we are\nbetter off without setting core.worktree ourselves, but that is a\ndifferent issue.\n\n> diff --git a/builtin/init-db.c b/builtin/init-db.c\n> index d5d7558..0d5cc76 100644\n> --- a/builtin/init-db.c\n> +++ b/builtin/init-db.c\n> @@ -23,6 +23,7 @@ static int init_is_bare_repository = 0;\n> static int init_shared_repository = -1;\n> static const char *init_db_template_dir;\n> static const char *git_link;\n> +static const char *original_git_dir;\n> \n> static void copy_templates_1(struct strbuf *path, struct strbuf *template,\n> \t\t\t DIR *dir)\n> @@ -263,7 +264,7 @@ static int create_default_files(const char *template_path)\n> \t\t/* allow template config file to override the default */\n> \t\tif (log_all_ref_updates == -1)\n> \t\t\tgit_config_set(\"core.logallrefupdates\", \"true\");\n> -\t\tif (needs_work_tree_config(get_git_dir(), work_tree))\n> +\t\tif (needs_work_tree_config(original_git_dir, work_tree))\n> \t\t\tgit_config_set(\"core.worktree\", work_tree);\n> \t}\n> \n> @@ -314,6 +315,8 @@ static void create_object_directory(void)\n> int set_git_dir_init(const char *git_dir, const char *real_git_dir,\n> \t\t int exist_ok)\n> {\n> +\toriginal_git_dir = xstrdup(real_path(git_dir));\n> +\n> \tif (real_git_dir) {\n> \t\tstruct stat st;\n\nThe function being extern bothers me. The create_default_files()\nfunction, which is the only thing consumes this variable, is called\nonly from init_db(), and I'd prefer to see some way to guarantee\nthat everybody who calls init_db() calls set_git_dir_init()\nbeforehand. Right now, cmd_init_db() and cmd_clone() are the only\nones that call init_db() and they both call set_dir_git_init(); if a\nnew caller starts calling init_db() and forgets to call the other\none, that caller will be buggy.\n\nPerhaps a comment before init_db() to tell callers to always call\nthe other one is the least thing necessary?\n\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index 488564b..b8fc588 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -400,9 +400,11 @@ test_expect_success 're-init from a linked worktree' '\n> \t\ttest_commit first &&\n> \t\tgit worktree add ../linked-worktree &&\n> \t\tmv .git/info/exclude expected-exclude &&\n> +\t\tcp .git/config expected-config &&\n> \t\tfind .git/worktrees -print | sort >expected &&\n> \t\tgit -C ../linked-worktree init &&\n> \t\ttest_cmp expected-exclude .git/info/exclude &&\n> +\t\ttest_cmp expected-config .git/config &&\n> \t\tfind .git/worktrees -print | sort >actual &&\n> \t\ttest_cmp expected actual\n> \t)\n"},{"id":"302367","messageId":"CACsJy8BQvo_XSXZ3m8=A=0tik3qPd8k9cyC4G408JK+AhtYStQ@mail.gmail.com","threadId":"43910","inReplyTo":"xmqqd1jx854z.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/3] init: do not set core.worktree more often than necessary","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-22T10:06:11Z","receivedAt":"2016-09-22T10:06:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Sep 22, 2016 at 1:44 AM, Junio C Hamano wrote:\n>> @@ -314,6 +315,8 @@ static void create_object_directory(void)\n>> int set_git_dir_init(const char *git_dir, const char *real_git_dir,\n>> int exist_ok)\n>> {\n>> + original_git_dir = xstrdup(real_path(git_dir));\n>> +\n>> if (real_git_dir) {\n>> struct stat st;\n>\n> The function being extern bothers me. The create_default_files()\n> function, which is the only thing consumes this variable, is called\n> only from init_db(), and I'd prefer to see some way to guarantee\n> that everybody who calls init_db() calls set_git_dir_init()\n> beforehand. Right now, cmd_init_db() and cmd_clone() are the only\n> ones that call init_db() and they both call set_dir_git_init(); if a\n> new caller starts calling init_db() and forgets to call the other\n> one, that caller will be buggy.\n>\n> Perhaps a comment before init_db() to tell callers to always call\n> the other one is the least thing necessary?\n\nGood thinking. We could go a step further, baking it as assert() to\ncatch new/incorrect call sequences automatically.\n\nOr we could combine the two functions init_db() and set_git_dir_init()\ninto one. I prefer this one, but having problem with finding a good\nname for it because the new function would prepare $GIT_DIR for the\nentire process and init the repository. Maybe enter_and_init_db(),\nenter_and_init_repo()? If no good name is found, I'll go back to\neither adding comment or assert().\n-- \nDuy\n"},{"id":"302380","messageId":"xmqqbmzf6e0r.fsf@gitster.mtv.corp.google.com","threadId":"43910","inReplyTo":"CACsJy8BQvo_XSXZ3m8=A=0tik3qPd8k9cyC4G408JK+AhtYStQ@mail.gmail.com","subject":"Re: [PATCH v2 2/3] init: do not set core.worktree more often than necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-22T17:27:32Z","receivedAt":"2016-09-22T17:27:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n>> Perhaps a comment before init_db() to tell callers to always call\n>> the other one is the least thing necessary?\n>\n> Good thinking. We could go a step further, baking it as assert() to\n> catch new/incorrect call sequences automatically.\n>\n> Or we could combine the two functions init_db() and set_git_dir_init()\n> into one. I prefer this one, but having problem with finding a good\n> name for it ...\n\nIt probably is a reasonable way forward to name the combined\nfunction that takes the parameters that are taken by both functions\nin their current incarnation init_db(). After all, initializing the\nGit repository database involves telling the function where the .git\ndirectory is, among other things like where to take the template for\na newly created repository is.\n\n"},{"id":"302440","messageId":"20160923111206.8596-1-pclouds@gmail.com","threadId":"43910","inReplyTo":"xmqqbmzf6e0r.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 4/3] init: combine set_git_dir_init() and init_db() into one","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-23T11:12:06Z","receivedAt":"2016-09-23T11:13:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Commit \"init: do not set core.worktree more often than necessary\" adds a\nsubtle dependency between set_git_dir_init() and init_db(). The former\n_must_ be called before init_db() so that original_git_dir can be set\nproperly. If something else, like enter_repo() or setup_git_directory(),\nis used instead, the trick in that commit breaks down.\n\nTo eliminate the possibility that init_db() in future may be called\nwithout set_git_dir_init(), init_db() now calls that function internally\n(and does not allow anybody else to use it).\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n I think a separate commit for this is better than combining back to\n 2/3 so we can explain the problem properly (without making 2/3 commit\n message even longer)\n\n Not sure if you want to s/contains/contain/ in 2/3 by yourself or I\n should resend the whole series. Let me know.\n\n builtin/clone.c | 15 +++++++--------\n builtin/init-db.c | 18 +++++++++++-------\n cache.h | 5 +++--\n 3 files changed, 21 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 6616392..29b1832 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -928,23 +928,22 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tset_git_work_tree(work_tree);\n \t}\n \n-\tjunk_git_dir = git_dir;\n+\tjunk_git_dir = real_git_dir ? real_git_dir : git_dir;\n \tif (safe_create_leading_directories_const(git_dir) < 0)\n \t\tdie(_(\"could not create leading directories of '%s'\"), git_dir);\n \n-\tset_git_dir_init(git_dir, real_git_dir, 0);\n-\tif (real_git_dir) {\n-\t\tgit_dir = real_git_dir;\n-\t\tjunk_git_dir = real_git_dir;\n-\t}\n-\n \tif (0 <= option_verbosity) {\n \t\tif (option_bare)\n \t\t\tfprintf(stderr, _(\"Cloning into bare repository '%s'...\\n\"), dir);\n \t\telse\n \t\t\tfprintf(stderr, _(\"Cloning into '%s'...\\n\"), dir);\n \t}\n-\tinit_db(option_template, INIT_DB_QUIET);\n+\n+\tinit_db(git_dir, real_git_dir, option_template, INIT_DB_QUIET);\n+\n+\tif (real_git_dir)\n+\t\tgit_dir = real_git_dir;\n+\n \twrite_config(&option_config);\n \n \tgit_config(git_default_config, NULL);\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex d70fc45..ee7942f 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -312,8 +312,9 @@ static void create_object_directory(void)\n \tstrbuf_release(&path);\n }\n \n-int set_git_dir_init(const char *git_dir, const char *real_git_dir,\n-\t\t int exist_ok)\n+static int set_git_dir_init(const char *git_dir,\n+\t\t\t const char *real_git_dir,\n+\t\t\t int exist_ok)\n {\n \toriginal_git_dir = xstrdup(real_path(git_dir));\n \n@@ -362,10 +363,14 @@ static void separate_git_dir(const char *git_dir)\n \twrite_file(git_link, \"gitdir: %s\", git_dir);\n }\n \n-int init_db(const char *template_dir, unsigned int flags)\n+int init_db(const char *git_dir, const char *real_git_dir,\n+\t const char *template_dir, unsigned int flags)\n {\n \tint reinit;\n-\tconst char *git_dir = get_git_dir();\n+\n+\tset_git_dir_init(git_dir, real_git_dir, flags & INIT_DB_EXIST_OK);\n+\n+\tgit_dir = get_git_dir();\n \n \tif (git_link)\n \t\tseparate_git_dir(git_dir);\n@@ -585,7 +590,6 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\tset_git_work_tree(work_tree);\n \t}\n \n-\tset_git_dir_init(git_dir, real_git_dir, 1);\n-\n-\treturn init_db(template_dir, flags);\n+\tflags |= INIT_DB_EXIST_OK;\n+\treturn init_db(git_dir, real_git_dir, template_dir, flags);\n }\ndiff --git a/cache.h b/cache.h\nindex b2d77f3..7fc875f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -525,9 +525,10 @@ extern void verify_non_filename(const char *prefix, const char *name);\n extern int path_inside_repo(const char *prefix, const char *path);\n \n #define INIT_DB_QUIET 0x0001\n+#define INIT_DB_EXIST_OK 0x0002\n \n-extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int);\n-extern int init_db(const char *template_dir, unsigned int flags);\n+extern int init_db(const char *git_dir, const char *real_git_dir,\n+\t\t const char *template_dir, unsigned int flags);\n \n extern void sanitize_stdfds(void);\n extern int daemonize(void);\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302445","messageId":"xmqqzimy1w6g.fsf@gitster.mtv.corp.google.com","threadId":"43910","inReplyTo":"20160923111206.8596-1-pclouds@gmail.com","subject":"Re: [PATCH v2 4/3] init: combine set_git_dir_init() and init_db() into one","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-23T15:18:47Z","receivedAt":"2016-09-23T15:19:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> I think a separate commit for this is better than combining back to\n> 2/3 so we can explain the problem properly (without making 2/3 commit\n> message even longer)\n>\n> Not sure if you want to s/contains/contain/ in 2/3 by yourself or I\n> should resend the whole series. Let me know.\n\nOK, I just amended it before applying this on top.\n\n> +\tflags |= INIT_DB_EXIST_OK;\n> +\treturn init_db(git_dir, real_git_dir, template_dir, flags);\n\nI do not think of anything better, but EXIST_OK does not sound\ngrammatical. \"REINIT\" is not quite it--we are merely allowing\nthe function to re-init if there already is a repository. And\nOK_TO_REINIT is a bit too long. Let's take the patch as-is for\nnow.\n\nThanks.\n\n"},{"id":"302469","messageId":"xmqqshsqz0s1.fsf@gitster.mtv.corp.google.com","threadId":"43910","inReplyTo":"20160923111206.8596-1-pclouds@gmail.com","subject":"Re: [PATCH v2 4/3] init: combine set_git_dir_init() and init_db() into one","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-23T22:53:02Z","receivedAt":"2016-09-23T22:53:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> I think a separate commit for this is better than combining back to\n> 2/3 so we can explain the problem properly (without making 2/3 commit\n> message even longer)\n>\n> Not sure if you want to s/contains/contain/ in 2/3 by yourself or I\n> should resend the whole series. Let me know.\n\nI think this 4/3 is not quite enough to fix the damage to the code\ncaused by 2/3.\n\nGiven that\n\n - set_git_dir_init() is is the only one that sets original_git_dir,\n\n - create_default_files() is the only one that uses\n original_git_dir, and\n\n - init_db() is the only one that calls set_git_dir_init() and\n create_default_files()\n\nafter 4/3 is applied, we should be able to remove the global\nvariable 2/3 introduced, make init_db() receive that information as\nthe return value of set_git_dir_init(), and pass that as a parameter\nto create_default_files().\n"},{"id":"302484","messageId":"xmqq8tuhyvoa.fsf@gitster.mtv.corp.google.com","threadId":"43910","inReplyTo":"xmqqshsqz0s1.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 4/3] init: combine set_git_dir_init() and init_db() into one","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-24T18:55:33Z","receivedAt":"2016-09-24T18:55:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I think this 4/3 is not quite enough to fix the damage to the code\n> caused by 2/3.\n> ...\n> after 4/3 is applied, we should be able to remove the global\n> variable 2/3 introduced, make init_db() receive that information as\n> the return value of set_git_dir_init(), and pass that as a parameter\n> to create_default_files().\n\nThat would look something like this squashed into 4/3, I think. I\nam not sure if a commit that squashes 2/3, 3/3, 4/3 and this update\ntogether is harder to understand than keeping 2/3, 3/3 and a fixed\n4/3 separate, though. The end result looks much better structured\nthan before 2/3 is applied to my quick scan-through of the code.\n\nIn any case, the log message of 2/3 needs to be updated to retitle\nit, I think. \"do not ... more often than necessary\" makes it sound\nas if we were doing things that did not make any difference in the\nend result, wasting cycles. But what you actually wanted to achieve\nwas not to \"avoid unnecessary work\"--doing so gave a broken\nbehaviour and that was what you were fixing, \"do not record broken\ncore.worktree\", perhaps?\n\nThe solution (if we squash 2-4 and the fixup below) is to stop\nfeeding get_git_dir() to needs_work_tree_config(), because the\nparameter to the latter is the path to \".git\" that presumably is at\nthe top of the working tree, and get_git_dir() is not that when\n\"gitdir\" file is involved. So a rewritten log message may say\nsomething like...\n\n init: do not set unnecessary core.worktree\n\n The function needs_work_tree_config() that is called from\n create_default_files() is supposed to be fed the path to \".git\"\n that looks as if it is at the top of the working tree, and\n decide if that location matches the actual worktree being used.\n This comparison allows \"git init\" to decide if core.worktree\n needs to be recorded in the working tree.\n\n In the current code, however, we feed the return value from\n get_git_dir(), which can be totally different from what the\n function expects when \"gitdir\" file is involved. Instead of\n giving the path to the \".git\" at the top of the working tree, we\n end up feeding the actual path that the file points at.\n\n This original location of \".git\" however is only known to a\n helper function set_git_dir_init() that must be called before\n init_db() is called (they both have only two callsites, one in\n \"git init\" and the other in \"git clone\"), and in the current\n code, this original location is not visible to its callers.\n\n By doing the following two things:\n\n * Move call to set_git_dir_init() to init_db(), as the two must\n always be called in this order, and adjust its current\n callers.\n\n * Make set_git_dir_init() return the original location of \".git\"\n to the caller, which is init_db(), and have it passed to\n create_default_files() as a new parameter.\n\n pass the correct location down to needs_work_tree_config() to fix\n this.\n\nThis suggests that 2/3, 3/3 and fixed 4/3 could be done in two\nlogical steps. The first bullet point can be done as a separate\npreparatory step, and on top of that, the second bullet point can be\ndone as a separate \"fix\".\n\n\n\n builtin/init-db.c | 21 +++++++++++----------\n 1 file changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex ee7942f..527722c 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -23,7 +23,6 @@ static int init_is_bare_repository = 0;\n static int init_shared_repository = -1;\n static const char *init_db_template_dir;\n static const char *git_link;\n-static const char *original_git_dir;\n \n static void copy_templates_1(struct strbuf *path, struct strbuf *template,\n \t\t\t DIR *dir)\n@@ -172,7 +171,8 @@ static int needs_work_tree_config(const char *git_dir, const char *work_tree)\n \treturn 1;\n }\n \n-static int create_default_files(const char *template_path)\n+static int create_default_files(const char *template_path,\n+\t\t\t\tconst char *original_git_dir)\n {\n \tstruct stat st1;\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -312,11 +312,11 @@ static void create_object_directory(void)\n \tstrbuf_release(&path);\n }\n \n-static int set_git_dir_init(const char *git_dir,\n-\t\t\t const char *real_git_dir,\n-\t\t\t int exist_ok)\n+static char *set_git_dir_init(const char *git_dir,\n+\t\t\t const char *real_git_dir,\n+\t\t\t int exist_ok)\n {\n-\toriginal_git_dir = xstrdup(real_path(git_dir));\n+\tchar *original_git_dir = xstrdup(real_path(git_dir));\n \n \tif (real_git_dir) {\n \t\tstruct stat st;\n@@ -339,7 +339,7 @@ static int set_git_dir_init(const char *git_dir,\n \t\tgit_link = NULL;\n \t}\n \tstartup_info->have_repository = 1;\n-\treturn 0;\n+\treturn original_git_dir;\n }\n \n static void separate_git_dir(const char *git_dir)\n@@ -367,9 +367,10 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t const char *template_dir, unsigned int flags)\n {\n \tint reinit;\n+\tchar *original_git_dir;\n \n-\tset_git_dir_init(git_dir, real_git_dir, flags & INIT_DB_EXIST_OK);\n-\n+\tflags |= INIT_DB_EXIST_OK;\n+\toriginal_git_dir = set_git_dir_init(git_dir, real_git_dir, flags);\n \tgit_dir = get_git_dir();\n \n \tif (git_link)\n@@ -386,7 +387,7 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t */\n \tcheck_repository_format();\n \n-\treinit = create_default_files(template_dir);\n+\treinit = create_default_files(template_dir, original_git_dir);\n \n \tcreate_object_directory();\n \n"},{"id":"302497","messageId":"20160925031329.GA9124@ash","threadId":"43910","inReplyTo":"xmqq8tuhyvoa.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 4/3] init: combine set_git_dir_init() and init_db() into one","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-25T03:13:29Z","receivedAt":"2016-09-25T03:14:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Sep 24, 2016 at 11:55:33AM -0700, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > I think this 4/3 is not quite enough to fix the damage to the code\n> > caused by 2/3.\n> > ...\n> > after 4/3 is applied, we should be able to remove the global\n> > variable 2/3 introduced, make init_db() receive that information as\n> > the return value of set_git_dir_init(), and pass that as a parameter\n> > to create_default_files().\n> \n> That would look something like this squashed into 4/3, I think. I\n> am not sure if a commit that squashes 2/3, 3/3, 4/3 and this update\n> together is harder to understand than keeping 2/3, 3/3 and a fixed\n> 4/3 separate, though. The end result looks much better structured\n> than before 2/3 is applied to my quick scan-through of the code.\n> ...\n\nHow about this?\n\n [1/5] init: correct re-initialization from a linked worktree\n [2/5] init: call set_git_dir_init() from within init_db()\n [3/5] init: kill set_git_dir_init()\n [4/5] init: do not set unnecessary core.worktree\n [5/5] init: kill git_link variable\n\nI went a bit further, merging set_git_dir_init() back to init_db() so\nI can kill \"git_link\" variable cleanly in 5/5. 3/5 does make init_db()\na bit longer, but not to the alarming level yet. By 4/5,\nset_git_dir_init() is gone, so init_db() just needs to save and pass\noriginal_git_dir down to needs_work_tree_config().\n--\nDuy\n"},{"id":"302498","messageId":"20160925031440.10435-1-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160925031329.GA9124@ash","subject":"[PATCH v3 1/5] init: correct re-initialization from a linked worktree","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-25T03:14:36Z","receivedAt":"2016-09-25T03:16:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When 'git init' is called from a linked worktree, we treat '.git'\ndir (which is $GIT_COMMON_DIR/worktrees/something) as the main\n'.git' (i.e. $GIT_COMMON_DIR) and populate the whole repository skeleton\nin there. It does not harm anything (*) but it is still wrong.\n\nSince 'git init' calls set_git_dir() at preparation time, which\nindirectly calls get_common_dir() and correctly detects multiple\nworktree setup, all git_path_buf() calls in create_default_files() will\nreturn correct paths in both single and multiple worktree setups. The\nonly thing left is copy_templates(), which targets $GIT_DIR, not\n$GIT_COMMON_DIR.\n\nFix that with get_git_common_dir(). This function will return $GIT_DIR\nin single-worktree setup, so we don't have to make a special case for\nmultiple-worktree here.\n\n(*) It does in fact, thanks to another bug. More on that later.\n\nNoticed-by: Max Nordlund \nHelped-by: Michael J Gruber \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/init-db.c | 2 +-\n t/t0001-init.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex cc09fca..d5d7558 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -138,7 +138,7 @@ static void copy_templates(const char *template_dir)\n \t\tgoto close_free_return;\n \t}\n \n-\tstrbuf_addstr(&path, get_git_dir());\n+\tstrbuf_addstr(&path, get_git_common_dir());\n \tstrbuf_complete(&path, '/');\n \tcopy_templates_1(&path, &template_path, dir);\n close_free_return:\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 8ffbbea..488564b 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -393,4 +393,19 @@ test_expect_success 'remote init from does not use config from cwd' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 're-init from a linked worktree' '\n+\tgit init main-worktree &&\n+\t(\n+\t\tcd main-worktree &&\n+\t\ttest_commit first &&\n+\t\tgit worktree add ../linked-worktree &&\n+\t\tmv .git/info/exclude expected-exclude &&\n+\t\tfind .git/worktrees -print | sort >expected &&\n+\t\tgit -C ../linked-worktree init &&\n+\t\ttest_cmp expected-exclude .git/info/exclude &&\n+\t\tfind .git/worktrees -print | sort >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n test_done\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302499","messageId":"20160925031440.10435-2-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160925031440.10435-1-pclouds@gmail.com","subject":"[PATCH v3 2/5] init: call set_git_dir_init() from within init_db()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-25T03:14:37Z","receivedAt":"2016-09-25T03:16:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The next commit requires that set_git_dir_init() must be called before\ninit_db(). Let's make sure nobody can do otherwise.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/clone.c | 15 +++++++--------\n builtin/init-db.c | 18 +++++++++++-------\n cache.h | 5 +++--\n 3 files changed, 21 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 6616392..29b1832 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -928,23 +928,22 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tset_git_work_tree(work_tree);\n \t}\n \n-\tjunk_git_dir = git_dir;\n+\tjunk_git_dir = real_git_dir ? real_git_dir : git_dir;\n \tif (safe_create_leading_directories_const(git_dir) < 0)\n \t\tdie(_(\"could not create leading directories of '%s'\"), git_dir);\n \n-\tset_git_dir_init(git_dir, real_git_dir, 0);\n-\tif (real_git_dir) {\n-\t\tgit_dir = real_git_dir;\n-\t\tjunk_git_dir = real_git_dir;\n-\t}\n-\n \tif (0 <= option_verbosity) {\n \t\tif (option_bare)\n \t\t\tfprintf(stderr, _(\"Cloning into bare repository '%s'...\\n\"), dir);\n \t\telse\n \t\t\tfprintf(stderr, _(\"Cloning into '%s'...\\n\"), dir);\n \t}\n-\tinit_db(option_template, INIT_DB_QUIET);\n+\n+\tinit_db(git_dir, real_git_dir, option_template, INIT_DB_QUIET);\n+\n+\tif (real_git_dir)\n+\t\tgit_dir = real_git_dir;\n+\n \twrite_config(&option_config);\n \n \tgit_config(git_default_config, NULL);\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex d5d7558..5cb05d9 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -311,8 +311,9 @@ static void create_object_directory(void)\n \tstrbuf_release(&path);\n }\n \n-int set_git_dir_init(const char *git_dir, const char *real_git_dir,\n-\t\t int exist_ok)\n+static int set_git_dir_init(const char *git_dir,\n+\t\t\t const char *real_git_dir,\n+\t\t\t int exist_ok)\n {\n \tif (real_git_dir) {\n \t\tstruct stat st;\n@@ -359,10 +360,14 @@ static void separate_git_dir(const char *git_dir)\n \twrite_file(git_link, \"gitdir: %s\", git_dir);\n }\n \n-int init_db(const char *template_dir, unsigned int flags)\n+int init_db(const char *git_dir, const char *real_git_dir,\n+\t const char *template_dir, unsigned int flags)\n {\n \tint reinit;\n-\tconst char *git_dir = get_git_dir();\n+\n+\tset_git_dir_init(git_dir, real_git_dir, flags & INIT_DB_EXIST_OK);\n+\n+\tgit_dir = get_git_dir();\n \n \tif (git_link)\n \t\tseparate_git_dir(git_dir);\n@@ -582,7 +587,6 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\tset_git_work_tree(work_tree);\n \t}\n \n-\tset_git_dir_init(git_dir, real_git_dir, 1);\n-\n-\treturn init_db(template_dir, flags);\n+\tflags |= INIT_DB_EXIST_OK;\n+\treturn init_db(git_dir, real_git_dir, template_dir, flags);\n }\ndiff --git a/cache.h b/cache.h\nindex b2d77f3..7fc875f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -525,9 +525,10 @@ extern void verify_non_filename(const char *prefix, const char *name);\n extern int path_inside_repo(const char *prefix, const char *path);\n \n #define INIT_DB_QUIET 0x0001\n+#define INIT_DB_EXIST_OK 0x0002\n \n-extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int);\n-extern int init_db(const char *template_dir, unsigned int flags);\n+extern int init_db(const char *git_dir, const char *real_git_dir,\n+\t\t const char *template_dir, unsigned int flags);\n \n extern void sanitize_stdfds(void);\n extern int daemonize(void);\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302500","messageId":"20160925031440.10435-3-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160925031440.10435-1-pclouds@gmail.com","subject":"[PATCH v3 3/5] init: kill set_git_dir_init()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-25T03:14:38Z","receivedAt":"2016-09-25T03:16:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is a pure code move, necessary to kill the global variable git_link\nlater (and also helps a bit in the next patch).\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/init-db.c | 50 +++++++++++++++++++++-----------------------------\n 1 file changed, 21 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 5cb05d9..68c1ae3 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -311,34 +311,6 @@ static void create_object_directory(void)\n \tstrbuf_release(&path);\n }\n \n-static int set_git_dir_init(const char *git_dir,\n-\t\t\t const char *real_git_dir,\n-\t\t\t int exist_ok)\n-{\n-\tif (real_git_dir) {\n-\t\tstruct stat st;\n-\n-\t\tif (!exist_ok && !stat(git_dir, &st))\n-\t\t\tdie(_(\"%s already exists\"), git_dir);\n-\n-\t\tif (!exist_ok && !stat(real_git_dir, &st))\n-\t\t\tdie(_(\"%s already exists\"), real_git_dir);\n-\n-\t\t/*\n-\t\t * make sure symlinks are resolved because we'll be\n-\t\t * moving the target repo later on in separate_git_dir()\n-\t\t */\n-\t\tgit_link = xstrdup(real_path(git_dir));\n-\t\tset_git_dir(real_path(real_git_dir));\n-\t}\n-\telse {\n-\t\tset_git_dir(real_path(git_dir));\n-\t\tgit_link = NULL;\n-\t}\n-\tstartup_info->have_repository = 1;\n-\treturn 0;\n-}\n-\n static void separate_git_dir(const char *git_dir)\n {\n \tstruct stat st;\n@@ -364,9 +336,29 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t const char *template_dir, unsigned int flags)\n {\n \tint reinit;\n+\tint exist_ok = flags & INIT_DB_EXIST_OK;\n \n-\tset_git_dir_init(git_dir, real_git_dir, flags & INIT_DB_EXIST_OK);\n+\tif (real_git_dir) {\n+\t\tstruct stat st;\n \n+\t\tif (!exist_ok && !stat(git_dir, &st))\n+\t\t\tdie(_(\"%s already exists\"), git_dir);\n+\n+\t\tif (!exist_ok && !stat(real_git_dir, &st))\n+\t\t\tdie(_(\"%s already exists\"), real_git_dir);\n+\n+\t\t/*\n+\t\t * make sure symlinks are resolved because we'll be\n+\t\t * moving the target repo later on in separate_git_dir()\n+\t\t */\n+\t\tgit_link = xstrdup(real_path(git_dir));\n+\t\tset_git_dir(real_path(real_git_dir));\n+\t}\n+\telse {\n+\t\tset_git_dir(real_path(git_dir));\n+\t\tgit_link = NULL;\n+\t}\n+\tstartup_info->have_repository = 1;\n \tgit_dir = get_git_dir();\n \n \tif (git_link)\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302501","messageId":"20160925031440.10435-4-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160925031440.10435-1-pclouds@gmail.com","subject":"[PATCH v3 4/5] init: do not set unnecessary core.worktree","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-25T03:14:39Z","receivedAt":"2016-09-25T03:16:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The function needs_work_tree_config() that is called from\ncreate_default_files() is supposed to be fed the path to \".git\" that\nlooks as if it is at the top of the working tree, and decide if that\nlocation matches the actual worktree being used. This comparison allows\n\"git init\" to decide if core.worktree needs to be recorded in the\nworking tree.\n\nIn the current code, however, we feed the return value from\nget_git_dir(), which can be totally different from what the function\nexpects when \"gitdir\" file is involved. Instead of giving the path to\nthe \".git\" at the top of the working tree, we end up feeding the actual\npath that the file points at.\n\nThis original location of \".git\" however is only known to init_db().\nMake init_db() save it and have it passed to create_default_files() as a\nnew parameter, which passes the correct location down to\nneeds_work_tree_config() to fix this.\n\nNoticed-by: Max Nordlund \nHelped-by: Michael J Gruber \nHelped-by: Junio C Hamano \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/init-db.c | 9 ++++++---\n t/t0001-init.sh | 2 ++\n 2 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 68c1ae3..8069cd2 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -171,7 +171,8 @@ static int needs_work_tree_config(const char *git_dir, const char *work_tree)\n \treturn 1;\n }\n \n-static int create_default_files(const char *template_path)\n+static int create_default_files(const char *template_path,\n+\t\t\t\tconst char *original_git_dir)\n {\n \tstruct stat st1;\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -263,7 +264,7 @@ static int create_default_files(const char *template_path)\n \t\t/* allow template config file to override the default */\n \t\tif (log_all_ref_updates == -1)\n \t\t\tgit_config_set(\"core.logallrefupdates\", \"true\");\n-\t\tif (needs_work_tree_config(get_git_dir(), work_tree))\n+\t\tif (needs_work_tree_config(original_git_dir, work_tree))\n \t\t\tgit_config_set(\"core.worktree\", work_tree);\n \t}\n \n@@ -337,6 +338,7 @@ int init_db(const char *git_dir, const char *real_git_dir,\n {\n \tint reinit;\n \tint exist_ok = flags & INIT_DB_EXIST_OK;\n+\tchar *original_git_dir = xstrdup(real_path(git_dir));\n \n \tif (real_git_dir) {\n \t\tstruct stat st;\n@@ -375,7 +377,7 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t */\n \tcheck_repository_format();\n \n-\treinit = create_default_files(template_dir);\n+\treinit = create_default_files(template_dir, original_git_dir);\n \n \tcreate_object_directory();\n \n@@ -412,6 +414,7 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t\t git_dir, len && git_dir[len-1] != '/' ? \"/\" : \"\");\n \t}\n \n+\tfree(original_git_dir);\n \treturn 0;\n }\n \ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 488564b..b8fc588 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -400,9 +400,11 @@ test_expect_success 're-init from a linked worktree' '\n \t\ttest_commit first &&\n \t\tgit worktree add ../linked-worktree &&\n \t\tmv .git/info/exclude expected-exclude &&\n+\t\tcp .git/config expected-config &&\n \t\tfind .git/worktrees -print | sort >expected &&\n \t\tgit -C ../linked-worktree init &&\n \t\ttest_cmp expected-exclude .git/info/exclude &&\n+\t\ttest_cmp expected-config .git/config &&\n \t\tfind .git/worktrees -print | sort >actual &&\n \t\ttest_cmp expected actual\n \t)\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"302502","messageId":"20160925031440.10435-5-pclouds@gmail.com","threadId":"43910","inReplyTo":"20160925031440.10435-1-pclouds@gmail.com","subject":"[PATCH v3 5/5] init: kill git_link variable","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-09-25T03:14:40Z","receivedAt":"2016-09-25T03:16:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/init-db.c | 16 ++++------------\n 1 file changed, 4 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 8069cd2..37e318b 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -22,7 +22,6 @@\n static int init_is_bare_repository = 0;\n static int init_shared_repository = -1;\n static const char *init_db_template_dir;\n-static const char *git_link;\n \n static void copy_templates_1(struct strbuf *path, struct strbuf *template,\n \t\t\t DIR *dir)\n@@ -312,7 +311,7 @@ static void create_object_directory(void)\n \tstrbuf_release(&path);\n }\n \n-static void separate_git_dir(const char *git_dir)\n+static void separate_git_dir(const char *git_dir, const char *git_link)\n {\n \tstruct stat st;\n \n@@ -349,22 +348,15 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t\tif (!exist_ok && !stat(real_git_dir, &st))\n \t\t\tdie(_(\"%s already exists\"), real_git_dir);\n \n-\t\t/*\n-\t\t * make sure symlinks are resolved because we'll be\n-\t\t * moving the target repo later on in separate_git_dir()\n-\t\t */\n-\t\tgit_link = xstrdup(real_path(git_dir));\n \t\tset_git_dir(real_path(real_git_dir));\n+\t\tgit_dir = get_git_dir();\n+\t\tseparate_git_dir(git_dir, original_git_dir);\n \t}\n \telse {\n \t\tset_git_dir(real_path(git_dir));\n-\t\tgit_link = NULL;\n+\t\tgit_dir = get_git_dir();\n \t}\n \tstartup_info->have_repository = 1;\n-\tgit_dir = get_git_dir();\n-\n-\tif (git_link)\n-\t\tseparate_git_dir(git_dir);\n \n \tsafe_create_dir(git_dir, 0);\n \n-- \n2.8.2.524.g6ff3d78\n\n"}]}