[pull] master from git:master #140
Merged
+30
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When "git replay" replays a commit it copies the extended headers across from the original commit. However, if the original commit was signed, we do not want to copy the header associated with the signature is it wont be valid for the new commit. The code already knows to avoid coping the "gpgsig" header but does not know to avoid copying the "gpgsig-sha256" header. Add that header to the list of exclusions to match what "git commit --amend" does. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In this comparison, we want to know whether the number of lines is greater than 1. Our test_line_count function passes the first argument as the comparison operator to test, so what we want is a numerical comparison, not a string comparison. While this does not produce a functional problem now, it could very well if we expected two or more items, in which case the value "10" would not match when it should. Furthermore, the "<" and ">" comparisons are new in POSIX 1003.1-2024 and we don't want to require such a new version of POSIX since many popular and supported operating systems were released before that version of POSIX was released. Finally, zsh's builtin test operator does not like the greater-than sign in "test", since it is only supported in the double-bracket extension. This has been reported and will be addressed in a future version, but since our code is also technically incorrect, as well as not very compatible, let's fix it by using a numeric comparison. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This test starts a SOCKS server in Perl in the background and then kills it after the tests are done. However, when using zsh (in sh mode) in the tests, the start_socks function hangs until the background process is killed. Note that this does not reproduce in a simple shell script, so there is likely some interaction between job handling, our heavy use of eval in the test framework, and possibly other complexities of our test framework. What is clear, however, is that switching from a compound statement to a subshell fixes the problem entirely and the test passes with no problem, so do that. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The find_longest_common_sequence() function in patience diff is inefficient as it calls binary_search() for every unique line it encounters when deciding where to put it in the sequence. From instrumentation (using xctrace) on popular repositories, binary_search() takes up 50-60% of the run time within patience_diff() when performing a diff. To optimize this, add a boundary condition check before binary_search() is called to see if the encountered unique line is located after the entire currently tracked longest subsequence. If so, skip the unnecessary binary search and simply append the entry to the end of sequence. Given that most files compared in a diff are usually quite similar to each other, this condition is very common, and should be hit much more frequently than the binary search. Below are some end-to-end performance results by timing `git log --shortstat --oneline -500 --patience` on different repositories with the old and new code. Generally speaking this seems to give at least 8-10% speed up. The "binary search hit %" column describes how often the algorithm enters the binary search path instead of the new faster path. Even in the WebKit case we can see that it's quite rare (1.46%). | Repo | Speed difference | binary search hit % | |----------|------------------|---------------------| | vim | 1.27x | 0.01% | | pytorch | 1.16x | 0.02% | | cpython | 1.14x | 0.06% | | ripgrep | 1.14x | 0.03% | | git | 1.13x | 0.12% | | vscode | 1.09x | 0.10% | | WebKit | 1.08x | 1.46% | The benchmarks were done using hyperfine, on an Apple M1 Max laptop, with git compiled with `-O3 -flto`. Signed-off-by: Yee Cheng Chin <ychin.git@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There was significant confusion in the git-replay manual about what constitutes a revision range. As noted in f302c1e (revisions(7): clarify that most commands take a single revision range, 2021-05-18): Commands that are specifically designed to take two distinct ranges (e.g. "git range-diff R1 R2" to compare two ranges) do exist, but they are exceptions. Unless otherwise noted, all "git" commands that operate on a set of commits work on a single revision range. `git replay` is not an exception, but a few places in the manual were written as though it were. These appear to have come in revisions to the original series, between v3->v4 (see https://lore.kernel.org/git/CAP8UFD3bpLrVW97DH7j=V9H2GsTSAkksC9L3QujQERFk_kLnZA@mail.gmail.com/ , "More than one <revision-range> can be passed") and between v6->v7 (https://lore.kernel.org/git/20231115143327.2441397-1-christian.couder@gmail.com/, "Takes ranges of commits"), and I missed both of these revisions when reviewing. Fix them now. There was also a reference to the "Commit Limiting options below", but this page has no such section of options; strike the misleading reference. It is worth noting that we are documenting existing behavior, rather than optimal behavior. Junio has multiple times suggested introducing alternative ways to walk revisions and use them in `git replay --advance`, e.g. at * https://lore.kernel.org/git/xmqqy1mqo6kv.fsf@gitster.g/ * https://lore.kernel.org/git/xmqq8rb3is8c.fsf@gitster.g/ * https://lore.kernel.org/git/xmqqtsydj2zk.fsf@gitster.g/ (item (2)) If/when we introduce some new revision walking flag that implements one of these alternate types of revision walks, we can update the --advance option and this manual appropriately. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git replay" forgot to omit the "gpgsig-sha256" extended header from the resulting commit the same way it omits "gpgsig", which has been corrected. * pw/replay-exclude-gpgsig-fix: replay: do not copy "gpgsign-sha256" header
A few tests have been updated to work under the shell compatible mode of zsh. * bc/zsh-testsuite: t5564: fix test hang under zsh's sh mode t0614: use numerical comparison with test_line_count
The way patience diff finds LCS has been optimized. * yc/xdiff-patience-optim: xdiff: optimize patience diff's LCS search
The use of "revision" (a connected set of commits) has been clarified in the "git replay" documentation. * en/replay-doc-revision-range: Documentation/git-replay.adoc: fix errors around revision range
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )