[PATCH v1 0/4] environment.c: migrate 'trust_executable_bit' into 'repo_config_values'

[PATCH v1 0/4] environment.c: migrate 'trust_executable_bit' into 'repo_config_values'

[Tian Yuchen]

The 'core.filemode' configuration, which is stored as a global
variable 'trust_executable_bit', is a core filesystem capability flag.

Move it into 'repo_config_values' to tie it to the specific
repository instance it was read from. Eager parsing is maintained
because this flag is heavily consulted in hot paths.

To avoid falling back to 'the_repository', refactor the signatures
of helper functions:

 - ce_mode_from_stat()
 - st_mode_from_ce()
 - fake_lstat()
 - check_removed()

These functions now accept a 'struct index_state *istate', ensuring
the correct context is seamlessly passed down to the lowest levels.

Note: 'repo_config_values()' still does not support any 'struct
repository' other than 'the_repository'. In other words, this series
of patches is laying the groundwork for the eventual elimination of
'the_repository'.

Previous related work:

 - [PATCH 2/6] config: add trust_executable_bit to global config

 - [PATCH] Refactor 'trust_executable_bit' to repository-scoped setting 
 (This series of patches was unsuccessful because the target location selected
 was 'struct repo_settings', which our analysis indicated was not the
 optimal choice. For further details, please see: [1])

[1] https://lore.kernel.org/git/837b5360b40f992351f489a0ae05fedf49884c6e.1685716420.git.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/20260301190017.53539-1-dronarajgyawali@gmail.com/
[3] https://lore.kernel.org/git/xmqq1pht6nyx.fsf@gitster.g/

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>

Tian Yuchen (4):
  read-cache: remove redundant extern declarations
  read-cache: move 'ce_mode_from_stat()' to 'read-cache.c'
  environment: move 'trust_executable_bit' into repo_config_values
  read-cache: pass 'istate' to stat/mode helper functions

 apply.c                |  6 ++++--
 builtin/update-index.c |  2 +-
 diff-lib.c             | 20 +++++++++---------
 environment.c          |  4 ++--
 environment.h          |  2 +-
 read-cache-ll.h        |  2 +-
 read-cache.c           | 47 ++++++++++++++++++++++++++++++++----------
 read-cache.h           | 17 +++------------
 8 files changed, 58 insertions(+), 42 deletions(-)

-- 
2.43.0

[PATCH v1 1/4] read-cache: remove redundant extern declarations

[Tian Yuchen]

The 'read-cache.c' file already includes 'environment.h', which provides
the extern declarations for variables like 'trust_executable_bit' and
'has_symlinks'.

Remove the redundant extern declarations inside 'st_mode_from_ce()' to
clean up the code.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 read-cache.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 38a04b8de3..c44e4d128f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -204,8 +204,6 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
 
 static unsigned int st_mode_from_ce(const struct cache_entry *ce)
 {
-	extern int trust_executable_bit, has_symlinks;
-
 	switch (ce->ce_mode & S_IFMT) {
 	case S_IFLNK:
 		return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
-- 
2.43.0

[PATCH v1 2/4] read-cache: move 'ce_mode_from_stat()' to 'read-cache.c'

[Tian Yuchen]

The ce_mode_from_stat() function is declared as a static inline function
in 'read-cache.h'. As we want to migrate configuration variables, this
helper function will need access to corresponding repository-specific
configuration logic. Move the implementation to 'read-cache.c' to
cleanly encapsulate its dependencies.

Note that the 'extern int trust_executable_bit, has_symlinks;' line is
discarded because it's not necessary when the function lives in
"read-cache.c".

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 read-cache.c | 13 +++++++++++++
 read-cache.h | 16 ++--------------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index c44e4d128f..54150fe756 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -202,6 +202,19 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
 	}
 }
 
+unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
+{
+	if (!has_symlinks && S_ISREG(mode) &&
+	    ce && S_ISLNK(ce->ce_mode))
+		return ce->ce_mode;
+	if (!trust_executable_bit && S_ISREG(mode)) {
+		if (ce && S_ISREG(ce->ce_mode))
+			return ce->ce_mode;
+		return create_ce_mode(0666);
+	}
+	return create_ce_mode(mode);
+}
+
 static unsigned int st_mode_from_ce(const struct cache_entry *ce)
 {
 	switch (ce->ce_mode & S_IFMT) {
diff --git a/read-cache.h b/read-cache.h
index 043da1f1aa..3c4af2faeb 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -5,20 +5,8 @@
 #include "object.h"
 #include "pathspec.h"
 
-static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
-					     unsigned int mode)
-{
-	extern int trust_executable_bit, has_symlinks;
-	if (!has_symlinks && S_ISREG(mode) &&
-	    ce && S_ISLNK(ce->ce_mode))
-		return ce->ce_mode;
-	if (!trust_executable_bit && S_ISREG(mode)) {
-		if (ce && S_ISREG(ce->ce_mode))
-			return ce->ce_mode;
-		return create_ce_mode(0666);
-	}
-	return create_ce_mode(mode);
-}
+unsigned int ce_mode_from_stat(const struct cache_entry *ce,
+				unsigned int mode);
 
 static inline int ce_to_dtype(const struct cache_entry *ce)
 {
-- 
2.43.0

[PATCH v1 3/4] environment: move 'trust_executable_bit' into repo_config_values

[Tian Yuchen]

Move the global 'trust_executable_bit' configuration into the
repository-specific 'repo_config_values' struct.

For now, associated functions in read-cache.c access this configuration
by explicitly falling back to 'the_repository'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 apply.c       |  4 +++-
 environment.c |  4 ++--
 environment.h |  2 +-
 read-cache.c  | 15 +++++++++++----
 4 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/apply.c b/apply.c
index 249248d4f2..73ca9907f8 100644
--- a/apply.c
+++ b/apply.c
@@ -3890,10 +3890,12 @@ static int check_preimage(struct apply_state *state,
 	}
 
 	if (!state->cached && !previous) {
+		struct repo_config_values *cfg = repo_config_values(the_repository);
+
 		if (*ce && !(*ce)->ce_mode)
 			BUG("ce_mode == 0 for path '%s'", old_name);
 
-		if (trust_executable_bit || !S_ISREG(st->st_mode))
+		if (cfg->trust_executable_bit || !S_ISREG(st->st_mode))
 			st_mode = ce_mode_from_stat(*ce, st->st_mode);
 		else if (*ce)
 			st_mode = (*ce)->ce_mode;
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..94f74f39e6 100644
--- a/environment.c
+++ b/environment.c
@@ -41,7 +41,6 @@
 static int pack_compression_seen;
 static int zlib_compression_seen;
 
-int trust_executable_bit = 1;
 int trust_ctime = 1;
 int check_stat = 1;
 int has_symlinks = 1;
@@ -305,7 +304,7 @@ int git_default_core_config(const char *var, const char *value,
 
 	/* This needs a better name */
 	if (!strcmp(var, "core.filemode")) {
-		trust_executable_bit = git_config_bool(var, value);
+		cfg->trust_executable_bit = git_config_bool(var, value);
 		return 0;
 	}
 	if (!strcmp(var, "core.trustctime")) {
@@ -720,5 +719,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
 	cfg->apply_sparse_checkout = 0;
+	cfg->trust_executable_bit = 1;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 }
diff --git a/environment.h b/environment.h
index 123a71cdc8..72c400923d 100644
--- a/environment.h
+++ b/environment.h
@@ -90,6 +90,7 @@ struct repository;
 struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
+	int trust_executable_bit;
 	int apply_sparse_checkout;
 
 	/* section "branch" config values */
@@ -160,7 +161,6 @@ int is_bare_repository(void);
 extern char *git_work_tree_cfg;
 
 /* Environment bits from configuration mechanism */
-extern int trust_executable_bit;
 extern int trust_ctime;
 extern int check_stat;
 extern int has_symlinks;
diff --git a/read-cache.c b/read-cache.c
index 54150fe756..18af533649 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -204,10 +204,12 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
 
 unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
 {
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
 	if (!has_symlinks && S_ISREG(mode) &&
 	    ce && S_ISLNK(ce->ce_mode))
 		return ce->ce_mode;
-	if (!trust_executable_bit && S_ISREG(mode)) {
+	if (!cfg->trust_executable_bit && S_ISREG(mode)) {
 		if (ce && S_ISREG(ce->ce_mode))
 			return ce->ce_mode;
 		return create_ce_mode(0666);
@@ -217,11 +219,13 @@ unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
 
 static unsigned int st_mode_from_ce(const struct cache_entry *ce)
 {
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
 	switch (ce->ce_mode & S_IFMT) {
 	case S_IFLNK:
 		return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
 	case S_IFREG:
-		return (ce->ce_mode & (trust_executable_bit ? 0755 : 0644)) | S_IFREG;
+		return (ce->ce_mode & (cfg->trust_executable_bit ? 0755 : 0644)) | S_IFREG;
 	case S_IFGITLINK:
 		return S_IFDIR | 0755;
 	case S_IFDIR:
@@ -321,6 +325,7 @@ static int ce_modified_check_fs(struct index_state *istate,
 static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
 {
 	unsigned int changed = 0;
+	struct repo_config_values *cfg = repo_config_values(the_repository);
 
 	if (ce->ce_flags & CE_REMOVE)
 		return MODE_CHANGED | DATA_CHANGED | TYPE_CHANGED;
@@ -331,7 +336,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
 		/* We consider only the owner x bit to be relevant for
 		 * "mode changes"
 		 */
-		if (trust_executable_bit &&
+		if (cfg->trust_executable_bit &&
 		    (0100 & (ce->ce_mode ^ st->st_mode)))
 			changed |= MODE_CHANGED;
 		break;
@@ -732,6 +737,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
 	unsigned hash_flags = pretend ? 0 : INDEX_WRITE_OBJECT;
 
+	struct repo_config_values *cfg = repo_config_values(the_repository);
+
 	if (flags & ADD_CACHE_RENORMALIZE)
 		hash_flags |= INDEX_RENORMALIZE;
 
@@ -752,7 +759,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 		ce->ce_flags |= CE_INTENT_TO_ADD;
 
 
-	if (trust_executable_bit && has_symlinks) {
+	if (cfg->trust_executable_bit && has_symlinks) {
 		ce->ce_mode = create_ce_mode(st_mode);
 	} else {
 		/* If there is an existing entry, pick the mode bits and type
-- 
2.43.0

[PATCH v1 4/4] read-cache: pass 'istate' to stat/mode helper functions

[Tian Yuchen]

In the previous commit, the gloabl 'trust_executable_bit' was
migrated into 'repo_config_values', but low-level helpers in
read-cache.c still relied on 'the_repository' to access it.

Refactor the signatures of ce_mode_from_stat(), st_mode_from_ce(),
fake_lstat(), and check_removed() to accept a 'struct
index_state *istate'. This allows these functions to retrieve the
repository context via 'istate->repo'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 apply.c                |  4 ++--
 builtin/update-index.c |  2 +-
 diff-lib.c             | 20 ++++++++++----------
 read-cache-ll.h        |  2 +-
 read-cache.c           | 31 +++++++++++++++++++------------
 read-cache.h           |  3 ++-
 6 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/apply.c b/apply.c
index 73ca9907f8..a81bb29a6f 100644
--- a/apply.c
+++ b/apply.c
@@ -3890,13 +3890,13 @@ static int check_preimage(struct apply_state *state,
 	}
 
 	if (!state->cached && !previous) {
-		struct repo_config_values *cfg = repo_config_values(the_repository);
+		struct repo_config_values *cfg = repo_config_values(state->repo);
 
 		if (*ce && !(*ce)->ce_mode)
 			BUG("ce_mode == 0 for path '%s'", old_name);
 
 		if (cfg->trust_executable_bit || !S_ISREG(st->st_mode))
-			st_mode = ce_mode_from_stat(*ce, st->st_mode);
+			st_mode = ce_mode_from_stat(state->repo->index, *ce, st->st_mode);
 		else if (*ce)
 			st_mode = (*ce)->ce_mode;
 		else
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 8a5907767b..3f6967bd84 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -293,7 +293,7 @@ static int add_one_path(const struct cache_entry *old, const char *path, int len
 	ce->ce_flags = create_ce_flags(0);
 	ce->ce_namelen = len;
 	fill_stat_cache_info(the_repository->index, ce, st);
-	ce->ce_mode = ce_mode_from_stat(old, st->st_mode);
+	ce->ce_mode = ce_mode_from_stat(the_repository->index, old, st->st_mode);
 
 	if (index_path(the_repository->index, &ce->oid, path, st,
 		       info_only ? 0 : INDEX_WRITE_OBJECT)) {
diff --git a/diff-lib.c b/diff-lib.c
index ae91027a02..95fd3ba4b9 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -39,14 +39,14 @@
  * exists for ce that is a submodule -- it is a submodule that is not
  * checked out).  Return negative for an error.
  */
-static int check_removed(const struct cache_entry *ce, struct stat *st)
+static int check_removed(struct index_state *istate, const struct cache_entry *ce, struct stat *st)
 {
 	int stat_err;
 
 	if (!(ce->ce_flags & CE_FSMONITOR_VALID))
 		stat_err = lstat(ce->name, st);
 	else
-		stat_err = fake_lstat(ce, st);
+		stat_err = fake_lstat(istate, ce, st);
 	if (stat_err < 0) {
 		if (!is_missing_file_error(errno))
 			return -1;
@@ -158,9 +158,9 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
 			int num_compare_stages = 0;
 			struct stat st;
 
-			changed = check_removed(ce, &st);
+			changed = check_removed(revs->repo->index, ce, &st);
 			if (!changed)
-				wt_mode = ce_mode_from_stat(ce, st.st_mode);
+				wt_mode = ce_mode_from_stat(revs->repo->index, ce, st.st_mode);
 			else {
 				if (changed < 0) {
 					perror(ce->name);
@@ -193,7 +193,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
 					num_compare_stages++;
 					oidcpy(&dpath->parent[stage - 2].oid,
 					       &nce->oid);
-					dpath->parent[stage-2].mode = ce_mode_from_stat(nce, mode);
+					dpath->parent[stage-2].mode = ce_mode_from_stat(revs->repo->index, nce, mode);
 					dpath->parent[stage-2].status =
 						DIFF_STATUS_MODIFIED;
 				}
@@ -249,7 +249,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
 		} else {
 			struct stat st;
 
-			changed = check_removed(ce, &st);
+			changed = check_removed(revs->repo->index, ce, &st);
 			if (changed) {
 				if (changed < 0) {
 					perror(ce->name);
@@ -262,7 +262,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
 				continue;
 			} else if (revs->diffopt.ita_invisible_in_index &&
 				   ce_intent_to_add(ce)) {
-				newmode = ce_mode_from_stat(ce, st.st_mode);
+				newmode = ce_mode_from_stat(revs->repo->index, ce, st.st_mode);
 				diff_addremove(&revs->diffopt, '+', newmode,
 					       null_oid(the_hash_algo), 0, ce->name, 0);
 				continue;
@@ -270,7 +270,7 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
 
 			changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
 							    ce_option, &dirty_submodule);
-			newmode = ce_mode_from_stat(ce, st.st_mode);
+			newmode = ce_mode_from_stat(revs->repo->index, ce, st.st_mode);
 		}
 
 		if (!changed && !dirty_submodule) {
@@ -324,7 +324,7 @@ static int get_stat_data(const struct cache_entry *ce,
 	if (!cached && !ce_uptodate(ce)) {
 		int changed;
 		struct stat st;
-		changed = check_removed(ce, &st);
+		changed = check_removed(diffopt->repo->index, ce, &st);
 		if (changed < 0)
 			return -1;
 		else if (changed) {
@@ -338,7 +338,7 @@ static int get_stat_data(const struct cache_entry *ce,
 		changed = match_stat_with_submodule(diffopt, ce, &st,
 						    0, dirty_submodule);
 		if (changed) {
-			mode = ce_mode_from_stat(ce, st.st_mode);
+			mode = ce_mode_from_stat(diffopt->repo->index, ce, st.st_mode);
 			oid = null_oid(the_hash_algo);
 		}
 	}
diff --git a/read-cache-ll.h b/read-cache-ll.h
index 2c8b4b21b1..9fb9bedfbf 100644
--- a/read-cache-ll.h
+++ b/read-cache-ll.h
@@ -442,7 +442,7 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
  * for lstat() for a tracked path that is known to be up-to-date via
  * some out-of-line means (like fsmonitor).
  */
-int fake_lstat(const struct cache_entry *ce, struct stat *st);
+int fake_lstat(struct index_state *istate, const struct cache_entry *ce, struct stat *st);
 
 #define REFRESH_REALLY                   (1 << 0) /* ignore_valid */
 #define REFRESH_UNMERGED                 (1 << 1) /* allow unmerged */
diff --git a/read-cache.c b/read-cache.c
index 18af533649..28e7f24382 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -202,9 +202,12 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
 	}
 }
 
-unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
+unsigned int ce_mode_from_stat(struct index_state *istate,
+			       const struct cache_entry *ce,
+			       unsigned int mode)
 {
-	struct repo_config_values *cfg = repo_config_values(the_repository);
+	struct repository *repo = (istate && istate->repo) ? istate->repo : the_repository;
+	struct repo_config_values *cfg = repo_config_values(repo);
 
 	if (!has_symlinks && S_ISREG(mode) &&
 	    ce && S_ISLNK(ce->ce_mode))
@@ -217,9 +220,10 @@ unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
 	return create_ce_mode(mode);
 }
 
-static unsigned int st_mode_from_ce(const struct cache_entry *ce)
+static unsigned int st_mode_from_ce(struct index_state *istate, const struct cache_entry *ce)
 {
-	struct repo_config_values *cfg = repo_config_values(the_repository);
+	struct repository *repo = (istate && istate->repo) ? istate->repo : the_repository;
+	struct repo_config_values *cfg = repo_config_values(repo);
 
 	switch (ce->ce_mode & S_IFMT) {
 	case S_IFLNK:
@@ -235,10 +239,10 @@ static unsigned int st_mode_from_ce(const struct cache_entry *ce)
 	}
 }
 
-int fake_lstat(const struct cache_entry *ce, struct stat *st)
+int fake_lstat(struct index_state *istate, const struct cache_entry *ce, struct stat *st)
 {
 	fake_lstat_data(&ce->ce_stat_data, st);
-	st->st_mode = st_mode_from_ce(ce);
+	st->st_mode = st_mode_from_ce(istate, ce);
 
 	/* always succeed as lstat() replacement */
 	return 0;
@@ -322,10 +326,12 @@ static int ce_modified_check_fs(struct index_state *istate,
 	return 0;
 }
 
-static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
+static int ce_match_stat_basic(struct index_state *istate,
+			       const struct cache_entry *ce, struct stat *st)
 {
 	unsigned int changed = 0;
-	struct repo_config_values *cfg = repo_config_values(the_repository);
+	struct repository *repo = (istate && istate->repo) ? istate->repo : the_repository;
+	struct repo_config_values *cfg = repo_config_values(repo);
 
 	if (ce->ce_flags & CE_REMOVE)
 		return MODE_CHANGED | DATA_CHANGED | TYPE_CHANGED;
@@ -430,7 +436,7 @@ int ie_match_stat(struct index_state *istate,
 	if (ce_intent_to_add(ce))
 		return DATA_CHANGED | TYPE_CHANGED | MODE_CHANGED;
 
-	changed = ce_match_stat_basic(ce, st);
+	changed = ce_match_stat_basic(istate, ce, st);
 
 	/*
 	 * Within 1 second of this sequence:
@@ -737,7 +743,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
 	unsigned hash_flags = pretend ? 0 : INDEX_WRITE_OBJECT;
 
-	struct repo_config_values *cfg = repo_config_values(the_repository);
+	struct repository *repo = (istate && istate->repo) ? istate->repo : the_repository;
+	struct repo_config_values *cfg = repo_config_values(repo);
 
 	if (flags & ADD_CACHE_RENORMALIZE)
 		hash_flags |= INDEX_RENORMALIZE;
@@ -769,7 +776,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 		int pos = index_name_pos_also_unmerged(istate, path, namelen);
 
 		ent = (0 <= pos) ? istate->cache[pos] : NULL;
-		ce->ce_mode = ce_mode_from_stat(ent, st_mode);
+		ce->ce_mode = ce_mode_from_stat(istate, ent, st_mode);
 	}
 
 	/* When core.ignorecase=true, determine if a directory of the same name but differing
@@ -2592,7 +2599,7 @@ static void ce_smudge_racily_clean_entry(struct index_state *istate,
 
 	if (lstat(ce->name, &st) < 0)
 		return;
-	if (ce_match_stat_basic(ce, &st))
+	if (ce_match_stat_basic(istate, ce, &st))
 		return;
 	if (ce_modified_check_fs(istate, ce, &st)) {
 		/* This is "racily clean"; smudge it.  Note that this
diff --git a/read-cache.h b/read-cache.h
index 3c4af2faeb..61299ed95b 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -5,7 +5,8 @@
 #include "object.h"
 #include "pathspec.h"
 
-unsigned int ce_mode_from_stat(const struct cache_entry *ce,
+unsigned int ce_mode_from_stat(struct index_state *istate,
+				const struct cache_entry *ce,
 				unsigned int mode);
 
 static inline int ce_to_dtype(const struct cache_entry *ce)
-- 
2.43.0

Re: [PATCH v1 3/4] environment: move 'trust_executable_bit' into repo_config_values

[Christian Couder]

On Sat, May 30, 2026 at 6:05 PM Tian Yuchen <cat@malon.dev> wrote:

> @@ -720,5 +719,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
>  {
>         cfg->attributes_file = NULL;
>         cfg->apply_sparse_checkout = 0;
> +       cfg->trust_executable_bit = 1;

Here `trust_executable_bit` is processed after `apply_sparse_checkout` ...

>         cfg->branch_track = BRANCH_TRACK_REMOTE;
>  }
> diff --git a/environment.h b/environment.h
> index 123a71cdc8..72c400923d 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -90,6 +90,7 @@ struct repository;
>  struct repo_config_values {
>         /* section "core" config values */
>         char *attributes_file;
> +       int trust_executable_bit;
>         int apply_sparse_checkout;

... but here it is before `apply_sparse_checkout`.

I think it would make more sense to put `trust_executable_bit` after
`apply_sparse_checkout` here.

>         /* section "branch" config values */

Re: [PATCH v1 4/4] read-cache: pass 'istate' to stat/mode helper functions

[Christian Couder]

On Sat, May 30, 2026 at 6:05 PM Tian Yuchen <cat@malon.dev> wrote:
>
> In the previous commit, the gloabl 'trust_executable_bit' was

s/gloabl/global/

> migrated into 'repo_config_values', but low-level helpers in
> read-cache.c still relied on 'the_repository' to access it.
>
> Refactor the signatures of ce_mode_from_stat(), st_mode_from_ce(),
> fake_lstat(), and check_removed() to accept a 'struct
> index_state *istate'. This allows these functions to retrieve the
> repository context via 'istate->repo'.

The cover letter contains:

"In other words, this series of patches is laying the groundwork for
the eventual elimination of 'the_repository'."

but I think it would be also interesting to have something similar
here, as this is especially relevant to this commit.

For example maybe add something like "which will help with removing
'the_repository' in the future" to the last sentence?

Re: [PATCH v1 3/4] environment: move 'trust_executable_bit' into repo_config_values

[Junio C Hamano]

Tian Yuchen <cat@malon.dev> writes:

> diff --git a/apply.c b/apply.c
> index 249248d4f2..73ca9907f8 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -3890,10 +3890,12 @@ static int check_preimage(struct apply_state *state,
>  	}
>  
>  	if (!state->cached && !previous) {
> +		struct repo_config_values *cfg = repo_config_values(the_repository);
> +
>  		if (*ce && !(*ce)->ce_mode)
>  			BUG("ce_mode == 0 for path '%s'", old_name);
>  
> -		if (trust_executable_bit || !S_ISREG(st->st_mode))
> +		if (cfg->trust_executable_bit || !S_ISREG(st->st_mode))
>  			st_mode = ce_mode_from_stat(*ce, st->st_mode);
>  		else if (*ce)
>  			st_mode = (*ce)->ce_mode;
> diff --git a/read-cache.c b/read-cache.c
> index 54150fe756..18af533649 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -204,10 +204,12 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
>  
>  unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
>  {
> +	struct repo_config_values *cfg = repo_config_values(the_repository);
> +
>  	if (!has_symlinks && S_ISREG(mode) &&
>  	    ce && S_ISLNK(ce->ce_mode))
>  		return ce->ce_mode;
> -	if (!trust_executable_bit && S_ISREG(mode)) {
> +	if (!cfg->trust_executable_bit && S_ISREG(mode)) {
>  		if (ce && S_ISREG(ce->ce_mode))
>  			return ce->ce_mode;
>  		return create_ce_mode(0666);

How hot are the code paths that call into this helper function?  In
the original under some condition, it was possible to return without
even consulting the trust_executable_bit variable, but in the
updated code, the helper unconditionally makes a call to the
repo_config_values() helper function even before it knows it needs
to know the value of trust_executable_bit.

> @@ -217,11 +219,13 @@ unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
>  
>  static unsigned int st_mode_from_ce(const struct cache_entry *ce)
>  {
> +	struct repo_config_values *cfg = repo_config_values(the_repository);
> +
>  	switch (ce->ce_mode & S_IFMT) {
>  	case S_IFLNK:
>  		return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
>  	case S_IFREG:
> -		return (ce->ce_mode & (trust_executable_bit ? 0755 : 0644)) | S_IFREG;
> +		return (ce->ce_mode & (cfg->trust_executable_bit ? 0755 : 0644)) | S_IFREG;
>  	case S_IFGITLINK:
>  		return S_IFDIR | 0755;
>  	case S_IFDIR:

Ditto.

> @@ -321,6 +325,7 @@ static int ce_modified_check_fs(struct index_state *istate,
>  static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
>  {
>  	unsigned int changed = 0;
> +	struct repo_config_values *cfg = repo_config_values(the_repository);
>  
>  	if (ce->ce_flags & CE_REMOVE)
>  		return MODE_CHANGED | DATA_CHANGED | TYPE_CHANGED;
> @@ -331,7 +336,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
>  		/* We consider only the owner x bit to be relevant for
>  		 * "mode changes"
>  		 */
> -		if (trust_executable_bit &&
> +		if (cfg->trust_executable_bit &&
>  		    (0100 & (ce->ce_mode ^ st->st_mode)))
>  			changed |= MODE_CHANGED;
>  		break;

Ditto.

> @@ -732,6 +737,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
>  			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
>  	unsigned hash_flags = pretend ? 0 : INDEX_WRITE_OBJECT;
>  
> +	struct repo_config_values *cfg = repo_config_values(the_repository);
> +

Lose the excess blank line before the new declaration.

>  	if (flags & ADD_CACHE_RENORMALIZE)
>  		hash_flags |= INDEX_RENORMALIZE;
>  
> @@ -752,7 +759,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
>  		ce->ce_flags |= CE_INTENT_TO_ADD;
>  
>  
> -	if (trust_executable_bit && has_symlinks) {
> +	if (cfg->trust_executable_bit && has_symlinks) {
>  		ce->ce_mode = create_ce_mode(st_mode);
>  	} else {
>  		/* If there is an existing entry, pick the mode bits and type

Almost all of these places that care about trust_executable_bit also
cares about has_symlinks.  I wonder if they should be converted to
repo-local settings in the same series.

Re: [PATCH v1 3/4] environment: move 'trust_executable_bit' into repo_config_values

[Tian Yuchen]

Hi Junio,

Thanks for the feedback!

On 5/31/26 07:17, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
> 
>> diff --git a/apply.c b/apply.c
>> index 249248d4f2..73ca9907f8 100644
>> --- a/apply.c
>> +++ b/apply.c
>> @@ -3890,10 +3890,12 @@ static int check_preimage(struct apply_state *state,
>>   	}
>>   
>>   	if (!state->cached && !previous) {
>> +		struct repo_config_values *cfg = repo_config_values(the_repository);
>> +
>>   		if (*ce && !(*ce)->ce_mode)
>>   			BUG("ce_mode == 0 for path '%s'", old_name);
>>   
>> -		if (trust_executable_bit || !S_ISREG(st->st_mode))
>> +		if (cfg->trust_executable_bit || !S_ISREG(st->st_mode))
>>   			st_mode = ce_mode_from_stat(*ce, st->st_mode);
>>   		else if (*ce)
>>   			st_mode = (*ce)->ce_mode;
>> diff --git a/read-cache.c b/read-cache.c
>> index 54150fe756..18af533649 100644
>> --- a/read-cache.c
>> +++ b/read-cache.c
>> @@ -204,10 +204,12 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
>>   
>>   unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
>>   {
>> +	struct repo_config_values *cfg = repo_config_values(the_repository);
>> +
>>   	if (!has_symlinks && S_ISREG(mode) &&
>>   	    ce && S_ISLNK(ce->ce_mode))
>>   		return ce->ce_mode;
>> -	if (!trust_executable_bit && S_ISREG(mode)) {
>> +	if (!cfg->trust_executable_bit && S_ISREG(mode)) {
>>   		if (ce && S_ISREG(ce->ce_mode))
>>   			return ce->ce_mode;
>>   		return create_ce_mode(0666);
> 
> How hot are the code paths that call into this helper function?  In
> the original under some condition, it was possible to return without
> even consulting the trust_executable_bit variable, but in the
> updated code, the helper unconditionally makes a call to the
> repo_config_values() helper function even before it knows it needs
> to know the value of trust_executable_bit.

That sounds reasonable to me. I’ll adjust the conditional logic in some 
of the statements so that they short-circuit appropriately to avoid 
performance overhead.

> 
>> @@ -217,11 +219,13 @@ unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
>>   
>>   static unsigned int st_mode_from_ce(const struct cache_entry *ce)
>>   {
>> +	struct repo_config_values *cfg = repo_config_values(the_repository);
>> +
>>   	switch (ce->ce_mode & S_IFMT) {
>>   	case S_IFLNK:
>>   		return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
>>   	case S_IFREG:
>> -		return (ce->ce_mode & (trust_executable_bit ? 0755 : 0644)) | S_IFREG;
>> +		return (ce->ce_mode & (cfg->trust_executable_bit ? 0755 : 0644)) | S_IFREG;
>>   	case S_IFGITLINK:
>>   		return S_IFDIR | 0755;
>>   	case S_IFDIR:
> 
> Ditto.
> 
>> @@ -321,6 +325,7 @@ static int ce_modified_check_fs(struct index_state *istate,
>>   static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
>>   {
>>   	unsigned int changed = 0;
>> +	struct repo_config_values *cfg = repo_config_values(the_repository);
>>   
>>   	if (ce->ce_flags & CE_REMOVE)
>>   		return MODE_CHANGED | DATA_CHANGED | TYPE_CHANGED;
>> @@ -331,7 +336,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
>>   		/* We consider only the owner x bit to be relevant for
>>   		 * "mode changes"
>>   		 */
>> -		if (trust_executable_bit &&
>> +		if (cfg->trust_executable_bit &&
>>   		    (0100 & (ce->ce_mode ^ st->st_mode)))
>>   			changed |= MODE_CHANGED;
>>   		break;
> 
> Ditto.
> 
>> @@ -732,6 +737,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
>>   			  (intent_only ? ADD_CACHE_NEW_ONLY : 0));
>>   	unsigned hash_flags = pretend ? 0 : INDEX_WRITE_OBJECT;
>>   
>> +	struct repo_config_values *cfg = repo_config_values(the_repository);
>> +
> 
> Lose the excess blank line before the new declaration.
> 
>>   	if (flags & ADD_CACHE_RENORMALIZE)
>>   		hash_flags |= INDEX_RENORMALIZE;
>>   
>> @@ -752,7 +759,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
>>   		ce->ce_flags |= CE_INTENT_TO_ADD;
>>   
>>   
>> -	if (trust_executable_bit && has_symlinks) {
>> +	if (cfg->trust_executable_bit && has_symlinks) {
>>   		ce->ce_mode = create_ce_mode(st_mode);
>>   	} else {
>>   		/* If there is an existing entry, pick the mode bits and type
> 
> Almost all of these places that care about trust_executable_bit also
> cares about has_symlinks.  I wonder if they should be converted to
> repo-local settings in the same series.

That’s true: I had actually planned to start migrating has_symlinks as 
soon as this series was approved. Since you think it would be better to 
merge them into a single series, I’ll go ahead and do that ;)

Thanks, yuchen

Re: [PATCH v1 3/4] environment: move 'trust_executable_bit' into repo_config_values

[Tian Yuchen]

On 6/1/26 18:10, Tian Yuchen wrote:

> That’s true: I had actually planned to start migrating has_symlinks as 
> soon as this series was approved. Since you think it would be better to 
> merge them into a single series, I’ll go ahead and do that ;)
> 

I’ve found that migrating has_symlinks seems to be quite a tricky 
business. Some callers in certain files pass very few parameters, and 
the call stack is quite deep, if I am correct. so I feel that adding a 
repo for this purpose might be overkill. Perhaps it would be better to 
focus on trust_executable_bit for now?

Regards, yuchen