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.
[...]
> --- 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.
> + 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 <[email protected]>
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
