Skip to main content

Browse the archive

swh:1:snp:47f1e8bb459169b0feb652a9c3d9cbabd8526d4a
sort by:
Revision Author Date Message Commit Date
c17cf77 Junio C Hamano 14 February 2020, 20:42:27 UTC Merge branch 'bc/run-command-nullness-after-free-fix' into maint C pedantry ;-) fix. * bc/run-command-nullness-after-free-fix: run-command: avoid undefined behavior in exists_in_PATH 14 February 2020, 20:42:27 UTC
d0ebd64 Junio C Hamano 14 February 2020, 20:42:27 UTC Merge branch 'en/string-list-can-be-custom-sorted' into maint API-doc update. * en/string-list-can-be-custom-sorted: string-list: note in docs that callers can specify sorting function 14 February 2020, 20:42:27 UTC
9eddeae Junio C Hamano 14 February 2020, 20:42:27 UTC Merge branch 'jt/sha1-file-remove-oi-skip-cached' into maint 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 14 February 2020, 20:42:27 UTC
3bba763 Junio C Hamano 14 February 2020, 20:42:27 UTC Merge branch 'hw/commit-advise-while-rejecting' into maint "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 14 February 2020, 20:42:27 UTC
08809c0 Johannes Schindelin 13 February 2020, 21:49:53 UTC mingw: add a helper function to attach GDB to the current process When debugging Git, the criss-cross spawning of processes can make things quite a bit difficult, especially when a Unix shell script is thrown in the mix that calls a `git.exe` that then segfaults. To help debugging such things, we introduce the `open_in_gdb()` function which can be called at a code location where the segfault happens (or as close as one can get); This will open a new MinTTY window with a GDB that already attached to the current process. Inspired by Derrick Stolee. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 14 February 2020, 18:02:07 UTC
bfe2bbb Johannes Schindelin 13 February 2020, 21:51:44 UTC t5580: test cloning without file://, test fetching via UNC paths On Windows, it is quite common to work with network drives. The format of the paths to network drives (or "network shares", or UNC paths) is: \\<server>\<share>\... We already have a couple regression tests revolving around those types of paths, but we missed cloning and fetching from UNC paths without leading `file://` (and with backslashes instead of forward slashes). This lil' patch closes that gap. It gets a bit silly to add the commands to the name of the test script, so let's just rename it while we're testing more UNC stuff. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 14 February 2020, 17:59:41 UTC
076ee3e Johannes Schindelin 12 February 2020, 11:27:55 UTC tests: fix --write-junit-xml with subshells In t0000, more precisely in its `test_bool_env` test case, there are two subshells that are supposed to fail. To be even _more_ precise, they fail by calling the `error` function, and that is okay, because it is in a subshell, and it is expected that those two subshell invocations fail. However, the `error` function also tries to finalize the JUnit XML (if that XML was asked for, via `--write-junit-xml`. As a consequence, the XML is edited to add a `time` attribute for the `testsuite` tag. And since there are two expected `error` calls in addition to the final `test_done`, the `finalize_junit_xml` function is called three times and naturally the `time` attribute is added _three times_. Azure Pipelines is not happy with that, complaining thusly: ##[warning]Failed to read D:\a\1\s\t\out\TEST-t0000-basic.xml. Error : 'time' is a duplicate attribute name. Line 2, position 82.. One possible way to address this would be to unset `write_junit_xml` in the `test_bool_env` test case. But that would be fragile, as other `error` calls in subshells could be introduced. So let's just modify `finalize_junit_xml` to remove any `time` attribute before adding the authoritative one. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 February 2020, 17:19:42 UTC
2b0f19f Johannes Schindelin 11 February 2020, 18:56:18 UTC convert: fix typo Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 11 February 2020, 20:03:09 UTC
3018389 Johannes Schindelin 09 February 2020, 22:36:16 UTC ci: ignore rubygems warning in the "Documentation" job A recent update in the Linux VM images used by Azure Pipelines surfaced a new problem in the "Documentation" job. Apparently, this warning appears 396 times on `stderr` when running `make doc`: /usr/lib/ruby/vendor_ruby/rubygems/defaults/operating_system.rb:10: warning: constant Gem::ConfigMap is deprecated This problem was already reported to the `rubygems` project via https://github.com/rubygems/rubygems/issues/3068. As there is nothing Git can do about this warning, and as the "Documentation" job reports this warning as a failure, let's just silence it and move on. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 10 February 2020, 17:52:27 UTC
eb31044 Johannes Berg 07 February 2020, 22:16:40 UTC pack-format: correct multi-pack-index description The description of the multi-pack-index contains a small bug, if all offsets are < 2^32 then there will be no LOFF chunk, not only if they're all < 2^31 (since the highest bit is only needed as the "LOFF-escape" when that's actually needed.) Correct this, and clarify that in that case only offsets up to 2^31-1 can be stored in the OOFF chunk. Signed-off-by: Johannes Berg <johannes@sipsolutions.net> Acked-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 10 February 2020, 17:01:48 UTC
9299f84 Martin Ågren 06 February 2020, 20:53:01 UTC diff-options.txt: avoid "regex" overload in example When we exemplify the difference between `-G` and `-S` (using `--pickaxe-regex`), we do so using an example diff and git-diff invocation involving "regexec", "regexp", "regmatch", ... The example is correct, but we can make it easier to untangle by avoiding writing "regex.*" unless it's really needed to make our point. Use some made-up, non-regexy words instead. Reported-by: Adam Dinwoodie <adam@dinwoodie.org> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 09 February 2020, 17:25:24 UTC
7378ec9 Masaya Suzuki 07 February 2020, 20:42:25 UTC doc: describe Git bundle format The bundle format was not documented. Describe the format with ABNF and explain the meaning of each part. Signed-off-by: Masaya Suzuki <masayasuzuki@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 February 2020, 20:47:02 UTC
303b3c1 Emily Shaffer 07 February 2020, 00:48:33 UTC submodule: add newline on invalid submodule error Since 'err' contains output for multiple submodules and is printed all at once by fetch_populated_submodules(), errors for each submodule should be newline separated for readability. The same strbuf is added to with a newline in the other half of the conditional where this error is detected, so make the two consistent. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 February 2020, 19:05:45 UTC
395518c Jacques Bodin-Hullin 05 February 2020, 13:07:23 UTC parse-options: lose an unnecessary space in an error message Signed-off-by: Jacques Bodin-Hullin <j.bodinhullin@monsieurbiz.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 05 February 2020, 18:49:29 UTC
a217810 Jeff King 03 February 2020, 14:40:55 UTC index-pack: downgrade twice-resolved REF_DELTA to die() When we're resolving a REF_DELTA, we compare-and-swap its type from REF_DELTA to whatever real type the base object has, as discussed in ab791dd138 (index-pack: fix race condition with duplicate bases, 2014-08-29). If the old type wasn't a REF_DELTA, we consider that a BUG(). But as discussed in that commit, we might see this case whenever we try to resolve an object twice, which may happen because we have multiple copies of the base object. So this isn't a bug at all, but rather a sign that the input pack is broken. And indeed, this case is triggered already in t5309.5 and t5309.6, which create packs with delta cycles and duplicate bases. But we never noticed because those tests are marked expect_failure. Those tests were added by b2ef3d9ebb (test index-pack on packs with recoverable delta cycles, 2013-08-23), which was leaving the door open for cases that we theoretically _could_ handle. And when we see an already-resolved object like this, in theory we could keep going after confirming that the previously resolved child->real_type matches base->obj->real_type. But: - enforcing the "only resolve once" rule here saves us from an infinite loop in other parts of the code. If we keep going, then the delta cycle in t5309.5 causes us to loop infinitely, as find_ref_delta_children() doesn't realize which objects have already been resolved. So there would be more changes needed to make this case work, and in the meantime we'd be worse off. - any pack that triggers this is broken anyway. It either has a duplicate base object, or it has a cycle which causes us to bring in a duplicate via --fix-thin. In either case, we'd end up rejecting the pack in write_idx_file(), which also detects duplicates. So the tests have little value in documenting what we _could_ be doing (and have been neglected for 6+ years). Let's switch them to confirming that we handle this case cleanly (and switch out the BUG() for a more informative die() so that we do so). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 February 2020, 21:19:11 UTC
dbc2747 Johan Herland 03 February 2020, 21:04:45 UTC notes.c: fix off-by-one error when decreasing notes fanout As noted in the previous commit, the nature of the fanout heuristic in the notes code causes the exact point at which we increase or decrease the notes fanout to vary with the objects being annotated. Since the object ids generated by the test environment are deterministic (by design), the notes generated and tested by t3305 are always the same, and we therefore happen to see the same fanout behavior from one run to the next. Coincidentally, if we were to change the test environment slightly (say by making a test commit on an unrelated branch before we start the t3305 test proper), we not only see the fanout switch happen at different points, we also manage to trigger a _bug_ in the notes code where the fanout 1 -> 0 switch is not applied uniformly across the notes tree, but instead yields a notes tree like this: ... bdeafb301e44b0e4db0f738a2d2a7beefdb70b70 bff2d39b4f7122bd4c5caee3de353a774d1e632a d3/8ec8f851adf470131178085bfbaab4b12ad2a7 e0b173960431a3e692ae929736df3c9b73a11d5b eb3c3aede523d729990ac25c62a93eb47c21e2e3 ... The bug occurs when we are writing out a notes tree with a newly decreased fanout, and the notes tree contains unexpanded subtrees that should be consolidated into the parent tree as a consequence of the decreased fanout): Subtrees that happen to sit at an _even_ level in the internal notes 16-tree structure (in other words: subtrees whose path - "d3" in the example above - is unique in the first nibble - i.e. there are no other note paths that start with "d") are _not_ unpacked as part of the tree writeout. This error will repeat itself in subsequent note trees until the subtree is forced to be unpacked. In t3305 this only happens when the d38ec8f8 note is itself removed from the tree. The error is not severe (no information is lost, and the notes code is able to read/decode this tree and manipulate it correctly), but this is nonetheless a bug in the current implementation that should be fixed. That said, fixing the off-by-one error is not without complications: We must take into account that the load_subtree() call from for_each_note_helper() (that is now done to correctly unpack the subtree while we're writing out the notes tree) may end up inserting unpacked non-notes into the linked list of non_note entries held by the struct notes_tree. Since we are in the process of writing out the notes tree, this linked list is currently in the process of being traversed by write_each_non_note_until(). The unpacked non-notes are necessarily inserted between the last non-note we wrote out, and the next non-note to be written. Hence, we cannot simply hold the next_non_note to write in struct write_each_note_data (as we would then silently skip these newly inserted notes), but must instead always follow the ->next pointer from the last non-note we wrote. (This part was caught by an existing test in t3304.) Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de> Cc: Brian M. Carlson <sandals@crustytoothpaste.net> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 February 2020, 20:20:43 UTC
e1c5253 Johan Herland 03 February 2020, 21:04:44 UTC t3305: check notes fanout more carefully and robustly In short, before this patch, this test script: - creates many notes - verifies that all notes in the notes tree has a fanout of 1 - removes most notes - verifies that the notes in the notes tree now has a fanout of 0 The fanout verification only happened twice: after creating all the notes, and after removing most of them. This patch strengthens the test by checking the fanout after _each_ added/removed note: We assert that the switch from fanout 0 -> 1 happens exactly once while adding notes (and that the switch pervades the entire notes tree). Likewise, we assert that the switch from fanout 1 -> 0 happens exactly once while removing notes. Additionally, we decrease the number of notes left after removal, from 50 to 15 notes, in order to ensure that fanout 1 -> 0 transition keeps happening regardless of external factors[1]. [1]: Currently (with the SHA1 hash function and the deterministic object ids of the test environment) the fanout heuristic in the notes code happens to switch from 0 -> 1 at 109 notes, and from 1 -> 0 at 59 notes. However, changing the hash function or other external factors will vary these numbers, and the latter may - in theory - go as low as 15. For more details, please see the discussion at https://public-inbox.org/git/20200125230035.136348-4-sandals@crustytoothpaste.net/ Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de> Cc: Brian M. Carlson <sandals@crustytoothpaste.net> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 February 2020, 20:20:27 UTC
e469afe Martin Ågren 03 February 2020, 20:36:50 UTC git-filter-branch.txt: wrap "maths" notation in backticks In this paragraph, we have a few instances of the '^' character, which we give as "\^". This renders well with AsciiDoc ("^"), but Asciidoctor renders it literally as "\^". Dropping the backslashes renders fine with Asciidoctor, but not AsciiDoc... An earlier version of this patch used "{caret}" instead of "^", which avoided these escaping problems. The rendering was still so-so, though -- these expressions end up set as normal text, similarly to when one provides, e.g., computer code in the middle of running text, without properly marking it with `backticks` to be monospaced. As noted by Jeff King, this suggests actually wrapping these expressions in backticks, setting them in monospace. The lone "5" could be left as is or wrapped as `5`. Spell it out as "five" instead -- this generally looks better anyway for small numbers in the middle of text like this. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 February 2020, 20:17:18 UTC
04e5b3f Kyle Meyer 31 January 2020, 17:08:43 UTC submodule foreach: replace $path with $sm_path in example f0fd0dc5c5 (submodule foreach: document '$sm_path' instead of '$path', 2018-05-08) updated the documentation to advise callers to favor $sm_path over the deprecated synonym $path. However, the example in that section still uses $path. Update it to use $sm_path. Signed-off-by: Kyle Meyer <kyle@kyleam.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 31 January 2020, 20:57:18 UTC
0ad7144 Denton Liu 14 January 2020, 03:23:58 UTC .mailmap: map Yi-Jyun Pan's email 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> 30 January 2020, 18:23:45 UTC
c56c48d Philippe Blain 30 January 2020, 13:37:28 UTC grep: ignore --recurse-submodules if --no-index is given Since grep learned to recurse into submodules in 0281e487fd (grep: optionally recurse into submodules, 2016-12-16), using --recurse-submodules along with --no-index makes Git die(). This is unfortunate because if submodule.recurse is set in a user's ~/.gitconfig, invoking `git grep --no-index` either inside or outside a Git repository results in fatal: option not supported with --recurse-submodules Let's allow using these options together, so that setting submodule.recurse globally does not prevent using `git grep --no-index`. Using `--recurse-submodules` should not have any effect if `--no-index` is used inside a repository, as Git will recurse into the checked out submodule directories just like into regular directories. Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 30 January 2020, 18:15:58 UTC
cf82bff Jeff King 25 January 2020, 05:44:29 UTC obstack: avoid computing offsets from NULL pointer As with the previous two commits, UBSan with clang-11 complains about computing offsets from a NULL pointer. The failures in t4013 (and elsewhere) look like this: kwset.c:102:23: runtime error: applying non-zero offset 107820859019600 to null pointer ... not ok 79 - git log -SF master # magic is (not used) That line is not enlightening: ... = obstack_alloc(&kwset->obstack, sizeof (struct trie)); because obstack is implemented almost entirely in macros, and the actual problem is five macros deep (I temporarily converted them to inline functions to get better compiler errors, which was tedious but worked reasonably well). The actual problem is in these pointer-alignment macros: /* If B is the base of an object addressed by P, return the result of aligning P to the next multiple of A + 1. B and P must be of type char *. A + 1 must be a power of 2. */ #define __BPTR_ALIGN(B, P, A) ((B) + (((P) - (B) + (A)) & ~(A))) /* Similar to _BPTR_ALIGN (B, P, A), except optimize the common case where pointers can be converted to integers, aligned as integers, and converted back again. If PTR_INT_TYPE is narrower than a pointer (e.g., the AS/400), play it safe and compute the alignment relative to B. Otherwise, use the faster strategy of computing the alignment relative to 0. */ #define __PTR_ALIGN(B, P, A) \ __BPTR_ALIGN (sizeof (PTR_INT_TYPE) < sizeof (void *) ? (B) : (char *) 0, \ P, A) If we have a sufficiently-large integer pointer type, then we do the computation using a NULL pointer constant. That turns __BPTR_ALIGN() into something like: NULL + (P - NULL + A) & ~A and UBSan is complaining about adding the full value of P to that initial NULL. We can fix this by doing our math as an integer type, and then casting the result back to a pointer. The problem case only happens when we know that the integer type is large enough, so there should be no issue with truncation. Another option would be just simplify out all the 0's from __BPTR_ALIGN() for the NULL-pointer case. That probably wouldn't work for a platform where the NULL pointer isn't all-zeroes, but Git already wouldn't work on such a platform (due to our use of memset to set pointers in structs to NULL). But I tried here to keep as close to the original as possible. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 29 January 2020, 07:13:25 UTC
3cd309c Jeff King 25 January 2020, 05:39:29 UTC xdiff: avoid computing non-zero offset from NULL pointer As with the previous commit, clang-11's UBSan complains about computing offsets from a NULL pointer, causing some tests to fail. In this case, though, we're actually computing a non-zero offset, which is even more dubious. From t7810: xdiff-interface.c:268:14: runtime error: applying non-zero offset 1 to null pointer ... not ok 131 - grep -p with userdiff The problem is our parsing of the funcname config. We count the number of lines in the string, allocate an array, and then loop over our allocated entries, parsing each line and moving our cursor to one past the trailing newline for the next iteration. But the final line will not generally have a trailing newline (since it's a config value), and hence we go to one past NULL. In practice this is OK, since our loop should terminate before we look at the value. But even computing such an invalid pointer technically violates the standard. We can fix it by leaving the pointer at NULL if we're at the end, rather than one-past. And while we're thinking about it, we can also document the variant by asserting that our initial line-count matches the second-pass of parsing. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 29 January 2020, 07:13:25 UTC
d20bc01 Jeff King 29 January 2020, 05:46:47 UTC avoid computing zero offsets from NULL pointer The Undefined Behavior Sanitizer in clang-11 seems to have learned a new trick: it complains about computing offsets from a NULL pointer, even if that offset is 0. This causes numerous test failures. For example, from t1090: unpack-trees.c:1355:41: runtime error: applying zero offset to null pointer ... not ok 6 - in partial clone, sparse checkout only fetches needed blobs The code in question looks like this: struct cache_entry **cache_end = cache + nr; ... while (cache != cache_end) and we sometimes pass in a NULL and 0 for "cache" and "nr". This is conceptually fine, as "cache_end" would be equal to "cache" in this case, and we wouldn't enter the loop at all. But computing even a zero offset violates the C standard. And given the fact that UBSan is noticing this behavior, this might be a potential problem spot if the compiler starts making unexpected assumptions based on undefined behavior. So let's just avoid it, which is pretty easy. In some cases we can just switch to iterating with a numeric index (as we do in sequencer.c here). In other cases (like the cache_end one) the use of an end pointer is more natural; we can keep that by just explicitly checking for the NULL/0 case when assigning the end pointer. Note that there are two ways you can write this latter case, checking for the pointer: cache_end = cache ? cache + nr : cache; or the size: cache_end = nr ? cache + nr : cache; For the case of a NULL/0 ptr/len combo, they are equivalent. But writing it the second way (as this patch does) has the property that if somebody were to incorrectly pass a NULL pointer with a non-zero length, we'd continue to notice and segfault, rather than silently pretending the length was zero. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 29 January 2020, 07:12:48 UTC
7edee32 David Turner 27 January 2020, 18:58:56 UTC git rm submodule: succeed if .gitmodules index stat info is zero The bug was that ie_match_stat() was used to compare if the stat info for the file was compatible with the stat info in the index, rather using ie_modified() to check if the file was in fact different from the version in the index. A version of this (with deinit instead of rm) was reported here: https://lore.kernel.org/git/CAPOqYV+C-P9M2zcUBBkD2LALPm4K3sxSut+BjAkZ9T1AKLEr+A@mail.gmail.com/ It seems that in that case, the user's clone command left the index with empty stat info. The mailing list was unable to reproduce this. But we (Two Sigma) hit the bug while using some plumbing commands, so I'm fixing it. I manually confirmed that the fix also repairs deinit in this scenario. Signed-off-by: David Turner <dturner@twosigma.com> Reported-by: Thomas Bétous <th.betous@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 28 January 2020, 22:46:14 UTC
7210ca4 Junio C Hamano 27 January 2020, 21:19:53 UTC .mailmap: fix GGG authoship screwup Signed-off-by: Junio C Hamano <gitster@pobox.com> 27 January 2020, 21:19:53 UTC
b54128b Jonathan Tan 13 January 2020, 20:28:23 UTC t5616: make robust to delta base change Commit 6462d5eb9a ("fetch: remove fetch_if_missing=0", 2019-11-08) contains a test that relies on having to lazily fetch the delta base of a blob, but assumes that the tree being fetched (as part of the test) is sent as a non-delta object. This assumption may not hold in the future; for example, a change in the length of the object hash might result in the tree being sent as a delta instead. Make the test more robust by relying on having to lazily fetch the delta base of the tree instead, and by making no assumptions on whether the blobs are sent as delta or non-delta. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 27 January 2020, 20:42:40 UTC
4c616c2 Junio C Hamano 25 January 2020, 23:57:45 UTC merge-recursive: use subtraction to flip stage The flip_stage() helper uses a bit-flipping xor to switch between "2" and "3". While clever, this relies on a property of those two numbers that is mostly coincidence. Let's write it as a subtraction; that's more clear and would extend to other numbers if somebody copies the logic. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 27 January 2020, 19:17:41 UTC
ee79874 Jeff King 25 January 2020, 05:37:23 UTC merge-recursive: silence -Wxor-used-as-pow warning The merge-recursive code uses stage number constants like this: add = &ci->ren1->dst_entry->stages[2 ^ 1]; ... add = &ci->ren2->dst_entry->stages[3 ^ 1]; The xor has the effect of flipping the "1" bit, so that "2 ^ 1" becomes "3" and "3 ^ 1" becomes "2", which correspond to the "ours" and "theirs" stages respectively. Unfortunately, clang-10 and up issue a warning for this code: merge-recursive.c:1759:40: error: result of '2 ^ 1' is 3; did you mean '1 << 1' (2)? [-Werror,-Wxor-used-as-pow] add = &ci->ren1->dst_entry->stages[2 ^ 1]; ~~^~~ 1 << 1 merge-recursive.c:1759:40: note: replace expression with '0x2 ^ 1' to silence this warning We could silence it by using 0x2, as the compiler mentions. Or by just using the constants "2" and "3" directly. But after digging into it, I do think this bit-flip is telling us something. If we just wrote: add = &ci->ren2->dst_entry->stages[2]; for the second one, you might think that "ren2" and "2" correspond. But they don't. The logic is: ren2 is theirs, which is stage 3, but we are interested in the opposite side's stage, so flip it to 2. So let's keep the bit-flipping, but let's also put it behind a named function, which will make its purpose a bit clearer. This also has the side effect of suppressing the warning (and an optimizing compiler should be able to easily turn it into a constant as before). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 27 January 2020, 19:15:35 UTC
a0ba800 Denton Liu 14 January 2020, 02:49:38 UTC .mailmap: fix erroneous authorship for Johannes Schindelin 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> 27 January 2020, 18:32:11 UTC
2602762 Johannes Schindelin 23 January 2020, 12:28:19 UTC rebase -i: also avoid SHA-1 collisions with missingCommitsCheck When `rebase.missingCommitsCheck` is in effect, we use the backup of the todo list that was copied just before the user was allowed to edit it. That backup is, of course, just as susceptible to the hash collision as the todo list itself: a reworded commit could make a previously unambiguous short commit ID ambiguous all of a sudden. So let's not just copy the todo list, but let's instead write out the backup with expanded commit IDs. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 January 2020, 20:48:12 UTC
b699226 Johannes Schindelin 23 January 2020, 12:28:18 UTC rebase -i: re-fix short SHA-1 collision In 66ae9a57b88 (t3404: rebase -i: demonstrate short SHA-1 collision, 2013-08-23), we added a test case that demonstrated how it is possible that a previously unambiguous short commit ID could become ambiguous *during* a rebase. In 75c69766554 (rebase -i: fix short SHA-1 collision, 2013-08-23), we fixed that problem simply by writing out the todo list with expanded commit IDs (except *right* before letting the user edit the todo list, in which case we shorten them, but we expand them right after the file was edited). However, the bug resurfaced as a side effect of 393adf7a6f6 (sequencer: directly call pick_commits() from complete_action(), 2019-11-24): as of this commit, the sequencer no longer re-reads the todo list after writing it out with expanded commit IDs. The only redeeming factor is that the todo list is already parsed at that stage, including all the commits corresponding to the commands, therefore the sequencer can continue even if the internal todo list has short commit IDs. That does not prevent problems, though: the sequencer writes out the `done` and `git-rebase-todo` files incrementally (i.e. overwriting the todo list with a version that has _short_ commit IDs), and if a merge conflict happens, or if an `edit` or a `break` command is encountered, a subsequent `git rebase --continue` _will_ re-read the todo list, opening an opportunity for the "short SHA-1 collision" bug again. To avoid that, let's make sure that we do expand the commit IDs in the todo list as soon as we have parsed it after letting the user edit it. Additionally, we improve the 'short SHA-1 collide' test case in t3404 to test specifically for the case where the rebase is resumed. We also hard-code the expected colliding short SHA-1s, to document the expectation (and to make it easier on future readers). Note that we specifically test that the short commit ID is used in the `git-rebase-todo.tmp` file: this file is created by the fake editor in the test script and reflects the state that would have been presented to the user to edit. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 January 2020, 20:48:11 UTC
d859dca Johannes Schindelin 23 January 2020, 12:28:17 UTC parse_insn_line(): improve error message when parsing failed In the case that a `get_oid()` call failed, we showed some rather bogus part of the line instead of the precise string we sent to said function. That makes it rather hard for users to understand what is going wrong, so let's fix that. While at it, return a negative value from `parse_insn_line()` in case of an error, as per our convention. This function's only caller, `todo_list_parse_insn_buffer()`, cares only whether that return value is non-zero or not, i.e. does not need to be changed. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 January 2020, 20:48:05 UTC
a9472af Philippe Blain 21 January 2020, 15:01:17 UTC submodule.c: use get_git_dir() instead of get_git_common_dir() Ever since df56607dff (git-common-dir: make "modules/" per-working-directory directory, 2014-11-30), submodules in linked worktrees are cloned to $GIT_DIR/modules, i.e. $GIT_COMMON_DIR/worktrees/<name>/modules. However, this convention was not followed when the worktree updater commands checkout, reset and read-tree learned to recurse into submodules. Specifically, submodule.c::submodule_move_head, introduced in 6e3c1595c6 (update submodules: add submodule_move_head, 2017-03-14) and submodule.c::submodule_unset_core_worktree, (re)introduced in 898c2e65b7 (submodule: unset core.worktree if no working tree is present, 2018-12-14) use get_git_common_dir() instead of get_git_dir() to get the path of the submodule repository. This means that, for example, 'git checkout --recurse-submodules <branch>' in a linked worktree will correctly checkout <branch>, detach the submodule's HEAD at the commit recorded in <branch> and update the submodule working tree, but the submodule HEAD that will be moved is the one in $GIT_COMMON_DIR/modules/<name>/, i.e. the submodule repository of the main superproject working tree. It will also rewrite the gitfile in the submodule working tree of the linked worktree to point to $GIT_COMMON_DIR/modules/<name>/. This leads to an incorrect (and confusing!) state in the submodule working tree of the main superproject worktree. Additionally, if switching to a commit where the submodule is not present, submodule_unset_core_worktree will be called and will incorrectly remove 'core.wortree' from the config file of the submodule in the main superproject worktree, $GIT_COMMON_DIR/modules/<name>/config. Fix this by constructing the path to the submodule repository using get_git_dir() in both submodule_move_head and submodule_unset_core_worktree. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 January 2020, 21:39:01 UTC
129510a Philippe Blain 21 January 2020, 15:01:16 UTC t2405: clarify test descriptions and simplify test When 'checkout --to' functionality was moved to 'worktree add', tests were adapted in f194b1ef6e (tests: worktree: retrofit "checkout --to" tests for "worktree add", 2015-07-06). The calls were changed to 'worktree add' in this test (then t7410), but the test descriptions were not updated, keeping 'checkout' instead of using the new terminology (linked worktrees). Also, in the test each worktree is created in $TRASH_DIRECTORY/<leading-directory>/main, where the name of <leading-directory> carries some information about what behavior each test verifies. This directory structure is not mandatory for the tests; the worktrees can live next to one another in the trash directory. Clarify the tests by using the right terminology, and remove the unnecessary leading directories such that all superproject worktrees are directly next to one another in the trash directory. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 January 2020, 21:37:04 UTC
4eaadc8 Philippe Blain 21 January 2020, 15:01:15 UTC t2405: use git -C and test_commit -C instead of subshells The subshells used in the setup phase of this test are unnecessary. Remove them by using 'git -C' and 'test_commit -C'. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 January 2020, 21:36:05 UTC
773c60a Philippe Blain 21 January 2020, 15:01:14 UTC t7410: rename to t2405-worktree-submodule.sh This test was added in df56607dff (git-common-dir: make "modules/" per-working-directory directory, 2014-11-30), back when the 'git worktree' command did not exist and 'git checkout --to' was used to create supplementary worktrees. Since this file contains tests for the interaction of 'git worktree' with submodules, rename it to t2405-worktree-submodule.sh, following the naming scheme for tests checking the behavior of various commands with submodules. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 January 2020, 21:36:03 UTC
7a2dc95 brian m. carlson 22 January 2020, 03:45:43 UTC docs: mention when increasing http.postBuffer is valuable Users in a wide variety of situations find themselves with HTTP push problems. Oftentimes these issues are due to antivirus software, filtering proxies, or other man-in-the-middle situations; other times, they are due to simple unreliability of the network. However, a common solution to HTTP push problems found online is to increase http.postBuffer. This works for none of the aforementioned situations and is only useful in a small, highly restricted number of cases: essentially, when the connection does not properly support HTTP/1.1. Document when raising this value is appropriate and what it actually does, and discourage people from using it as a general solution for push problems, since it is not effective there. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 January 2020, 20:27:49 UTC
1b13e90 brian m. carlson 22 January 2020, 03:45:42 UTC doc: dissuade users from trying to ignore tracked files It is quite common for users to want to ignore the changes to a file that Git tracks. Common scenarios for this case are IDE settings and configuration files, which should generally not be tracked and possibly generated from tracked files using a templating mechanism. However, users learn about the assume-unchanged and skip-worktree bits and try to use them to do this anyway. This is problematic, because when these bits are set, many operations behave as the user expects, but they usually do not help when git checkout needs to replace a file. There is no sensible behavior in this case, because sometimes the data is precious, such as certain configuration files, and sometimes it is irrelevant data that the user would be happy to discard. Since this is not a supported configuration and users are prone to misuse the existing features for unintended purposes, causing general sadness and confusion, let's document the existing behavior and the pitfalls in the documentation for git update-index so that users know they should explore alternate solutions. In addition, let's provide a recommended solution to dealing with the common case of configuration files, since there are well-known approaches used successfully in many environments. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 January 2020, 20:27:49 UTC
69e104d brian m. carlson 22 January 2020, 03:45:41 UTC doc: provide guidance on user.name format It's a frequent misconception that the user.name variable controls authentication in some way, and as a result, beginning users frequently attempt to change it when they're having authentication troubles. Document that the convention is that this variable represents some form of a human's personal name, although that is not required. In addition, address concerns about whether Unicode is supported. Use the term "personal name" as this is likely to draw the intended contrast, be applicable across cultures which may have different naming conventions, and be easily understandable to people who do not speak English as their first language. Indicate that "some form" is conventionally used, as people may use a nickname or preferred name instead of a full legal name. Point users who may be confused about authentication to an appropriate configuration option instead. Provide a shortened form of this information in the configuration option description. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 January 2020, 20:27:08 UTC
813f602 brian m. carlson 22 January 2020, 03:45:40 UTC docs: expand on possible and recommended user config options In the section on setting author and committer information, we omit the author.* and committer.* variables, so mention them for completeness. In addition, guide users to the typical case: simply setting user.name and user.email, which are recommended if one does not need complex configuration. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 January 2020, 20:27:08 UTC
bc94e58 brian m. carlson 22 January 2020, 03:45:39 UTC doc: move author and committer information to git-commit(1) While at one time it made perfect sense to store information about configuring author and committer information in the documentation for git commit-tree, in modern Git that operation is seldom used. Most users will use git commit and expect to find comprehensive documentation about its use in the manual page for that command. Considering that there is significant confusion about how one is to use the user.name and user.email variables, let's put as much documentation as possible into an obvious place where users will be more likely to find it. In addition, expand the environment variables section to describe their use more fully. Even though we now describe all of the options there and in the configuration settings documentation, preserve the existing text in git-commit.txt so that people can easily reason about the ordering of the various options they can use. Explain the use of the author.* and committer.* options as well. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 January 2020, 20:27:08 UTC
856249c brian m. carlson 20 January 2020, 21:21:56 UTC docs: use "currently" for the present time In many languages, the adverb with the root "actual" means "at the present time." However, this usage is considered dated or even archaic in English, and for referring to events occurring at the present time, we usually prefer "currently" or "presently". "Actually" is commonly used in modern English only for the meaning of "in fact" or to express a contrast with what is expected. Since the documentation refers to the available options at the present time (that is, at the time of writing) instead of drawing a contrast, let's switch to "currently," which both is commonly used and sounds less formal than "presently." Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 21 January 2020, 18:43:41 UTC
b40a502 Derrick Stolee 21 January 2020, 01:38:12 UTC fetch: document and test --refmap="" To prevent long blocking time during a 'git fetch' call, a user may want to set up a schedule for background 'git fetch' processes. However, these runs will update the refs/remotes branches due to the default refspec set in the config when Git adds a remote. Hence the user will not notice when remote refs are updated during their foreground fetches. In fact, they may _want_ those refs to stay put so they can work with the refs from their last foreground fetch call. This can be accomplished by overriding the configured refspec using '--refmap=' along with a custom refspec: git fetch --refmap='' <remote> +refs/heads/*:refs/hidden/<remote>/* to populate a custom ref space and download a pack of the new reachable objects. This kind of call allows a few things to happen: 1. We download a new pack if refs have updated. 2. Since the refs/hidden branches exist, GC will not remove the newly-downloaded data. 3. With fetch.writeCommitGraph enabled, the refs/hidden refs are used to update the commit-graph file. To avoid the refs/hidden directory from filling without bound, the --prune option can be included. When providing a refspec like this, the --prune option does not delete remote refs and instead only deletes refs in the target refspace. Update the documentation to clarify how '--refmap=""' works and create tests to guarantee this behavior remains in the future. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 21 January 2020, 18:24:48 UTC
2323784 Junio C Hamano 16 January 2020, 23:18:46 UTC Sync with maint * maint: msvc: accommodate for vcpkg's upgrade to OpenSSL v1.1.x 16 January 2020, 23:18:46 UTC
e4837b4 Jeff King 16 January 2020, 18:19:40 UTC t7800: don't rely on reuse_worktree_file() 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> 16 January 2020, 22:48:26 UTC
fbce03d Jeff King 16 January 2020, 18:34:23 UTC t4018: drop "debugging" cat from hunk-header tests 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> 16 January 2020, 22:46:28 UTC
f65d07f Jeff King 16 January 2020, 17:51:38 UTC Makefile: use compat regex with SANITIZE=address 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> 16 January 2020, 22:19:39 UTC
849e43c Johannes Schindelin 16 January 2020, 08:33:07 UTC built-in add -i: accept open-ended ranges again The interactive `add` command allows selecting multiple files for some of its sub-commands, via unique prefixes, indices or index ranges. When re-implementing `git add -i` in C, we even added a code comment talking about ranges with a missing end index, such as `2-`, but the code did not actually accept those, as pointed out in https://github.com/git-for-windows/git/issues/2466#issuecomment-574142760. Let's fix this, and add a test case to verify that this stays fixed forever. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 January 2020, 22:10:23 UTC
d660a30 Johannes Schindelin 16 January 2020, 08:33:06 UTC built-in add -i: do not try to `patch`/`diff` an empty list of files When the user does not select any files to `patch` or `diff`, there is no need to call `run_add_p()` on them. Even worse: we _have_ to avoid calling `parse_pathspec()` with an empty list because that would trigger this error: BUG: pathspec.c:557: PATHSPEC_PREFER_CWD requires arguments So let's avoid doing any work on a list of files that is empty anyway. This fixes https://github.com/git-for-windows/git/issues/2466. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 January 2020, 22:10:21 UTC
a4ffbbb Ralf Thielow 15 January 2020, 18:07:01 UTC submodule.c: mark more strings for translation Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 January 2020, 22:02:45 UTC
0cbb605 Jeff King 16 January 2020, 20:21:56 UTC dir: point treat_leading_path() warning to the right place Commit 777b420347 (dir: synchronize treat_leading_path() and read_directory_recursive(), 2019-12-19) tried to add two warning comments in those functions, pointing at each other. But the one in treat_leading_path() just points at itself. Let's fix that. Since the comment also redirects the reader for more details to "the commit that added this warning", and since we're now modifying the warning (creating a new commit without those details), let's mention the actual commit id. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 January 2020, 20:56:13 UTC
ad6f215 Jeff King 16 January 2020, 20:21:55 UTC dir: restructure in a way to avoid passing around a struct dirent Restructure the code slightly to avoid passing around a struct dirent anywhere, which also enables us to avoid trying to manufacture one. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 January 2020, 20:56:13 UTC
2270533 Elijah Newren 16 January 2020, 20:21:54 UTC dir: treat_leading_path() and read_directory_recursive(), round 2 I was going to title this "dir: more synchronizing of treat_leading_path() and read_directory_recursive()", a nod to commit 777b42034764 ("dir: synchronize treat_leading_path() and read_directory_recursive()", 2019-12-19), but the title was too long. Anyway, first the backstory... fill_directory() has always had a slightly error-prone interface: it returns a subset of paths which *might* match the specified pathspec; it was intended to prune away some paths which didn't match the specified pathspec and keep at least all the ones that did match it. Given this interface, callers were responsible to post-process the results and check whether each actually matched the pathspec. builtin/clean.c did this. It would first prune out duplicates (e.g. if "dir" was returned as well as all files under "dir/", then it would simplify this to just "dir"), and after pruning duplicates it would compare the remaining paths to the specified pathspec(s). This post-processing itself could run into problems, though, as noted in commit 404ebceda01c ("dir: also check directories for matching pathspecs", 2019-09-17): For the case of git-clean and a set of pathspecs of "dir/file" and "more", this caused a problem because we'd end up with dir entries for both of "dir" "dir/file" Then correct_untracked_entries() would try to helpfully prune duplicates for us by removing "dir/file" since it's under "dir", leaving us with "dir" Since the original pathspec only had "dir/file", the only entry left doesn't match and leaves nothing to be removed. (Note that if only one pathspec was specified, e.g. only "dir/file", then the common_prefix_len optimizations in fill_directory would cause us to bypass this problem, making it appear in simple tests that we could correctly remove manually specified pathspecs.) That commit fixed the issue -- when multiple pathspecs were specified -- by making sure fill_directory() wouldn't return both "dir" and "dir/file" outside the common_prefix_len optimization path. This is where it starts to get fun. In commit b9670c1f5e6b ("dir: fix checks on common prefix directory", 2019-12-19), we noticed that the common_prefix_len wasn't doing appropriate checks and letting all kinds of stuff through, resulting in recursing into .git/ directories and other craziness. So it started locking down and doing checks on pathnames within that code path. That continued with commit 777b42034764 ("dir: synchronize treat_leading_path() and read_directory_recursive()", 2019-12-19), which noted the following: Our optimization to avoid calling into read_directory_recursive() when all pathspecs have a common leading directory mean that we need to match the logic that read_directory_recursive() would use if we had just called it from the root. Since it does more than call treat_path() we need to copy that same logic. ...and then it more forcefully addressed the issue with this wonderfully ironic statement: Needing to duplicate logic like this means it is guaranteed someone will eventually need to make further changes and forget to update both locations. It is tempting to just nuke the leading_directory special casing to avoid such bugs and simplify the code, but unpack_trees' verify_clean_subdirectory() also calls read_directory() and does so with a non-empty leading path, so I'm hesitant to try to restructure further. Add obnoxious warnings to treat_leading_path() and read_directory_recursive() to try to warn people of such problems. You would think that with such a strongly worded description, that its author would have actually ensured that the logic in treat_leading_path() and read_directory_recursive() did actually match and that *everything* that was needed had at least been copied over at the time that this paragraph was written. But you'd be wrong, I messed it up by missing part of the logic. Copy the missing bits to fix the new final test in t7300. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 January 2020, 20:56:13 UTC
f365bf4 Derrick Stolee 16 January 2020, 20:21:53 UTC clean: demonstrate a bug with pathspecs b9670c1f5e (dir: fix checks on common prefix directory, 2019-12-19) modified the way pathspecs are handled when handling a directory during "git clean -f <path>". While this improved the behavior for known test breakages, it also regressed in how the clean command handles cleaning a specified file. Add a test case that demonstrates this behavior. This test passes before b9670c1f5e then fails after. Helped-by: Kevin Willford <Kevin.Willford@microsoft.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 January 2020, 20:56:12 UTC
b6d4d82 Johannes Schindelin 15 January 2020, 22:57:34 UTC msvc: accommodate for vcpkg's upgrade to OpenSSL v1.1.x 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> 16 January 2020, 20:18:23 UTC
d0654dc Junio C Hamano 13 January 2020, 18:16:43 UTC Git 2.25 Signed-off-by: Junio C Hamano <gitster@pobox.com> 13 January 2020, 18:16:43 UTC
b4615e4 Junio C Hamano 12 January 2020, 21:28:13 UTC Merge tag 'l10n-2.25.0-rnd1' of git://github.com/git-l10n/git-po 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 12 January 2020, 21:28:13 UTC
4d92452 Junio C Hamano 12 January 2020, 20:27:41 UTC Revert "Merge branch 'ra/rebase-i-more-options'" 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> 12 January 2020, 21:25:18 UTC
ddc12c4 Jiang Xin 30 December 2019, 00:56:49 UTC l10n: zh_CN: for git v2.25.0 l10n round 1 Translate 119 new messages (4800t0f0u) for git 2.25.0. Signed-off-by: Jiang Xin <worldhello.net@gmail.com> 12 January 2020, 11:22:02 UTC
e23b95e Jiang Xin 11 January 2020, 08:04:21 UTC Merge branch 'master' of github.com:Softcatala/git-po into git-po-master * 'master' of github.com:Softcatala/git-po: l10n: Update Catalan translation 11 January 2020, 08:04:21 UTC
1cf4836 Junio C Hamano 10 January 2020, 22:45:26 UTC Merge branch 'js/mingw-loosen-overstrict-tree-entry-checks' 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 10 January 2020, 22:45:27 UTC
d78a196 Junio C Hamano 10 January 2020, 22:45:26 UTC Merge branch 'ma/config-advice-markup-fix' Documentation markup fix. * ma/config-advice-markup-fix: config/advice.txt: fix description list separator 10 January 2020, 22:45:26 UTC
a20ae3e Jordi Mas 10 January 2020, 21:21:55 UTC l10n: Update Catalan translation Signed-off-by: Jordi Mas <jmas@softcatala.org> 10 January 2020, 21:21:55 UTC
49e268e Johannes Schindelin via GitGitGadget 09 January 2020, 13:30:34 UTC mingw: safeguard better against backslashes in file names 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> 10 January 2020, 20:29:07 UTC
4c6c797 Derrick Stolee via GitGitGadget 10 January 2020, 01:59:30 UTC unpack-trees: correctly compute result count 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> 10 January 2020, 19:34:36 UTC
63a5650 Matthias Rüster 05 January 2020, 14:55:00 UTC l10n: de.po: Update German translation v2.25.0 round 1 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> 10 January 2020, 11:04:03 UTC
75449c1 Thomas Braun 15 December 2019, 17:35:48 UTC l10n: de.po: Reword generation numbers 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> 10 January 2020, 11:04:03 UTC
6b6a980 Alexander Shopov 09 January 2020, 11:45:26 UTC l10n: bg.po: Updated Bulgarian translation (4800t) Signed-off-by: Alexander Shopov <ash@kambanaria.org> 09 January 2020, 15:32:25 UTC
3901d2c Martin Ågren 08 January 2020, 20:08:44 UTC config/advice.txt: fix description list separator 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> 08 January 2020, 21:38:24 UTC
7a6a90c Junio C Hamano 08 January 2020, 20:43:54 UTC Git 2.25-rc2 Signed-off-by: Junio C Hamano <gitster@pobox.com> 08 January 2020, 20:44:13 UTC
1f5f3ff Junio C Hamano 08 January 2020, 20:44:12 UTC Merge branch 'ds/graph-assert-fix' 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 08 January 2020, 20:44:13 UTC
a4e4140 Junio C Hamano 08 January 2020, 20:44:12 UTC Merge branch 'tm/doc-submodule-absorb-fix' Typofix. * tm/doc-submodule-absorb-fix: doc: submodule: fix typo for command absorbgitdirs 08 January 2020, 20:44:12 UTC
202f68b Junio C Hamano 08 January 2020, 20:44:12 UTC Merge branch 'pm/am-in-body-header-doc-update' Doc update. * pm/am-in-body-header-doc-update: am: document that Date: can appear as an in-body header 08 January 2020, 20:44:12 UTC
7e65f86 Junio C Hamano 08 January 2020, 20:44:11 UTC Merge branch 'jb/doc-multi-pack-idx-fix' Typofix. * jb/doc-multi-pack-idx-fix: multi-pack-index: correct configuration in documentation 08 January 2020, 20:44:12 UTC
c5dc206 Junio C Hamano 08 January 2020, 20:44:11 UTC Merge branch 'do/gitweb-typofix-in-comments' Typofix. * do/gitweb-typofix-in-comments: gitweb: fix a couple spelling errors in comments 08 January 2020, 20:44:11 UTC
fe47c9c Junio C Hamano 08 January 2020, 19:17:16 UTC Merge https://github.com/prati0100/git-gui * 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 08 January 2020, 19:18:06 UTC
a1087c9 Derrick Stolee 07 January 2020, 21:27:02 UTC graph: fix lack of color in horizontal lines 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> 08 January 2020, 17:37:18 UTC
0d251c3 Derrick Stolee 07 January 2020, 21:27:01 UTC graph: drop assert() for merge with two collapsing parents 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> 08 January 2020, 17:35:07 UTC
4d8cab9 Jeff King 08 January 2020, 07:10:09 UTC transport: don't flush when disconnecting stateless-rpc helper 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> 08 January 2020, 17:32:38 UTC
573117d Emily Shaffer 08 January 2020, 02:31:27 UTC unpack-trees: watch for out-of-range index position 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> 08 January 2020, 17:31:18 UTC
e701bab Jeff King 08 January 2020, 11:43:44 UTC restore: invalidate cache-tree when removing entries with --staged 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> 08 January 2020, 17:03:36 UTC
1a7e454 Heba Waly 08 January 2020, 00:31:36 UTC doc/gitcore-tutorial: fix prose to match example command 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> 08 January 2020, 16:56:40 UTC
63ab08f brian m. carlson 07 January 2020, 01:36:40 UTC run-command: avoid undefined behavior in exists_in_PATH 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> 07 January 2020, 19:59:07 UTC
065027e Elijah Newren 07 January 2020, 15:07:30 UTC string-list: note in docs that callers can specify sorting function 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> 07 January 2020, 17:12:36 UTC
26f924d Elijah Newren 07 January 2020, 06:57:57 UTC unpack-trees: exit check_updates() early if updates are not wanted check_updates() has a lot of code that repeatedly checks whether o->update or o->dry_run are set. (Note that o->dry_run is a near-synonym for !o->update, but not quite as per commit 2c9078d05bf2 ("unpack-trees: add the dry_run flag to unpack_trees_options", 2011-05-25).) In fact, this function almost turns into a no-op whenever the condition !o->update || o->dry_run is met. Simplify the code by checking this condition at the beginning of the function, and when it is true, do the few things that are relevant and return early. There are a few things that make the conversion not quite obvious: * The fact that check_updates() does not actually turn into a no-op when updates are not wanted may be slightly surprising. However, commit 33ecf7eb61 (Discard "deleted" cache entries after using them to update the working tree, 2008-02-07) put the discarding of unused cache entries in check_updates() so we still need to keep the call to remove_marked_cache_entries(). It's possible this call belongs in another function, but it is certainly needed as tests will fail if it is removed. * The original called remove_scheduled_dirs() unconditionally. Technically, commit 7847892716 (unlink_entry(): introduce schedule_dir_for_removal(), 2009-02-09) should have made that call conditional, but it didn't matter in practice because remove_scheduled_dirs() becomes a no-op when all the calls to unlink_entry() are skipped. As such, we do not need to call it. * When (o->dry_run && o->update), the original would have two calls to git_attr_set_direction() surrounding a bunch of skipped updates. These two calls to git_attr_set_direction() cancel each other out and thus can be omitted when o->dry_run is true just as they already are when !o->update. * The code would previously call setup_collided_checkout_detection() and report_collided_checkout() even when o->dry_run. However, this was just an expensive no-op because setup_collided_checkout_detection() merely cleared the CE_MATCHED flag for each cache entry, and report_collided_checkout() reported which ones had it set. Since a dry-run would skip all the checkout_entry() calls, CE_MATCHED would never get set and thus no collisions would be reported. Since we can't detect the collisions anyway without doing updates, skipping the collisions detection setup and reporting is an optimization. * The code previously would call get_progress() and display_progress() even when (!o->update || o->dry_run). This served to show how long it took to skip all the updates, which is somewhat useless. Since we are skipping the updates, we can skip showing how long it takes to skip them. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 January 2020, 16:37:37 UTC
042ed3e Junio C Hamano 06 January 2020, 22:17:38 UTC The final batch before -rc2 Signed-off-by: Junio C Hamano <gitster@pobox.com> 06 January 2020, 22:17:51 UTC
0f1930c Junio C Hamano 06 January 2020, 22:17:51 UTC Merge branch 'ds/sparse-cone' Code cleanup. * ds/sparse-cone: Documentation/git-sparse-checkout.txt: fix a typo sparse-checkout: use extern for global variables 06 January 2020, 22:17:51 UTC
037f067 Junio C Hamano 06 January 2020, 22:17:51 UTC Merge branch 'ds/commit-graph-set-size-mult' The code to write split commit-graph file(s) upon fetching computed bogus value for the parameter used in splitting the resulting files, which has been corrected. * ds/commit-graph-set-size-mult: commit-graph: prefer default size_mult when given zero 06 January 2020, 22:17:51 UTC
f25f04e Junio C Hamano 06 January 2020, 22:17:51 UTC Merge branch 'en/merge-recursive-oid-eq-simplify' Code cleanup. * en/merge-recursive-oid-eq-simplify: merge-recursive: remove unnecessary oid_eq function 06 January 2020, 22:17:51 UTC
c20d4fd Junio C Hamano 06 January 2020, 22:17:51 UTC Merge branch 'ds/sparse-list-in-cone-mode' "git sparse-checkout list" subcommand learned to give its output in a more concise form when the "cone" mode is in effect. * ds/sparse-list-in-cone-mode: sparse-checkout: document interactions with submodules sparse-checkout: list directories in cone mode 06 January 2020, 22:17:51 UTC
a578ef9 Junio C Hamano 06 January 2020, 22:17:50 UTC Merge branch 'js/mingw-loosen-overstrict-tree-entry-checks' An earlier update to Git for Windows declared that a tree object is invalid if it has a path component with backslash in it, which was overly strict, which has been corrected. The only protection the Windows users need is to prevent such path (or any path that their filesystem cannot check out) from entering the index. * js/mingw-loosen-overstrict-tree-entry-checks: mingw: only test index entries for backslashes, not tree entries 06 January 2020, 22:17:50 UTC
c4117fc Junio C Hamano 06 January 2020, 22:17:50 UTC Merge branch 'pb/clarify-line-log-doc' Doc update. * pb/clarify-line-log-doc: doc: log, gitk: line-log arguments must exist in starting revision doc: log, gitk: document accepted line-log diff formats 06 January 2020, 22:17:50 UTC
556f025 Junio C Hamano 06 January 2020, 22:17:50 UTC Merge branch 'ew/packfile-syscall-optim' Code cleanup. * ew/packfile-syscall-optim: packfile: replace lseek+read with pread packfile: remove redundant fcntl F_GETFD/F_SETFD 06 January 2020, 22:17:50 UTC
5814d44 Thomas Menzel 06 January 2020, 14:50:13 UTC doc: submodule: fix typo for command absorbgitdirs The sentence wants to talk about the superproject's possesive, not plural form. Signed-off-by: Thomas Menzel <dev@tomsit.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 06 January 2020, 17:24:46 UTC
7fdc5f2 Christopher Diaz Riveros 06 January 2020, 14:18:43 UTC l10n: es: 2.25.0 round #1 Signed-off-by: Christopher Diaz Riveros <christopher.diaz.riv@gmail.com> 06 January 2020, 14:18:43 UTC
f8740c5 Paul Menzel 03 January 2020, 15:05:46 UTC am: document that Date: can appear as an in-body header Similar to "From:" and "Subject:" already mentioned in the documentation, "Date:" can also appear as an in-body header to override the value in the e-mail headers. Document it. Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 January 2020, 23:12:39 UTC
4e2c4c0 Denis Ovsienko 04 January 2020, 17:39:26 UTC gitweb: fix a couple spelling errors in comments Signed-off-by: Denis Ovsienko <denis@ovsienko.info> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 January 2020, 23:09:33 UTC
421c0ff Johannes Berg 04 January 2020, 12:43:14 UTC multi-pack-index: correct configuration in documentation It's core.multiPackIndex, not pack.multiIndex. Signed-off-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 January 2020, 23:02:06 UTC
757ff35 Taylor Blau 02 January 2020, 22:51:40 UTC Documentation/git-sparse-checkout.txt: fix a typo This typo was introduced in 94c0956b60 (sparse-checkout: create builtin with 'list' subcommand, 2019-11-21). Signed-off-by: Taylor Blau <me@ttaylorr.com> Acked-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 04 January 2020, 22:40:29 UTC
back to top