All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Karthik Nayak <karthik.188@gmail.com>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 6/6] repository: drop `initialize_the_repository()`
Date: Thu, 18 Apr 2024 14:14:33 +0200	[thread overview]
Message-ID: <ed722b9b4b2d0d173eff2945812e6a15d6470c38.1713442061.git.ps@pks.im> (raw)
In-Reply-To: <cover.1713442061.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 4465 bytes --]

Now that we have dropped `the_index`, `initialize_the_repository()`
doesn't really do a lot anymore except for setting up the pointer for
`the_repository` and then calling `initialize_repository()`. The former
can be replaced by statically initializing the pointer though, which
basically makes this function moot.

Convert callers to instead call `initialize_repository(the_repository)`
and drop `initialize_thee_repository()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 common-main.c                |  2 +-
 oss-fuzz/fuzz-commit-graph.c |  3 ++-
 repository.c                 | 29 +++++++++++++++++++++--------
 repository.h                 |  2 +-
 t/helper/test-read-cache.c   |  2 +-
 5 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/common-main.c b/common-main.c
index 033778b3c5..b86f40600f 100644
--- a/common-main.c
+++ b/common-main.c
@@ -48,7 +48,7 @@ int main(int argc, const char **argv)
 	setlocale(LC_CTYPE, "");
 	git_setup_gettext();
 
-	initialize_the_repository();
+	initialize_repository(the_repository);
 
 	attr_start();
 
diff --git a/oss-fuzz/fuzz-commit-graph.c b/oss-fuzz/fuzz-commit-graph.c
index 2992079dd9..fe15e2c225 100644
--- a/oss-fuzz/fuzz-commit-graph.c
+++ b/oss-fuzz/fuzz-commit-graph.c
@@ -11,7 +11,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
 {
 	struct commit_graph *g;
 
-	initialize_the_repository();
+	initialize_repository(the_repository);
+
 	/*
 	 * Initialize the_repository with commit-graph settings that would
 	 * normally be read from the repository's gitdir. We want to avoid
diff --git a/repository.c b/repository.c
index 089edbffa2..2118f563e3 100644
--- a/repository.c
+++ b/repository.c
@@ -17,22 +17,35 @@
 
 /* The main repository */
 static struct repository the_repo;
-struct repository *the_repository;
+struct repository *the_repository = &the_repo;
 
-static void initialize_repository(struct repository *repo)
+void initialize_repository(struct repository *repo)
 {
 	repo->objects = raw_object_store_new();
 	repo->remote_state = remote_state_new();
 	repo->parsed_objects = parsed_object_pool_new();
 	ALLOC_ARRAY(repo->index, 1);
 	index_state_init(repo->index, repo);
-}
 
-void initialize_the_repository(void)
-{
-	the_repository = &the_repo;
-	initialize_repository(the_repository);
-	repo_set_hash_algo(&the_repo, GIT_HASH_SHA1);
+	/*
+	 * Unfortunately, we need to keep this hack around for the time being:
+	 *
+	 *   - Not setting up the hash algorithm for `the_repository` leads to
+	 *     crashes because `the_hash_algo` is a macro that expands to
+	 *     `the_repository->hash_algo`. So if Git commands try to access
+	 *     `the_hash_algo` without a Git directory we crash.
+	 *
+	 *   - Setting up the hash algorithm to be SHA1 by default breaks other
+	 *     commands when running with SHA256.
+	 *
+	 * This is another point in case why having global state is a bad idea.
+	 * Eventually, we should remove this hack and stop setting the hash
+	 * algorithm in this function altogether. Instead, it should only ever
+	 * be set via our repository setup procedures. But that requires more
+	 * work.
+	 */
+	if (repo == the_repository)
+		repo_set_hash_algo(repo, GIT_HASH_SHA1);
 }
 
 static void expand_base_dir(char **out, const char *in,
diff --git a/repository.h b/repository.h
index 6f4af15417..41ed22543a 100644
--- a/repository.h
+++ b/repository.h
@@ -207,7 +207,7 @@ void repo_set_worktree(struct repository *repo, const char *path);
 void repo_set_hash_algo(struct repository *repo, int algo);
 void repo_set_compat_hash_algo(struct repository *repo, int compat_algo);
 void repo_set_ref_storage_format(struct repository *repo, unsigned int format);
-void initialize_the_repository(void);
+void initialize_repository(struct repository *repo);
 RESULT_MUST_BE_USED
 int repo_init(struct repository *r, const char *gitdir, const char *worktree);
 
diff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c
index 458efa88a6..e803c43ece 100644
--- a/t/helper/test-read-cache.c
+++ b/t/helper/test-read-cache.c
@@ -9,7 +9,7 @@ int cmd__read_cache(int argc, const char **argv)
 	int i, cnt = 1;
 	const char *name = NULL;
 
-	initialize_the_repository();
+	initialize_repository(the_repository);
 
 	if (argc > 1 && skip_prefix(argv[1], "--print-and-refresh=", &name)) {
 		argc--;
-- 
2.44.GIT


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2024-04-18 12:14 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-15 11:42 [PATCH 0/5] global: drop external `the_index` variable Patrick Steinhardt
2024-04-15 11:42 ` [PATCH 1/5] t/helper: stop using `the_index` Patrick Steinhardt
2024-04-17 17:23   ` Karthik Nayak
2024-04-15 11:42 ` [PATCH 2/5] builtin: " Patrick Steinhardt
2024-04-17 17:32   ` Karthik Nayak
2024-04-18 12:16     ` Patrick Steinhardt
2024-04-15 11:42 ` [PATCH 3/5] repository: initialize index in `repo_init()` Patrick Steinhardt
2024-04-17 17:38   ` Karthik Nayak
2024-04-18 12:16     ` Patrick Steinhardt
2024-04-15 11:43 ` [PATCH 4/5] builtin/clone: stop using `the_index` Patrick Steinhardt
2024-04-15 11:43 ` [PATCH 5/5] repository: drop global `the_index` variable Patrick Steinhardt
2024-04-15 13:55 ` [PATCH 0/5] global: drop external " Phillip Wood
2024-04-15 14:15   ` Patrick Steinhardt
2024-04-15 17:50   ` Junio C Hamano
2024-04-16  5:27     ` Patrick Steinhardt
2024-04-17 17:40 ` Karthik Nayak
2024-04-18 12:16   ` Patrick Steinhardt
2024-04-18 12:14 ` [PATCH v2 0/6] global: drop " Patrick Steinhardt
2024-04-18 12:14   ` [PATCH v2 1/6] t/helper: stop using `the_index` Patrick Steinhardt
2024-04-18 12:14   ` [PATCH v2 2/6] builtin: " Patrick Steinhardt
2024-04-18 12:14   ` [PATCH v2 3/6] repository: initialize index in `repo_init()` Patrick Steinhardt
2024-04-18 12:14   ` [PATCH v2 4/6] builtin/clone: stop using `the_index` Patrick Steinhardt
2024-04-18 12:14   ` [PATCH v2 5/6] repository: drop `the_index` variable Patrick Steinhardt
2024-04-18 12:14   ` Patrick Steinhardt [this message]
2024-04-18 19:36   ` [PATCH v2 0/6] global: " Junio C Hamano
2024-04-19  4:25     ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ed722b9b4b2d0d173eff2945812e6a15d6470c38.1713442061.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=phillip.wood123@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.