git/environment.c, branch v2.45.2

Merge branch 'jk/core-comment-string'

2024-04-05T17:49:49Z

core.commentChar used to be limited to a single byte, but has been updated to allow an arbitrary multi-byte sequence. * jk/core-comment-string: config: add core.commentString config: allow multi-byte core.commentChar environment: drop comment_line_char compatibility macro wt-status: drop custom comment-char stringification sequencer: handle multi-byte comment characters when writing todo list find multi-byte comment chars in unterminated buffers find multi-byte comment chars in NUL-terminated strings prefer comment_line_str to comment_line_char for printing strbuf: accept a comment string for strbuf_add_commented_lines() strbuf: accept a comment string for strbuf_commented_addf() strbuf: accept a comment string for strbuf_stripspace() environment: store comment_line_char as a string strbuf: avoid shadowing global comment_line_char name commit: refactor base-case of adjust_comment_line_char() strbuf: avoid static variables in strbuf_add_commented_lines() strbuf: simplify comment-handling in add_lines() helper config: forbid newline as core.commentChar

environment: store comment_line_char as a string

2024-03-12T20:28:10Z

We'd like to eventually support multi-byte comment prefixes, but the comment_line_char variable is referenced in many spots, making the transition difficult. Let's start by storing the character in a NUL-terminated string. That will let us switch code over incrementally to the string format, and we can easily support the existing code with a macro wrapper (since we'll continue to allow only a single-byte prefix, this will behave identically). Once all references to the "char" variable have been converted, we can drop it and enable longer strings. We'll still have to touch all of the spots that create or set the variable in this patch, but there are only a few (reading the config, and the "auto" character selector). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'jc/no-lazy-fetch'

2024-03-07T23:59:40Z

"git --no-lazy-fetch cmd" allows to run "cmd" while disabling lazy fetching of objects from the promisor remote, which may be handy for debugging. * jc/no-lazy-fetch: git: extend --no-lazy-fetch to work across subprocesses git: document GIT_NO_REPLACE_OBJECTS environment variable git: --no-lazy-fetch option

git: extend --no-lazy-fetch to work across subprocesses

2024-02-27T17:53:14Z

Modeling after how the `--no-replace-objects` option is made usable across subprocess spawning (e.g., cURL based remote helpers are spawned as a separate process while running "git fetch"), allow the `--no-lazy-fetch` option to be passed across process boundaries. Do not model how the value of GIT_NO_REPLACE_OBJECTS environment variable is ignored, though. Just use the usual git_env_bool() to allow "export GIT_NO_LAZY_FETCH=0" and "unset GIT_NO_LAZY_FETCH" to be equivalents. Also do not model how the request is not propagated to subprocesses we spawn (e.g. "git clone --local" that spawns a new process to work in the origin repository, while the original one working in the newly created one) by the "--no-replace-objects" option, as this "do not lazily fetch from the promisor" is more about a per-request debugging aid, not "this repository's promisor should not be relied upon" property specific to a repository. Signed-off-by: Junio C Hamano

config: use git_config_string() for core.checkRoundTripEncoding

2023-12-08T23:26:22Z

Since this code path was recently converted to check for a NULL value, it now behaves exactly like git_config_string(). We can shorten the code a bit by using that helper. Note that git_config_string() takes a const pointer, but our storage variable is non-const. We're better off making this "const", though, since the default value points to a string literal (and thus it would be an error if anybody tried to write to it). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

max_tree_depth: lower it for MSVC to avoid stack overflows

2023-11-01T23:58:28Z

There seems to be some internal stack overflow detection in MSVC's `malloc()` machinery that seems to be independent of the `stack reserve` and `heap reserve` sizes specified in the executable (editable via `EDITBIN /STACK: ` and `EDITBIN /HEAP: `). In the newly test cases added by `jk/tree-name-and-depth-limit`, this stack overflow detection is unfortunately triggered before Git can print out the error message about too-deep trees and exit gracefully. Instead, it exits with `STATUS_STACK_OVERFLOW`. This corresponds to the numeric value -1073741571, something the MSYS2 runtime we sadly need to use to run Git's test suite cannot handle and which it internally maps to the exit code 127. Git's test suite, in turn, mistakes this to mean that the command was not found, and fails both test cases. Here is an example stack trace from an example run: [0x0] ntdll!RtlpAllocateHeap+0x31 0x4212603f50 0x7ff9d6d4cd49 [0x1] ntdll!RtlpAllocateHeapInternal+0x6c9 0x42126041b0 0x7ff9d6e14512 [0x2] ntdll!RtlDebugAllocateHeap+0x102 0x42126042b0 0x7ff9d6dcd8b0 [0x3] ntdll!RtlpAllocateHeap+0x7ec70 0x4212604350 0x7ff9d6d4cd49 [0x4] ntdll!RtlpAllocateHeapInternal+0x6c9 0x42126045b0 0x7ff9596ed480 [0x5] ucrtbased!heap_alloc_dbg_internal+0x210 0x42126046b0 0x7ff9596ed20d [0x6] ucrtbased!heap_alloc_dbg+0x4d 0x4212604750 0x7ff9596f037f [0x7] ucrtbased!_malloc_dbg+0x2f 0x42126047a0 0x7ff9596f0dee [0x8] ucrtbased!malloc+0x1e 0x42126047d0 0x7ff730fcc1ef [0x9] git!do_xmalloc+0x2f 0x4212604800 0x7ff730fcc2b9 [0xa] git!do_xmallocz+0x59 0x4212604840 0x7ff730fca779 [0xb] git!xmallocz_gently+0x19 0x4212604880 0x7ff7311b0883 [0xc] git!unpack_compressed_entry+0x43 0x42126048b0 0x7ff7311ac9a4 [0xd] git!unpack_entry+0x554 0x42126049a0 0x7ff7311b0628 [0xe] git!cache_or_unpack_entry+0x58 0x4212605250 0x7ff7311ad3a8 [0xf] git!packed_object_info+0x98 0x42126052a0 0x7ff7310a92da [0x10] git!do_oid_object_info_extended+0x3fa 0x42126053b0 0x7ff7310a44e7 [0x11] git!oid_object_info_extended+0x37 0x4212605460 0x7ff7310a38ba [0x12] git!repo_read_object_file+0x9a 0x42126054a0 0x7ff7310a6147 [0x13] git!read_object_with_reference+0x97 0x4212605560 0x7ff7310b4656 [0x14] git!fill_tree_descriptor+0x66 0x4212605620 0x7ff7310dc0a5 [0x15] git!traverse_trees_recursive+0x3f5 0x4212605680 0x7ff7310dd831 [0x16] git!unpack_callback+0x441 0x4212605790 0x7ff7310b4c95 [0x17] git!traverse_trees+0x5d5 0x42126058a0 0x7ff7310dc0f2 [0x18] git!traverse_trees_recursive+0x442 0x4212605980 0x7ff7310dd831 [0x19] git!unpack_callback+0x441 0x4212605a90 0x7ff7310b4c95 [0x1a] git!traverse_trees+0x5d5 0x4212605ba0 0x7ff7310dc0f2 [0x1b] git!traverse_trees_recursive+0x442 0x4212605c80 0x7ff7310dd831 [0x1c] git!unpack_callback+0x441 0x4212605d90 0x7ff7310b4c95 [0x1d] git!traverse_trees+0x5d5 0x4212605ea0 0x7ff7310dc0f2 [0x1e] git!traverse_trees_recursive+0x442 0x4212605f80 0x7ff7310dd831 [0x1f] git!unpack_callback+0x441 0x4212606090 0x7ff7310b4c95 [0x20] git!traverse_trees+0x5d5 0x42126061a0 0x7ff7310dc0f2 [0x21] git!traverse_trees_recursive+0x442 0x4212606280 0x7ff7310dd831 [...] [0xfad] git!cmd_main+0x2a2 0x42126ff740 0x7ff730fb6345 [0xfae] git!main+0xe5 0x42126ff7c0 0x7ff730fbff93 [0xfaf] git!wmain+0x2a3 0x42126ff830 0x7ff731318859 [0xfb0] git!invoke_main+0x39 0x42126ff8a0 0x7ff7313186fe [0xfb1] git!__scrt_common_main_seh+0x12e 0x42126ff8f0 0x7ff7313185be [0xfb2] git!__scrt_common_main+0xe 0x42126ff960 0x7ff7313188ee [0xfb3] git!wmainCRTStartup+0xe 0x42126ff990 0x7ff9d5ed257d [0xfb4] KERNEL32!BaseThreadInitThunk+0x1d 0x42126ff9c0 0x7ff9d6d6aa78 [0xfb5] ntdll!RtlUserThreadStart+0x28 0x42126ff9f0 0x0 I verified manually that `traverse_trees_cur_depth` was 562 when that happened, which is far below the 2048 that were already accepted into Git as a hard limit. Despite many attempts to figure out which of the internals trigger this `STATUS_STACK_OVERFLOW` and how to maybe increase certain sizes to avoid running into this issue and let Git behave the same way as under Linux, I failed to find any build-time/runtime knob we could turn to that effect. Note: even switching to using a different allocator (I used mimalloc because that's what Git for Windows uses for its GCC builds) does not help, as the zlib code used to unpack compressed pack entries _still_ uses the regular `malloc()`. And runs into the same issue. Note also: switching to using a different allocator _also_ for zlib code seems _also_ not to help. I tried that, and it still exited with `STATUS_STACK_OVERFLOW` that seems to have been triggered by a `mi_assert_internal()`, i.e. an internal assertion of mimalloc... So the best bet to work around this for now seems to just lower the maximum allowed tree depth _even further_ for MSVC builds. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano

lower core.maxTreeDepth default to 2048

2023-08-31T22:51:08Z

On my Linux system, all of our recursive tree walking algorithms can run up to the 4096 default limit without segfaulting. But not all platforms will have stack sizes as generous (nor might even Linux if we kick off a recursive walk within a thread). In particular, several of the tests added in the previous few commits fail in our Windows CI environment. Through some guess-and-check pushing, I found that 3072 is still too much, but 2048 is OK. These are obviously vague heuristics, and there is nothing to promise that another system might not have trouble at even lower values. But it seems unlikely anybody will be too angry about a 2048-depth limit (this is close to the default max-pathname limit on Linux even for a pathological path like "a/a/a/..."). So let's just lower it. Some alternatives are: - configure separate defaults for Windows versus other platforms. - just skip the tests on Windows. This leaves Windows users with the annoying case that they can be crashed by running out of stack space, but there shouldn't be any security implications (they can't go deep enough to hit integer overflow problems). Since the original default was arbitrary, it seems less confusing to just lower it, keeping behavior consistent across platforms. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

add core.maxTreeDepth config

2023-08-31T22:51:07Z

Most of our tree traversal algorithms use recursion to visit sub-trees. For pathologically large trees, this can cause us to run out of stack space and abort in an uncontrolled way. Let's put our own limit here so that we can fail gracefully rather than segfaulting. In similar cases where we recursed along the commit graph, we rewrote the algorithms to avoid recursion and keep any stack data on the heap. But the commit graph is meant to grow without bound, whereas it's not an imposition to put a limit on the maximum size of tree we'll handle. And this has a bonus side effect: coupled with a limit on individual tree entry names, this limits the total size of a path we may encounter. This gives us an extra protection against code handling long path names which may suffer from integer overflows in the size (which could then be exploited by malicious trees). The default of 4096 is set to be much longer than anybody would care about in the real world. Even with single-letter interior tree names (like "a/b/c"), such a path is at least 8191 bytes. While most operating systems will let you create such a path incrementally, trying to reference the whole thing in a system call (as Git would do when actually trying to access it) will result in ENAMETOOLONG. Coupled with the recent fsck.largePathname warning, the maximum total pathname Git will handle is (by default) 16MB. This config option doesn't do anything yet; future patches will convert various algorithms to respect the limit. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

Merge branch 'rs/pack-objects-parseopt-fix'

2023-07-28T16:45:22Z

Command line parser fix. * rs/pack-objects-parseopt-fix: pack-objects: fix --no-quiet pack-objects: fix --no-keep-true-parents

pack-objects: fix --no-keep-true-parents

2023-07-21T17:02:59Z

Since 99fb6e04cb (pack-objects: convert to use parse_options(), 2012-02-01) git pack-objects has accepted --no-keep-true-parents, but this option does the same as --keep-true-parents. That's because it's defined using OPT_SET_INT with a value of 0, which sets 0 when negated as well. Turn --no-keep-true-parents into the opposite of --keep-true-parents by using OPT_BOOL and storing the option's status directly in a variable named "grafts_keep_true_parents" instead of in negative form in "grafts_replace_parents". Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano