Re: [PATCH] builtins: reset startup_info->have_run_setup_gitdir when unsetting up repository
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Mar 28, 2010, 00:34 UTC
- Message-ID
- <20100328003416.GA8011@progeny.tock>
- In-Reply-To
- <1269681184-1992-2-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy wrote:
Show 12 quoted lines
> - alias handling will always be done before help_unknown_cmd() > - alias handling code will search and set up repository if found > - alias handline code will not undo repository setup > > These ensure that repository will always be set up (or attempted to > set up) before help_unknown_cmd(), so there is no issue. But the setup > dependency here is subtle. It may break some day if someone reorders > the loop, for example. > > Make a note about this. > > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Makes sense, thanks.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Nguyễn Thái Ngọc Duy wrote:
Show 5 quoted lines
> startup_info->have_run_setup_gitdir is used to guard unallowed access > to repository. When a repository has been set up and the real command > does not expect any setup, we should revert to the original "fresh" > state, including startup_info->have_run_setup_gitdir. Otherwise, the > next attempt to set up repository will fail.
[...]
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
The test this adds fails for current master, too.
The cause: core.worktree gets set to point to the trash directory. This results from a facet of the test I was not paying attention to --- the trash directory is a git repository itself, which poisons git with some extra information it hadn’t learned to forget yet.
The third and fourth tests below fail in master for this reason.
The first test passes everywhere; it is just checking basic behavior.
The second test passes in master and fails in nd/setup without the git.c hunk of your patch. It is a more targeted test, meant to check only for the problem your patch solves.
Of course, with your patch applied, all four tests pass. Thanks.
Reviewed-and-tested-by: Jonathan Nieder <jrnieder@gmail.com>
diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 6757734..6d6766e 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -33,6 +33,59 @@ test_expect_success 'plain' ' check_config plain/.git false unset ' +test_expect_success 'plain nested in bare' ' + ( + unset GIT_DIR GIT_WORK_TREE && + git init --bare bare-ancestor.git && + cd bare-ancestor.git && + mkdir plain-nested && + cd plain-nested && + git init + ) && + check_config bare-ancestor.git/plain-nested/.git false unset +' + +test_expect_success 'plain through aliased command' ' + mkdir alias-config && + echo "[alias] aliasedinit = init" >alias-config/.gitconfig && + ( + unset GIT_DIR GIT_WORK_TREE GIT_CONFIG_NOGLOBAL && + GIT_CEILING_DIRECTORIES=$(pwd) && + HOME=$(pwd)/alias-config && + export GIT_CEILING_DIRECTORIES HOME && + mkdir plain-aliased && + cd plain-aliased && + git aliasedinit + ) && + check_config plain-aliased/.git false unset +' + +test_expect_success 'plain nested through aliased command' ' + ( + unset GIT_DIR GIT_WORK_TREE && + git init plain-ancestor-aliased && + cd plain-ancestor-aliased && + echo "[alias] aliasedinit = init" >>.git/config && + mkdir plain-nested && + cd plain-nested && + git aliasedinit + ) && + check_config plain-ancestor-aliased/plain-nested/.git false unset +' + +test_expect_success 'plain nested in bare through aliased command' ' + ( + unset GIT_DIR GIT_WORK_TREE && + git init --bare bare-ancestor-aliased.git && + cd bare-ancestor-aliased.git && + echo "[alias] aliasedinit = init" >>config && + mkdir plain-nested && + cd plain-nested && + git aliasedinit + ) && + check_config bare-ancestor-aliased.git/plain-nested/.git false unset +' + test_expect_success 'plain with GIT_WORK_TREE' ' if ( unset GIT_DIR