Re: [PATCH 01/10] environment: move access to core.maxTreeDepth into repo settings

["René Scharfe" <l.s.r@web.de>]

On 1/12/26 10:21 AM, Patrick Steinhardt wrote:
> On Fri, Jan 09, 2026 at 10:30:12PM +0100, René Scharfe wrote:
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index b0673d1a450..bebcf9f698c 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -578,6 +578,30 @@ static inline bool strip_suffix(const char *str, const char *suffix,
>>  #define DEFAULT_PACKED_GIT_LIMIT \
>>  	((1024L * 1024L) * (size_t)(sizeof(void*) >= 8 ? (32 * 1024L * 1024L) : 256))
>>  
>> +#ifdef _MSC_VER
>> +  /*
>> +   * When traversing into too-deep trees, Visual C-compiled Git seems to
>> +   * run into some internal stack overflow detection in the
>> +   * `RtlpAllocateHeap()` function that is called from within
>> +   * `git_inflate_init()`'s call tree. The following value seems to be
>> +   * low enough to avoid that by letting Git exit with an error before
>> +   * the stack overflow can occur.
>> +   */
>> +#define DEFAULT_MAX_ALLOWED_TREE_DEPTH 512
>> +#elif defined(GIT_WINDOWS_NATIVE) && defined(__clang__) && defined(__aarch64__)
>> +  /*
>> +   * Similar to Visual C, it seems that on Windows/ARM64 the clang-based
>> +   * builds have a smaller stack space available. When running out of
>> +   * that stack space, a `STATUS_STACK_OVERFLOW` is produced. When the
>> +   * Git command was run from an MSYS2 Bash, this unfortunately results
>> +   * in an exit code 127. Let's prevent that by lowering the maximal
>> +   * tree depth; This value seems to be low enough.
>> +   */
>> +#define DEFAULT_MAX_ALLOWED_TREE_DEPTH 1280
>> +#else
>> +#define DEFAULT_MAX_ALLOWED_TREE_DEPTH 2048
>> +#endif
>> +
>>  int git_open_cloexec(const char *name, int flags);
>>  #define git_open(name) git_open_cloexec(name, O_RDONLY)
>>  
> 
> Why is this new constant defined in "git-compat-util.h"?

No particular reason, I just put it next to the existing similar macros
DEFAULT_PACKED_GIT_WINDOW_SIZE and DEFAULT_PACKED_GIT_LIMIT.

> That feels way
> too global for my taste for a value that should ultimately only be
> required by "repo-settings.c". Is there a reason why it's not declared
> in "repo-settings.h"?

No.  The same goes for the above two, I guess.  Or they could go to
packfile.h and DEFAULT_MAX_ALLOWED_TREE_DEPTH to tree.h.
> I think we could even get away with declaring it in "repo-settings.c".
> The expectation is that the settings need to be prepared anyway, so we
> can simply set up the default when parsing the config.

True, REPO_SETTINGS_INIT is the only user of 
DEFAULT_MAX_ALLOWED_TREE_DEPTH outside of repo-settings.c and is itself
only referenced there, so we could make it private, or even inline it to
its only user, repo_settings_clear().

René