git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] setup.c: don't setup in discover_git_directory()

From
Glen Choo via GitGitGadget <gitgitgadget@gmail.com>
Date
Jun 14, 2023, 19:35 UTC
Message-ID
<pull.1526.git.git.1686771358484.gitgitgadget@gmail.com>
From: Glen Choo <chooglen@google.com>

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.

Signed-off-by: Glen Choo <chooglen@google.com>
---
    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/kl6llegnfccw.fsf@chooglen-macbookpro.roam.corp.google.com
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(-)
diff --git a/setup.c b/setup.c
index 458582207ea..bbd95f52c0f 100644
--- a/setup.c
+++ b/setup.c
@@ -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
Next: Johannes Schindelin
Message 1 of 3 in “setup.c: don't setup in discover_git_directory()”
  1. setup.c: don't setup in discover_git_directory()Glen Choo via GitGitGadget, Jun 14, 2023
  2. Johannes SchindelinJun 16, 2023
  3. Junio C HamanoJun 16, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.