Re: [PATCH] init: don't set core.worktree when initializing /.git
flat view
From: Jonathan Nieder <hidden>
Date: 2016-06-15 23:04:19
Jeff King wrote:
No tests, as we would need to be able to write to "/" to do so.
t1509-root-worktree.sh is supposed to test the repository-at-/ case. But I wouldn't be surprised if it's bitrotted, since people don't set up a throwaway chroot or VM for tests too often. [...]
The current behavior isn't _wrong_, in the sense that it's OK to set core.worktree when we don't need to. But I think it is unnecessarily confusing to users who expect to be able to move .git directories around, because it usually just works. So I'd call this an extremely minor bug.
This belongs in the commit message. [...]
quoted hunk ↗ jump to hunk
--- a/builtin/init-db.c +++ b/builtin/init-db.c@@ -182,6 +182,21 @@ static int git_init_db_config(const char *k, const char *v, void *cb) return 0; } +/* + * If the git_dir is not directly inside the working tree, then git will not + * find it by default, and we need to set the worktree explicitly. + */ +static int needs_work_tree_config(const char *git_dir, const char *work_tree) +{ + /* special case that is missed by the general rule below */
(optional) I'd leave out this comment --- it seems obvious enough in context and the purpose of the comment is unobvious without looking at the history.
quoted hunk ↗ jump to hunk
+ if (!strcmp(work_tree, "/") && !strcmp(git_dir, "/.git")) + return 0; + if (skip_prefix(git_dir, work_tree, &git_dir) && + !strcmp(git_dir, "/.git")) + return 0;
work_tree has been cleaned up with real_path, so in the normal case it contains getcwd() output which will not end with a / unless it has to. The only exception I can see is when git hits the MAXDEPTH limit for symlink resolution (5 nested symlinks), in which case we take what we find instead of erroring out, which looks like a bug. We have called set_git_dir_init so git_dir has been cleaned up by real_path in the same way. Good. With or without the commit message, comment, and test improvements mentioned above, Reviewed-by: Jonathan Nieder <redacted> Thanks.