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>,
	Justin Tobler <jltobler@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 13/21] environment: guard state depending on a repository
Date: Fri, 30 Aug 2024 11:09:32 +0200	[thread overview]
Message-ID: <9a3f466b53040d693ed9b2c2390d391a5420aad1.1725008898.git.ps@pks.im> (raw)
In-Reply-To: <cover.1725008897.git.ps@pks.im>

In "environment.h" we have quite a lot of functions and variables that
either explicitly or implicitly depend on `the_repository`.

The implicit set of stateful declarations includes for example variables
which get populated when parsing a repository's Git configuration. This
set of variables is broken by design, as their state often depends on
the last repository config that has been parsed. So they may or may not
represent the state of `the_repository`.

Fixing that is quite a big undertaking, and later patches in this series
will demonstrate a solution for a first small set of those variables. So
for now, let's guard these with `USE_THE_REPOSITORY_VARIABLE` so that
callers are aware of the implicit dependency.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 compat/mingw.c             |  2 ++
 compat/win32/path-utils.c  |  2 ++
 config.c                   |  2 ++
 environment.h              | 25 ++++++++++++++++++++++++-
 name-hash.c                |  3 +++
 path.c                     |  2 ++
 preload-index.c            |  3 +++
 prompt.c                   |  2 ++
 refs/files-backend.c       |  2 ++
 sparse-index.c             |  2 ++
 statinfo.c                 |  2 ++
 t/helper/test-path-utils.c |  2 ++
 tree-diff.c                |  3 +++
 userdiff.c                 |  2 ++
 14 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 29d3f09768c..5c2080c04c1 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "../git-compat-util.h"
 #include "win32.h"
 #include <aclapi.h>
diff --git a/compat/win32/path-utils.c b/compat/win32/path-utils.c
index b658ca3f811..966ef779b9c 100644
--- a/compat/win32/path-utils.c
+++ b/compat/win32/path-utils.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "../../git-compat-util.h"
 #include "../../environment.h"
 
diff --git a/config.c b/config.c
index 043e1c8a078..f3066c37477 100644
--- a/config.c
+++ b/config.c
@@ -6,6 +6,8 @@
  *
  */
 
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "abspath.h"
 #include "advice.h"
diff --git a/environment.h b/environment.h
index f1a7c645db5..934859e1c59 100644
--- a/environment.h
+++ b/environment.h
@@ -102,6 +102,28 @@ int use_optional_locks(void);
 const char *get_git_namespace(void);
 const char *strip_namespace(const char *namespaced_ref);
 
+/*
+ * TODO: All the below state either explicitly or implicitly relies on
+ * `the_repository`. We should eventually get rid of these and make the
+ * dependency on a repository explicit:
+ *
+ *   - `setup_git_env()` ideally shouldn't exist as it modifies global state,
+ *     namely the environment. The current process shouldn't ever access that
+ *     state via envvars though, but should instead consult a `struct
+ *     repository`. When spawning new processes, we would ideally also pass a
+ *     `struct repository` and then set up the environment variables for the
+ *     child process, only.
+ *
+ *   - `have_git_dir()` should not have to exist at all. Instead, we should
+ *     decide on whether or not we have a `struct repository`.
+ *
+ *   - All the global config variables should become tied to a repository. Like
+ *     this, we'd correctly honor repository-local configuration and be able to
+ *     distinguish configuration values from different repositories.
+ *
+ * Please do not add new global config variables here.
+ */
+# ifdef USE_THE_REPOSITORY_VARIABLE
 void setup_git_env(const char *git_dir);
 
 /*
@@ -213,4 +235,5 @@ extern const char *comment_line_str;
 extern char *comment_line_str_to_free;
 extern int auto_comment_line_char;
 
-#endif
+# endif /* USE_THE_REPOSITORY_VARIABLE */
+#endif /* ENVIRONMENT_H */
diff --git a/name-hash.c b/name-hash.c
index 3a58ce03d9c..95528e3bcd2 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -5,6 +5,9 @@
  *
  * Copyright (C) 2008 Linus Torvalds
  */
+
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "environment.h"
 #include "gettext.h"
diff --git a/path.c b/path.c
index a3bf25b7def..93491bab141 100644
--- a/path.c
+++ b/path.c
@@ -2,6 +2,8 @@
  * Utilities for paths and pathnames
  */
 
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "abspath.h"
 #include "environment.h"
diff --git a/preload-index.c b/preload-index.c
index 63fd35d64b1..7926eb09a69 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -1,6 +1,9 @@
 /*
  * Copyright (C) 2008 Linus Torvalds
  */
+
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "pathspec.h"
 #include "dir.h"
diff --git a/prompt.c b/prompt.c
index 8935fe4dfb9..f21c5bf1c7e 100644
--- a/prompt.c
+++ b/prompt.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "parse.h"
 #include "environment.h"
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 1cff65f6ae5..1bbb550f3af 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "../git-compat-util.h"
 #include "../copy.h"
 #include "../environment.h"
diff --git a/sparse-index.c b/sparse-index.c
index 9958656ded1..542ca5f411c 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "environment.h"
 #include "gettext.h"
diff --git a/statinfo.c b/statinfo.c
index 3c6bc049c15..30a164b0e68 100644
--- a/statinfo.c
+++ b/statinfo.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "environment.h"
 #include "statinfo.h"
diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index bf0e23ed505..f57c8d706aa 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "test-tool.h"
 #include "abspath.h"
 #include "environment.h"
diff --git a/tree-diff.c b/tree-diff.c
index 9252481df36..5eab8af631b 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -1,6 +1,9 @@
 /*
  * Helper functions for tree diff generation
  */
+
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "diff.h"
 #include "diffcore.h"
diff --git a/userdiff.c b/userdiff.c
index 989629149f6..d43d8360d17 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "config.h"
 #include "userdiff.h"
-- 
2.46.0.421.g159f2d50e7.dirty


  parent reply	other threads:[~2024-08-30  9:09 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 ` [PATCH 18/21] environment: stop storing "core.logAllRefUpdates" globally Patrick Steinhardt
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   ` Patrick Steinhardt [this message]
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=9a3f466b53040d693ed9b2c2390d391a5420aad1.1725008898.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=calvinwan@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jltobler@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 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).