diff --git a/Documentation/RelNotes/2.56.0.adoc b/Documentation/RelNotes/2.56.0.adoc index 87ac9067f194eb..9073520c251035 100644 --- a/Documentation/RelNotes/2.56.0.adoc +++ b/Documentation/RelNotes/2.56.0.adoc @@ -298,6 +298,22 @@ Performance, Internal Implementation, Development Support etc. the image upgrade. (merge 1a1579c42d jk/ci-static-analysis-image-bump later to maint). + * The alias tests in 't/t0014-alias.sh' have been updated to dynamically + query the list of deprecated commands using 'git + --list-cmds=deprecated' to avoid test failures when running with + 'WITH_BREAKING_CHANGES' in a build directory that contains stale + executables of formerly deprecated commands. + (merge bc57ecb915 jk/t0014-dynamic-deprecated-cmds later to maint). + + * The code path that deals with relative paths in the diff-lib has + been cleaned up. + + * The get_commit_action() function has been refactored to be a pure + predicate by moving the side-effecting line-level log range folding to + simplify_commit(). This ensures that evaluating a commit's action + before the walk reaches it does not prematurely mutate its tracked + line ranges, making it safer for potential lookahead evaluations. + Fixes since v2.55 ----------------- @@ -496,14 +512,26 @@ Fixes since v2.55 and another prevented the editor from opening when the final command in a chain containing 'fixup -c' was skipped. - * The alias tests in 't/t0014-alias.sh' have been updated to dynamically - query the list of deprecated commands using 'git - --list-cmds=deprecated' to avoid test failures when running with - 'WITH_BREAKING_CHANGES' in a build directory that contains stale - executables of formerly deprecated commands. - (merge bc57ecb915 jk/t0014-dynamic-deprecated-cmds later to maint). - * Git for Windows has been updated to avoid auto-detecting the symlink type if the target path starts with a slash, preventing NTLM credential leaks when checking out repositories with crafted symbolic links pointing to network shares. + + * 'git cat-file --batch-command' that asked for 'contents' without + 'type' segfaults, which has been corrected. + (merge 2abc7f0304 jk/cat-file-batch-wo-type-fix later to maint). + + * A memory leak in 'git merge' when run without arguments (which + triggers the default-to-upstream path) has been fixed. A test has + been added to cover this case. + (merge 68cce04a02 tc/merge-default-to-upstream-leakfix later to maint). + + * A boundary case check in reachability bitmap traversal has been + corrected to properly handle the object at position zero, which was + previously skipped, leading to redundant bitmap loading. + (merge b56b48301e dl/pack-bitmap-position-zero later to maint). + + * A crash in the 'sparse-index' collapse code when encountering an + invalidated cache-tree node (due to an intent-to-add path) has been + fixed by avoiding collapsing such subtrees. + (merge eede1e69fe ds/sparse-index-ita-crash later to maint). diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 884b6d5ad348b5..e85e2cfc1b0484 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -789,8 +789,11 @@ static void parse_cmd_contents(struct batch_options *opt, struct strbuf *output, struct expand_data *data) { + enum object_type *saved_typep = data->info.typep; + data->info.typep = &data->type; opt->batch_mode = BATCH_MODE_CONTENTS; batch_one_object(line, output, opt, data); + data->info.typep = saved_typep; } static void parse_cmd_info(struct batch_options *opt, diff --git a/builtin/merge.c b/builtin/merge.c index 58d1b7bb07d90f..5b4eb23a833295 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1373,7 +1373,7 @@ int cmd_merge(int argc, struct commit_list *common = NULL; const char *best_strategy = NULL, *wt_strategy = NULL; struct commit_list *remoteheads = NULL, *p; - void *branch_to_free; + void *branch_to_free, *argv_to_free = NULL; int orig_argc = argc; int merge_log_config = -1; @@ -1517,8 +1517,10 @@ int cmd_merge(int argc, option_commit = 1; if (!argc) { - if (default_to_upstream) + if (default_to_upstream) { argc = setup_with_upstream(&argv); + argv_to_free = argv; + } else die(_("No commit specified and merge.defaultToUpstream not set.")); } else if (argc == 1 && !strcmp(argv[0], "-")) { @@ -1880,6 +1882,7 @@ int cmd_merge(int argc, } strbuf_release(&buf); free(branch_to_free); + free(argv_to_free); free(pull_twohead); free(pull_octopus); discard_index(the_repository->index); diff --git a/diff-lib.c b/diff-lib.c index ac4e310438e710..086476bd77c76a 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -508,11 +508,9 @@ static void do_oneway_diff(struct unpack_trees_options *o, * For diffing, the index is more important, and we only have a * single tree. * - * We're supposed to advance o->pos to skip what we have already processed. - * * This wrapper makes it all more readable, and takes care of all * the fairly complex unpack_trees() semantic requirements, including - * the skipping, the path matching, the type conflict cases etc. + * the path matching, the type conflict cases etc. */ static int oneway_diff(const struct cache_entry * const *src, struct unpack_trees_options *o) @@ -540,6 +538,11 @@ static int oneway_diff(const struct cache_entry * const *src, if (!idx && !tree) BUG("oneway_diff with neither idx nor tree"); + if (revs->diffopt.prefix && + strncmp((idx ? idx : tree)->name, revs->diffopt.prefix, + revs->diffopt.prefix_length)) + return 0; + if (ce_path_match(revs->diffopt.repo->index, idx ? idx : tree, &revs->prune_data, NULL)) { diff --git a/pack-bitmap.c b/pack-bitmap.c index d8dc4ae8d1633c..e85bd69ba446aa 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -1569,7 +1569,7 @@ static struct bitmap *find_objects(struct bitmap_index *bitmap_git, if (base) { int pos = bitmap_position(bitmap_git, &object->oid); - if (pos > 0 && bitmap_get(base, pos)) { + if (pos >= 0 && bitmap_get(base, pos)) { object->flags |= SEEN; continue; } diff --git a/revision.c b/revision.c index 526bcf3fb5aeb9..50dc8b199137c7 100644 --- a/revision.c +++ b/revision.c @@ -4196,37 +4196,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; @@ -4335,7 +4337,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/sparse-index.c b/sparse-index.c index 1ed769b78d8de1..c1fa231a89fc07 100644 --- a/sparse-index.c +++ b/sparse-index.c @@ -113,10 +113,17 @@ static int convert_to_sparse_rec(struct index_state *istate, continue; } + span = ct->down[pos]->cache_tree->entry_count; + if (span < 0) { + /* cache-tree entry is invalidated, cannot collapse. */ + istate->cache[num_converted++] = ce; + i++; + continue; + } + strbuf_setlen(&child_path, 0); strbuf_add(&child_path, ce->name, slash - ce->name + 1); - span = ct->down[pos]->cache_tree->entry_count; count = convert_to_sparse_rec(istate, num_converted, i, i + span, child_path.buf, child_path.len, 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/t1006-cat-file.sh b/t/t1006-cat-file.sh index 0c1dc0fb400d7d..72fc8acc611927 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -1340,6 +1340,14 @@ test_expect_success 'batch-command flush without --buffer' ' test_grep "^fatal:.*flush is only for --buffer mode.*" err ' +test_expect_success 'batch-command contents auto-handles type' ' + echo "HEAD" | + git cat-file --batch="%(objectname)" >expect && + echo "contents HEAD" | + git cat-file --batch-command="%(objectname)" >actual && + test_cmp expect actual +' + perl_script=' use warnings; use strict; diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 4140c4d8ef2436..446c1776cb478b 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -384,6 +384,54 @@ test_expect_success 'add, commit, checkout' ' test_all_match git checkout - ' +test_expect_success 'intent-to-add entries outside sparse-checkout' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + test_sparse_match git sparse-checkout set deep folder1 && + run_on_sparse mkdir -p folder1 && + run_on_all ../edit-contents folder1/newita && + test_sparse_match git add -N folder1/newita && + + test_sparse_match git sparse-checkout set deep && + test_sparse_match git status --porcelain=v2 && + test_sparse_match git ls-files --stage +' + +test_expect_success 'intent-to-add with --sparse outside sparse-checkout' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>$1 + EOF + + run_on_all mkdir -p folder1 && + run_on_all ../edit-contents folder1/newita && + test_all_match git add --sparse --intent-to-add folder1/newita && + + test_all_match git status --porcelain=v2 && + test_all_match git ls-files --stage && + test_all_match git diff --cached --stat && + + # Ensure sparse index stores correct sparse directories and + # intent-to-add path. + git -C sparse-index ls-files --format="%(path)" --sparse >out && + + # These paths should be present in index as-is. + test_grep "^before/\$" out && + test_grep "^folder1/newita\$" out && + test_grep "^folder2/\$" out && + test_grep "^x/\$" out && + + # folder/0/ could theoretically be collapsed to a sparse + # directory entry, but the current implementation avoids the + # reduction because of folder1/newita + test_grep "^folder1/0/0/0\$" out +' + test_expect_success 'git add, checkout, and reset with -p' ' init_repos && diff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh index 64ad7a2949bf0c..975f9218b05cd7 100755 --- a/t/t3705-add-sparse-checkout.sh +++ b/t/t3705-add-sparse-checkout.sh @@ -233,4 +233,30 @@ test_expect_success 'refuse to add non-skip-worktree file from sparse dir' ' test_cmp expect stderr ' +test_expect_success 'intent-to-add entry and sparse index' ' + test_when_finished "git sparse-checkout disable" && + test_when_finished "git reset --hard" && + + git sparse-checkout disable && + mkdir -p in out && + echo base >in/file && + echo base >out/file && + git add in/file out/file && + git commit -m "in and out directories" && + + # enable sparse-checkout, but with all child directories. + git config index.sparse true && + git sparse-checkout set in out && + + # create a new path and set intent-to-add bit + echo new >out/newita && + git add -N out/newita && + + # collapse sparse-checkout, and make sure that the sparse index + # maintains the intent-to-add bit. + git sparse-checkout set in && + git ls-files --error-unmatch out/newita && + git status --porcelain +' + test_done diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index e61ef9b21104b2..d0a834ed8f5bba 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 diff --git a/t/t5333-pseudo-merge-bitmaps.sh b/t/t5333-pseudo-merge-bitmaps.sh index 305d6771082d55..5b2a17f90a1091 100755 --- a/t/t5333-pseudo-merge-bitmaps.sh +++ b/t/t5333-pseudo-merge-bitmaps.sh @@ -50,7 +50,15 @@ test_expect_success 'bitmap traversal without pseudo-merges' ' test_pseudo_merges_cascades 0 merges && test_must_be_empty merges && - test_cmp expect actual + test_cmp expect actual && + + : >trace2.txt && + GIT_TRACE2_EVENT=$PWD/trace2.txt \ + git rev-list --objects --use-bitmap-index HEAD HEAD >/dev/null && + + # The first HEAD initializes base from its position-zero bitmap. The + # duplicate root should not count as another bitmap hit. + test_trace2_data bitmap bitmap/hits 1