Nguyen Thai Ngoc Duy wrote:
> 2011/1/19 Jonathan Nieder <jrnieder@xxxxxxxxx>:
>> @@ -411,6 +411,16 @@ static const char *setup_discovered_git_dir(const char *gitdir,
>> Â Â Â Âif (check_repository_format_gently(gitdir, nongit_ok))
>> Â Â Â Â Â Â Â Âreturn NULL;
>>
>> + Â Â Â /* Accept --work-tree to support old scripts that played with fire. */
>> + Â Â Â if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
>
> Can we leave git_work_tree_cfg out? If this code is to support misused
> scripts, then $GIT_WORK_TREE alone ought to be enough.
Sure, it would be easy to exclude git_work_tree_cfg here. I guess the
relevant question is
Maaartin wrote:
> On 11-01-19 13:42, Jonathan Nieder wrote:
>> Unfortunately the existence of GIT_WORK_TREE makes it tempting to
>> use without setting GIT_DIR.
>
> Maybe I'm asking nonsense, but why should I always use both?
That is, why do we want to discourage setting the work tree without
GIT_DIR in the first place?
The issues seem to be:
1. Previously there was some confusion about what path the worktree
is relative to. Now setup_explicit_git_dir makes it clear:
+ GIT_WORK_TREE and --work-tree are relative to the original cwd;
+ the "[core] worktree" setting is relative to the gitdir.
2. Previously there was some confusion about the right order of
operations --- move to worktree first, or find the git dir?
Now the setup procedure is clearly "first find git dir, then
act on worktree".
3. A global "[core] worktree" setting with relative path is
nonsensical since there is not necessarily a gitdir for it to be
relative to.
4. Likewise, setting GIT_WORK_TREE when outside any git repository
has no clear meaning.
5. The interaction with core.bare and implicit bareness are not
obvious. Clearly core.bare should conflict with core.worktree,
but can GIT_WORK_TREE override that? Maybe
check_repository_format_gently is the right place for this check
(rather than the setup procedure).
(1) and (2) have been resolved by your work (nice!), (3) seems like
a case of "don't do that, then", and (4) out to error out in
setup_nongit (though my patch doesn't take care of that). Given an
answer to (5) we could wholeheartedly and consistently support
worktree with implicit gitdir, as a new feature.
Is that a fair summary?
[...]
>> + Â Â Â Â Â Â Â if (offset != len && !is_absolute_path(gitdir))
>> + Â Â Â Â Â Â Â Â Â Â Â gitdir = xstrdup(make_absolute_path(gitdir));
>
> The behavior regarding relative $GIT_WORK_TREE before nd/setup series
> is inconsistent. If setup_git_directory() is used, work_tree is
> relative to user's cwd. In other cases, when get_git_work_tree() is
> called, work_tree is made absolute relative to _current_ cwd (usually
> at discovered work_tree root). Which way do you want to keep?
The former. But this is worrisome: scripts using cd_to_toplevel are
getting the latter behavior. Maybe a warning for relative
GIT_WORK_TREE is in order, like you hinted before.
--
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