setup.c: don't setup in discover_git_directory()
PATCH 1/1 setup.c: don't setup in discover_git_directory()
From: Glen Choo <[email protected]>
discover_git_directory() started modifying the_repository in ebaf3bcf1ae (repository: move global r_f_p_c to repo struct, 2021-06-17), when, in the repository setup process, we started copying members from the "struct repository_format" we're inspecting to the appropriate "struct repository". However, discover_git_directory() isn't actually used in the setup process (its only caller in the Git binary is read_early_config()), so it shouldn't be doing this setup at all!
As explained by 16ac8b8db6 (setup: introduce the discover_git_directory() function, 2017-03-13) and the comment on its declaration, discover_git_directory() is intended to be an entrypoint into setup.c machinery that allows the Git directory to be discovered without side effects, e.g. so that read_early_config() can read ".git/config" before the_repository has been set up.
Fortunately, we didn't start to rely on this unintended behavior between then and now, so we let's just remove it. It isn't harming anyone, but it's confusing.
---
setup.c: don't setup in discover_git_directory()
This is the scissors patch I sent on Victoria's series [1], but rebased onto "master" since that series hasn't been merged yet. The merge conflict resolution is to delete all of the conflicting lines:
- the_repository->repository_format_worktree_config =
- candidate.worktree_config;
-
IOW it's the original scissors patch if queued on top of Victoria's series, but it might be cleaner to invert that, i.e. if we pretended that this was in "master" already, there wouldn't be reason to add those lines to begin with.
[1]
https://lore.kernel.org/git/[email protected]
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1526%2Fchooglen%2Fpush-nknkwmnkxolv-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1526/chooglen/push-nknkwmnkxolv-v1
Pull-Request: https://github.com/git/git/pull/1526
setup.c | 5 ----- 1 file changed, 5 deletions(-)
setup.c +0 -5
@@ -1423,11 +1423,6 @@ int discover_git_directory(struct strbuf *commondir,return -1 ; } -/* take ownership of candidate.partial_clone */ - the_repository->repository_format_partial_clone = - candidate.partial_clone; - candidate.partial_clone =NULL ; -clear_repository_format (&candidate);return 0 ; }
base-commit: fe86abd7511a9a6862d5706c6fa1d9b57a63ba09
Signature · 1 line
gitgitgadget
Hi Glen,
Quoted 17 lines · Glen Choo via GitGitGadget
From: Glen Choo <[email protected]>
discover_git_directory()started modifyingthe_repositoryinebaf3bcf1ae(repository: move globalr_f_p_cto repo struct, 2021-06-17), when, in the repository setup process, we started copying members from the "structrepository_format" we're inspecting to the appropriate "struct repository". However,discover_git_directory()isn't actually used in the setup process (its only caller in the Git binary isread_early_config()), so it shouldn't be doing this setup at all!As explained by
16ac8b8db6(setup: introduce thediscover_git_directory()function, 2017-03-13) and the comment on its declaration,discover_git_directory()is intended to be an entrypoint intosetup.cmachinery that allows the Git directory to be discovered without side effects, e.g. so thatread_early_config()can read ".git/config" beforethe_repositoryhas been set up.
Quoted 5 lines
Fortunately, we didn't start to rely on this unintended behavior between then and now, so we let's just remove it. It isn't harming anyone, but it's confusing.
Signed-off-by Glen Choo <[email protected]>
As the author of the commit whose rationale was quoted above, I am delighted to provide my ACK to both commit message and diff.
Thanks,
Johannes
Quoted 48 lines
---
setup.c: don't setup indiscover_git_directory()This is the scissors patch I sent on Victoria's series [1], but rebased onto "master" since that series hasn't been merged yet. The merge conflict resolution is to delete all of the conflicting lines:
-
the_repository->repository_format_worktree_config=
- candidate.worktree_config;
-IOW it's the original scissors patch if queued on top of Victoria's series, but it might be cleaner to invert that, i.e. if we pretended that this was in "master" already, there wouldn't be reason to add those lines to begin with.
[1]
https://lore.kernel.org/git/[email protected]Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1526%2Fchooglen%2Fpush-nknkwmnkxolv-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1526/chooglen/push-nknkwmnkxolv-v1
Pull-Request: https://github.com/git/git/pull/1526setup.c | 5 ----- 1 file changed, 5 deletions(-)setup.c +0
-5@@ -1423,11 +1423,6 @@ int discover_git_directory(struct strbuf *commondir,return -1 ; } -/* take ownership of candidate.partial_clone */ - the_repository->repository_format_partial_clone = - candidate.partial_clone; - candidate.partial_clone =NULL ; -clear_repository_format (&candidate);return 0 ; }base-commit:
fe86abd7511a9a6862d5706c6fa1d9b57a63ba09--
gitgitgadget
Quoted 11 lines · Johannes Schindelin
Quoted 11 lines
As explained by
16ac8b8db6(setup: introduce thediscover_git_directory()function, 2017-03-13) and the comment on its declaration,discover_git_directory()is intended to be an entrypoint intosetup.cmachinery that allows the Git directory to be discovered without side effects, e.g. so thatread_early_config()can read ".git/config" beforethe_repositoryhas been set up.Fortunately, we didn't start to rely on this unintended behavior between then and now, so we let's just remove it. It isn't harming anyone, but it's confusing.
Quoted 7 lines
Quoted 1 line
Signed-off-by Glen Choo <[email protected]>As the author of the commit whose rationale was quoted above, I am delighted to provide my ACK to both commit message and diff.
Thanks,
Johannes
Thanks, both, for writing and reviewing.
Queued.