summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2023-11-02Merge branch 'ds/scalar-updates' into maint-2.42Junio C Hamano5-44/+115
Scalar updates. * ds/scalar-updates: scalar reconfigure: help users remove buggy repos setup: add discover_git_directory_reason() scalar: add --[no-]src option
2023-11-02Merge branch 'mp/rebase-label-length-limit' into maint-2.42Junio C Hamano4-6/+62
Overly long label names used in the sequencer machinery are now chopped to fit under filesystem limitation. * mp/rebase-label-length-limit: rebase: allow overriding the maximal length of the generated labels sequencer: truncate labels to accommodate loose refs
2023-11-02Merge branch 'js/ci-coverity' into maint-2.42Junio C Hamano1-0/+163
GitHub CI workflow has learned to trigger Coverity check. * js/ci-coverity: coverity: detect and report when the token or project is incorrect coverity: allow running on macOS coverity: support building on Windows coverity: allow overriding the Coverity project coverity: cache the Coverity Build Tool ci: add a GitHub workflow to submit Coverity scans
2023-11-02Merge branch 'jk/test-lsan-denoise-output' into maint-2.42Junio C Hamano1-0/+1
Tests with LSan from time to time seem to emit harmless message that makes our tests unnecessarily flakey; we work it around by filtering the uninteresting output. * jk/test-lsan-denoise-output: test-lib: ignore uninteresting LSan output
2023-11-02Merge branch 'js/ci-san-skip-p4-and-svn-tests' into maint-2.42Junio C Hamano1-0/+2
Flakey "git p4" tests, as well as "git svn" tests, are now skipped in the (rather expensive) sanitizer CI job. * js/ci-san-skip-p4-and-svn-tests: ci(linux-asan-ubsan): let's save some time
2023-11-02Merge branch 'tb/mark-more-tests-as-leak-free' into maint-2.42Junio C Hamano4-0/+5
Tests that are known to pass with LSan are now marked as such. * tb/mark-more-tests-as-leak-free: leak tests: mark t5583-push-branches.sh as leak-free leak tests: mark t3321-notes-stripspace.sh as leak-free leak tests: mark a handful of tests as leak-free
2023-11-02max_tree_depth: lower it for MSVC to avoid stack overflowsJohannes Schindelin1-1/+14
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:<n> <exe>` and `EDITBIN /HEAP:<n> <exe>`). 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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-02merge-file: add an option to process object IDsbrian m. carlson3-17/+122
git merge-file knows how to merge files on the file system already. It would be helpful, however, to allow it to also merge single blobs. Teach it an `--object-id` option which means that its arguments are object IDs and not files to allow it to do so. We handle the empty blob specially since read_mmblob doesn't read it directly and otherwise users cannot specify an empty ancestor. Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-02git-merge-file doc: drop "-file" from argument placeholdersMartin Ågren1-10/+11
`git merge-file` takes three positional arguments. Each of them is documented as `<foo-file>`. In preparation for teaching this command to alternatively take three object IDs, make these placeholders a bit more generic by dropping the "-file" parts. Instead, clarify early that the three arguments are filenames. Even after the next commit, we can afford to present this file-centric view up front and in the general discussion, since it will remain the default one. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01Documentation/gitformat-pack.txt: fix incorrect MIDX documentationTaylor Blau1-4/+5
Back in 32f3c541e3 (multi-pack-index: write pack names in chunk, 2018-07-12) the MIDX's "Packfile Names" (or "PNAM", for short) chunk was described as containing an array of string entries. e0d1bcf825 notes that this is the only chunk in the MIDX format's specification that is not guaranteed to be 4-byte aligned, and so should be placed last. This isn't quite accurate: the entries within the PNAM chunk are not guaranteed to be 4-byte aligned since they are arbitrary strings, but the chunk itself is 4-byte aligned since the ending is padded with NUL bytes. That padding has always been there since 32f3c541e3 via midx.c::write_midx_pack_names(), which ended with: i = MIDX_CHUNK_ALIGNMENT - (written % MIDX_CHUNK_ALIGNMENT) if (i < MIDX_CHUNK_ALIGNMENT) { unsigned char padding[MIDX_CHUNK_ALIGNMENT]; memset(padding, 0, sizeof(padding)) hashwrite(f, padding, i); written += i; } In fact, 32f3c541e3's log message itself describes the chunk in its first paragraph with: Since filenames are not well structured, add padding to keep good alignment in later chunks. So these have always been externally aligned. Correct the corresponding part of our documentation to reflect that. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01Documentation/gitformat-pack.txt: fix typoTaylor Blau1-1/+1
e0d1bcf825 (multi-pack-index: add format details, 2018-07-12) describes the MIDX's "PNAM" chunk as having entries which are "null-terminated strings". This is a typo, as strings are terminated with a NUL character, which is a distinct concept from "NULL" or "null", which we typically reserve for the void pointer to address 0. Correct the documentation accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01clang-format: fix typo in commentAditya Neelamraju1-3/+3
Signed-off-by: Aditya Neelamraju <adityanv97@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01t: use git-show-ref(1) to check for ref existencePatrick Steinhardt5-27/+94
Convert tests that use `test_path_is_file` and `test_path_is_missing` to instead use a set of helpers `test_ref_exists` and `test_ref_missing`. These helpers are implemented via the newly introduced `git show-ref --exists` command. Thus, we can avoid intimate knowledge of how the ref backend stores references on disk. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: add new mode to check for reference existencePatrick Steinhardt3-6/+117
While we have multiple ways to show the value of a given reference, we do not have any way to check whether a reference exists at all. While commands like git-rev-parse(1) or git-show-ref(1) can be used to check for reference existence in case the reference resolves to something sane, neither of them can be used to check for existence in some other scenarios where the reference does not resolve cleanly: - References which have an invalid name cannot be resolved. - References to nonexistent objects cannot be resolved. - Dangling symrefs can be resolved via git-symbolic-ref(1), but this requires the caller to special case existence checks depending on whether or not a reference is symbolic or direct. Furthermore, git-rev-list(1) and other commands do not let the caller distinguish easily between an actually missing reference and a generic error. Taken together, this seems like sufficient motivation to introduce a separate plumbing command to explicitly check for the existence of a reference without trying to resolve its contents. This new command comes in the form of `git show-ref --exists`. This new mode will exit successfully when the reference exists, with a specific exit code of 2 when it does not exist, or with 1 when there has been a generic error. Note that the only way to properly implement this command is by using the internal `refs_read_raw_ref()` function. While the public function `refs_resolve_ref_unsafe()` can be made to behave in the same way by passing various flags, it does not provide any way to obtain the errno with which the reference backend failed when reading the reference. As such, it becomes impossible for us to distinguish generic errors from the explicit case where the reference wasn't found. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: explicitly spell out different modes in synopsisPatrick Steinhardt2-4/+10
The synopsis treats the `--verify` and the implicit mode the same. They are slightly different though: - They accept different sets of flags. - The implicit mode accepts patterns while the `--verify` mode accepts references. Split up the synopsis for these two modes such that we can disambiguate those differences. While at it, drop "--quiet" from the pattern mode's synopsis. It does not make a lot of sense to list patterns, but squelch the listing output itself. The description for "--quiet" is adapted accordingly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: ensure mutual exclusiveness of subcommandsPatrick Steinhardt2-0/+13
The git-show-ref(1) command has three different modes, of which one is implicit and the other two can be chosen explicitly by passing a flag. But while these modes are standalone and cause us to execute completely separate code paths, we gladly accept the case where a user asks for both `--exclude-existing` and `--verify` at the same time even though it is not obvious what will happen. Spoiler: we ignore `--verify` and execute the `--exclude-existing` mode. Let's explicitly detect this invalid usage and die in case both modes were requested. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: refactor options for patterns subcommandPatrick Steinhardt1-13/+22
The patterns subcommand is the last command that still uses global variables to track its options. Convert it to use a structure instead with the same motivation as preceding commits. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: stop using global vars for `show_one()`Patrick Steinhardt1-22/+40
The `show_one()` function implicitly receives a bunch of options which are tracked via global variables. This makes it hard to see which subcommands of git-show-ref(1) actually make use of these options. Introduce a `show_one_options` structure that gets passed down to this function. This allows us to get rid of more global state and makes it more explicit which subcommands use those options. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: stop using global variable to count matchesPatrick Steinhardt1-3/+4
When passing patterns to git-show-ref(1) we're checking whether any reference matches -- if none do, we indicate this condition via an unsuccessful exit code. We're using a global variable to count these matches, which is required because the counter is getting incremented in a callback function. But now that we have the `struct show_ref_data` in place, we can get rid of the global variable and put the counter in there instead. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: refactor `--exclude-existing` optionsPatrick Steinhardt1-34/+44
It's not immediately obvious options which options are applicable to what subcommand in git-show-ref(1) because all options exist as global state. This can easily cause confusion for the reader. Refactor options for the `--exclude-existing` subcommand to be contained in a separate structure. This structure is stored on the stack and passed down as required. Consequently, it clearly delimits the scope of those options and requires the reader to worry less about global state. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: fix dead code when passing patternsPatrick Steinhardt1-4/+1
When passing patterns to `git show-ref` we have some code that will cause us to die if `verify && !quiet` is true. But because `verify` indicates a different subcommand of git-show-ref(1) that causes us to execute `cmd_show_ref__verify()` and not `cmd_show_ref__patterns()`, the condition cannot ever be true. Let's remove this dead code. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: fix leaking string bufferPatrick Steinhardt1-1/+3
Fix a leaking string buffer in `git show-ref --exclude-existing`. While the buffer is technically not leaking because its variable is declared as static, there is no inherent reason why it should be. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: split up different subcommandsPatrick Steinhardt1-47/+54
While not immediately obvious, git-show-ref(1) actually implements three different subcommands: - `git show-ref <patterns>` can be used to list references that match a specific pattern. - `git show-ref --verify <refs>` can be used to list references. These are _not_ patterns. - `git show-ref --exclude-existing` can be used as a filter that reads references from standard input, performing some conversions on each of them. Let's make this more explicit in the code by splitting up the three subcommands into separate functions. This also allows us to address the confusingly named `patterns` variable, which may hold either patterns or reference names. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01builtin/show-ref: convert pattern to a local variablePatrick Steinhardt1-18/+28
The `pattern` variable is a global variable that tracks either the reference names (not patterns!) for the `--verify` mode or the patterns for the non-verify mode. This is a bit confusing due to the slightly different meanings. Convert the variable to be local. While this does not yet fix the double meaning of the variable, this change allows us to address it in a subsequent patch more easily by explicitly splitting up the different subcommands of git-show-ref(1). Note that we introduce a `struct show_ref_data` to pass the patterns to `show_ref()`. While this is overengineered now, we will extend this structure in a subsequent patch. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01rev-list: add commit object support in `--missing` optionKarthik Nayak5-2/+101
The `--missing` object option in rev-list currently works only with missing blobs/trees. For missing commits the revision walker fails with a fatal error. Let's extend the functionality of `--missing` option to also support commit objects. This is done by adding a `missing_objects` field to `rev_info`. This field is an `oidset` to which we'll add the missing commits as we encounter them. The revision walker will now continue the traversal and call `show_commit()` even for missing commits. In rev-list we can then check if the commit is a missing commit and call the existing code for parsing `--missing` objects. A scenario where this option would be used is to find the boundary objects between different object directories. Consider a repository with a main object directory (GIT_OBJECT_DIRECTORY) and one or more alternate object directories (GIT_ALTERNATE_OBJECT_DIRECTORIES). In such a repository, using the `--missing=print` option while disabling the alternate object directory allows us to find the boundary objects between the main and alternate object directory. Helped-by: Patrick Steinhardt <ps@pks.im> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01rev-list: move `show_commit()` to the bottomKarthik Nayak1-43/+42
The `show_commit()` function already depends on `finish_commit()`, and in the upcoming commit, we'll also add a dependency on `finish_object__ma()`. Since in C symbols must be declared before they're used, let's move `show_commit()` below both `finish_commit()` and `finish_object__ma()`, so the code is cleaner as a whole without the need for declarations. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01revision: rename bit to `do_not_die_on_missing_objects`Karthik Nayak4-11/+12
The bit `do_not_die_on_missing_tree` is used in revision.h to ensure the revision walker does not die when encountering a missing tree. This is currently exclusively set within `builtin/rev-list.c` to ensure the `--missing` option works with missing trees. In the upcoming commits, we will extend `--missing` to also support missing commits. So let's rename the bit to `do_not_die_on_missing_objects`, which is object type agnostic and can be used for both trees/commits. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01Merge branch 'ps/do-not-trust-commit-graph-blindly-for-existence' into ↵Junio C Hamano5-2/+84
kn/rev-list-missing-fix * ps/do-not-trust-commit-graph-blindly-for-existence: commit: detect commits that exist in commit-graph but not in the ODB commit-graph: introduce envvar to disable commit existence checks
2023-11-01commit: detect commits that exist in commit-graph but not in the ODBPatrick Steinhardt2-1/+42
Commit graphs can become stale and contain references to commits that do not exist in the object database anymore. Theoretically, this can lead to a scenario where we are able to successfully look up any such commit via the commit graph even though such a lookup would fail if done via the object database directly. As the commit graph is mostly intended as a sort of cache to speed up parsing of commits we do not want to have diverging behaviour in a repository with and a repository without commit graphs, no matter whether they are stale or not. As commits are otherwise immutable, the only thing that we really need to care about is thus the presence or absence of a commit. To address potentially stale commit data that may exist in the graph, our `lookup_commit_in_graph()` function will check for the commit's existence in both the commit graph, but also in the object database. So even if we were able to look up the commit's data in the graph, we would still pretend as if the commit didn't exist if it is missing in the object database. We don't have the same safety net in `parse_commit_in_graph_one()` though. This function is mostly used internally in "commit-graph.c" itself to validate the commit graph, and this usage is fine. We do expose its functionality via `parse_commit_in_graph()` though, which gets called by `repo_parse_commit_internal()`, and that function is in turn used in many places in our codebase. For all I can see this function is never used to directly turn an object ID into a commit object without additional safety checks before or after this lookup. What it is being used for though is to walk history via the parent chain of commits. So when commits in the parent chain of a graph walk are missing it is possible that we wouldn't notice if that missing commit was part of the commit graph. Thus, a query like `git rev-parse HEAD~2` can succeed even if the intermittent commit is missing. It's unclear whether there are additional ways in which such stale commit graphs can lead to problems. In any case, it feels like this is a bigger bug waiting to happen when we gain additional direct or indirect callers of `repo_parse_commit_internal()`. So let's fix the inconsistent behaviour by checking for object existence via the object database, as well. This check of course comes with a performance penalty. The following benchmarks have been executed in a clone of linux.git with stable tags added: Benchmark 1: git -c core.commitGraph=true rev-list --topo-order --all (git = master) Time (mean ± σ): 2.913 s ± 0.018 s [User: 2.363 s, System: 0.548 s] Range (min … max): 2.894 s … 2.950 s 10 runs Benchmark 2: git -c core.commitGraph=true rev-list --topo-order --all (git = pks-commit-graph-inconsistency) Time (mean ± σ): 3.834 s ± 0.052 s [User: 3.276 s, System: 0.556 s] Range (min … max): 3.780 s … 3.961 s 10 runs Benchmark 3: git -c core.commitGraph=false rev-list --topo-order --all (git = master) Time (mean ± σ): 13.841 s ± 0.084 s [User: 13.152 s, System: 0.687 s] Range (min … max): 13.714 s … 13.995 s 10 runs Benchmark 4: git -c core.commitGraph=false rev-list --topo-order --all (git = pks-commit-graph-inconsistency) Time (mean ± σ): 13.762 s ± 0.116 s [User: 13.094 s, System: 0.667 s] Range (min … max): 13.645 s … 14.038 s 10 runs Summary git -c core.commitGraph=true rev-list --topo-order --all (git = master) ran 1.32 ± 0.02 times faster than git -c core.commitGraph=true rev-list --topo-order --all (git = pks-commit-graph-inconsistency) 4.72 ± 0.05 times faster than git -c core.commitGraph=false rev-list --topo-order --all (git = pks-commit-graph-inconsistency) 4.75 ± 0.04 times faster than git -c core.commitGraph=false rev-list --topo-order --all (git = master) We look at a ~30% regression in general, but in general we're still a whole lot faster than without the commit graph. To counteract this, the new check can be turned off with the `GIT_COMMIT_GRAPH_PARANOIA` envvar. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-11-01commit-graph: introduce envvar to disable commit existence checksPatrick Steinhardt4-1/+42
Our `lookup_commit_in_graph()` helper tries to look up commits from the commit graph and, if it doesn't exist there, falls back to parsing it from the object database instead. This is intended to speed up the lookup of any such commit that exists in the database. There is an edge case though where the commit exists in the graph, but not in the object database. To avoid returning such stale commits the helper function thus double checks that any such commit parsed from the graph also exists in the object database. This makes the function safe to use even when commit graphs aren't updated regularly. We're about to introduce the same pattern into other parts of our code base though, namely `repo_parse_commit_internal()`. Here the extra sanity check is a bit of a tougher sell: `lookup_commit_in_graph()` was a newly introduced helper, and as such there was no performance hit by adding this sanity check. If we added `repo_parse_commit_internal()` with that sanity check right from the beginning as well, this would probably never have been an issue to begin with. But by retrofitting it with this sanity check now we do add a performance regression to preexisting code, and thus there is a desire to avoid this or at least give an escape hatch. In practice, there is no inherent reason why either of those functions should have the sanity check whereas the other one does not: either both of them are able to detect this issue or none of them should be. This also means that the default of whether we do the check should likely be the same for both. To err on the side of caution, we thus rather want to make `repo_parse_commit_internal()` stricter than to loosen the checks that we already have in `lookup_commit_in_graph()`. The escape hatch is added in the form of a new GIT_COMMIT_GRAPH_PARANOIA environment variable that mirrors GIT_REF_PARANOIA. If enabled, which is the default, we will double check that commits looked up in the commit graph via `lookup_commit_in_graph()` also exist in the object database. This same check will also be added in `repo_parse_commit_internal()`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-31test framework: further deprecate test_i18ngrepJunio C Hamano1-4/+6
As an attempt to come up with a useful mechanism to ensure that certain messages are left untranslated [*], we earlier wrote GIT_TEST_GETTEXT_POISON off as a failed experiment. But the output from the test helper was easier to use while debugging failed tests, compared to the same test writtein with the plain-vanilla "grep". Especially when a test that expects a certain string to appear in the output (e.g. "this test must fail with this message") fails, "grep message output" would just silently fail and in a &&-chained sequence of commands, it is hard to tell which step failed. test_i18ngrep explicitly said "we wanted to see a line that match this pattern but did not see a hit in this file". What we have as test_i18ngrep in our tree still retains this verbose output (even though we got rid of the "poison" support). Let's rename it to test_grep (because it is no longer about i18n at all) and then make test_i18ngrep a thin wrapper around it. Existing callers of test_i18ngrep can be mechanically rewritten to instead use test_grep over time, but it does not have to be done in this commit. [Footnote] * The idea was that human-facing messages are often translated, but there are messages that should never be translated. We use "grep" only for the latter kind of messages, and then run tests in "poison" mode that spew garbage for translatable messages. If such a test run fails, it means these messages tested with "grep" were marked for translation by mistake. test_i18ngrep was to be used for other messages that are to be translated, and was to always "succeed" when runing under the "poison" mode. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-31Merge branch 'jm/bisect-run-synopsis-fix'Junio C Hamano2-4/+4
Doc and usage message update. * jm/bisect-run-synopsis-fix: doc/git-bisect: clarify `git bisect run` syntax
2023-10-31Merge branch 'ii/branch-error-messages-update'Junio C Hamano4-47/+47
Error message update. * ii/branch-error-messages-update: builtin/branch.c: adjust error messages to coding guidelines
2023-10-30upload-pack: add tracing for fetchesRobert Coup2-11/+55
Information on how users are accessing hosted repositories can be helpful to server operators. For example, being able to broadly differentiate between fetches and initial clones; the use of shallow repository features; or partial clone filters. a29263c (fetch-pack: add tracing for negotiation rounds, 2022-08-02) added some information on have counts to fetch-pack itself to help diagnose negotiation; but from a git-upload-pack (server) perspective, there's no means of accessing such information without using GIT_TRACE_PACKET to examine the protocol packets. Improve this by emitting a Trace2 JSON event from upload-pack with summary information on the contents of a fetch request. * haves, wants, and want-ref counts can help determine (broadly) between fetches and clones, and the use of single-branch, etc. * shallow clone depth, tip counts, and deepening options. * any partial clone filter type. Signed-off-by: Robert Coup <robert@coup.net.nz> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-30The twenty-second batchJunio C Hamano1-0/+19
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-10-30Merge branch 'ms/doc-push-fix'Junio C Hamano1-1/+1
Docfix. * ms/doc-push-fix: git-push doc: more visibility for -q option
2023-10-30Merge branch 'jk/send-email-fix-addresses-from-composed-messages'Junio C Hamano3-85/+99
The codepath to handle recipient addresses `git send-email --compose` learns from the user was completely broken, which has been corrected. * jk/send-email-fix-addresses-from-composed-messages: send-email: handle to/cc/bcc from --compose message Revert "send-email: extract email-parsing code into a subroutine" doc/send-email: mention handling of "reply-to" with --compose
2023-10-30Merge branch 'ob/rebase-cleanup'Junio C Hamano1-30/+14
Code clean-up. * ob/rebase-cleanup: rebase: move parse_opt_keep_empty() down rebase: handle --strategy via imply_merge() as well rebase: simplify code related to imply_merge()
2023-10-30Merge branch 'jc/commit-new-underscore-index-fix'Junio C Hamano1-4/+4
Message fix. * jc/commit-new-underscore-index-fix: commit: do not use cryptic "new_index" in end-user facing messages
2023-10-30Merge branch 'wx/merge-ort-comment-typofix'Junio C Hamano1-1/+1
Typofix. * wx/merge-ort-comment-typofix: merge-ort.c: fix typo 'neeed' to 'needed'
2023-10-30Merge branch 'ps/git-repack-doc-fixes'Junio C Hamano1-6/+3
Doc updates. * ps/git-repack-doc-fixes: doc/git-repack: don't mention nonexistent "--unpacked" option doc/git-repack: fix syntax for `-g` shorthand option
2023-10-30Merge branch 'kh/pathspec-error-wo-repository-fix'Junio C Hamano2-1/+33
The pathspec code carelessly dereferenced NULL while emitting an error message, which has been corrected. * kh/pathspec-error-wo-repository-fix: grep: die gracefully when outside repository
2023-10-30Merge branch 'ni/die-message-fix-for-git-add'Junio C Hamano1-5/+5
Message updates. * ni/die-message-fix-for-git-add: builtin/add.c: clean up die() messages
2023-10-30Merge branch 'jc/am-doc-whitespace-action-fix'Junio C Hamano1-2/+2
Docfix. * jc/am-doc-whitespace-action-fix: am: align placeholder for --whitespace option with apply
2023-10-30Merge branch 'mm/p4-symlink-with-lfs'Junio C Hamano1-0/+4
"git p4" tried to store symlinks to LFS when told, but has been fixed not to do so, because it does not make sense. * mm/p4-symlink-with-lfs: git-p4 shouldn't attempt to store symlinks in LFS
2023-10-30Merge branch 'da/t7601-style-fix'Junio C Hamano1-12/+12
Coding style update. * da/t7601-style-fix: t7601: use "test_path_is_file" etc. instead of "test -f"
2023-10-30Merge branch 'jc/update-list-references-to-lore'Junio C Hamano1-1/+1
Doc update. * jc/update-list-references-to-lore: doc: update list archive reference to use lore.kernel.org
2023-10-30Merge branch 'jc/attr-tree-config'Junio C Hamano7-3/+119
The attribute subsystem learned to honor `attr.tree` configuration that specifies which tree to read the .gitattributes files from. * jc/attr-tree-config: attr: add attr.tree for setting the treeish to read attributes from attr: read attributes from HEAD when bare repo
2023-10-30Merge branch 'sn/typo-grammo-phraso-fixes'Junio C Hamano14-42/+41
Many typos, ungrammatical sentences and wrong phrasing have been fixed. * sn/typo-grammo-phraso-fixes: t/README: fix multi-prerequisite example doc/gitk: s/sticked/stuck/ git-jump: admit to passing merge mode args to ls-files doc/diff-options: improve wording of the log.diffMerges mention doc: fix some typos, grammar and wording issues
2023-10-29reflog: fix expire --single-worktreeRené Scharfe2-3/+26
33d7bdd645 (builtin/reflog.c: use parse-options api for expire, delete subcommands, 2022-01-06) broke the option --single-worktree of git reflog expire and added a non-printable short flag for it, presumably by accident. While before it set the variable "all_worktrees" to 0, now it sets it to 1, its default value. --no-single-worktree is required now to set it to 0. Fix it by replacing the variable with one that has the opposite meaning, to avoid the negation and its potential for confusion. The new variable "single_worktree" directly captures whether --single-worktree was given. Also remove the unprintable short flag SOH (start of heading) because it is undocumented, hard to use and is likely to have been added by mistake in connection with the negation bug above. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>