@matheusbsbs / matheus-bagliano / commits / f8781bfda3

2.36 gitk/diff-tree --stdin regression fix

This only surfaced as a regression after 2.36 release, but the breakage was already there with us for at least a year. The diff_free() call is to be used after we completely finished with a diffopt structure. After "git diff A B" finishes producing output, calling it before process exit is fine. But there are commands that prepares diff_options struct once, compares two sets of paths, releases resources that were used to do the comparison, then reuses the same diff_option struct to go on to compare the next two sets of paths, like "git log -p". After "git log -p" finishes showing a single commit, calling it before it goes on to the next commit is NOT fine. There is a mechanism, the .no_free member in diff_options struct, to help "git log" to avoid calling diff_free() after showing each commit and instead call it just one. When the mechanism was introduced in e900d494 (diff: add an API for deferred freeing, 2021-02-11), however, we forgot to do the same to "diff-tree --stdin", which *is* a moral equivalent to "git log". During 2.36 release cycle, we started clearing the pathspec in diff_free(), so programs like gitk that runs git diff-tree --stdin -- <pathspec> downstream of a pipe, processing one commit after another, started showing irrelevant comparison outside the given <pathspec> from the second commit. The same commit, by forgetting to teach the .no_free mechanism, broke "diff-tree --stdin -I<regexp>" and nobody noticed it for over a year, presumably because it is so seldom used an option. But <pathspec> is a different story. The breakage was very prominently visible and was reported immediately after 2.36 was released. Fix this breakage by mimicking how "git log" utilizes the .no_free member so that "diff-tree --stdin" behaves more similarly to "log". Protect the fix with a few new tests. Reported-by: Matthias Aßhauer <mha1993@live.de> Helped-by: René Scharfe <l.s.r@web.de> Helped-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Apr 26, 2022 at 09:11 UTC f8781bfda31756acdc0ae77da7e70337aedae7c9

3 files changed +18

builtin/diff-tree.c

@@ -195,6 +195,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)

195 int saved_dcctc = 0;

196

197 opt->diffopt.rotate_to_strict = 0;

198 + opt->diffopt.no_free = 1;

199 if (opt->diffopt.detect_rename) {

200 if (!the_index.cache)

201 repo_read_index(the_repository);

@@ -217,6 +218,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)

218 }

219 opt->diffopt.degraded_cc_to_c = saved_dcctc;

220 opt->diffopt.needed_rename_limit = saved_nrl;

221 + opt->diffopt.no_free = 0;

222 + diff_free(&opt->diffopt);

223 }

224

225 return diff_result_code(&opt->diffopt, 0);

log-tree.c

@@ -1098,6 +1098,7 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)

1098 opt->loginfo = &log;

1099 opt->diffopt.no_free = 1;

1100

1101 + /* NEEDSWORK: no restoring of no_free? Why? */

1102 if (opt->line_level_traverse)

1103 return line_log_print(opt, commit);

1104

t/t4013-diff-various.sh

+14

@@ -542,6 +542,20 @@ test_expect_success 'diff-tree --stdin with log formatting' '

542 test_cmp expect actual

543 '

544

545 +test_expect_success 'diff-tree --stdin with pathspec' '

546 + cat >expect <<-EOF &&

547 + Third

548 +

549 + dir/sub

550 + Second

551 +

552 + dir/sub

553 + EOF

554 + git rev-list master^ |

555 + git diff-tree -r --stdin --name-only --format=%s dir >actual &&

556 + test_cmp expect actual

557 +'

558 +

559 test_expect_success 'diff -I<regex>: setup' '

560 git checkout master &&

561 test_seq 50 >file0 &&