Re: [PATCH 6/6] max_tree_depth: lower it for clangarm64 on Windows

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Patrick,

On Tue, 22 Apr 2025, Patrick Steinhardt wrote:

> On Mon, Apr 21, 2025 at 12:39:10PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > diff --git a/environment.c b/environment.c
> > index 9e4c7781be0..cc853950bb2 100644
> > --- a/environment.c
> > +++ b/environment.c
> > @@ -82,9 +82,21 @@ int max_allowed_tree_depth =
> >  	 * the stack overflow can occur.
> >  	 */
> >  	512;
> > +#else
> > +#if defined(GIT_WINDOWS_NATIVE) && defined(__clang__) && defined(__aarch64__)
> 
> Tiny nit, only because it puzzled me for a second: this should probably
> be `#elif`.

I will change it.

> > +	/*
> > +	 * 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.
> > +	 */
> > +	1280;
> >  #else
> >  	2048;
> >  #endif
> > +#endif
> 
> Hm. This whole construct feels rather awful, if you ask me. Instead of
> papering over the issue it would be nice if we eventually fixed the root
> cause, which is that we use recursion on a data structure that has an
> unbounded depth in theory.

True.

It is also quite awful that I cannot find a way to represent
`STATUS_STACK_OVERFLOW` by anything else than exit code 127, which always
misleads me into thinking that an executable or a DLL might be missing.
But I did not find any.

> Anyway, that is clearly outside of the scope of this patch series, so
> the bandaid is good enough for now.

True enough!

Thank you,
Johannes