environment.c: migrate 'trust_executable_bit' into 'repo_config_values' by malon7782 · Pull Request #2312 · git/git

malon7782 · 2026-05-26T10:06:26Z

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.

Patch 1: Cleans up redundant local 'extern' declarations for 'trust_executable_bit' and 'has_symlinks' in 'read-cache.c'.
Patch 2: Moves 'ce_mode_from_stat()' from 'read-cache.h' to 'read-cache.c', removing its inline status.
Patch 3: Refactors the signatures of low-level helpers.
Patch 4: Performs the actual migration of 'trust_executable_bit' into 'repo_config_values'.

Previous related work: https://lore.kernel.org/git/837b5360b40f992351f489a0ae05fedf49884c6e.1685716420.git.gitgitgadget@gmail.com/

Mentored-by: Christian Couder christian.couder@gamil.com
Mentored-by: Ayush Chandekar ayu.chandekar@gmail.com
Signed-off-by: Tian Yuchen cat@malon.dev

Anjalo133

T

chriscool · 2026-05-27T06:44:12Z

 		return ce->ce_mode;
 	default:
-		BUG("unsupported ce_mode: %o", ce->ce_mode);
+		bug("unsupported ce_mode: %o", ce->ce_mode);


Why is BUG() changed into bug()?

chriscool · 2026-05-27T06:45:42Z

 #define INDEX_FORMAT_UB 4

 struct cache_entry {
+


Why is this blank line added?

chriscool · 2026-05-27T06:49:43Z

+	struct repository *repo = (istate && istate->repo) ? istate->repo : the_repository;
+	struct repo_config_values *cfg = repo_config_values(repo);
+
+	extern int has_symlinks;


"read-cache.c" already includes "environment.h" which has extern int has_symlinks; so I am not sure this is useful here. I think it could be dropped.

chriscool · 2026-05-27T06:50:15Z

+static unsigned int st_mode_from_ce(struct index_state *istate, const struct cache_entry *ce)
 {
-	extern int trust_executable_bit, has_symlinks;
+	extern int has_symlinks;


"read-cache.c" already includes "environment.h" which has extern int has_symlinks; so I am not sure this is useful here. I think it could be dropped.

chriscool · 2026-05-27T06:57:55Z

It seems to me that the single commit in this branch is doing too many things:

Moving ce_mode_from_stat() from "read-cache.h" to "read-cache.c" making it non inline.
Moving trust_executable_bit from a global into struct repo_config_values.
Passing a struct index_state *istate to many functions.

I think the commit could be split into three commits to help with reviews.

Also the usefulness of 3. is dubious for now because repo_config_values() still contains:

	if (repo != the_repository)
		BUG("trying to read config from wrong repository instance");

So 3. makes it look like some functions could work for a repo that isn't the_repository, but it's in fact not true. At least 3. should be explained more thoroughly in a dedicated commit message.

It looks like there could even be a small cleanup patch before the main three patches to remove the:

extern int trust_executable_bit, has_symlinks;

which was not necessary since "read-cache.c" already does #include "environment.h".

chriscool · 2026-05-27T07:00:09Z

 #include "object.h"
 #include "pathspec.h"
+#include "repository.h"
+#include "environment.h"


Not sure that including both is necessary.

In fact it seems to compile correctly without them.

chriscool · 2026-05-27T07:03:45Z

Also about the commit message, maybe you can start to systematically add Mentored-by: ... trailers for both of your co-mentors. I think it will simplify things to add them systematically rather than keeping track about who helped you on which commit.

chriscool · 2026-05-27T07:15:40Z


 		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->repo->index, ent, st_mode);


It looks like this could simply be:

ce->ce_mode = ce_mode_from_stat(istate, ent, st_mode);

chriscool · 2026-05-27T07:35:37Z

The nice thing with a pull request (or merge request) is that its description can contain what will be in the cover letter of the patch series.

In the cover letter it's nice to talk about previous work related to the series.

About the trust_executable_bit, there was this patch before:

https://lore.kernel.org/git/20260301190017.53539-1-dronarajgyawali@gmail.com/

and it looks like you participated in the discussion.

This one is maybe relevant too:

https://lore.kernel.org/git/837b5360b40f992351f489a0ae05fedf49884c6e.1685716420.git.gitgitgadget@gmail.com/

So you might want to mention them in the cover letter/this PR's description.

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@gamil.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev>

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. Mentored-by: Christian Couder <christian.couder@gamil.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev>

In preparation for moving global configuration variables into the repository-specific 'repo_config_values' struct, low-level stat and mode helper functions need access to repository context. 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@gamil.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev>

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. For callers where 'istate->repo' might be NULL, fall back safely to 'the_repository'. Mentored-by: Christian Couder <christian.couder@gamil.com> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com> Signed-off-by: Tian Yuchen <cat@malon.dev>

gitgitgadget-git · 2026-05-27T10:39:50Z

There is an issue in commit 9ef58e0:
read-cache: pass 'istate' to stat/mode helper functions

Lines in the body of the commit messages should be wrapped between 60 and 76 characters.
Indented lines, and lines without whitespace, are exempt

chriscool · 2026-05-27T14:27:20Z

 static unsigned int st_mode_from_ce(const struct cache_entry *ce)
 {
-	extern int trust_executable_bit, has_symlinks;



I think the blank line following the extern int ... line could be removed too.

gitgitgadget-git · 2026-05-28T13:05:21Z

There is an issue in commit 9ef58e0:
read-cache: pass 'istate' to stat/mode helper functions

Lines in the body of the commit messages should be wrapped between 60 and 76 characters.
Indented lines, and lines without whitespace, are exempt

gitgitgadget-git · 2026-05-28T13:41:46Z

There is an issue in commit 9ef58e0:
read-cache: pass 'istate' to stat/mode helper functions

Lines in the body of the commit messages should be wrapped between 60 and 76 characters.
Indented lines, and lines without whitespace, are exempt

github-project-automation Bot added this to Op-Amp Frequency Response and Applications Suite May 26, 2026

github-project-automation Bot moved this to Todo in Op-Amp Frequency Response and Applications Suite May 26, 2026

malon7782 force-pushed the refactor-environment branch 3 times, most recently from 898ae86 to 373dfb2 Compare May 26, 2026 11:11

Anjalo133 reviewed May 26, 2026

View reviewed changes

chriscool reviewed May 27, 2026

View reviewed changes

Tian Yuchen added 4 commits May 27, 2026 18:33

malon7782 force-pushed the refactor-environment branch from 373dfb2 to 51ca686 Compare May 27, 2026 10:39

chriscool reviewed May 27, 2026

View reviewed changes

malon7782 mentioned this pull request May 28, 2026

environment: migrate trust_executable_bit (test) #2316

Open

malon7782 force-pushed the refactor-environment branch 2 times, most recently from 7207106 to 51ca686 Compare May 28, 2026 13:04

malon7782 closed this May 28, 2026

malon7782 force-pushed the refactor-environment branch from 51ca686 to 56a4f3c Compare May 28, 2026 13:29

github-project-automation Bot moved this from Todo to Done in Op-Amp Frequency Response and Applications Suite May 28, 2026

malon7782 reopened this May 28, 2026

Conversation

malon7782 commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Anjalo133 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriscool commented May 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriscool commented May 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriscool commented May 27, 2026

Uh oh!

gitgitgadget-git Bot commented May 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitgitgadget-git Bot commented May 28, 2026

Uh oh!

gitgitgadget-git Bot commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

malon7782 commented May 26, 2026 •

edited

Loading