Skip to content

environment: migrate trust_executable_bit (test)#2316

Open
malon7782 wants to merge 4 commits into
git:masterfrom
malon7782:environment-test
Open

environment: migrate trust_executable_bit (test)#2316
malon7782 wants to merge 4 commits into
git:masterfrom
malon7782:environment-test

Conversation

@malon7782
Copy link
Copy Markdown
Contributor

@malon7782 malon7782 commented May 28, 2026 •
edited
Loading

This commit series is currently for test use. Will refine associated descriptions once completed.
Based on @chriscool's suggestion, this branch is a reordered version of #2312

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

@gitgitgadget-git
Copy link
Copy Markdown

gitgitgadget-git Bot commented May 28, 2026

There is an issue in commit 87e5acf:
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

@malon7782 malon7782 force-pushed the environment-test branch 2 times, most recently from 7207106 to d2f2b49 Compare May 28, 2026 16:20
@chriscool
Copy link
Copy Markdown
Contributor

chriscool commented May 29, 2026

It looks like in the description and commit messages gamil.com is used instead of gmail.com in my email address (christian.couder@gamil.com instead of christian.couder@gmail.com).

@chriscool
Copy link
Copy Markdown
Contributor

chriscool commented May 29, 2026

The commit title read-cache: move 'ce_move_from_stat()' to 'read-cache.c' has a typo too. It should talk about ce_mode_from_stat() not ce_move_from_stat().

chriscool
chriscool reviewed May 29, 2026
View reviewed changes
Comment thread read-cache.c
chriscool
chriscool reviewed May 29, 2026
View reviewed changes
Comment thread read-cache.h
chriscool
chriscool reviewed May 29, 2026
View reviewed changes
Comment thread read-cache.c Outdated
malon7782 added 2 commits May 29, 2026 18:17
@malon7782
read-cache: remove redundant extern declarations
e17a715 
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>
@malon7782
read-cache: move 'ce_mode_from_stat()' to 'read-cache.c'
2af2684 
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>
malon7782 added 2 commits May 29, 2026 18:56
@malon7782
environment: move 'trust_executable_bit' into repo_config_values
8a121f2 
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>
@malon7782
read-cache: pass 'istate' to stat/mode helper functions
f02c007 
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>
@chriscool
Copy link
Copy Markdown
Contributor

chriscool commented May 29, 2026 •
edited
Loading

It seems to me that the goal of the "read-cache: pass 'istate' to stat/mode helper functions" is to help with getting rid of the_repository in the long term, but it is not mentioned clearly in its commit message.

I think saying something like "This is a step in the long term direction of getting rid of the_repository variable." could help justify the commit.

In the commit message you might also want to take into account what I said in this comment:

#2312 (comment)

@chriscool
Copy link
Copy Markdown
Contributor

chriscool commented May 29, 2026

I am not sure why the description doesn't also mention:

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

I also think it would be nice if there was a small explanation of what the previous works on this tried and why they weren't successful.

@malon7782 malon7782 mentioned this pull request May 29, 2026
Open
malon7782
malon7782 commented May 30, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@malon7782 malon7782 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Specified the wrong range when replacing quotes via Vim, which has resulted in inconsistent quote types in the filenames here.

"read-cache.c" vs 'read-cache.h'

How did I miss it? :((

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@chriscool chriscool chriscool left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@malon7782 @chriscool