summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2020-02-14Merge branch 'rt/submodule-i18n' into maintLibravatar Junio C Hamano1-14/+14
Comments update. * rt/submodule-i18n: submodule.c: mark more strings for translation
2020-02-14Merge branch 'jk/test-fixes' into maintLibravatar Junio C Hamano2-4/+2
Test fixes. * jk/test-fixes: t7800: don't rely on reuse_worktree_file() t4018: drop "debugging" cat from hunk-header tests
2020-02-14Merge branch 'jk/asan-build-fix' into maintLibravatar Junio C Hamano2-0/+8
Work around test breakages caused by custom regex engine used in libasan, when address sanitizer is used with more recent versions of gcc and clang. * jk/asan-build-fix: Makefile: use compat regex with SANITIZE=address
2020-02-14Merge branch 'ds/sparse-cone' into maintLibravatar Junio C Hamano2-2/+3
The code recently added in this release to move to the entry beyond the ones in the same directory in the index in the sparse-cone mode did not count the number of entries to skip over incorrectly, which has been corrected. * ds/sparse-cone: .mailmap: fix GGG authoship screwup unpack-trees: correctly compute result count
2020-02-14Merge branch 'nd/switch-and-restore' into maintLibravatar Junio C Hamano2-0/+19
"git restore --staged" did not correctly update the cache-tree structure, resulting in bogus trees to be written afterwards, which has been corrected. * nd/switch-and-restore: restore: invalidate cache-tree when removing entries with --staged
2020-02-14Merge branch 'jk/no-flush-upon-disconnecting-slrpc-transport' into maintLibravatar Junio C Hamano2-1/+13
Reduce unnecessary round-trip when running "ls-remote" over the stateless RPC mechanism. * jk/no-flush-upon-disconnecting-slrpc-transport: transport: don't flush when disconnecting stateless-rpc helper
2020-02-14Merge branch 'hw/tutorial-favor-switch-over-checkout' into maintLibravatar Junio C Hamano1-1/+1
Complete an update to tutorial that encourages "git switch" over "git checkout" that was done only half-way. * hw/tutorial-favor-switch-over-checkout: doc/gitcore-tutorial: fix prose to match example command
2020-02-14Merge branch 'es/unpack-trees-oob-fix' into maintLibravatar Junio C Hamano1-2/+4
The code that tries to skip over the entries for the paths in a single directory using the cache-tree was not careful enough against corrupt index file. * es/unpack-trees-oob-fix: unpack-trees: watch for out-of-range index position
2020-02-14Merge branch 'bc/run-command-nullness-after-free-fix' into maintLibravatar Junio C Hamano1-1/+2
C pedantry ;-) fix. * bc/run-command-nullness-after-free-fix: run-command: avoid undefined behavior in exists_in_PATH
2020-02-14Merge branch 'en/string-list-can-be-custom-sorted' into maintLibravatar Junio C Hamano1-2/+4
API-doc update. * en/string-list-can-be-custom-sorted: string-list: note in docs that callers can specify sorting function
2020-02-14Merge branch 'jt/sha1-file-remove-oi-skip-cached' into maintLibravatar Junio C Hamano2-22/+18
has_object_file() said "no" given an object registered to the system via pretend_object_file(), making it inconsistent with read_object_file(), causing lazy fetch to attempt fetching an empty tree from promisor remotes. * jt/sha1-file-remove-oi-skip-cached: sha1-file: remove OBJECT_INFO_SKIP_CACHED
2020-02-14Merge branch 'hw/commit-advise-while-rejecting' into maintLibravatar Junio C Hamano2-0/+10
"git commit" gives output similar to "git status" when there is nothing to commit, but without honoring the advise.statusHints configuration variable, which has been corrected. * hw/commit-advise-while-rejecting: commit: honor advice.statusHints when rejecting an empty commit
2020-01-30.mailmap: map Yi-Jyun Pan's emailLibravatar Denton Liu1-0/+1
In 13185fd241 (l10n: zh_TW.po: update translation for v2.25.0 round 1, 2019-12-31), the author mistakenly used their GitHub username for authorship information instead of their real name. However, a commit with their real name exists prior to this: 9917eca794 (l10n: zh_TW: add translation for v2.24.0, 2019-11-20). Map their email to their real name so that these contributions can be counted together. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-27.mailmap: fix GGG authoship screwupLibravatar Junio C Hamano1-0/+1
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-27.mailmap: fix erroneous authorship for Johannes SchindelinLibravatar Denton Liu1-0/+1
In 49e268e23e (mingw: safeguard better against backslashes in file names, 2020-01-09), the commit author is listed as "Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>", which is erroneous. Fix the authorship by mapping the erroneous authorship to his canonical authorship information. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16Sync with maintLibravatar Junio C Hamano2-4/+4
* maint: msvc: accommodate for vcpkg's upgrade to OpenSSL v1.1.x
2020-01-16t7800: don't rely on reuse_worktree_file()Libravatar Jeff King1-3/+2
A test in t7800 tries to make sure that when git-difftool runs an external tool that fails, it stops looking at files. Our fake failing tool prints the file name it was asked to diff before exiting non-zero, and then we confirm the output contains only that file. However, this subtly relies on our internal reuse_worktree_file(). Because we're diffing between branches, the command run by difftool might see: - the git-stored filename (e.g., "file"), if we decided that the working tree contents were up-to-date with the object in the index and HEAD, and we could reuse them - a temporary filename (e.g. "/tmp/abc123_file") if we had to dump the contents from the object database If the latter case happens, then the test fails, because it's expecting the string "file". I discovered this when debugging something unrelated with reuse_worktree_file(). I _thought_ it should be able to be triggered by a racy-git situation, but running: ./t7800-difftool.sh --stress --run=2,13 never seems to fail. However, by my reading of reuse_worktree_file(), this would probably always fail under Cygwin, because it sets NO_FAST_WORKING_DIRECTORY. At any rate, since reuse_worktree_file() is meant to be an optimization that may or may not trigger, our test should be robust either way. Instead of checking the filename, let's just make sure we got a single line of output (which would not be true if we continued after the first failure). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16t4018: drop "debugging" cat from hunk-header testsLibravatar Jeff King1-1/+0
We run a series of hunk-header tests in a loop, and each one does this: test_when_finished 'cat actual' && # for debugging only This is pretty pointless. When the test succeeds, we waste time running a useless cat process. If you're debugging a failure with "-i", then we won't run the when-finished part at all. So it helps only if you're running with something like "--verbose-log". Since we expect the tests to succeed most of the time, a better way to do this would be a helper that checks the output and dumps "actual" only when it fails. But it's probably not even worth the effort, as anyone debugging a failure could just run with "-i" and investigate the "actual" file themselves. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16Makefile: use compat regex with SANITIZE=addressLibravatar Jeff King2-0/+8
Recent versions of the gcc and clang Address Sanitizer produce test failures related to regexec(). This triggers with gcc-10 and clang-8 (but not gcc-9 nor clang-7). Running: make CC=gcc-10 SANITIZE=address test results in failures in t4018, t3206, and t4062. The cause seems to be that when built with ASan, we use a different version of regexec() than normal. And this version doesn't understand the REG_STARTEND flag. Here's my evidence supporting that. The failure in t4062 is an ASan warning: expecting success of 4062.2 '-G matches': git diff --name-only -G "^(0{64}){64}$" HEAD^ >out && test 4096-zeroes.txt = "$(cat out)" ================================================================= ==672994==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7fa76f672000 at pc 0x7fa7726f75b6 bp 0x7ffe41bdda70 sp 0x7ffe41bdd220 READ of size 4097 at 0x7fa76f672000 thread T0 #0 0x7fa7726f75b5 (/lib/x86_64-linux-gnu/libasan.so.6+0x4f5b5) #1 0x562ae0c9c40e in regexec_buf /home/peff/compile/git/git-compat-util.h:1117 #2 0x562ae0c9c40e in diff_grep /home/peff/compile/git/diffcore-pickaxe.c:52 #3 0x562ae0c9cc28 in pickaxe_match /home/peff/compile/git/diffcore-pickaxe.c:166 [...] In this case we're looking in a buffer which was mmap'd via reuse_worktree_file(), and whose size is 4096 bytes. But libasan's regex tries to look at byte 4097 anyway! If we tweak Git like this: diff --git a/diff.c b/diff.c index 8e2914c031..cfae60c120 100644 --- a/diff.c +++ b/diff.c @@ -3880,7 +3880,7 @@ static int reuse_worktree_file(struct index_state *istate, */ if (ce_uptodate(ce) || (!lstat(name, &st) && !ie_match_stat(istate, ce, &st, 0))) - return 1; + return 0; return 0; } to use a regular buffer (with a trailing NUL) instead of an mmap, then the complaint goes away. The other failures are actually diff output with an incorrect funcname header. If I instrument xdiff to show the funcname matching like so: diff --git a/xdiff-interface.c b/xdiff-interface.c index 8509f9ea22..f6c3dc1986 100644 --- a/xdiff-interface.c +++ b/xdiff-interface.c @@ -197,6 +197,7 @@ struct ff_regs { struct ff_reg { regex_t re; int negate; + char *printable; } *array; }; @@ -218,7 +219,12 @@ static long ff_regexp(const char *line, long len, for (i = 0; i < regs->nr; i++) { struct ff_reg *reg = regs->array + i; - if (!regexec_buf(&reg->re, line, len, 2, pmatch, 0)) { + int ret = regexec_buf(&reg->re, line, len, 2, pmatch, 0); + warning("regexec %s:\n regex: %s\n buf: %.*s", + ret == 0 ? "matched" : "did not match", + reg->printable, + (int)len, line); + if (!ret) { if (reg->negate) return -1; break; @@ -264,6 +270,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags) expression = value; if (regcomp(&reg->re, expression, cflags)) die("Invalid regexp to look for hunk header: %s", expression); + reg->printable = xstrdup(expression); free(buffer); value = ep + 1; } then when compiling with ASan and gcc-10, running the diff from t4018.66 produces this: $ git diff -U1 cpp-skip-access-specifiers warning: regexec did not match: regex: ^[ ]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*]) buf: private: warning: regexec matched: regex: ^((::[[:space:]]*)?[A-Za-z_].*)$ buf: private: diff --git a/cpp-skip-access-specifiers b/cpp-skip-access-specifiers index 4d4a9db..ebd6f42 100644 --- a/cpp-skip-access-specifiers +++ b/cpp-skip-access-specifiers @@ -6,3 +6,3 @@ private: void DoSomething(); int ChangeMe; }; void DoSomething(); - int ChangeMe; + int IWasChanged; }; That first regex should match (and is negated, so it should be telling us _not_ to match "private:"). But it wouldn't if regexec() is looking at the whole buffer, and not just the length-limited line we've fed to regexec_buf(). So this is consistent again with REG_STARTEND being ignored. The correct output (compiling without ASan, or gcc-9 with Asan) looks like this: warning: regexec matched: regex: ^[ ]*[A-Za-z_][A-Za-z_0-9]*:[[:space:]]*($|/[/*]) buf: private: [...more lines that we end up not using...] warning: regexec matched: regex: ^((::[[:space:]]*)?[A-Za-z_].*)$ buf: class RIGHT : public Baseclass diff --git a/cpp-skip-access-specifiers b/cpp-skip-access-specifiers index 4d4a9db..ebd6f42 100644 --- a/cpp-skip-access-specifiers +++ b/cpp-skip-access-specifiers @@ -6,3 +6,3 @@ class RIGHT : public Baseclass void DoSomething(); - int ChangeMe; + int IWasChanged; }; So it really does seem like libasan's regex engine is ignoring REG_STARTEND. We should be able to work around it by compiling with NO_REGEX, which would use our local regexec(). But to make matters even more interesting, this isn't enough by itself. Because ASan has support from the compiler, it doesn't seem to intercept our call to regexec() at the dynamic library level. It actually recognizes when we are compiling a call to regexec() and replaces it with ASan-specific code at that point. And unlike most of our other compat code, where we might have git_mmap() or similar, the actual symbol name in the compiled compat/regex code is regexec(). So just compiling with NO_REGEX isn't enough; we still end up in libasan! We can work around that by having the preprocessor replace regexec with git_regexec (both in the callers and in the actual implementation), and we truly end up with a call to our custom regex code, even when compiling with ASan. That's probably a good thing to do anyway, as it means anybody looking at the symbols later (e.g., in a debugger) would have a better indication of which function is which. So we'll do the same for the other common regex functions (even though just regexec() is enough to fix this ASan problem). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16submodule.c: mark more strings for translationLibravatar Ralf Thielow1-14/+14
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-16msvc: accommodate for vcpkg's upgrade to OpenSSL v1.1.xLibravatar Johannes Schindelin2-4/+4
With the upgrade, the library names changed from libeay32/ssleay32 to libcrypto/libssl. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-13Git 2.25Libravatar Junio C Hamano1-1/+1
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-12Merge tag 'l10n-2.25.0-rnd1' of git://github.com/git-l10n/git-poLibravatar Junio C Hamano12-24312/+53740
l10n-2.25.0-rnd1 * tag 'l10n-2.25.0-rnd1' of git://github.com/git-l10n/git-po: l10n: zh_CN: for git v2.25.0 l10n round 1 l10n: Update Catalan translation l10n: de.po: Update German translation v2.25.0 round 1 l10n: de.po: Reword generation numbers l10n: bg.po: Updated Bulgarian translation (4800t) l10n: es: 2.25.0 round #1 l10n: sv.po: Update Swedish translation (4800t0f0u) l10n: fr.po v2.25.0 rnd 1 l10n: vi(4800t): Updated Vietnamese translation v2.25.0 l10n: zh_TW.po: update translation for v2.25.0 round 1 l10n: it.po: update the Italian translation for Git 2.25.0 l10n: git.pot: v2.25.0 round 1 (119 new, 13 removed) l10n: Update Catalan translation l10n: zh_TW: add translation for v2.24.0
2020-01-12Revert "Merge branch 'ra/rebase-i-more-options'"Libravatar Junio C Hamano7-327/+28
This reverts commit 5d9324e0f4210bb7d52bcb79efe3935703083f72, reversing changes made to c58ae96fc4bb11916b62a96940bb70bb85ea5992. The topic turns out to be too buggy for real use. cf. <f2fe7437-8a48-3315-4d3f-8d51fe4bb8f1@gmail.com>
2020-01-12l10n: zh_CN: for git v2.25.0 l10n round 1Libravatar Jiang Xin1-2275/+2807
Translate 119 new messages (4800t0f0u) for git 2.25.0. Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
2020-01-11Merge branch 'master' of github.com:Softcatala/git-po into git-po-masterLibravatar Jiang Xin1-3757/+4260
* 'master' of github.com:Softcatala/git-po: l10n: Update Catalan translation
2020-01-10Merge branch 'js/mingw-loosen-overstrict-tree-entry-checks'Libravatar Junio C Hamano2-7/+7
Further tweak to a "no backslash in indexed paths" for Windows port we applied earlier. * js/mingw-loosen-overstrict-tree-entry-checks: mingw: safeguard better against backslashes in file names
2020-01-10Merge branch 'ma/config-advice-markup-fix'Libravatar Junio C Hamano1-1/+1
Documentation markup fix. * ma/config-advice-markup-fix: config/advice.txt: fix description list separator
2020-01-10l10n: Update Catalan translationLibravatar Jordi Mas1-3757/+4260
Signed-off-by: Jordi Mas <jmas@softcatala.org>
2020-01-10mingw: safeguard better against backslashes in file namesLibravatar Johannes Schindelin via GitGitGadget2-7/+7
In 224c7d70fa1 (mingw: only test index entries for backslashes, not tree entries, 2019-12-31), we relaxed the check for backslashes in tree entries to check only index entries. However, the code change was incorrect: it was added to `add_index_entry_with_check()`, not to `add_index_entry()`, so under certain circumstances it was possible to side-step the protection. Besides, the description of that commit purported that all index entries would be checked when in fact they were only checked when being added to the index (there are code paths that do not do that, constructing "transient" index entries). In any case, it was pointed out in one insightful review at https://github.com/git-for-windows/git/pull/2437#issuecomment-566771835 that it would be a much better idea to teach `verify_path()` to perform the check for a backslash. This is safer, even if it comes with two notable drawbacks: - `verify_path()` cannot say _what_ is wrong with the path, therefore the user will no longer be told that there was a backslash in the path, only that the path was invalid. - The `git apply` command also calls the `verify_path()` function, and might have been able to handle Windows-style paths (i.e. with backslashes instead of forward slashes). This will no longer be possible unless the user (temporarily) sets `core.protectNTFS=false`. Note that `git add <windows-path>` will _still_ work because `normalize_path_copy_len()` will convert the backslashes to forward slashes before hitting the code path that creates an index entry. The clear advantage is that `verify_path()`'s purpose is to check the validity of the file name, therefore we naturally tap into all the code paths that need safeguarding, also implicitly into future code paths. The benefits of that approach outweigh the downsides, so let's move the check from `add_index_entry_with_check()` to `verify_path()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-10unpack-trees: correctly compute result countLibravatar Derrick Stolee via GitGitGadget1-2/+2
The clear_ce_flags_dir() method processes the cache entries within a common directory. The returned int is the number of cache entries processed by that directory. When using the sparse-checkout feature in cone mode, we can skip the pattern matching for entries in the directories that are entirely included or entirely excluded. eb42feca (unpack-trees: hash less in cone mode, 2019-11-21) introduced this performance feature. The old mechanism relied on the counts returned by calling clear_ce_flags_1(), but the new mechanism calculated the number of rows by subtracting "cache_end" from "cache" to find the size of the range. However, the equation is wrong because it divides by sizeof(struct cache_entry *). This is not how pointer arithmetic works! A coverity build of Git for Windows in preparation for the 2.25.0 release found this issue with the warning, "Pointer differences, such as cache_end - cache, are automatically scaled down by the size (8 bytes) of the pointed-to type (struct cache_entry *). Most likely, the division by sizeof(struct cache_entry *) is extraneous and should be eliminated." This warning is correct. This leaves us with the question "how did this even work?" The problem that occurs with this incorrect pointer arithmetic is a performance-only bug, and a very slight one at that. Since the entry count returned by clear_ce_flags_dir() is reduced by a factor of 8, the loop in clear_ce_flags_1() will re-process entries from those directories. By inserting global counters into unpack-tree.c and tracing them with trace2_data_intmax() (in a private change, for testing), I was able to see count how many times the loop inside clear_ce_flags_1() processed an entry and how many times clear_ce_flags_dir() was called. Each of these are reduced by at least a factor of 8 with the current change. A factor larger than 8 happens when multiple levels of directories are repeated. Specifically, in the Linux kernel repo, the command git sparse-checkout set LICENSES restricts the working directory to only the files at root and in the LICENSES directory. Here are the measured counts: clear_ce_flags_1 loop blocks: Before: 11,520 After: 1,621 clear_ce_flags_dir calls: Before: 7,048 After: 606 While these are dramatic counts, the time spent in clear_ce_flags_1() is under one millisecond in each case, so the improvement is not measurable as an end-to-end time. Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-10l10n: de.po: Update German translation v2.25.0 round 1Libravatar Matthias Rüster1-2282/+2844
Signed-off-by: Matthias Rüster <matthias.ruester@gmail.com> Reviewed-by: Ralf Thielow <ralf.thielow@gmail.com> Reviewed-by: Phillip Szelat <phillip.szelat@gmail.com>
2020-01-10l10n: de.po: Reword generation numbersLibravatar Thomas Braun1-1/+1
The english term generation is here not used in the sense of "to generate" but in the sense of "generations of beings". This corrects the initial translation from cf4c0c25 (l10n: update German translation, 2018-12-06). Fixed-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
2020-01-09l10n: bg.po: Updated Bulgarian translation (4800t)Libravatar Alexander Shopov1-2353/+2905
Signed-off-by: Alexander Shopov <ash@kambanaria.org>
2020-01-08config/advice.txt: fix description list separatorLibravatar Martin Ågren1-1/+1
The whole submoduleAlternateErrorStrategyDie item is interpreted as being part of the supporting content of the preceding item. This is because we don't give a double-colon "::" for the separator, but just a single colon, ":". Let's fix that. There are a few other matches for [^:]:\s*$ in Documentation/config, but I didn't spot any similar bugs among them. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08Git 2.25-rc2Libravatar Junio C Hamano2-1/+5
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08Merge branch 'ds/graph-assert-fix'Libravatar Junio C Hamano2-8/+80
Since recent updates to the log graph rendering code, drawing certain merges started triggering an assert on a condition that would no longer hold true, which has been corrected. * ds/graph-assert-fix: graph: fix lack of color in horizontal lines graph: drop assert() for merge with two collapsing parents
2020-01-08Merge branch 'tm/doc-submodule-absorb-fix'Libravatar Junio C Hamano1-1/+1
Typofix. * tm/doc-submodule-absorb-fix: doc: submodule: fix typo for command absorbgitdirs
2020-01-08Merge branch 'pm/am-in-body-header-doc-update'Libravatar Junio C Hamano1-2/+2
Doc update. * pm/am-in-body-header-doc-update: am: document that Date: can appear as an in-body header
2020-01-08Merge branch 'jb/doc-multi-pack-idx-fix'Libravatar Junio C Hamano1-1/+1
Typofix. * jb/doc-multi-pack-idx-fix: multi-pack-index: correct configuration in documentation
2020-01-08Merge branch 'do/gitweb-typofix-in-comments'Libravatar Junio C Hamano1-2/+2
Typofix. * do/gitweb-typofix-in-comments: gitweb: fix a couple spelling errors in comments
2020-01-08Merge https://github.com/prati0100/git-guiLibravatar Junio C Hamano10-232/+920
* https://github.com/prati0100/git-gui: git-gui: allow opening currently selected file in default app git-gui: allow closing console window with Escape git gui: fix branch name encoding error git-gui: revert untracked files by deleting them git-gui: update status bar to track operations git-gui: consolidate naming conventions
2020-01-08graph: fix lack of color in horizontal linesLibravatar Derrick Stolee2-4/+38
In some cases, horizontal lines in rendered graphs can lose their coloring. This is due to a use of graph_line_addch() instead of graph_line_write_column(). Using a ternary operator to pick the character is nice for compact code, but we actually need a column to provide the color. Add a test to t4215-log-skewed-merges.sh to prevent regression. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08graph: drop assert() for merge with two collapsing parentsLibravatar Derrick Stolee2-4/+42
When "git log --graph" shows a merge commit that has two collapsing lines, like: | | | | * | |_|_|/| |/| | |/ | | |/| | |/| | | * | | * | | | we trigger an assert(): graph.c:1228: graph_output_collapsing_line: Assertion `graph->mapping[i - 3] == target' failed. The assert was introduced by eaf158f8 ("graph API: Use horizontal lines for more compact graphs", 2009-04-21), which is quite old. This assert is trying to say that when we complete a horizontal line with a single slash, it is because we have reached our target. It is actually the _second_ collapsing line that hits this assert. The reason we are in this code path is because we are collapsing the first line, and in that case we are hitting our target now that the horizontal line is complete. However, the second line cannot be a horizontal line, so it will collapse without horizontal lines. In this case, it is inappropriate to assert that we have reached our target, as we need to continue for another column before reaching the target. Dropping the assert is safe here. The new behavior in 0f0f389f12 (graph: tidy up display of left-skewed merges, 2019-10-15) caused the behavior change that made this assertion failure possible. In addition to making the assert possible, it also changed how multiple edges collapse. In a larger example, the current code will output a collapse as follows: | | | | | | * | |_|_|_|_|/|\ |/| | | | |/ / | | | | |/| / | | | |/| |/ | | |/| |/| | |/| |/| | | | |/| | | | | * | | | However, the intended collapse should allow multiple horizontal lines as follows: | | | | | | * | |_|_|_|_|/|\ |/| | | | |/ / | | |_|_|/| / | |/| | | |/ | | | |_|/| | | |/| | | | | * | | | This behavior is not corrected by this change, but is noted for a later update. Helped-by: Jeff King <peff@peff.net> Reported-by: Bradley Smith <brad@brad-smith.co.uk> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08transport: don't flush when disconnecting stateless-rpc helperLibravatar Jeff King2-1/+13
Since ba227857d2 (Reduce the number of connects when fetching, 2008-02-04), when we disconnect a git transport, we send a final flush packet. This cleanly tells the other side that we're done, and avoids the other side complaining "the remote end hung up unexpectedly" (though we'd only see that for transports that pass along the server stderr, like ssh or local-host). But when we've initiated a v2 stateless-connect session over a transport helper, there's no point in sending this flush packet. Each operation we've performed is self-contained, and the other side is fine with us hanging up between operations. But much worse, by sending the flush packet we may cause the helper to issue an entirely new request _just_ to send the flush packet. So we can incur an extra network request just to say "by the way, we have nothing more to send". Let's drop this extra flush packet. As the test shows, this reduces the number of POSTs required for a v2 ls-remote over http from 2 to 1. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08unpack-trees: watch for out-of-range index positionLibravatar Emily Shaffer1-2/+4
It's possible in a case where the index file contains a tree extension but no blobs within that tree exist for index_pos_by_traverse_info() to segfault. If the name_entry passed into index_pos_by_traverse_info() has no blobs inside, AND is alphabetically later than all blobs currently in the index file, index_pos_by_traverse_info() will segfault. For example, an index file which looks something like this: aaa#0 bbb/aaa#0 [Extensions] TREE: zzz In this example, 'index_name_pos(..., "zzz/", ...)' will return '-4', indicating that "zzz/" could be inserted at position 3. However, when the checks which ensure that the insertion position of "zzz/" look for a blob at that position beginning with "zzz/", the index cache is accessed out of range, causing a segfault. This kind of index state is not typically generated during user operations, and is in fact an edge case of the state being checked for in the conditional where it was added. However, since the entry for the BUG() line is ambiguous, tell some additional context to help Git developers debug the failure later. When we know the name of the dir we were trying to look up, it becomes possible to examine the index file in a hex util to determine what went wrong; the position gives a hint about where to start looking. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08restore: invalidate cache-tree when removing entries with --stagedLibravatar Jeff King2-0/+19
When "git restore --staged <path>" removes a path that's in the index, it marks the entry with CE_REMOVE, but we don't do anything to invalidate the cache-tree. In the non-staged case, we end up in checkout_worktree(), which calls remove_marked_cache_entries(). That actually drops the entries from the index, as well as invalidating the cache-tree and untracked-cache. But with --staged, we never call checkout_worktree(), and the CE_REMOVE entries remain. Interestingly, they are dropped when we write out the index, but that means the resulting index is inconsistent: its cache-tree will not match the actual entries, and running "git commit" immediately after will create the wrong tree. We can solve this by calling remove_marked_cache_entries() ourselves before writing out the index. Note that we can't just hoist it out of checkout_worktree(); that function needs to iterate over the CE_REMOVE entries (to drop their matching worktree files) before removing them. One curiosity about the test: without this patch, it actually triggers a BUG() when running git-restore: BUG: cache-tree.c:810: new1 with flags 0x4420000 should not be in cache-tree But in the original problem report, which used a similar recipe, git-restore actually creates the bogus index (and the commit is created with the wrong tree). I'm not sure why the test here behaves differently than my out-of-suite reproduction, but what's here should catch either symptom (and the fix corrects both cases). Reported-by: Torsten Krah <krah.tm@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-08doc/gitcore-tutorial: fix prose to match example commandLibravatar Heba Waly1-1/+1
In 328c6cb853 (doc: promote "git switch", 2019-03-29), an example was changed to use "git switch" rather than "git checkout" but an instance of "git checkout" in the explanatory text preceding the example was overlooked. Fix this oversight. Signed-off-by: Heba Waly <heba.waly@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-07run-command: avoid undefined behavior in exists_in_PATHLibravatar brian m. carlson1-1/+2
In this function, we free the pointer we get from locate_in_PATH and then check whether it's NULL. However, this is undefined behavior if the pointer is non-NULL, since the C standard no longer permits us to use a valid pointer after freeing it. The only case in which the C standard would permit this to be defined behavior is if r were NULL, since it states that in such a case "no action occurs" as a result of calling free. It's easy to suggest that this is not likely to be a problem, but we know that GCC does aggressively exploit the fact that undefined behavior can never occur to optimize and rewrite code, even when that's contrary to the expectations of the programmer. It is, in fact, very common for it to omit NULL pointer checks, just as we have here. Since it's easy to fix, let's do so, and avoid a potential headache in the future. Noticed-by: Miriam R. <mirucam@gmail.com> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-07string-list: note in docs that callers can specify sorting functionLibravatar Elijah Newren1-2/+4
In commit 1959bf6430 (string_list API: document what "sorted" means, 2012-09-17), Documentation/technical/api-string-list.txt was updated to specify that strcmp() was used for sorting. In commit 8dd5afc926 (string-list: allow case-insensitive string list, 2013-01-07), a cmp member was added to struct string_list to allow callers to specify an alternative comparison function, but api-string-list.txt was not updated. In commit 4f665f2cf3 (string-list.h: move documentation from Documentation/api/ into header, 2017-09-26), the now out-dated api-string-list.txt documentation was moved into string-list.h. Update the docs to reflect the configurability of sorting. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>