Re: [PATCH 3/3] setup: always honor GIT_WORK_TREE and core.worktree
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 19, 2011, 19:13 UTC
- Message-ID
- <7vei88u2os.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20110119124230.GD23222@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 5 quoted lines
> In the usual case, git commands are run from within a git worktree. > The toplevel directory and associated .git dir are found by chdir-ing > to .. repeatedly until .git is a valid repository. > > That behavior can be overridden in two ways.
Hmm, I wouldn't call them "two ways".
We originally had GIT_DIR and people who wanted to use a working tree without an embedded .git (hence having to use GIT_DIR) complained that they cannot work from subdirectories while cd'ing around; GIT_WORK_TREE was added to augment the mechanism to allow them to specify where their root is. Saying "two ways" sounds as if it is sane to do one without doing the other, which is not the case here.
For the same reason (another tangent), although t1510 seems to have complex rules about "relative to", it does not make much sense to specify a path in GIT_WORK_TREE relative to user's $cwd. The mechanism is to allow people to chdir around inside their working tree that does not have an embedded .git directory to begin with.
Show 41 quoted lines
> diff --git a/setup.c b/setup.c
> index 3d73269..c0f5846 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -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) {
> + warning("pretending GIT_DIR was supplied alongside GIT_WORK_TREE");
> + if (offset != len && !is_absolute_path(gitdir))
> + gitdir = xstrdup(make_absolute_path(gitdir));
> + if (chdir(cwd))
> + die_errno("Could not come back to cwd");
> + return setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);
> + }
> +
> /* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */
> if (is_bare_repository_cfg > 0) {
> set_git_dir(offset == len ? gitdir : make_absolute_path(gitdir));
> @@ -443,6 +453,19 @@ static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongi
> if (check_repository_format_gently(".", nongit_ok))
> return NULL;
>
> + /*
> + * Accept --work-tree, reluctantly.
> + */
> + if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
> + const char *gitdir;
> +
> + warning("pretending GIT_DIR was supplied alongside GIT_WORK_TREE");
> + gitdir = offset == len ? "." : xmemdupz(cwd, offset);
> + if (chdir(cwd))
> + die_errno("Could not come back to cwd");
> + return setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);
> + }
> +
> inside_git_dir = 1;
> inside_work_tree = 0;
> if (offset != len) {This seems to be a good workaround to resurrect the historical buggy behaviour, but it makes the already ugly code even uglier. Can we do something about it?
I wonder if it would make it more readable if we restructure setup_git_directory_gently_1() slightly, perhaps by making the loop to discover the .git directory/file into a separate helper function that returns:
- the path to the discovered .git; - if the .git was found as an ancestor of the original $cwd (to handle "bare"); - if the discovery was aborted due to ceiling (or hitting root);
and make the caller decide what kind of setup to call outside the loop.
But that is a minor point.