Fix git-last-modified(1) #1968
Closed
Fix git-last-modified(1) #1968
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
git-last-modified(1) was recently merged into 'next', but upon use
I've[1] noticed it has a bug:
$ git last-modified
3f3e111 .gitignore
3f3e111 Makefile
3f3e111 builtin.h
3f3e111 command-list.txt
3f3e111 commit-graph.c
3f3e111 git.c
3f3e111 meson.build
56072ff fetch-pack.c
457534d progress.c
0d8f4cc for-each-ref.h
109c3df combine-diff.c
109c3df diff.c
109c3df diff.h
109c3df dir.c
9e5878f git-web--browse.sh
b2fb391 config.mak.in
36268b7 LGPL-2.1
1e58dba aclocal.m4
9517e6b levenshtein.h
703601d COPYING
BUG: ../builtin/last-modified.c:236: paths remaining beyond boundary in last-modified
zsh: IOT instruction (core dumped) git last-modified
I didn't see this before but the BUG() statement was only moved up in
the last version of the patch series, and it only comes into trouble
with merge commits.
This series fixes that issue.
This patch is based on 'next' at 1ba7204 (Merge branch
'kh/doc-markup-fixes' into next, 2025-09-03).
--- b4-submit-tracking ---
# This section is used internally by b4 prep for tracking purposes.
{
"series": {
"revision": 1,
"change-id": "20250902-toon-fix-last-modified-865172060265",
"prefixes": []
}
}
Function diff_tree_combined() copies the 'struct diff_options' from the input 'struct rev_info' to override some flags. One flag was 'recursive', which is set to 1. This has been the case since the inception of this function in af3feef (diff-tree -c: show a merge commit a bit more sensibly., 2006-01-24). From that commit there's no clear indication why recursive is overridden. But it seems to be incorrect. Looking at a test case in t4013. The expected test results are in t/t4013/. And take 'diff.noellipses-diff-tree_-c_--abbrev_master' as an example: $ git diff-tree -c --abbrev master 59d314ad6f356dd08601a4cd5e530381da3e3c64 ::100644 100644 100644 cead32e 7289e35 992913c MM dir/sub ::100644 100644 100644 b414108 f4615da 10a8a9f MM file0 $ Here we see the expected results include 'dir/sub', while argument '-r' (for recursive) was not given. So we should expect 'dir' instead. Stop overriding the 'recursive' flag in diff_tree_combined(). This changes the expected results for the example above to be: $ git diff-tree -c --abbrev master 59d314ad6f356dd08601a4cd5e530381da3e3c64 ::040000 040000 040000 65f5c9d f977ed4 0564e02 MM dir ::100644 100644 100644 b414108 f4615da 10a8a9f MM file0 $ This also fixes the recently added subcommand git-last-modified(1), which in some cases (when merges are involved) exited with the error: BUG: paths remaining beyond boundary in last-modified The last-modified machinery uses a hashmap for all the files it wants to get the last-modified commit for. Through log_tree_commit() the callback mark_path() is called. Here the incoming path is looked up in the hashmap, but because the diff-tree machinery internally switched to recursive (even if the last-modified machinery wasn't) the entry for 'dir' was never expelled from the hashmap, and the BUG() statement was hit. Because internally diff-tree no longer runs recursive, this results in a nice speedup when running `git last-modified` on git.git: Benchmark 1: next Time (mean ± σ): 6.068 s ± 0.038 s [User: 5.268 s, System: 0.187 s] Range (min … max): 6.007 s … 6.123 s 10 runs Benchmark 2: HEAD Time (mean ± σ): 3.057 s ± 0.009 s [User: 2.996 s, System: 0.055 s] Range (min … max): 3.047 s … 3.070 s 10 runs Summary HEAD ran 1.98 ± 0.01 times faster than next Besides that, set the flag 'recursive' in submodule.c to 1 because it calls diff_tree_combined_merge() which no longer enables the flag itself. Signed-off-by: Toon Claes <[email protected]>
|
There are issues in commit b0a9f07: |
Isn't it nice that the full log is written to |
|
I know enough. Let's close. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
git-last-modified(1) was recently merged into 'next', but upon use
I've[1] noticed it has a bug:
I didn't see this before but the BUG() statement was only moved up in
the last version of the patch series, and it only comes into trouble
with merge commits.
This series fixes that issue.
This patch is based on 'next' at 1ba7204 (Merge branch
'kh/doc-markup-fixes' into next, 2025-09-03).