diff --git a/revision.c b/revision.c index 0c95edef5947fa..5d650affc089a6 100644 --- a/revision.c +++ b/revision.c @@ -4175,37 +4175,39 @@ static timestamp_t comparison_date(const struct rev_info *revs, commit->date; } -enum commit_action get_commit_action(struct rev_info *revs, struct commit *commit) +/* + * Whether the commit is ignored by the cheap checks that read only its + * traversal flags and pack membership (e.g. already shown, or marked + * uninteresting), before any check that examines the commit's date, + * parents, message, or diff. + */ +static int commit_early_ignore(struct rev_info *revs, struct commit *commit) { if (commit->object.flags & SHOWN) - return commit_ignore; + return 1; if (revs->maximal_only && (commit->object.flags & CHILD_VISITED)) - return commit_ignore; + return 1; if (revs->unpacked && has_object_pack(revs->repo, &commit->object.oid)) - return commit_ignore; - if (revs->no_kept_objects) { - if (has_object_kept_pack(revs->repo, &commit->object.oid, - revs->keep_pack_cache_flags)) - return commit_ignore; - } + return 1; + if (revs->no_kept_objects && + has_object_kept_pack(revs->repo, &commit->object.oid, + revs->keep_pack_cache_flags)) + return 1; if (commit->object.flags & UNINTERESTING) + return 1; + return 0; +} + +/* + * Decide whether this commit is shown or ignored. Keep it a pure + * predicate: callers such as the commit graph depend on it having no + * side effects, so per-commit mutations (such as -L range tracking) + * belong in the caller, simplify_commit(), not here. + */ +enum commit_action get_commit_action(struct rev_info *revs, struct commit *commit) +{ + if (commit_early_ignore(revs, commit)) return commit_ignore; - if (revs->line_level_traverse && !want_ancestry(revs)) { - /* - * In case of line-level log with parent rewriting - * prepare_revision_walk() already took care of all line-level - * log filtering, and there is nothing left to do here. - * - * If parent rewriting was not requested, then this is the - * place to perform the line-level log filtering. Notably, - * this check, though expensive, must come before the other, - * cheaper filtering conditions, because the tracked line - * ranges must be adjusted even when the commit will end up - * being ignored based on other conditions. - */ - if (!line_log_process_ranges_arbitrary_commit(revs, commit)) - return commit_ignore; - } if (revs->min_age != -1 && comparison_date(revs, commit) > revs->min_age) return commit_ignore; @@ -4314,7 +4316,23 @@ struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit) { - enum commit_action action = get_commit_action(revs, commit); + enum commit_action action; + + /* + * For a line-level log without parent rewriting, fold each commit's + * ranges as the walk reaches it (parent rewriting does this eagerly in + * prepare_revision_walk()). Fold before get_commit_action() so the + * ranges carry across a commit that a later, cheaper check ignores; + * the commit_early_ignore() guard skips a commit get_commit_action() + * would ignore outright. + */ + if (revs->line_level_traverse && !want_ancestry(revs) && + !commit_early_ignore(revs, commit)) { + if (!line_log_process_ranges_arbitrary_commit(revs, commit)) + return commit_ignore; + } + + action = get_commit_action(revs, commit); if (action == commit_show && revs->prune && revs->dense && want_ancestry(revs)) { diff --git a/t/helper/test-revision-walking.c b/t/helper/test-revision-walking.c index 70051eeaf848e7..24d7f294178dda 100644 --- a/t/helper/test-revision-walking.c +++ b/t/helper/test-revision-walking.c @@ -13,9 +13,12 @@ #include "test-tool.h" #include "commit.h" #include "diff.h" +#include "line-log.h" +#include "object-name.h" #include "repository.h" #include "revision.h" #include "setup.h" +#include "string-list.h" static void print_commit(struct commit *commit) { @@ -51,6 +54,60 @@ static int run_revision_walk(void) return got_revision; } +/* + * Check that get_commit_action() is a pure predicate by evaluating it on a + * commit the walk has not reached yet. No git command makes that out-of-order + * call, so this probe does it deliberately, and reports whether the call + * mutated the peeked commit: a pure get_commit_action() leaves it untouched. + * We compare the commit's flags rather than the emitted commit list because + * range merges are idempotent, so a side effect would not change which commits + * are shown. Only meaningful for a plain "-L" walk with no parent rewriting. + */ +static int line_log_peek(const char **argv) +{ + struct repository *repo = the_repository; + struct rev_info rev; + struct string_list range_args = STRING_LIST_INIT_DUP; + struct object_id oid; + struct commit *peek; + const char *rev_argv[3]; + unsigned before, after; + + if (repo_get_oid(repo, argv[0], &oid)) + die("bad peek commit: %s", argv[0]); + peek = lookup_commit_reference(repo, &oid); + if (!peek || repo_parse_commit(repo, peek)) + die("cannot parse peek commit: %s", argv[0]); + + repo_init_revisions(repo, &rev, NULL); + rev.diffopt.flags.recursive = 1; + rev.line_level_traverse = 1; + string_list_append(&range_args, argv[1]); + + rev_argv[0] = "line-log-peek"; + rev_argv[1] = argv[2]; + rev_argv[2] = NULL; + setup_revisions(2, rev_argv, &rev, NULL); + + line_log_init(&rev, NULL, &range_args); + + if (rev.rewrite_parents || rev.children.name) + die("line-log-peek requires a non-ancestry (-L, no --graph) walk"); + + if (prepare_revision_walk(&rev)) + die("prepare_revision_walk failed"); + + before = peek->object.flags; + get_commit_action(&rev, peek); + after = peek->object.flags; + + printf("mutated %d\n", before != after); + + release_revisions(&rev); + string_list_clear(&range_args, 0); + return 0; +} + int cmd__revision_walking(int argc, const char **argv) { if (argc < 2) @@ -69,6 +126,12 @@ int cmd__revision_walking(int argc, const char **argv) return 0; } + if (!strcmp(argv[1], "line-log-peek")) { + if (argc != 5) + die("usage: test-tool revision-walking line-log-peek "); + return line_log_peek(argv + 2); + } + fprintf(stderr, "check usage\n"); return 1; } diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index ca4eb7bbc713ef..f4a7d8ab61d027 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -781,4 +781,24 @@ test_expect_success '--summary shows new file on root commit' ' test_grep "create mode 100644 file.c" actual ' +test_expect_success 'get_commit_action() does not mutate a not-yet-walked commit' ' + git init peek && + ( + cd peek && + test_write_lines 1 2 3 4 5 >f.c && + git add f.c && test_tick && git commit -m base && + test_write_lines 1 two 3 4 5 >f.c && + test_tick && git commit -am change && + + # Peek HEAD^, which the walk has not reached (the out-of-order + # call a lookahead makes), and confirm get_commit_action() leaves + # it untouched. A side effect is invisible in the commit list + # (range merges are idempotent), so the helper reports whether the + # call mutated the peeked commit at all. + echo "mutated 0" >expect && + test-tool revision-walking line-log-peek HEAD^ 1,3:f.c HEAD >actual && + test_cmp expect actual + ) +' + test_done