From d5a6f505e65444bafeae6d86568c589c3be8797b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 4 Sep 2025 14:49:55 +0200 Subject: [PATCH 01/53] blame: drop explicit check for commit graph Our blaming subsystem knows to use bloom filters from commit graphs to speed up the whole computation. The setup of this happens in `setup_blame_bloom_data()`, where we first verify that we even have a commit graph in the first place. This check is redundant though, as we call `get_bloom_filter_settings()` immediately afterwards which, which already knows to return a `NULL` pointer in case we don't have a commit graph. Drop the redundant check. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- blame.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/blame.c b/blame.c index f1c0670144b67c..cb0b08342308ef 100644 --- a/blame.c +++ b/blame.c @@ -2909,9 +2909,6 @@ void setup_blame_bloom_data(struct blame_scoreboard *sb) struct blame_bloom_data *bd; struct bloom_filter_settings *bs; - if (!sb->repo->objects->commit_graph) - return; - bs = get_bloom_filter_settings(sb->repo); if (!bs) return; From 307e30792b3fb256f58816c4ca67d647eb76c788 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 4 Sep 2025 14:49:56 +0200 Subject: [PATCH 02/53] revision: drop explicit check for commit graph When filtering down revisions by paths we know to use bloom filters from the commit graph, if we have any. The entry point for this is in `check_maybe_different_in_bloom_filter()`, where we first verify that: - We do have a commit graph. - That the commit is contained therein by checking that we have a proper generation number. - And that the graph contains a bloom filter. The first check is somewhat redundant though: if we don't have a commit graph, then the second check would already tell us that we don't have a generation number for the specific commit. In theory this could be seen as a performance optimization to short-circuit for scenarios where there is no commit graph. But in practice this shouldn't matter: if there is no commit graph, then the commit graph data slab would also be unpopulated and thus a lookup of the commit should happen in constant time. Drop the unnecessary check. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- revision.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/revision.c b/revision.c index 6ba8f670542ce3..6018f30a99e5f3 100644 --- a/revision.c +++ b/revision.c @@ -774,9 +774,6 @@ static int check_maybe_different_in_bloom_filter(struct rev_info *revs, struct bloom_filter *filter; int result = 0; - if (!revs->repo->objects->commit_graph) - return -1; - if (commit_graph_generation(commit) == GENERATION_NUMBER_INFINITY) return -1; From 199d452758605a3ff15ac7d900653b4de7455e24 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 4 Sep 2025 14:49:57 +0200 Subject: [PATCH 03/53] commit-graph: return the prepared commit graph from `prepare_commit_graph()` When making use of commit graphs, one needs to first prepare them by calling `prepare_commit_graph()`. Once that function was called and the commit graph was prepared successfully, the caller is now expected to access the graph directly via `struct object_database::commit_graph`. In a subsequent change, we're going to move the commit graph pointer from `struct object_database` into `struct odb_source`. With this change, semantics will change so that we use the commit graph of the first source that has one. Consequently, all callers that currently deference the `commit_graph` pointer would now have to loop around the list of sources to find the commit graph. This would become quite unwieldy. So instead of shifting the burden onto such callers, adapt `prepare_commit_graph()` to return the prepared commit graph, if any. Like this, callers are expected to call that function and then use the returned commit graph. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit-graph.c | 82 ++++++++++++++++++++------------------------------ 1 file changed, 32 insertions(+), 50 deletions(-) diff --git a/commit-graph.c b/commit-graph.c index 3cd9e73e2aa092..62260a2026d839 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -735,7 +735,7 @@ struct commit_graph *read_commit_graph_one(struct odb_source *source) * On the first invocation, this function attempts to load the commit * graph if the repository is configured to have one. */ -static int prepare_commit_graph(struct repository *r) +static struct commit_graph *prepare_commit_graph(struct repository *r) { struct odb_source *source; @@ -747,10 +747,10 @@ static int prepare_commit_graph(struct repository *r) * we want to disable even an already-loaded graph file. */ if (!r->gitdir || r->commit_graph_disabled) - return 0; + return NULL; if (r->objects->commit_graph_attempted) - return !!r->objects->commit_graph; + return r->objects->commit_graph; r->objects->commit_graph_attempted = 1; prepare_repo_settings(r); @@ -763,10 +763,10 @@ static int prepare_commit_graph(struct repository *r) * so that commit graph loading is not attempted again for this * repository.) */ - return 0; + return NULL; if (!commit_graph_compatible(r)) - return 0; + return NULL; odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { @@ -775,20 +775,17 @@ static int prepare_commit_graph(struct repository *r) break; } - return !!r->objects->commit_graph; + return r->objects->commit_graph; } int generation_numbers_enabled(struct repository *r) { uint32_t first_generation; struct commit_graph *g; - if (!prepare_commit_graph(r)) - return 0; - g = r->objects->commit_graph; - - if (!g->num_commits) - return 0; + g = prepare_commit_graph(r); + if (!g || !g->num_commits) + return 0; first_generation = get_be32(g->chunk_commit_data + g->hash_algo->rawsz + 8) >> 2; @@ -799,12 +796,9 @@ int generation_numbers_enabled(struct repository *r) int corrected_commit_dates_enabled(struct repository *r) { struct commit_graph *g; - if (!prepare_commit_graph(r)) - return 0; - g = r->objects->commit_graph; - - if (!g->num_commits) + g = prepare_commit_graph(r); + if (!g || !g->num_commits) return 0; return g->read_generation_data; @@ -1012,23 +1006,26 @@ static int find_commit_pos_in_graph(struct commit *item, struct commit_graph *g, int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c, uint32_t *pos) { - if (!prepare_commit_graph(r)) + struct commit_graph *g = prepare_commit_graph(r); + if (!g) return 0; - return find_commit_pos_in_graph(c, r->objects->commit_graph, pos); + return find_commit_pos_in_graph(c, g, pos); } struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id) { static int commit_graph_paranoia = -1; + struct commit_graph *g; struct commit *commit; uint32_t pos; if (commit_graph_paranoia == -1) commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 0); - if (!prepare_commit_graph(repo)) + g = prepare_commit_graph(repo); + if (!g) return NULL; - if (!search_commit_pos_in_graph(id, repo->objects->commit_graph, &pos)) + if (!search_commit_pos_in_graph(id, g, &pos)) return NULL; if (commit_graph_paranoia && !odb_has_object(repo->objects, id, 0)) return NULL; @@ -1039,7 +1036,7 @@ struct commit *lookup_commit_in_graph(struct repository *repo, const struct obje if (commit->object.parsed) return commit; - if (!fill_commit_in_graph(commit, repo->objects->commit_graph, pos)) + if (!fill_commit_in_graph(commit, g, pos)) return NULL; return commit; @@ -1062,6 +1059,7 @@ static int parse_commit_in_graph_one(struct commit_graph *g, int parse_commit_in_graph(struct repository *r, struct commit *item) { static int checked_env = 0; + struct commit_graph *g; if (!checked_env && git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE, 0)) @@ -1069,9 +1067,10 @@ int parse_commit_in_graph(struct repository *r, struct commit *item) GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE); checked_env = 1; - if (!prepare_commit_graph(r)) + g = prepare_commit_graph(r); + if (!g) return 0; - return parse_commit_in_graph_one(r->objects->commit_graph, item); + return parse_commit_in_graph_one(g, item); } void load_commit_graph_info(struct repository *r, struct commit *item) @@ -2519,6 +2518,7 @@ int write_commit_graph(struct odb_source *source, int replace = 0; struct bloom_filter_settings bloom_settings = DEFAULT_BLOOM_FILTER_SETTINGS; struct topo_level_slab topo_levels; + struct commit_graph *g; prepare_repo_settings(r); if (!r->settings.core_commit_graph) { @@ -2547,23 +2547,13 @@ int write_commit_graph(struct odb_source *source, init_topo_level_slab(&topo_levels); ctx.topo_levels = &topo_levels; - prepare_commit_graph(ctx.r); - if (ctx.r->objects->commit_graph) { - struct commit_graph *g = ctx.r->objects->commit_graph; - - while (g) { - g->topo_levels = &topo_levels; - g = g->base_graph; - } - } + g = prepare_commit_graph(ctx.r); + for (struct commit_graph *chain = g; chain; chain = chain->base_graph) + g->topo_levels = &topo_levels; if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS) ctx.changed_paths = 1; if (!(flags & COMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS)) { - struct commit_graph *g; - - g = ctx.r->objects->commit_graph; - /* We have changed-paths already. Keep them in the next graph */ if (g && g->bloom_filter_settings) { ctx.changed_paths = 1; @@ -2580,22 +2570,15 @@ int write_commit_graph(struct odb_source *source, bloom_settings.hash_version = bloom_settings.hash_version == 2 ? 2 : 1; if (ctx.split) { - struct commit_graph *g = ctx.r->objects->commit_graph; - - while (g) { + for (struct commit_graph *chain = g; chain; chain = chain->base_graph) ctx.num_commit_graphs_before++; - g = g->base_graph; - } if (ctx.num_commit_graphs_before) { ALLOC_ARRAY(ctx.commit_graph_filenames_before, ctx.num_commit_graphs_before); i = ctx.num_commit_graphs_before; - g = ctx.r->objects->commit_graph; - while (g) { - ctx.commit_graph_filenames_before[--i] = xstrdup(g->filename); - g = g->base_graph; - } + for (struct commit_graph *chain = g; chain; chain = chain->base_graph) + ctx.commit_graph_filenames_before[--i] = xstrdup(chain->filename); } if (ctx.opts) @@ -2604,8 +2587,7 @@ int write_commit_graph(struct odb_source *source, ctx.approx_nr_objects = repo_approximate_object_count(r); - if (ctx.append && ctx.r->objects->commit_graph) { - struct commit_graph *g = ctx.r->objects->commit_graph; + if (ctx.append && g) { for (i = 0; i < g->num_commits; i++) { struct object_id oid; oidread(&oid, g->chunk_oid_lookup + st_mult(g->hash_algo->rawsz, i), @@ -2651,7 +2633,7 @@ int write_commit_graph(struct odb_source *source, } else ctx.num_commit_graphs_after = 1; - ctx.trust_generation_numbers = validate_mixed_generation_chain(ctx.r->objects->commit_graph); + ctx.trust_generation_numbers = validate_mixed_generation_chain(g); compute_topological_levels(&ctx); if (ctx.write_generation_data) From 88bc3500e5be888c13757d12c4a5cb16e39ec673 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 4 Sep 2025 14:49:58 +0200 Subject: [PATCH 04/53] commit-graph: return commit graph from `repo_find_commit_pos_in_graph()` The function `repo_find_commit_pos_in_graph()` takes a commit as input and tries to figure out whether the given repository has a commit graph that contains that specific commit. If so, it returns the corresponding position of that commit inside the graph. Right now though we only return the position, but not the actual graph that the commit has been found in. This is sensible as repositories always have the graph in `struct repository::objects::commit_graph`. Consequently, the caller always knows where to find it. But in a subsequent change we're going to move the graph into the object sources. This would require callers of the function to loop through all sources to find the relevant commit graph. Refactor the code so that we instead return the commit-graph that the commit has been found with. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- bloom.c | 8 +++++--- commit-graph.c | 18 ++++++++++++------ commit-graph.h | 12 ++++++------ 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/bloom.c b/bloom.c index b86015f6d1babb..2d7b951e5bf245 100644 --- a/bloom.c +++ b/bloom.c @@ -452,10 +452,12 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r, filter = bloom_filter_slab_at(&bloom_filters, c); if (!filter->data) { + struct commit_graph *g; uint32_t graph_pos; - if (repo_find_commit_pos_in_graph(r, c, &graph_pos)) - load_bloom_filter_from_graph(r->objects->commit_graph, - filter, graph_pos); + + g = repo_find_commit_pos_in_graph(r, c, &graph_pos); + if (g) + load_bloom_filter_from_graph(g, filter, graph_pos); } if (filter->data && filter->len) { diff --git a/commit-graph.c b/commit-graph.c index 62260a2026d839..16dfe582295073 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1003,13 +1003,16 @@ static int find_commit_pos_in_graph(struct commit *item, struct commit_graph *g, } } -int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c, - uint32_t *pos) +struct commit_graph *repo_find_commit_pos_in_graph(struct repository *r, + struct commit *c, + uint32_t *pos) { struct commit_graph *g = prepare_commit_graph(r); if (!g) - return 0; - return find_commit_pos_in_graph(c, g, pos); + return NULL; + if (!find_commit_pos_in_graph(c, g, pos)) + return NULL; + return g; } struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id) @@ -1075,9 +1078,12 @@ int parse_commit_in_graph(struct repository *r, struct commit *item) void load_commit_graph_info(struct repository *r, struct commit *item) { + struct commit_graph *g; uint32_t pos; - if (repo_find_commit_pos_in_graph(r, item, &pos)) - fill_commit_graph_info(item, r->objects->commit_graph, pos); + + g = repo_find_commit_pos_in_graph(r, item, &pos); + if (g) + fill_commit_graph_info(item, g, pos); } static struct tree *load_tree_for_commit(struct commit_graph *g, diff --git a/commit-graph.h b/commit-graph.h index 4899b54ef88207..f6a54336415453 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -48,10 +48,9 @@ int open_commit_graph_chain(const char *chain_file, int *fd, struct stat *st, int parse_commit_in_graph(struct repository *r, struct commit *item); /* - * Fills `*pos` with the graph position of `c`, and returns 1 if `c` is - * found in the commit-graph belonging to `r`, or 0 otherwise. - * Initializes the commit-graph belonging to `r` if it hasn't been - * already. + * Fills `*pos` with the graph position of `c`, and returns the graph `c` is + * found in, or NULL otherwise. Initializes the commit-graphs belonging to + * `r` if it hasn't been already. * * Note: this is a low-level helper that does not alter any slab data * associated with `c`. Useful in circumstances where the slab data is @@ -59,8 +58,9 @@ int parse_commit_in_graph(struct repository *r, struct commit *item); * * In most cases, callers should use `parse_commit_in_graph()` instead. */ -int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c, - uint32_t *pos); +struct commit_graph *repo_find_commit_pos_in_graph(struct repository *r, + struct commit *c, + uint32_t *pos); /* * Look up the given commit ID in the commit-graph. This will only return a From 62490b6d85882e6a0ba434ab436640e31352ffee Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 4 Sep 2025 14:49:59 +0200 Subject: [PATCH 05/53] commit-graph: pass graphs that are to be merged as parameter When determining whether or not we want to merge a commit graph chain we retrieve the graph that is to be merged via the context's repository. With an upcoming change though it will become a bit more complex to figure out the commit graph, which would lead to code duplication. Prepare for this change by passing the graph that is to be merged as a parameter. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- commit-graph.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/commit-graph.c b/commit-graph.c index 16dfe582295073..0e25b140766289 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2226,7 +2226,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) return 0; } -static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) +static void split_graph_merge_strategy(struct write_commit_graph_context *ctx, + struct commit_graph *graph_to_merge) { struct commit_graph *g; uint32_t num_commits; @@ -2245,7 +2246,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) flags = ctx->opts->split_flags; } - g = ctx->r->objects->commit_graph; + g = graph_to_merge; num_commits = ctx->commits.nr; if (flags == COMMIT_GRAPH_SPLIT_REPLACE) ctx->num_commit_graphs_after = 1; @@ -2297,7 +2298,7 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) ctx->commit_graph_filenames_after[i] = xstrdup(ctx->commit_graph_filenames_before[i]); i = ctx->num_commit_graphs_before - 1; - g = ctx->r->objects->commit_graph; + g = graph_to_merge; while (g) { if (i < ctx->num_commit_graphs_after) @@ -2395,9 +2396,9 @@ static void sort_and_scan_merged_commits(struct write_commit_graph_context *ctx) stop_progress(&ctx->progress); } -static void merge_commit_graphs(struct write_commit_graph_context *ctx) +static void merge_commit_graphs(struct write_commit_graph_context *ctx, + struct commit_graph *g) { - struct commit_graph *g = ctx->r->objects->commit_graph; uint32_t current_graph_number = ctx->num_commit_graphs_before; while (g && current_graph_number >= ctx->num_commit_graphs_after) { @@ -2632,12 +2633,13 @@ int write_commit_graph(struct odb_source *source, goto cleanup; if (ctx.split) { - split_graph_merge_strategy(&ctx); + split_graph_merge_strategy(&ctx, g); if (!replace) - merge_commit_graphs(&ctx); - } else + merge_commit_graphs(&ctx, g); + } else { ctx.num_commit_graphs_after = 1; + } ctx.trust_generation_numbers = validate_mixed_generation_chain(g); From 3b9532dab2fe1db12d5a33c74a0256d03a4c4861 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Thu, 25 Sep 2025 15:10:37 +0000 Subject: [PATCH 06/53] add -p: mark split hunks as undecided When a hunk is split, each of the new hunks inherits whether it is selected or not from the original hunk. If a selected hunk is split all of the new hunks are marked as "selected" and the user is only prompted with the first of the split hunks. The user is not asked whether or not they wish to select the rest of the new hunks. This means that if they wish to deselect any of the new hunks apart from the first one they have to navigate back to the hunk they want to deselect before they can deselect it. This is unfortunate as the user is presumably splitting the original hunk because they only want to select some sub-set of it. Instead mark all the new hunks as "undecided" so that the user is prompted whether they wish to select each one in turn. In the case where the user only wants to change the selection of the first of the split hunks they will now have to do more work re-selecting the remaining split hunks. However, changing the selection of any of the other newly created hunks is now much simpler as the user no-longer has to navigate back to them in order to change their selected state. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- add-patch.c | 3 ++- t/t3701-add-interactive.sh | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/add-patch.c b/add-patch.c index 302e6ba7d9a353..61f42de9ea94e6 100644 --- a/add-patch.c +++ b/add-patch.c @@ -956,6 +956,7 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff, * sizeof(*hunk)); hunk = file_diff->hunk + hunk_index; hunk->splittable_into = 1; + hunk->use = UNDECIDED_HUNK; memset(hunk + 1, 0, (splittable_into - 1) * sizeof(*hunk)); header = &hunk->header; @@ -1057,7 +1058,7 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff, hunk++; hunk->splittable_into = 1; - hunk->use = hunk[-1].use; + hunk->use = UNDECIDED_HUNK; header = &hunk->header; header->old_count = header->new_count = context_line_count; diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 04d2a198352531..a6829fd0859d1c 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -1301,4 +1301,14 @@ do ' done +test_expect_success 'splitting previous hunk marks split hunks as undecided' ' + test_write_lines a " " b c d e f g h i j k >file && + git add file && + test_write_lines x " " b y d e f g h i j x >file && + test_write_lines n K s n y q | git add -p file && + git cat-file blob :file >actual && + test_write_lines a " " b y d e f g h i j k >expect && + test_cmp expect actual +' + test_done From 732650e263eb6bceda9988a8bbe75f311d908897 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Thu, 25 Sep 2025 15:10:38 +0000 Subject: [PATCH 07/53] add-patch: update hunk splitability after editing If, when the user edits a hunk, they change deletion lines into context lines or vice versa, then the number of hunks that the edited hunk can be split into may differ from the unedited hunk. This means that so we should recalculate `hunk->splittable_into` after the hunk has been edited. In practice users are unlikely to hit this bug as it is doubtful that a user who has edited a hunk will split it afterwards. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- add-patch.c | 12 +++++++++++- t/t3701-add-interactive.sh | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/add-patch.c b/add-patch.c index 61f42de9ea94e6..bcc2d7666ff872 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1185,19 +1185,29 @@ static ssize_t recount_edited_hunk(struct add_p_state *s, struct hunk *hunk, { struct hunk_header *header = &hunk->header; size_t i; + char ch, marker = ' '; + hunk->splittable_into = 0; header->old_count = header->new_count = 0; for (i = hunk->start; i < hunk->end; ) { - switch(normalize_marker(&s->plain.buf[i])) { + ch = normalize_marker(&s->plain.buf[i]); + switch (ch) { case '-': header->old_count++; + if (marker == ' ') + hunk->splittable_into++; + marker = ch; break; case '+': header->new_count++; + if (marker == ' ') + hunk->splittable_into++; + marker = ch; break; case ' ': header->old_count++; header->new_count++; + marker = ch; break; } diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index a6829fd0859d1c..13739a45820241 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -1311,4 +1311,25 @@ test_expect_success 'splitting previous hunk marks split hunks as undecided' ' test_cmp expect actual ' +test_expect_success 'splitting edited hunk' ' + # Before the first hunk is edited it can be split into two + # hunks, after editing it can be split into three hunks. + + write_script fake-editor.sh <<-\EOF && + sed "s/^ c/-c/" "$1" >"$1.tmp" && + mv "$1.tmp" "$1" + EOF + + test_write_lines a b c d e f g h i j k l m n >file && + git add file && + test_write_lines A b c d E f g h i j k l M n >file && + ( + test_set_editor "$(pwd)/fake-editor.sh" && + test_write_lines e K s j y n y q | git add -p file + ) && + git cat-file blob :file >actual && + test_write_lines a b d e f g h i j k l M n >expect && + test_cmp expect actual +' + test_done From 71fd6c695cd9fc9cc0a829d1579c7584c2ad9e18 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Thu, 25 Sep 2025 19:07:34 +0200 Subject: [PATCH 08/53] range-diff: rename other_arg to log_arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename `other_arg` to `log_arg` in `range_diff_options` and related places. “Other argument” comes from bd361918 (range-diff: pass through --notes to `git log`, 2019-11-20) which introduced Git notes handling to git-range-diff(1) by passing that option on to git-log(1). And that kind of name might be fine in a local context. However, it was initially spread among multiple files, and is now[1] part of the `range_diff_options` struct. It is, prima facie, difficult to guess what “other” means, especially when just looking at the struct. But with a little reading we find out that it is used for `--[no-]notes` and `--diff-merges`, which are both passed on to git-log(1). We should just rename it to reflect this role; `log_arg` suggests, along with the `strvec` type, that it is used to pass extra arguments to git-log(1). † 1: since f1ce6c19 (range-diff: combine all options in a single data structure, 2021-02-05) Suggested-by: Junio C Hamano Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- builtin/log.c | 8 ++++---- builtin/range-diff.c | 16 ++++++++-------- range-diff.c | 10 +++++----- range-diff.h | 2 +- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 5f552d14c0fe83..131512ac1af0f1 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1400,13 +1400,13 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file, * can be added later if deemed desirable. */ struct diff_options opts; - struct strvec other_arg = STRVEC_INIT; + struct strvec log_arg = STRVEC_INIT; struct range_diff_options range_diff_opts = { .creation_factor = rev->creation_factor, .dual_color = 1, .max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT, .diffopt = &opts, - .other_arg = &other_arg + .log_arg = &log_arg }; repo_diff_setup(the_repository, &opts); @@ -1414,9 +1414,9 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file, opts.use_color = rev->diffopt.use_color; diff_setup_done(&opts); fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title); - get_notes_args(&other_arg, rev); + get_notes_args(&log_arg, rev); show_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts); - strvec_clear(&other_arg); + strvec_clear(&log_arg); } } diff --git a/builtin/range-diff.c b/builtin/range-diff.c index aafcc99b96240f..f88b40e3607a0f 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -37,13 +37,13 @@ int cmd_range_diff(int argc, struct repository *repo UNUSED) { struct diff_options diffopt = { NULL }; - struct strvec other_arg = STRVEC_INIT; + struct strvec log_arg = STRVEC_INIT; struct strvec diff_merges_arg = STRVEC_INIT; struct range_diff_options range_diff_opts = { .creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT, .max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT, .diffopt = &diffopt, - .other_arg = &other_arg + .log_arg = &log_arg }; int simple_color = -1, left_only = 0, right_only = 0; struct option range_diff_options[] = { @@ -52,7 +52,7 @@ int cmd_range_diff(int argc, N_("percentage by which creation is weighted")), OPT_BOOL(0, "no-dual-color", &simple_color, N_("use simple diff colors")), - OPT_PASSTHRU_ARGV(0, "notes", &other_arg, + OPT_PASSTHRU_ARGV(0, "notes", &log_arg, N_("notes"), N_("passed to 'git log'"), PARSE_OPT_OPTARG), OPT_PASSTHRU_ARGV(0, "diff-merges", &diff_merges_arg, @@ -92,7 +92,7 @@ int cmd_range_diff(int argc, /* If `--diff-merges` was specified, imply `--merges` */ if (diff_merges_arg.nr) { range_diff_opts.include_merges = 1; - strvec_pushv(&other_arg, diff_merges_arg.v); + strvec_pushv(&log_arg, diff_merges_arg.v); } for (i = 0; i < argc; i++) @@ -124,7 +124,7 @@ int cmd_range_diff(int argc, strbuf_addf(&range1, "%s..%s", argv[0], argv[1]); strbuf_addf(&range2, "%s..%s", argv[0], argv[2]); - strvec_pushv(&other_arg, argv + + strvec_pushv(&log_arg, argv + (dash_dash < 0 ? 3 : dash_dash)); } else if (dash_dash == 2 || (dash_dash < 0 && argc > 1 && @@ -144,7 +144,7 @@ int cmd_range_diff(int argc, strbuf_addstr(&range1, argv[0]); strbuf_addstr(&range2, argv[1]); - strvec_pushv(&other_arg, argv + + strvec_pushv(&log_arg, argv + (dash_dash < 0 ? 2 : dash_dash)); } else if (dash_dash == 1 || (dash_dash < 0 && argc > 0 && @@ -175,7 +175,7 @@ int cmd_range_diff(int argc, strbuf_addf(&range1, "%s..%.*s", b, a_len, a); strbuf_addf(&range2, "%.*s..%s", a_len, a, b); - strvec_pushv(&other_arg, argv + + strvec_pushv(&log_arg, argv + (dash_dash < 0 ? 1 : dash_dash)); } else usage_msg_opt(_("need two commit ranges"), @@ -187,7 +187,7 @@ int cmd_range_diff(int argc, range_diff_opts.right_only = right_only; res = show_range_diff(range1.buf, range2.buf, &range_diff_opts); - strvec_clear(&other_arg); + strvec_clear(&log_arg); strvec_clear(&diff_merges_arg); strbuf_release(&range1); strbuf_release(&range2); diff --git a/range-diff.c b/range-diff.c index ca449a07693e85..57edff40a85f24 100644 --- a/range-diff.c +++ b/range-diff.c @@ -39,7 +39,7 @@ struct patch_util { * as struct object_id (will need to be free()d). */ static int read_patches(const char *range, struct string_list *list, - const struct strvec *other_arg, + const struct strvec *log_arg, unsigned int include_merges) { struct child_process cp = CHILD_PROCESS_INIT; @@ -69,8 +69,8 @@ static int read_patches(const char *range, struct string_list *list, if (!include_merges) strvec_push(&cp.args, "--no-merges"); strvec_push(&cp.args, range); - if (other_arg) - strvec_pushv(&cp.args, other_arg->v); + if (log_arg) + strvec_pushv(&cp.args, log_arg->v); cp.out = -1; cp.no_stdin = 1; cp.git_cmd = 1; @@ -594,9 +594,9 @@ int show_range_diff(const char *range1, const char *range2, if (range_diff_opts->left_only && range_diff_opts->right_only) res = error(_("options '%s' and '%s' cannot be used together"), "--left-only", "--right-only"); - if (!res && read_patches(range1, &branch1, range_diff_opts->other_arg, include_merges)) + if (!res && read_patches(range1, &branch1, range_diff_opts->log_arg, include_merges)) res = error(_("could not parse log for '%s'"), range1); - if (!res && read_patches(range2, &branch2, range_diff_opts->other_arg, include_merges)) + if (!res && read_patches(range2, &branch2, range_diff_opts->log_arg, include_merges)) res = error(_("could not parse log for '%s'"), range2); if (!res) { diff --git a/range-diff.h b/range-diff.h index 9d39818e349c91..9b70a80009e257 100644 --- a/range-diff.h +++ b/range-diff.h @@ -23,7 +23,7 @@ struct range_diff_options { unsigned include_merges:1; size_t max_memory; const struct diff_options *diffopt; /* may be NULL */ - const struct strvec *other_arg; /* may be NULL */ + const struct strvec *log_arg; /* may be NULL */ }; /* From 85bd88a7e8a8f7cd7c99b9db4a10b7a29498d258 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Thu, 25 Sep 2025 19:07:35 +0200 Subject: [PATCH 09/53] revision: add rdiff_log_arg to rev_info git-format-patch(1) supports Git notes by showing them beneath the patch/commit message, similar to git-log(1). The command also supports showing those same notes ref names in the range diff output. Note *the same* ref names; any Git notes options or configuration variables need to be handed off to the range-diff machinery. This works correctly in the case when the range diff is on the cover letter. But it does not work correctly when the output is a single patch with an embedded range diff. Concretely, git-format-patch(1) needs to pass `--[no-]notes` options on to the range-diff subprocess in `range-diff.c`. This is handled in `builtin/log.c` by the local variable `log_arg` in the case of mul- tiple commits, but not in the single commit case where there is no cover letter and the range diff is embedded in the patch output; the range diff is then made in `log-tree.c`, whither `log_arg` has not been propagated. This means that the range-diff subprocess reverts to its default behavior, which is to act like git-log(1) w.r.t. notes. We need to fix this. But first lay the groundwork by converting `log_arg` to a struct member; next we can simply use that member in `log-tree.c` without having to thread it from `builtin/log.c`. No functional changes. Helped-by: Junio C Hamano Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- builtin/log.c | 7 +++---- revision.h | 2 ++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 131512ac1af0f1..9eff62ce1110fe 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1400,13 +1400,12 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file, * can be added later if deemed desirable. */ struct diff_options opts; - struct strvec log_arg = STRVEC_INIT; struct range_diff_options range_diff_opts = { .creation_factor = rev->creation_factor, .dual_color = 1, .max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT, .diffopt = &opts, - .log_arg = &log_arg + .log_arg = &rev->rdiff_log_arg }; repo_diff_setup(the_repository, &opts); @@ -1414,9 +1413,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file, opts.use_color = rev->diffopt.use_color; diff_setup_done(&opts); fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title); - get_notes_args(&log_arg, rev); show_range_diff(rev->rdiff1, rev->rdiff2, &range_diff_opts); - strvec_clear(&log_arg); } } @@ -2328,6 +2325,7 @@ int cmd_format_patch(int argc, rev.rdiff_title = diff_title(&rdiff_title, reroll_count, _("Range-diff:"), _("Range-diff against v%d:")); + get_notes_args(&(rev.rdiff_log_arg), &rev); } /* @@ -2487,6 +2485,7 @@ int cmd_format_patch(int argc, rev.diffopt.no_free = 0; release_revisions(&rev); format_config_release(&cfg); + strvec_clear(&rev.rdiff_log_arg); return 0; } diff --git a/revision.h b/revision.h index 21e288c5baa2b5..ce30570d86a614 100644 --- a/revision.h +++ b/revision.h @@ -334,6 +334,7 @@ struct rev_info { /* range-diff */ const char *rdiff1; const char *rdiff2; + struct strvec rdiff_log_arg; int creation_factor; const char *rdiff_title; @@ -410,6 +411,7 @@ struct rev_info { .expand_tabs_in_log = -1, \ .commit_format = CMIT_FMT_DEFAULT, \ .expand_tabs_in_log_default = 8, \ + .rdiff_log_arg = STRVEC_INIT, \ } /** From 155986b49b52b9b5910edc0fd56ba46f0f1bed22 Mon Sep 17 00:00:00 2001 From: Kristoffer Haugsbakk Date: Thu, 25 Sep 2025 19:07:36 +0200 Subject: [PATCH 10/53] format-patch: handle range-diff on notes correctly for single patches (The two next paragraphs are taken from the previous commit.) git-format-patch(1) supports Git notes by showing them beneath the patch/commit message, similar to git-log(1). The command also supports showing those same notes ref names in the range diff output. Note *the same* ref names; any Git notes options or configuration variables need to be handed off to the range-diff machinery. This works correctly in the case when the range diff is on the cover letter. But it does not work correctly when the output is a single patch with an embedded range diff. Concretely, git-format-patch(1) needs to pass `--[no-]notes` options on to the range-diff subprocess in `range-diff.c`. Range diffs for single- commit series are handled in `log-tree.c`. But `log-tree.c` had no access to any `log_arg` variable before we added it to `rev_info` in the previous commit. Use that new struct member to fix this inconsistency. Signed-off-by: Kristoffer Haugsbakk Signed-off-by: Junio C Hamano --- log-tree.c | 3 ++- t/t3206-range-diff.sh | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/log-tree.c b/log-tree.c index 73d21f71764e94..3d38c748e45be9 100644 --- a/log-tree.c +++ b/log-tree.c @@ -718,7 +718,8 @@ static void show_diff_of_diff(struct rev_info *opt) .creation_factor = opt->creation_factor, .dual_color = 1, .max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT, - .diffopt = &opts + .diffopt = &opts, + .log_arg = &opt->rdiff_log_arg }; memcpy(&dq, &diff_queued_diff, sizeof(diff_queued_diff)); diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index e091df6d01da90..1e812df806bbbf 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -707,7 +707,7 @@ test_expect_success 'format-patch --range-diff does not compare notes by default ! grep "note" 0000-* ' -test_expect_success 'format-patch --notes=custom --range-diff only compares custom notes' ' +test_expect_success 'format-patch --notes=custom --range-diff --cover-letter only compares custom notes' ' test_when_finished "git notes remove topic unmodified || :" && git notes add -m "topic note" topic && git notes add -m "unmodified note" unmodified && @@ -721,6 +721,20 @@ test_expect_success 'format-patch --notes=custom --range-diff only compares cust ! grep "## Notes ##" 0000-* ' +# --range-diff on a single commit requires --no-cover-letter +test_expect_success 'format-patch --notes=custom --range-diff on single commit only compares custom notes' ' + test_when_finished "git notes remove HEAD unmodified || :" && + git notes add -m "topic note" HEAD && + test_when_finished "git notes --ref=custom remove HEAD unmodified || :" && + git notes add -m "unmodified note" unmodified && + git notes --ref=custom add -m "topic note (custom)" HEAD && + git notes --ref=custom add -m "unmodified note (custom)" unmodified && + git format-patch --notes=custom --range-diff=$prev \ + -1 --stdout >actual && + test_grep "## Notes (custom) ##" actual && + test_grep ! "## Notes ##" actual +' + test_expect_success 'format-patch --range-diff with --no-notes' ' test_when_finished "git notes remove topic unmodified || :" && git notes add -m "topic note" topic && From 43d5f52ac4a3b9cb6c5717af30be42a363fedf20 Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:48 +0000 Subject: [PATCH 11/53] xdiff: delete static forward declarations in xprepare Move xdl_prepare_env() later in the file to avoid the need for static forward declarations. Best-viewed-with: --color-moved Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xprepare.c | 116 ++++++++++++++++++++--------------------------- 1 file changed, 50 insertions(+), 66 deletions(-) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index e1d4017b2ddeac..249bfa678f4268 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -53,21 +53,6 @@ typedef struct s_xdlclassifier { -static int xdl_init_classifier(xdlclassifier_t *cf, long size, long flags); -static void xdl_free_classifier(xdlclassifier_t *cf); -static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t **rhash, - unsigned int hbits, xrecord_t *rec); -static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_t const *xpp, - xdlclassifier_t *cf, xdfile_t *xdf); -static void xdl_free_ctx(xdfile_t *xdf); -static int xdl_clean_mmatch(char const *dis, long i, long s, long e); -static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2); -static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2); -static int xdl_optimize_ctxs(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2); - - - - static int xdl_init_classifier(xdlclassifier_t *cf, long size, long flags) { cf->flags = flags; @@ -242,57 +227,6 @@ static void xdl_free_ctx(xdfile_t *xdf) { } -int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, - xdfenv_t *xe) { - long enl1, enl2, sample; - xdlclassifier_t cf; - - memset(&cf, 0, sizeof(cf)); - - /* - * For histogram diff, we can afford a smaller sample size and - * thus a poorer estimate of the number of lines, as the hash - * table (rhash) won't be filled up/grown. The number of lines - * (nrecs) will be updated correctly anyway by - * xdl_prepare_ctx(). - */ - sample = (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF - ? XDL_GUESS_NLINES2 : XDL_GUESS_NLINES1); - - enl1 = xdl_guess_lines(mf1, sample) + 1; - enl2 = xdl_guess_lines(mf2, sample) + 1; - - if (xdl_init_classifier(&cf, enl1 + enl2 + 1, xpp->flags) < 0) - return -1; - - if (xdl_prepare_ctx(1, mf1, enl1, xpp, &cf, &xe->xdf1) < 0) { - - xdl_free_classifier(&cf); - return -1; - } - if (xdl_prepare_ctx(2, mf2, enl2, xpp, &cf, &xe->xdf2) < 0) { - - xdl_free_ctx(&xe->xdf1); - xdl_free_classifier(&cf); - return -1; - } - - if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && - (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) && - xdl_optimize_ctxs(&cf, &xe->xdf1, &xe->xdf2) < 0) { - - xdl_free_ctx(&xe->xdf2); - xdl_free_ctx(&xe->xdf1); - xdl_free_classifier(&cf); - return -1; - } - - xdl_free_classifier(&cf); - - return 0; -} - - void xdl_free_env(xdfenv_t *xe) { xdl_free_ctx(&xe->xdf2); @@ -460,3 +394,53 @@ static int xdl_optimize_ctxs(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2 return 0; } + +int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, + xdfenv_t *xe) { + long enl1, enl2, sample; + xdlclassifier_t cf; + + memset(&cf, 0, sizeof(cf)); + + /* + * For histogram diff, we can afford a smaller sample size and + * thus a poorer estimate of the number of lines, as the hash + * table (rhash) won't be filled up/grown. The number of lines + * (nrecs) will be updated correctly anyway by + * xdl_prepare_ctx(). + */ + sample = (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF + ? XDL_GUESS_NLINES2 : XDL_GUESS_NLINES1); + + enl1 = xdl_guess_lines(mf1, sample) + 1; + enl2 = xdl_guess_lines(mf2, sample) + 1; + + if (xdl_init_classifier(&cf, enl1 + enl2 + 1, xpp->flags) < 0) + return -1; + + if (xdl_prepare_ctx(1, mf1, enl1, xpp, &cf, &xe->xdf1) < 0) { + + xdl_free_classifier(&cf); + return -1; + } + if (xdl_prepare_ctx(2, mf2, enl2, xpp, &cf, &xe->xdf2) < 0) { + + xdl_free_ctx(&xe->xdf1); + xdl_free_classifier(&cf); + return -1; + } + + if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && + (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) && + xdl_optimize_ctxs(&cf, &xe->xdf1, &xe->xdf2) < 0) { + + xdl_free_ctx(&xe->xdf2); + xdl_free_ctx(&xe->xdf1); + xdl_free_classifier(&cf); + return -1; + } + + xdl_free_classifier(&cf); + + return 0; +} From d1c028bdf75b9bcd380f85f74a34edcbaa060fee Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:49 +0000 Subject: [PATCH 12/53] xdiff: delete local variables and initialize/free xdfile_t directly These local variables are essentially a hand-rolled additional implementation of xdl_free_ctx() inlined into xdl_prepare_ctx(). Modify the code to use the existing xdl_free_ctx() function so there aren't two ways to free such variables. Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xprepare.c | 78 +++++++++++++++++++----------------------------- 1 file changed, 30 insertions(+), 48 deletions(-) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 249bfa678f4268..96134c9fbfe243 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -134,99 +134,81 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t } +static void xdl_free_ctx(xdfile_t *xdf) +{ + xdl_free(xdf->rhash); + xdl_free(xdf->rindex); + xdl_free(xdf->rchg - 1); + xdl_free(xdf->ha); + xdl_free(xdf->recs); + xdl_cha_free(&xdf->rcha); +} + + static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_t const *xpp, xdlclassifier_t *cf, xdfile_t *xdf) { - unsigned int hbits; - long nrec, hsize, bsize; + long bsize; unsigned long hav; char const *blk, *cur, *top, *prev; xrecord_t *crec; - xrecord_t **recs; - xrecord_t **rhash; - unsigned long *ha; - char *rchg; - long *rindex; - ha = NULL; - rindex = NULL; - rchg = NULL; - rhash = NULL; - recs = NULL; + xdf->ha = NULL; + xdf->rindex = NULL; + xdf->rchg = NULL; + xdf->rhash = NULL; + xdf->recs = NULL; if (xdl_cha_init(&xdf->rcha, sizeof(xrecord_t), narec / 4 + 1) < 0) goto abort; - if (!XDL_ALLOC_ARRAY(recs, narec)) + if (!XDL_ALLOC_ARRAY(xdf->recs, narec)) goto abort; - hbits = xdl_hashbits((unsigned int) narec); - hsize = 1 << hbits; - if (!XDL_CALLOC_ARRAY(rhash, hsize)) + xdf->hbits = xdl_hashbits((unsigned int) narec); + if (!XDL_CALLOC_ARRAY(xdf->rhash, 1 << xdf->hbits)) goto abort; - nrec = 0; + xdf->nrec = 0; if ((cur = blk = xdl_mmfile_first(mf, &bsize))) { for (top = blk + bsize; cur < top; ) { prev = cur; hav = xdl_hash_record(&cur, top, xpp->flags); - if (XDL_ALLOC_GROW(recs, nrec + 1, narec)) + if (XDL_ALLOC_GROW(xdf->recs, xdf->nrec + 1, narec)) goto abort; if (!(crec = xdl_cha_alloc(&xdf->rcha))) goto abort; crec->ptr = prev; crec->size = (long) (cur - prev); crec->ha = hav; - recs[nrec++] = crec; - if (xdl_classify_record(pass, cf, rhash, hbits, crec) < 0) + xdf->recs[xdf->nrec++] = crec; + if (xdl_classify_record(pass, cf, xdf->rhash, xdf->hbits, crec) < 0) goto abort; } } - if (!XDL_CALLOC_ARRAY(rchg, nrec + 2)) + if (!XDL_CALLOC_ARRAY(xdf->rchg, xdf->nrec + 2)) goto abort; if ((XDF_DIFF_ALG(xpp->flags) != XDF_PATIENCE_DIFF) && (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF)) { - if (!XDL_ALLOC_ARRAY(rindex, nrec + 1)) + if (!XDL_ALLOC_ARRAY(xdf->rindex, xdf->nrec + 1)) goto abort; - if (!XDL_ALLOC_ARRAY(ha, nrec + 1)) + if (!XDL_ALLOC_ARRAY(xdf->ha, xdf->nrec + 1)) goto abort; } - xdf->nrec = nrec; - xdf->recs = recs; - xdf->hbits = hbits; - xdf->rhash = rhash; - xdf->rchg = rchg + 1; - xdf->rindex = rindex; + xdf->rchg += 1; xdf->nreff = 0; - xdf->ha = ha; xdf->dstart = 0; - xdf->dend = nrec - 1; + xdf->dend = xdf->nrec - 1; return 0; abort: - xdl_free(ha); - xdl_free(rindex); - xdl_free(rchg); - xdl_free(rhash); - xdl_free(recs); - xdl_cha_free(&xdf->rcha); + xdl_free_ctx(xdf); return -1; } -static void xdl_free_ctx(xdfile_t *xdf) { - - xdl_free(xdf->rhash); - xdl_free(xdf->rindex); - xdl_free(xdf->rchg - 1); - xdl_free(xdf->ha); - xdl_free(xdf->recs); - xdl_cha_free(&xdf->rcha); -} - - void xdl_free_env(xdfenv_t *xe) { xdl_free_ctx(&xe->xdf2); From efaf553b1a4ea9cafcb9cab0697157091bc4825a Mon Sep 17 00:00:00 2001 From: Ezekiel Newren Date: Fri, 26 Sep 2025 22:41:50 +0000 Subject: [PATCH 13/53] xdiff: delete unnecessary fields from xrecord_t and xdfile_t xrecord_t.next, xdfile_t.hbits, xdfile_t.rhash are initialized, but never used for anything by the code. Remove them. Best-viewed-with: --color-words Signed-off-by: Ezekiel Newren Signed-off-by: Junio C Hamano --- xdiff/xprepare.c | 15 ++------------- xdiff/xtypes.h | 3 --- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 96134c9fbfe243..3576415c85cb7a 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -91,8 +91,7 @@ static void xdl_free_classifier(xdlclassifier_t *cf) { } -static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t **rhash, - unsigned int hbits, xrecord_t *rec) { +static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t *rec) { long hi; char const *line; xdlclass_t *rcrec; @@ -126,17 +125,12 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t rec->ha = (unsigned long) rcrec->idx; - hi = (long) XDL_HASHLONG(rec->ha, hbits); - rec->next = rhash[hi]; - rhash[hi] = rec; - return 0; } static void xdl_free_ctx(xdfile_t *xdf) { - xdl_free(xdf->rhash); xdl_free(xdf->rindex); xdl_free(xdf->rchg - 1); xdl_free(xdf->ha); @@ -155,7 +149,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ xdf->ha = NULL; xdf->rindex = NULL; xdf->rchg = NULL; - xdf->rhash = NULL; xdf->recs = NULL; if (xdl_cha_init(&xdf->rcha, sizeof(xrecord_t), narec / 4 + 1) < 0) @@ -163,10 +156,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ if (!XDL_ALLOC_ARRAY(xdf->recs, narec)) goto abort; - xdf->hbits = xdl_hashbits((unsigned int) narec); - if (!XDL_CALLOC_ARRAY(xdf->rhash, 1 << xdf->hbits)) - goto abort; - xdf->nrec = 0; if ((cur = blk = xdl_mmfile_first(mf, &bsize))) { for (top = blk + bsize; cur < top; ) { @@ -180,7 +169,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ crec->size = (long) (cur - prev); crec->ha = hav; xdf->recs[xdf->nrec++] = crec; - if (xdl_classify_record(pass, cf, xdf->rhash, xdf->hbits, crec) < 0) + if (xdl_classify_record(pass, cf, crec) < 0) goto abort; } } diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h index 8442bd436efeab..8b8467360ecfc0 100644 --- a/xdiff/xtypes.h +++ b/xdiff/xtypes.h @@ -39,7 +39,6 @@ typedef struct s_chastore { } chastore_t; typedef struct s_xrecord { - struct s_xrecord *next; char const *ptr; long size; unsigned long ha; @@ -48,8 +47,6 @@ typedef struct s_xrecord { typedef struct s_xdfile { chastore_t rcha; long nrec; - unsigned int hbits; - xrecord_t **rhash; long dstart, dend; xrecord_t **recs; char *rchg; From 5a12fd2a8c850df311aa149c9bad87b7cb002abb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= Date: Sat, 27 Sep 2025 21:39:45 +0200 Subject: [PATCH 14/53] doc: change the markup of paragraphs following a nested list item MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asciidoctor and asciidoc.py have different behaviors when a paragraph follows a nested list item. Asciidoctor has a bug[1] that makes it keep a plus sign (+) used to attached paragraphs at the beginning of the paragraph. This commit uses workarounds to avoid this problem by using second level definition lists and open blocks. [1]:https://github.com/asciidoctor/asciidoctor/issues/4704 Signed-off-by: Jean-Noël Avila Signed-off-by: Junio C Hamano --- Documentation/config/extensions.adoc | 23 +++++++++++------------ Documentation/pretty-formats.adoc | 6 ++++-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Documentation/config/extensions.adoc b/Documentation/config/extensions.adoc index 9e2f321a6d776f..49a7598ca5cd13 100644 --- a/Documentation/config/extensions.adoc +++ b/Documentation/config/extensions.adoc @@ -3,8 +3,7 @@ extensions.*:: `core.repositoryFormatVersion` is not `1`. See linkgit:gitrepository-layout[5]. + --- -compatObjectFormat:: +compatObjectFormat::: Specify a compatibility hash algorithm to use. The acceptable values are `sha1` and `sha256`. The value specified must be different from the value of `extensions.objectFormat`. This allows client level @@ -15,18 +14,18 @@ compatObjectFormat:: compatObjectFormat in addition to oids encoded with objectFormat to locally specify objects. -noop:: +noop::: This extension does not change git's behavior at all. It is useful only for testing format-1 compatibility. + For historical reasons, this extension is respected regardless of the `core.repositoryFormatVersion` setting. -noop-v1:: +noop-v1::: This extension does not change git's behavior at all. It is useful only for testing format-1 compatibility. -objectFormat:: +objectFormat::: Specify the hash algorithm to use. The acceptable values are `sha1` and `sha256`. If not specified, `sha1` is assumed. + @@ -34,7 +33,7 @@ Note that this setting should only be set by linkgit:git-init[1] or linkgit:git-clone[1]. Trying to change it after initialization will not work and will produce hard-to-diagnose issues. -partialClone:: +partialClone::: When enabled, indicates that the repo was created with a partial clone (or later performed a partial fetch) and that the remote may have omitted sending certain unwanted objects. Such a remote is called a @@ -46,14 +45,14 @@ The value of this key is the name of the promisor remote. For historical reasons, this extension is respected regardless of the `core.repositoryFormatVersion` setting. -preciousObjects:: +preciousObjects::: If enabled, indicates that objects in the repository MUST NOT be deleted (e.g., by `git-prune` or `git repack -d`). + For historical reasons, this extension is respected regardless of the `core.repositoryFormatVersion` setting. -refStorage:: +refStorage::: Specify the ref storage format to use. The acceptable values are: + include::../ref-storage-format.adoc[] @@ -63,13 +62,13 @@ Note that this setting should only be set by linkgit:git-init[1] or linkgit:git-clone[1]. Trying to change it after initialization will not work and will produce hard-to-diagnose issues. -relativeWorktrees:: +relativeWorktrees::: If enabled, indicates at least one worktree has been linked with relative paths. Automatically set if a worktree has been created or repaired with either the `--relative-paths` option or with the `worktree.useRelativePaths` config set to `true`. -worktreeConfig:: +worktreeConfig::: If enabled, then worktrees will load config settings from the `$GIT_DIR/config.worktree` file in addition to the `$GIT_COMMON_DIR/config` file. Note that `$GIT_COMMON_DIR` and @@ -83,11 +82,12 @@ When enabling this extension, you must be careful to move certain values from the common config file to the main working tree's `config.worktree` file, if present: + +-- * `core.worktree` must be moved from `$GIT_COMMON_DIR/config` to `$GIT_COMMON_DIR/config.worktree`. * If `core.bare` is true, then it must be moved from `$GIT_COMMON_DIR/config` to `$GIT_COMMON_DIR/config.worktree`. - +-- + It may also be beneficial to adjust the locations of `core.sparseCheckout` and `core.sparseCheckoutCone` depending on your desire for customizable @@ -100,4 +100,3 @@ details. + For historical reasons, this extension is respected regardless of the `core.repositoryFormatVersion` setting. --- diff --git a/Documentation/pretty-formats.adoc b/Documentation/pretty-formats.adoc index 9ed0417fc811e5..3d7a8885b696f6 100644 --- a/Documentation/pretty-formats.adoc +++ b/Documentation/pretty-formats.adoc @@ -232,7 +232,7 @@ ref names with custom decorations. The `decorate` string may be followed by a colon and zero or more comma-separated options. Option values may contain literal formatting codes. These must be used for commas (`%x2C`) and closing parentheses (`%x29`), due to their role in the option syntax. -+ + ** `prefix=`: Shown before the list of ref names. Defaults to "{nbsp}+(+". ** `suffix=`: Shown after the list of ref names. Defaults to "+)+". ** `separator=`: Shown between ref names. Defaults to "+,+{nbsp}". @@ -241,10 +241,12 @@ parentheses (`%x29`), due to their role in the option syntax. ** `tag=`: Shown before tag names. Defaults to "`tag:`{nbsp}". + +-- For example, to produce decorations with no wrapping or tag annotations, and spaces as separators: -+ + ++%(decorate:prefix=,suffix=,tag=,separator= )++ +-- ++%(describe++`[: