summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2021-07-08Merge branch 'ms/mergetools-kdiff3-on-windows'Libravatar Junio C Hamano1-0/+9
On Windows, mergetool has been taught to find kdiff3.exe just like it finds winmerge.exe. * ms/mergetools-kdiff3-on-windows: mergetools/kdiff3: make kdiff3 work on Windows too
2021-07-08Merge branch 'ab/cmd-foo-should-return'Libravatar Junio C Hamano9-14/+13
Code clean-up. * ab/cmd-foo-should-return: builtins + test helpers: use return instead of exit() in cmd_*
2021-07-08Merge branch 'ar/doc-libera-chat-in-my-first-contrib'Libravatar Junio C Hamano1-2/+2
Update MyFirst document. * ar/doc-libera-chat-in-my-first-contrib: MyFirstContribution: link #git-devel to Libera Chat
2021-07-08Merge branch 'ar/mailinfo-memcmp-to-skip-prefix'Libravatar Junio C Hamano1-2/+2
Code clean-up. * ar/mailinfo-memcmp-to-skip-prefix: mailinfo: use starts_with() when checking scissors
2021-07-08Merge branch 'jk/doc-max-pack-size'Libravatar Junio C Hamano3-10/+23
Doc update. * jk/doc-max-pack-size: doc: warn people against --max-pack-size
2021-07-08Merge branch 'ab/fix-columns-to-80-during-tests'Libravatar Junio C Hamano1-2/+3
Output from some of our tests were affected by the width of the terminal that they were run in, which has been corrected by exporting a fixed value in the COLUMNS environment. * ab/fix-columns-to-80-during-tests: test-lib.sh: set COLUMNS=80 for --verbose repeatability
2021-07-08Merge branch 'ar/more-typofix'Libravatar Junio C Hamano4-4/+4
Typofixes. * ar/more-typofix: git-worktree.txt: fix typo in example path t: fix typos in test messages blame: correct name of config option in docs
2021-07-08Merge branch 'fw/complete-cmd-idx-fix'Libravatar Junio C Hamano1-0/+1
Recent update to completion script (in contrib/) broke those who use the __git_complete helper to define completion to their custom command. * fw/complete-cmd-idx-fix: completion: bash: fix late declaration of __git_cmd_idx
2021-07-08Merge branch 'jk/test-without-readlink-1'Libravatar Junio C Hamano3-3/+9
Some test scripts assumed that readlink(1) was universally installed and available, which is not the case. * jk/test-without-readlink-1: t: use portable wrapper for readlink(1)
2021-07-08Merge branch 'jx/sideband-cleanup'Libravatar Junio C Hamano32-1013/+1036
The side-band demultiplexer that is used to display progress output from the remote end did not clear the line properly when the end of line hits at a packet boundary, which has been corrected. Also comes with test clean-ups. * jx/sideband-cleanup: test: refactor to use "get_abbrev_oid" to get abbrev oid test: refactor to use "test_commit" to create commits test: compare raw output, not mangle tabs and spaces sideband: don't lose clear-to-eol at packet boundary
2021-07-08Merge branch 'jk/test-avoid-globmatch-with-skip-patterns'Libravatar Junio C Hamano1-12/+22
We broke "GIT_SKIP_TESTS=t?000" to skip certain tests in recent update, which got fixed. * jk/test-avoid-globmatch-with-skip-patterns: test-lib: avoid accidental globbing in match_pattern_list()
2021-07-08Merge branch 'jv/userdiff-csharp-update'Libravatar Junio C Hamano1-1/+1
The userdiff pattern for C# learned the token "record". * jv/userdiff-csharp-update: userdiff: add support for C# record types
2021-07-08Merge branch 'ab/config-hooks-path-testfix'Libravatar Junio C Hamano1-0/+1
Test fix. * ab/config-hooks-path-testfix: pre-commit hook tests: don't leave "actual" nonexisting on failure
2021-07-08Merge branch 'fc/pull-cleanups'Libravatar Junio C Hamano1-15/+11
Code cleanup. * fc/pull-cleanups: pull: trivial whitespace style fix pull: trivial cleanup pull: cleanup autostash check
2021-07-08Merge branch 'jk/bitmap-tree-optim'Libravatar Junio C Hamano3-0/+22
Avoid duplicated work while building reachability bitmaps. * jk/bitmap-tree-optim: bitmaps: don't recurse into trees already in the bitmap
2021-07-08Merge branch 'ah/graph-typofix'Libravatar Junio C Hamano1-1/+1
Typofix in an error message. * ah/graph-typofix: graph: improve grammar of "invalid color" error message
2021-07-08Merge branch 'jx/t6020-with-older-bash'Libravatar Junio C Hamano1-19/+31
Work around inefficient glob substitution in older versions of bash by rewriting parts of a test. * jx/t6020-with-older-bash: t6020: fix incompatible parameter expansion
2021-07-08Merge branch 'ar/typofix'Libravatar Junio C Hamano14-16/+15
Typofixes. * ar/typofix: *: fix typos which duplicate a word
2021-07-08Merge branch 'jk/revision-squelch-gcc-warning'Libravatar Junio C Hamano1-2/+3
Warning fix. * jk/revision-squelch-gcc-warning: add_pending_object_with_path(): work around "gcc -O3" complaint
2021-07-08Merge branch 'ah/uninitialized-reads-fix'Libravatar Junio C Hamano3-4/+4
Make the codebase MSAN clean. * ah/uninitialized-reads-fix: builtin/checkout--worker: zero-initialise struct to avoid MSAN complaints split-index: use oideq instead of memcmp to compare object_id's bulk-checkin: make buffer reuse more obvious and safer
2021-07-08Merge branch 'js/no-more-multimail'Libravatar Junio C Hamano12-6303/+2
Remove multimail from contrib/ * js/no-more-multimail: multimail: stop shipping a copy
2021-07-08Merge branch 'js/subtree-on-windows-fix'Libravatar Junio C Hamano1-4/+8
Update "git subtree" to work better on Windows. * js/subtree-on-windows-fix: subtree: fix assumption about the directory separator subtree: fix the GIT_EXEC_PATH sanity check to work on Windows
2021-07-08Merge branch 'dd/svn-test-wo-locale-a'Libravatar Junio C Hamano6-22/+32
"git-svn" tests assumed that "locale -a", which is used to pick an available UTF-8 locale, is available everywhere. A knob has been introduced to allow testers to specify a suitable locale to use. * dd/svn-test-wo-locale-a: t: use user-specified utf-8 locale for testing svn
2021-07-08Merge branch 'fc/doc-default-to-upstream-config'Libravatar Junio C Hamano1-1/+1
Doc clean-up. * fc/doc-default-to-upstream-config: doc: merge: mention default of defaulttoupstream
2021-07-08Merge branch 'js/trace2-discard-event-docfix'Libravatar Junio C Hamano1-2/+2
Docfix. * js/trace2-discard-event-docfix: docs: fix api-trace2 doc for "too_many_files" event
2021-07-08Merge branch 'tb/complete-diff-anchored'Libravatar Junio C Hamano1-0/+1
The command line completion (in contrib/) learned that "git diff" takes the "--anchored" option. * tb/complete-diff-anchored: completion: add --anchored to diff's options
2021-07-08Merge branch 'tk/partial-clone-repack-doc'Libravatar Junio C Hamano1-5/+1
Docfix. * tk/partial-clone-repack-doc: Remove warning that repack only works on non-promisor packfiles
2021-07-01test-lib: avoid accidental globbing in match_pattern_list()Libravatar Jeff King1-12/+22
We have a custom match_pattern_list() function which we use for matching test names (like "t1234") against glob-like patterns (like "t1???") for $GIT_SKIP_TESTS, --verbose-only, etc. Those patterns may have multiple whitespace-separated elements (e.g., "t0* t1234 t5?78"). The callers of match_pattern_list thus pass the strings unquoted, so that the shell does the usual field-splitting into separate arguments. But this also means the shell will do the usual globbing for each argument, which can result in us seeing an expansion based on what's in the filesystem, rather than the real pattern. For example, if I have the path "t5000" in the filesystem, and you feed the pattern "t?000", that _should_ match the string "t0000", but it won't after the shell has expanded it to "t5000". This has been a bug ever since that function was introduced. But it didn't usually trigger since we typically use the function inside the trash directory, which has a very limited set of files that are unlikely to match. It became a lot easier to trigger after edc23840b0 (test-lib: bring $remove_trash out of retirement, 2021-05-10), because now we match $GIT_SKIP_TESTS before even entering the trash directory. So the t5000 example above can be seen with: GIT_SKIP_TESTS=t?000 ./t0000-basic.sh which should skip all tests but doesn't. We can fix this by using "set -f" to ask the shell not to glob (which is in POSIX, so should hopefully be portable enough). We only want to do this in a subshell (to avoid polluting the rest of the script), which means we need to get the whole string intact into the match_pattern_list function by quoting it. Arguably this is a good idea anyway, since it makes it much more obvious that we intend to split, and it's not simply sloppy scripting. Diagnosed-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-29test-lib.sh: set COLUMNS=80 for --verbose repeatabilityLibravatar Ævar Arnfjörð Bjarmason1-2/+3
Some tests will fail under --verbose because while we've unset COLUMNS since b1d645b58ac (tests: unset COLUMNS inherited from environment, 2012-03-27), we also look for the columns with an ioctl(.., TIOCGWINSZ, ...) on some platforms. By setting COLUMNS again we preempt the TIOCGWINSZ lookup in pager.c's term_columns(), it'll take COLUMNS over TIOCGWINSZ, This fixes t0500-progress-display.sh., which broke because of a combination of the this issue and the progress output reacting to the column width since 545dc345ebd (progress: break too long progress bar lines, 2019-04-12). The t5324-split-commit-graph.sh fails in a similar manner due to progress output, see [1] for details. The issue is not specific to progress.c, the diff code also checks COLUMNS and some of its tests can be made to fail in a similar manner[2], anything that invokes a pager is potentially affected. See ea77e675e56 (Make "git help" react to window size correctly, 2005-12-18) and ad6c3739a33 (pager: find out the terminal width before spawning the pager, 2012-02-12) for how the TIOCGWINSZ code ended up in pager.c 1. http://lore.kernel.org/git/20210624051253.GG6312@szeder.dev 2. https://lore.kernel.org/git/20210627074419.GH6312@szeder.dev/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-28git-worktree.txt: fix typo in example pathLibravatar Andrei Rybak1-1/+1
Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-28t: fix typos in test messagesLibravatar Andrei Rybak2-2/+2
Both in t4258 and in t9001, the code of the tests following shows the proper name for the configuration variables. So use the correct names in the test messages as well. Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-28blame: correct name of config option in docsLibravatar Andrei Rybak1-1/+1
As can be seen in files "Documentation/blame-options.txt" and "builtin/blame.c", the name of this configuration option is "blame.markUnblamableLines". Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-19pull: trivial whitespace style fixLibravatar Felipe Contreras1-3/+3
Two spaces unaligned to anything is not part of the coding-style. A single tab is. Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-19pull: trivial cleanupLibravatar Felipe Contreras1-4/+2
There's no need to store ran_ff. Now it's obvious from the conditionals. Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-19pull: cleanup autostash checkLibravatar Felipe Contreras1-9/+7
Currently "git pull --rebase" takes a shortcut in the case a fast-forward merge is possible; run_merge() is called with --ff-only. However, "git merge" didn't have an --autostash option, so, when "git pull --rebase --autostash" was called *and* the fast-forward merge shortcut was taken, then the pull failed. This was fixed in commit f15e7cf5cc (pull: ff --rebase --autostash works in dirty repo, 2017-06-01) by simply skipping the fast-forward merge shortcut. Later on "git merge" learned the --autostash option [a03b55530a (merge: teach --autostash option, 2020-04-07)], and so did "git pull" [d9f15d37f1 (pull: pass --autostash to merge, 2020-04-07)]. Therefore it's not necessary to skip the fast-forward merge shortcut anymore when called with --rebase --autostash. Let's always take the fast-forward merge shortcut by essentially reverting f15e7cf5cc. Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-19completion: bash: fix late declaration of __git_cmd_idxLibravatar Fabian Wermelinger1-0/+1
A recent update to contrib/completion/git-completion.bash causes bash to fail auto complete custom commands that are wrapped with __git_func_wrap. Declaring __git_cmd_idx=0 inside __git_func_wrap resolves the issue. Signed-off-by: Fabian Wermelinger <fabianw@mavt.ethz.ch> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-19t: use portable wrapper for readlink(1)Libravatar Jeff King3-3/+9
Not all systems have a readlink program available for use by the shell. This causes t3210 to fail on at least AIX. Let's provide a perl one-liner to do the same thing, and use it there. I also updated calls in t9802. Nobody reported failure there, but it's the same issue. Presumably nobody actually tests with p4 on AIX in the first place (if it is even available there). I left the use of readlink in the "--valgrind" setup in test-lib.sh, as valgrind isn't available on exotic platforms anyway (and I didn't want to increase dependencies between test-lib.sh and test-lib-functions.sh). There's one other curious case. Commit d2addc3b96 (t7800: readlink may not be available, 2016-05-31) fixed a similar case. We can't use our wrapper function there, though, as it's inside a sub-script triggered by Git. It uses a slightly different technique ("ls" piped to "sed"). I chose not to use that here as it gives confusing "ls -l" output if the file is unexpectedly not a symlink (which is OK for its limited use, but potentially confusing for general use within the test suite). The perl version emits the empty string. Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-17test: refactor to use "get_abbrev_oid" to get abbrev oidLibravatar Jiang Xin15-52/+72
Add new function "get_abbrev_oid" to get abbrev object ID. This function has a default value which helps to prepare a nonempty replace pattern for sed command. An empty replace pattern may cause sed fail to allocate memory. Refactor function "make_user_friendly_and_stable_output" to use "get_abbrev_oid" to get abbrev object ID. Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-17test: refactor to use "test_commit" to create commitsLibravatar Jiang Xin2-38/+12
Refactor function "create_commits_in" to use "test_commit" to create commit. Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-17test: compare raw output, not mangle tabs and spacesLibravatar Jiang Xin31-966/+972
Before comparing with the expect file, we used to call function "make_user_friendly_and_stable_output" to filter out trailing spaces in output. Ævar recommends using pattern "s/Z$//" to prepare expect file, and then compare it with raw output. Since we have fixed the issue of occasionally missing the clear-to-eol suffix when displaying sideband #2 messages, it is safe and stable to test against raw output. Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-17sideband: don't lose clear-to-eol at packet boundaryLibravatar Jiang Xin1-0/+23
When "demultiplex_sideband()" sees a nonempty message ending with CR or LF on the sideband #2, it adds "suffix" string to clear to the end of the current line, which helps when relaying a progress display whose records are terminated with CRs. But if it sees a single LF, no clear-to-end suffix should be appended, because this single LF is used to end the progress display by moving to the next line, and the final progress display above should be preserved. However, the code forgot that depending on the length of the payload line, such a CR may fall exactly at the packet boundary and the number of bytes before the CR from the beginning of the packet could be zero. In such a case, the message that was terminated by the CR were leftover in the "scratch" buffer in the previous call to the function and we still need to clear to the end of the current line. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Nicolas Pitre <nico@fluxnic.net> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-17t6020: fix incompatible parameter expansionLibravatar Jiang Xin1-19/+31
Ævar reported that the function `make_user_friendly_and_stable_output()` failed on a i386 box (gcc45) in the gcc farm boxes with error: sed: couldn't re-allocate memory It turns out that older versions of bash (4.3) or dash (0.5.7) cannot evaluate expression like `${A%${A#???????}}` used to get the leading 7 characters of variable A. Replace the incompatible parameter expansion so that t6020 works on older version of bash or dash. Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-16userdiff: add support for C# record typesLibravatar Julian Verdurmen1-1/+1
Records are added in C# 9 Code example : public record Person(string FirstName, string LastName); For more information, see: * https://docs.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-9 Signed-off-by: Julian Verdurmen <julian.verdurmen@outlook.com> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-16pre-commit hook tests: don't leave "actual" nonexisting on failureLibravatar Ævar Arnfjörð Bjarmason1-0/+1
Start by creating an "actual" file in a core.hooksPath test that has the hook echoing to the "actual" file. We later test_cmp that file to see what hooks were run. If we fail to run our hook(s) we'll have an empty list of hooks for the test_cmp instead of a nonexisting file. For the logic of this test that makes more sense. See 867ad08a261 (hooks: allow customizing where the hook directory is, 2016-05-04) for the commit that added these tests. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-15graph: improve grammar of "invalid color" error messageLibravatar Alex Henrie1-1/+1
Without the "d", it sounds like a command, not an error, and is liable to be translated incorrectly. Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-15builtin/checkout--worker: zero-initialise struct to avoid MSAN complaintsLibravatar Andrzej Hunt1-1/+1
report_result() sends a struct to the parent process, but that struct would contain uninitialised padding bytes. Running this code under MSAN rightly triggers a warning - but we don't particularly care about this warning because we control the receiving code, and we therefore know that those padding bytes won't be read on the receiving end. We could simply suppress this warning under MSAN with the approporiate ifdef'd attributes, but a less intrusive solution is to 0-initialise the struct, which guarantees that the padding will also be initialised. Interestingly, in the error-case branch, we only try to copy the first two members of pc_item_result, by copying only PC_ITEM_RESULT_BASE_SIZE bytes. However PC_ITEM_RESULT_BASE_SIZE is defined as 'offsetof(the_last_member)', which means that we're copying padding bytes after the end of the second last member. We could avoid doing this by redefining PC_ITEM_RESULT_BASE_SIZE as 'offsetof(second_last_member) + sizeof(second_last_member)', but there's no huge benefit to doing so (and this patch silences the MSAN warning in this scenario either way). MSAN output from t2080 (partially interleaved due to the parallel work :) ): Uninitialized bytes in __interceptor_write at offset 12 inside [0x7fff37d83408, 160) ==23279==WARNING: MemorySanitizer: use-of-uninitialized-value Uninitialized bytes in __interceptor_write at offset 12 inside [0x7ffdb8a07ec8, 160) ==23280==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0xd5ac28 in xwrite /home/ahunt/git/git/wrapper.c:256:8 #1 0xd5b327 in write_in_full /home/ahunt/git/git/wrapper.c:311:21 #2 0xb0a8c4 in do_packet_write /home/ahunt/git/git/pkt-line.c:221:6 #3 0xb0a5fd in packet_write /home/ahunt/git/git/pkt-line.c:242:6 #4 0x4f7441 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:69:2 #5 0x4f6be6 in worker_loop /home/ahunt/git/git/builtin/checkout--worker.c:100:3 #6 0x4f68d3 in cmd_checkout__worker /home/ahunt/git/git/builtin/checkout--worker.c:143:2 #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 #12 0x7f8778114349 in __libc_start_main (/lib64/libc.so.6+0x24349) #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120 Uninitialized value was created by an allocation of 'res' in the stack frame of function 'report_result' #0 0x4f72c0 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:55 SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ahunt/git/git/wrapper.c:256:8 in xwrite Exiting #0 0xd5ac28 in xwrite /home/ahunt/git/git/wrapper.c:256:8 #1 0xd5b327 in write_in_full /home/ahunt/git/git/wrapper.c:311:21 #2 0xb0a8c4 in do_packet_write /home/ahunt/git/git/pkt-line.c:221:6 #3 0xb0a5fd in packet_write /home/ahunt/git/git/pkt-line.c:242:6 #4 0x4f7441 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:69:2 #5 0x4f6be6 in worker_loop /home/ahunt/git/git/builtin/checkout--worker.c:100:3 #6 0x4f68d3 in cmd_checkout__worker /home/ahunt/git/git/builtin/checkout--worker.c:143:2 #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 #12 0x7f2749a0e349 in __libc_start_main (/lib64/libc.so.6+0x24349) #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120 Uninitialized value was created by an allocation of 'res' in the stack frame of function 'report_result' #0 0x4f72c0 in report_result /home/ahunt/git/git/builtin/checkout--worker.c:55 SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/ahunt/git/git/wrapper.c:256:8 in xwrite Signed-off-by: Andrzej Hunt <andrzej@ahunt.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-15split-index: use oideq instead of memcmp to compare object_id'sLibravatar Andrzej Hunt1-1/+2
cache_entry contains an object_id, and compare_ce_content() would include that field when calling memcmp on a subset of the cache_entry. Depending on which hashing algorithm is being used, only part of object_id.hash is actually being used, therefore including it in a memcmp() is incorrect. Instead we choose to exclude the object_id when calling memcmp(), and call oideq() separately. This issue was found when running t1700-split-index with MSAN, see MSAN output below (on my machine, offset 76 corresponds to 4 bytes after the start of object_id.hash). Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92) ==27914==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8 #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9 #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2 #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8 #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7 #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9 #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 #12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349) #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120 Uninitialized value was stored to memory at #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3 #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2 #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18 #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2 #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11 #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12 #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12 #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6 #8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17 #9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9 #10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8 #11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7 #12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2 #13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12 #14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9 #15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 Uninitialized value was created by a heap allocation #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3 #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8 #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9 #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6 #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3 #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17 #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8 #8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8 #9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2 #10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6 #11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11 #12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3 #13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4 #14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19 #15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11 #16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp Exiting Signed-off-by: Andrzej Hunt <andrzej@ahunt.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-15subtree: fix assumption about the directory separatorLibravatar Johannes Schindelin1-3/+4
On Windows, both forward and backslash are valid separators. In 22d550749361 (subtree: don't fuss with PATH, 2021-04-27), however, we added code that assumes that it can only be the forward slash. Let's fix that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-15subtree: fix the GIT_EXEC_PATH sanity check to work on WindowsLibravatar Johannes Schindelin1-1/+4
In 22d550749361 (subtree: don't fuss with PATH, 2021-04-27), `git subtree` was broken thoroughly on Windows. The reason is that it assumes Unix semantics, where `PATH` is colon-separated, and it assumes that `$GIT_EXEC_PATH:` is a verbatim prefix of `$PATH`. Neither are true, the latter in particular because `GIT_EXEC_PATH` is a Windows-style path, while `PATH` is a Unix-style path list. Let's make extra certain that `$GIT_EXEC_PATH` and the first component of `$PATH` refer to different entities before erroring out. We do that by using the `test <path1> -ef <path2>` command that verifies that the inode of `<path1>` and of `<path2>` is the same. Sadly, this construct is non-portable, according to https://pubs.opengroup.org/onlinepubs/009695399/utilities/test.html. However, it does not matter in practice because we still first look whether `$GIT_EXEC_PREFIX` is string-identical to the first component of `$PATH`. This will give us the expected result everywhere but in Git for Windows, and Git for Windows' own Bash _does_ handle the `-ef` operator. Just in case that we _do_ need to show the error message _and_ are running in a shell that lacks support for `-ef`, we simply suppress the error output for that part. This fixes https://github.com/git-for-windows/git/issues/3260 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-15bitmaps: don't recurse into trees already in the bitmapLibravatar Jeff King3-0/+22
If an object is already mentioned in a reachability bitmap we are building, then by definition so are all of the objects it can reach. We have an optimization to stop traversing commits when we see they are already in the bitmap, but we don't do the same for trees. It's generally unavoidable to recurse into trees for commits not yet covered by bitmaps (since most commits generally do have unique top-level trees). But they usually have subtrees that are shared with other commits (i.e., all of the subtrees the commit _didn't_ touch). And some of those commits (and their trees) may be covered by the bitmap. Usually this isn't _too_ big a deal, because we'll visit those subtrees only once in total for the whole walk. But if you have a large number of unbitmapped commits, and if your tree is big, then you may end up opening a lot of sub-trees for no good reason. We can use the same optimization we do for commits here: when we are about to open a tree, see if it's in the bitmap (either the one we are building, or the "seen" bitmap which covers the UNINTERESTING side of the bitmap when doing a set-difference). This works especially well because we'll visit all commits before hitting any trees. So even in a history like: A -- B if "A" has a bitmap on disk but "B" doesn't, we'll already have OR-ed in the results from A before looking at B's tree (so we really will only look at trees touched by B). For most repositories, the timings produced by p5310 are unspectacular. Here's linux.git: Test HEAD^ HEAD -------------------------------------------------------------------- 5310.4: simulated clone 6.00(5.90+0.10) 5.98(5.90+0.08) -0.3% 5310.5: simulated fetch 2.98(5.45+0.18) 2.85(5.31+0.18) -4.4% 5310.7: rev-list (commits) 0.32(0.29+0.03) 0.33(0.30+0.03) +3.1% 5310.8: rev-list (objects) 1.48(1.44+0.03) 1.49(1.44+0.05) +0.7% Any improvement there is within the noise (the +3.1% on test 7 has to be noise, since we are not recursing into trees, and thus the new code isn't even run). The results for git.git are likewise uninteresting. But here are numbers from some other real-world repositories (that are not public). This one's tree is comparable in size to linux.git, but has ~16k refs (and so less complete bitmap coverage): Test HEAD^ HEAD ------------------------------------------------------------------------- 5310.4: simulated clone 38.34(39.86+0.74) 33.95(35.53+0.76) -11.5% 5310.5: simulated fetch 2.29(6.31+0.35) 2.20(5.97+0.41) -3.9% 5310.7: rev-list (commits) 0.99(0.86+0.13) 0.96(0.85+0.11) -3.0% 5310.8: rev-list (objects) 11.32(11.04+0.27) 6.59(6.37+0.21) -41.8% And here's another with a very large tree (~340k entries), and a fairly large number of refs (~10k): Test HEAD^ HEAD ------------------------------------------------------------------------- 5310.3: simulated clone 53.83(54.71+1.54) 39.77(40.76+1.50) -26.1% 5310.4: simulated fetch 19.91(20.11+0.56) 19.79(19.98+0.67) -0.6% 5310.6: rev-list (commits) 0.54(0.44+0.11) 0.51(0.43+0.07) -5.6% 5310.7: rev-list (objects) 24.32(23.59+0.73) 9.85(9.49+0.36) -59.5% This patch provides substantial improvements in these larger cases, and have any drawbacks for smaller ones (the cost of the bitmap check is quite small compared to an actual tree traversal). Note that we have to add a version of revision.c's include_check callback which handles non-commits. We could possibly consolidate this into a single callback for all objects types, as there's only one user of the feature which would need converted (pack-bitmap.c:should_include). That would in theory let us avoid duplicating any logic. But when I tried it, the code ended up much worse to read, with lots of repeated "if it's a commit do this, otherwise do that". Having two separate callbacks splits that naturally, and matches the existing split of show_commit/show_object callbacks. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>