git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Calvin Wan <calvinwan@google.com>
Subject: [PATCH 18/21] environment: stop storing "core.logAllRefUpdates" globally
Date: Thu, 29 Aug 2024 11:39:22 +0200	[thread overview]
Message-ID: <b6a9eb981eed22298edaf37df01f30d2aeac8685.1724923648.git.ps@pks.im> (raw)
In-Reply-To: <cover.1724923648.git.ps@pks.im>

The value of "core.logAllRefUpdates" is being stored in the global
variable `log_all_ref_updates`. This design is somewhat aged nowadays,
where it is entirely possible to access multiple repositories in the
same process which all have different values for this setting. So using
a single global variable to track it is plain wrong.

Remove the global variable. Instead, we now provide a new function part
of the repo-settings subsystem that parses the value for a specific
repository. While that may require us to read the value multiple times,
we work around this by reading it once when the ref backends are set up
and caching the value there.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/checkout.c      |  2 ++
 config.c                | 10 ----------
 environment.c           |  1 -
 environment.h           |  2 --
 refs/files-backend.c    |  4 +++-
 refs/reftable-backend.c | 12 +++++++-----
 repo-settings.c         | 16 ++++++++++++++++
 repo-settings.h         |  3 +++
 setup.c                 |  2 +-
 9 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index a0214486471..84a2dbeee79 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -950,6 +950,8 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 	const char *old_desc, *reflog_msg;
 	if (opts->new_branch) {
 		if (opts->new_orphan_branch) {
+			enum log_refs_config log_all_ref_updates =
+				repo_settings_get_log_all_ref_updates(the_repository);
 			char *refname;
 
 			refname = mkpathdup("refs/heads/%s", opts->new_orphan_branch);
diff --git a/config.c b/config.c
index f3066c37477..47101c3e977 100644
--- a/config.c
+++ b/config.c
@@ -1452,16 +1452,6 @@ static int git_default_core_config(const char *var, const char *value,
 		return 0;
 	}
 
-	if (!strcmp(var, "core.logallrefupdates")) {
-		if (value && !strcasecmp(value, "always"))
-			log_all_ref_updates = LOG_REFS_ALWAYS;
-		else if (git_config_bool(var, value))
-			log_all_ref_updates = LOG_REFS_NORMAL;
-		else
-			log_all_ref_updates = LOG_REFS_NONE;
-		return 0;
-	}
-
 	if (!strcmp(var, "core.warnambiguousrefs")) {
 		warn_ambiguous_refs = git_config_bool(var, value);
 		return 0;
diff --git a/environment.c b/environment.c
index 64ae13ef240..992d87e0d60 100644
--- a/environment.c
+++ b/environment.c
@@ -77,7 +77,6 @@ int sparse_expect_files_outside_of_patterns;
 int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
-enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;
 int max_allowed_tree_depth =
 #ifdef _MSC_VER
 	/*
diff --git a/environment.h b/environment.h
index 0b4e5afc36d..315fd319951 100644
--- a/environment.h
+++ b/environment.h
@@ -181,8 +181,6 @@ extern int core_apply_sparse_checkout;
 extern int core_sparse_checkout_cone;
 extern int sparse_expect_files_outside_of_patterns;
 
-extern enum log_refs_config log_all_ref_updates;
-
 enum rebase_setup_type {
 	AUTOREBASE_NEVER = 0,
 	AUTOREBASE_LOCAL,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c143cde8aa4..6ad495744b0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -74,6 +74,7 @@ struct files_ref_store {
 	unsigned int store_flags;
 
 	char *gitcommondir;
+	enum log_refs_config log_all_ref_updates;
 
 	struct ref_cache *loose;
 
@@ -106,6 +107,7 @@ static struct ref_store *files_ref_store_init(struct repository *repo,
 	refs->gitcommondir = strbuf_detach(&sb, NULL);
 	refs->packed_ref_store =
 		packed_ref_store_init(repo, refs->gitcommondir, flags);
+	refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo);
 
 	chdir_notify_reparent("files-backend $GIT_DIR", &refs->base.gitdir);
 	chdir_notify_reparent("files-backend $GIT_COMMONDIR",
@@ -1703,7 +1705,7 @@ static int log_ref_setup(struct files_ref_store *refs,
 			 const char *refname, int force_create,
 			 int *logfd, struct strbuf *err)
 {
-	enum log_refs_config log_refs_cfg = log_all_ref_updates;
+	enum log_refs_config log_refs_cfg = refs->log_all_ref_updates;
 	struct strbuf logfile_sb = STRBUF_INIT;
 	char *logfile;
 
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index c78186423a1..043e19439f6 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -52,6 +52,7 @@ struct reftable_ref_store {
 	struct reftable_write_options write_options;
 
 	unsigned int store_flags;
+	enum log_refs_config log_all_ref_updates;
 	int err;
 };
 
@@ -157,21 +158,21 @@ static struct reftable_stack *stack_for(struct reftable_ref_store *store,
 	}
 }
 
-static int should_write_log(struct ref_store *refs, const char *refname)
+static int should_write_log(struct reftable_ref_store *refs, const char *refname)
 {
-	enum log_refs_config log_refs_cfg = log_all_ref_updates;
+	enum log_refs_config log_refs_cfg = refs->log_all_ref_updates;
 	if (log_refs_cfg == LOG_REFS_UNSET)
 		log_refs_cfg = is_bare_repository() ? LOG_REFS_NONE : LOG_REFS_NORMAL;
 
 	switch (log_refs_cfg) {
 	case LOG_REFS_NONE:
-		return refs_reflog_exists(refs, refname);
+		return refs_reflog_exists(&refs->base, refname);
 	case LOG_REFS_ALWAYS:
 		return 1;
 	case LOG_REFS_NORMAL:
 		if (should_autocreate_reflog(log_refs_cfg, refname))
 			return 1;
-		return refs_reflog_exists(refs, refname);
+		return refs_reflog_exists(&refs->base, refname);
 	default:
 		BUG("unhandled core.logAllRefUpdates value %d", log_refs_cfg);
 	}
@@ -278,6 +279,7 @@ static struct ref_store *reftable_be_init(struct repository *repo,
 	base_ref_store_init(&refs->base, repo, gitdir, &refs_be_reftable);
 	strmap_init(&refs->worktree_stacks);
 	refs->store_flags = store_flags;
+	refs->log_all_ref_updates = repo_settings_get_log_all_ref_updates(repo);
 
 	refs->write_options.hash_id = repo->hash_algo->format_id;
 	refs->write_options.default_permissions = calc_shared_perm(0666 & ~mask);
@@ -1220,7 +1222,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
 		} else if (!(u->flags & REF_SKIP_CREATE_REFLOG) &&
 			   (u->flags & REF_HAVE_NEW) &&
 			   (u->flags & REF_FORCE_CREATE_REFLOG ||
-			    should_write_log(&arg->refs->base, u->refname))) {
+			    should_write_log(arg->refs, u->refname))) {
 			struct reftable_log_record *log;
 			int create_reflog = 1;
 
diff --git a/repo-settings.c b/repo-settings.c
index 3a76ba276c9..1322fd2f972 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -124,3 +124,19 @@ void prepare_repo_settings(struct repository *r)
 	 */
 	r->settings.command_requires_full_index = 1;
 }
+
+enum log_refs_config repo_settings_get_log_all_ref_updates(struct repository *repo)
+{
+	const char *value;
+
+	if (!repo_config_get_string_tmp(repo, "core.logallrefupdates", &value)) {
+		if (value && !strcasecmp(value, "always"))
+			return LOG_REFS_ALWAYS;
+		else if (git_config_bool("core.logallrefupdates", value))
+			return LOG_REFS_NORMAL;
+		else
+			return LOG_REFS_NONE;
+	}
+
+	return LOG_REFS_UNSET;
+}
diff --git a/repo-settings.h b/repo-settings.h
index d03b6e57f0c..76adb96a669 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -65,4 +65,7 @@ struct repo_settings {
 
 void prepare_repo_settings(struct repository *r);
 
+/* Read the value for "core.logAllRefUpdates". */
+enum log_refs_config repo_settings_get_log_all_ref_updates(struct repository *repo);
+
 #endif /* REPO_SETTINGS_H */
diff --git a/setup.c b/setup.c
index a4a9fbb3a2a..c37dc4f7c48 100644
--- a/setup.c
+++ b/setup.c
@@ -2354,7 +2354,7 @@ static int create_default_files(const char *template_path,
 	else {
 		git_config_set("core.bare", "false");
 		/* allow template config file to override the default */
-		if (log_all_ref_updates == LOG_REFS_UNSET)
+		if (repo_settings_get_log_all_ref_updates(the_repository) == LOG_REFS_UNSET)
 			git_config_set("core.logallrefupdates", "true");
 		if (needs_work_tree_config(original_git_dir, work_tree))
 			git_config_set("core.worktree", work_tree);
-- 
2.46.0.421.g159f2d50e7.dirty


  parent reply	other threads:[~2024-08-29  9:39 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-29  9:38 [PATCH 00/21] environment: guard reliance on `the_repository` Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 01/21] environment: make `get_git_dir()` accept a repository Patrick Steinhardt
2024-08-29 20:15   ` Justin Tobler
2024-08-30  7:42     ` Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 02/21] environment: make `get_git_common_dir()` " Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 03/21] environment: make `get_object_directory()` " Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 04/21] environment: make `get_index_file()` " Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 05/21] environment: make `get_graft_file()` " Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 06/21] environment: make `get_git_work_tree()` " Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 07/21] config: document `read_early_config()` and `read_very_early_config()` Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 08/21] config: make dependency on repo in `read_early_config()` explicit Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 09/21] environment: move `odb_mkstemp()` into object layer Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 10/21] environment: make `get_git_namespace()` self-contained Patrick Steinhardt
2024-08-29  9:38 ` [PATCH 11/21] environment: move `set_git_dir()` and related into setup layer Patrick Steinhardt
2024-08-29  9:39 ` [PATCH 12/21] environment: reorder header to split out `the_repository`-free section Patrick Steinhardt
2024-08-29  9:39 ` [PATCH 13/21] environment: guard state depending on a repository Patrick Steinhardt
2024-08-29  9:39 ` [PATCH 14/21] repo-settings: split out declarations into a standalone header Patrick Steinhardt
2024-08-29  9:39 ` [PATCH 15/21] branch: stop modifying `log_all_ref_updates` variable Patrick Steinhardt
2024-08-29  9:39 ` [PATCH 16/21] refs: stop modifying global " Patrick Steinhardt
2024-08-29  9:39 ` [PATCH 17/21] repo-settings: track defaults close to `struct repo_settings` Patrick Steinhardt
2024-08-29  9:39 ` Patrick Steinhardt [this message]
2024-08-29  9:39 ` [PATCH 19/21] environment: stop storing "core.preferSymlinkRefs" globally Patrick Steinhardt
2024-08-29  9:39 ` [PATCH 20/21] environment: stop storing "core.warnAmbiguousRefs" globally Patrick Steinhardt
2024-08-29  9:39 ` [PATCH 21/21] environment: stop storing "core.notesRef" globally Patrick Steinhardt
2024-08-29 19:59 ` [PATCH 00/21] environment: guard reliance on `the_repository` Junio C Hamano
2024-08-30  6:58   ` Patrick Steinhardt
2024-08-30 16:32     ` Junio C Hamano
2024-09-02  9:29       ` Patrick Steinhardt
2024-08-30  9:08 ` [PATCH v2 " Patrick Steinhardt
2024-08-30  9:08   ` [PATCH v2 01/21] environment: make `get_git_dir()` accept a repository Patrick Steinhardt
2024-09-11 21:12     ` karthik nayak
2024-09-12 11:17       ` Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 02/21] environment: make `get_git_common_dir()` " Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 03/21] environment: make `get_object_directory()` " Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 04/21] environment: make `get_index_file()` " Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 05/21] environment: make `get_graft_file()` " Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 06/21] environment: make `get_git_work_tree()` " Patrick Steinhardt
2024-09-11 15:15     ` Justin Tobler
2024-09-12 11:16       ` Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 07/21] config: document `read_early_config()` and `read_very_early_config()` Patrick Steinhardt
2024-09-11 15:59     ` Justin Tobler
2024-09-12 11:17       ` Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 08/21] config: make dependency on repo in `read_early_config()` explicit Patrick Steinhardt
2024-09-04  1:46     ` James Liu
2024-09-04  7:14       ` Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 09/21] environment: move `odb_mkstemp()` into object layer Patrick Steinhardt
2024-09-11 21:26     ` karthik nayak
2024-09-12 11:17       ` Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 10/21] environment: make `get_git_namespace()` self-contained Patrick Steinhardt
2024-09-11 16:21     ` Justin Tobler
2024-08-30  9:09   ` [PATCH v2 11/21] environment: move `set_git_dir()` and related into setup layer Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 12/21] environment: reorder header to split out `the_repository`-free section Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 13/21] environment: guard state depending on a repository Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 14/21] repo-settings: split out declarations into a standalone header Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 15/21] repo-settings: track defaults close to `struct repo_settings` Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 16/21] branch: stop modifying `log_all_ref_updates` variable Patrick Steinhardt
2024-09-11 17:14     ` Justin Tobler
2024-09-12 11:17       ` Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 17/21] refs: stop modifying global " Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 18/21] environment: stop storing "core.logAllRefUpdates" globally Patrick Steinhardt
2024-09-12 11:10     ` karthik nayak
2024-08-30  9:09   ` [PATCH v2 19/21] environment: stop storing "core.preferSymlinkRefs" globally Patrick Steinhardt
2024-08-30  9:09   ` [PATCH v2 20/21] environment: stop storing "core.warnAmbiguousRefs" globally Patrick Steinhardt
2024-09-04  2:10     ` James Liu
2024-08-30  9:10   ` [PATCH v2 21/21] environment: stop storing "core.notesRef" globally Patrick Steinhardt
2024-09-04  2:12   ` [PATCH v2 00/21] environment: guard reliance on `the_repository` James Liu
2024-09-04  7:14     ` Patrick Steinhardt
2024-09-12 11:14   ` karthik nayak
2024-09-12 11:17     ` Patrick Steinhardt
2024-09-12 11:29 ` [PATCH v3 " Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 01/21] environment: make `get_git_dir()` accept a repository Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 02/21] environment: make `get_git_common_dir()` " Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 03/21] environment: make `get_object_directory()` " Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 04/21] environment: make `get_index_file()` " Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 05/21] environment: make `get_graft_file()` " Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 06/21] environment: make `get_git_work_tree()` " Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 07/21] config: document `read_early_config()` and `read_very_early_config()` Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 08/21] config: make dependency on repo in `read_early_config()` explicit Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 09/21] environment: move object database functions into object layer Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 10/21] environment: make `get_git_namespace()` self-contained Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 11/21] environment: move `set_git_dir()` and related into setup layer Patrick Steinhardt
2024-09-12 11:29   ` [PATCH v3 12/21] environment: reorder header to split out `the_repository`-free section Patrick Steinhardt
2024-09-12 11:30   ` [PATCH v3 13/21] environment: guard state depending on a repository Patrick Steinhardt
2024-09-12 11:30   ` [PATCH v3 14/21] repo-settings: split out declarations into a standalone header Patrick Steinhardt
2024-09-12 11:30   ` [PATCH v3 15/21] repo-settings: track defaults close to `struct repo_settings` Patrick Steinhardt
2024-09-12 11:30   ` [PATCH v3 16/21] branch: stop modifying `log_all_ref_updates` variable Patrick Steinhardt
2024-09-12 11:30   ` [PATCH v3 17/21] refs: stop modifying global " Patrick Steinhardt
2024-09-12 11:30   ` [PATCH v3 18/21] environment: stop storing "core.logAllRefUpdates" globally Patrick Steinhardt
2024-09-12 11:30   ` [PATCH v3 19/21] environment: stop storing "core.preferSymlinkRefs" globally Patrick Steinhardt
2024-09-12 11:30   ` [PATCH v3 20/21] environment: stop storing "core.warnAmbiguousRefs" globally Patrick Steinhardt
2024-09-12 11:30   ` [PATCH v3 21/21] environment: stop storing "core.notesRef" globally Patrick Steinhardt
2024-09-12 20:40   ` [PATCH v3 00/21] environment: guard reliance on `the_repository` Junio C Hamano

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=b6a9eb981eed22298edaf37df01f30d2aeac8685.1724923648.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=calvinwan@google.com \
    --cc=git@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).