Re: [PATCH 01/10] environment: move access to core.maxTreeDepth into repo settings
- From
René Scharfe <l.s.r@web.de>
- Date
- Jan 12, 2026, 19:37 UTC
- Message-ID
- <cf8330f2-e624-4d58-9250-57f0dbe7a0d2@web.de>
- In-Reply-To
- <aWS9JLb3UyxOwg9Z@pks.im>
On 1/12/26 10:21 AM, Patrick Steinhardt wrote:
Show 38 quoted lines
> 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é