From 026ad6016070748a66ed9a977ad90efc08df2225 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 21 Oct 2025 13:25:55 -0500 Subject: [PATCH 01/31] builtin/repo: rename repo_info() to cmd_repo_info() Subcommand functions are often prefixed with `cmd_` to denote that they are an entrypoint. Rename repo_info() to cmd_repo_info() accordingly. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/repo.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/repo.c b/builtin/repo.c index bbb0966f2d..eeeab8fbd2 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -136,8 +136,8 @@ static int parse_format_cb(const struct option *opt, return 0; } -static int repo_info(int argc, const char **argv, const char *prefix, - struct repository *repo) +static int cmd_repo_info(int argc, const char **argv, const char *prefix, + struct repository *repo) { enum output_format format = FORMAT_KEYVALUE; struct option options[] = { @@ -161,7 +161,7 @@ int cmd_repo(int argc, const char **argv, const char *prefix, { parse_opt_subcommand_fn *fn = NULL; struct option options[] = { - OPT_SUBCOMMAND("info", &fn, repo_info), + OPT_SUBCOMMAND("info", &fn, cmd_repo_info), OPT_END() }; -- GitLab From eafc03dbe316478acff5eef3b70c037de4758f08 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 21 Oct 2025 13:25:56 -0500 Subject: [PATCH 02/31] ref-filter: allow NULL filter pattern When setting up `struct ref_filter` for filter_refs(), the `name_patterns` field must point to an array of pattern strings even if no patterns are required. To improve this interface, treat a NULL `name_patterns` field the same as when it points to an empty array. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- ref-filter.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 520d2539c9..2cb5a166d6 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2664,7 +2664,7 @@ static int match_name_as_path(const char **pattern, const char *refname, /* Return 1 if the refname matches one of the patterns, otherwise 0. */ static int filter_pattern_match(struct ref_filter *filter, const char *refname) { - if (!*filter->name_patterns) + if (!filter->name_patterns || !*filter->name_patterns) return 1; /* No pattern always matches */ if (filter->match_as_path) return match_name_as_path(filter->name_patterns, refname, @@ -2751,7 +2751,7 @@ static int for_each_fullref_in_pattern(struct ref_filter *filter, return for_each_fullref_with_seek(filter, cb, cb_data, 0); } - if (!filter->name_patterns[0]) { + if (!filter->name_patterns || !filter->name_patterns[0]) { /* no patterns; we have to look at everything */ return for_each_fullref_with_seek(filter, cb, cb_data, 0); } -- GitLab From 6d1997f6cbc10ac03bc450630de4410762f77b6f Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 21 Oct 2025 13:25:57 -0500 Subject: [PATCH 03/31] ref-filter: export ref_kind_from_refname() When filtering refs, `ref_kind_from_refname()` is used to determine the ref type. In a subsequent commit, this same logic is reused when counting refs by type. Export the function to prepare for this change. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- ref-filter.c | 2 +- ref-filter.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 2cb5a166d6..30cc488d8a 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2833,7 +2833,7 @@ struct ref_array_item *ref_array_push(struct ref_array *array, return ref; } -static int ref_kind_from_refname(const char *refname) +int ref_kind_from_refname(const char *refname) { unsigned int i; diff --git a/ref-filter.h b/ref-filter.h index f22ca94b49..4ed1edf09a 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -135,6 +135,8 @@ struct ref_format { OPT_STRVEC(0, "exclude", &(var)->exclude, \ N_("pattern"), N_("exclude refs which match pattern")) +/* Get the reference kind from the provided reference name. */ +int ref_kind_from_refname(const char *refname); /* * API for filtering a set of refs. Based on the type of refs the user * has requested, we iterate through those refs and apply filters -- GitLab From bbb2b9334856ae0a2b18e65e5924a42c31a83c6b Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 21 Oct 2025 13:25:58 -0500 Subject: [PATCH 04/31] builtin/repo: introduce structure subcommand The structure of a repository's history can have huge impacts on the performance and health of the repository itself. Currently, Git lacks a means to surface repository metrics regarding its structure/shape via a single command. Acquiring this information requires users to be familiar with the relevant data points and the various Git commands required to surface them. To fill this gap, supplemental tools such as git-sizer(1) have been developed. To allow users to more readily identify repository structure related information, introduce the "structure" subcommand in git-repo(1). The goal of this subcommand is to eventually provide similar functionality to git-sizer(1), but natively in Git. The initial version of this command only iterates through all references in the repository and tracks the count of branches, tags, remote refs, and other reference types. The corresponding information is displayed in a human-friendly table formatted in a very similar manner to git-sizer(1). The width of each table column is adjusted automatically to satisfy the requirements of the widest row contained. Subsequent commits will surface additional relevant data points to output and also provide other more machine-friendly output formats. Based-on-patch-by: Derrick Stolee Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- Documentation/git-repo.adoc | 10 ++ builtin/repo.c | 200 ++++++++++++++++++++++++++++++++++++ t/meson.build | 1 + t/t1901-repo-structure.sh | 61 +++++++++++ 4 files changed, 272 insertions(+) create mode 100755 t/t1901-repo-structure.sh diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc index 209afd1b61..8193298dd5 100644 --- a/Documentation/git-repo.adoc +++ b/Documentation/git-repo.adoc @@ -9,6 +9,7 @@ SYNOPSIS -------- [synopsis] git repo info [--format=(keyvalue|nul)] [-z] [...] +git repo structure DESCRIPTION ----------- @@ -43,6 +44,15 @@ supported: + `-z` is an alias for `--format=nul`. +`structure`:: + Retrieve statistics about the current repository structure. The + following kinds of information are reported: ++ +* Reference counts categorized by type + ++ +The table output format may change and is not intended for machine parsing. + INFO KEYS --------- In order to obtain a set of values from `git repo info`, you should provide diff --git a/builtin/repo.c b/builtin/repo.c index eeeab8fbd2..e77e8db563 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -4,12 +4,16 @@ #include "environment.h" #include "parse-options.h" #include "quote.h" +#include "ref-filter.h" #include "refs.h" #include "strbuf.h" +#include "string-list.h" #include "shallow.h" +#include "utf8.h" static const char *const repo_usage[] = { "git repo info [--format=(keyvalue|nul)] [-z] [...]", + "git repo structure", NULL }; @@ -156,12 +160,208 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix, return print_fields(argc, argv, repo, format); } +struct ref_stats { + size_t branches; + size_t remotes; + size_t tags; + size_t others; +}; + +struct stats_table { + struct string_list rows; + + int name_col_width; + int value_col_width; +}; + +/* + * Holds column data that gets stored for each row. + */ +struct stats_table_entry { + char *value; +}; + +static void stats_table_vaddf(struct stats_table *table, + struct stats_table_entry *entry, + const char *format, va_list ap) +{ + struct strbuf buf = STRBUF_INIT; + struct string_list_item *item; + char *formatted_name; + int name_width; + + strbuf_vaddf(&buf, format, ap); + formatted_name = strbuf_detach(&buf, NULL); + name_width = utf8_strwidth(formatted_name); + + item = string_list_append_nodup(&table->rows, formatted_name); + item->util = entry; + + if (name_width > table->name_col_width) + table->name_col_width = name_width; + if (entry) { + int value_width = utf8_strwidth(entry->value); + if (value_width > table->value_col_width) + table->value_col_width = value_width; + } +} + +static void stats_table_addf(struct stats_table *table, const char *format, ...) +{ + va_list ap; + + va_start(ap, format); + stats_table_vaddf(table, NULL, format, ap); + va_end(ap); +} + +static void stats_table_count_addf(struct stats_table *table, size_t value, + const char *format, ...) +{ + struct stats_table_entry *entry; + va_list ap; + + CALLOC_ARRAY(entry, 1); + entry->value = xstrfmt("%" PRIuMAX, (uintmax_t)value); + + va_start(ap, format); + stats_table_vaddf(table, entry, format, ap); + va_end(ap); +} + +static inline size_t get_total_reference_count(struct ref_stats *stats) +{ + return stats->branches + stats->remotes + stats->tags + stats->others; +} + +static void stats_table_setup_structure(struct stats_table *table, + struct ref_stats *refs) +{ + size_t ref_total; + + ref_total = get_total_reference_count(refs); + stats_table_addf(table, "* %s", _("References")); + stats_table_count_addf(table, ref_total, " * %s", _("Count")); + stats_table_count_addf(table, refs->branches, " * %s", _("Branches")); + stats_table_count_addf(table, refs->tags, " * %s", _("Tags")); + stats_table_count_addf(table, refs->remotes, " * %s", _("Remotes")); + stats_table_count_addf(table, refs->others, " * %s", _("Others")); +} + +static void stats_table_print_structure(const struct stats_table *table) +{ + const char *name_col_title = _("Repository structure"); + const char *value_col_title = _("Value"); + int name_col_width = utf8_strwidth(name_col_title); + int value_col_width = utf8_strwidth(value_col_title); + struct string_list_item *item; + + if (table->name_col_width > name_col_width) + name_col_width = table->name_col_width; + if (table->value_col_width > value_col_width) + value_col_width = table->value_col_width; + + printf("| %-*s | %-*s |\n", name_col_width, name_col_title, + value_col_width, value_col_title); + printf("| "); + for (int i = 0; i < name_col_width; i++) + putchar('-'); + printf(" | "); + for (int i = 0; i < value_col_width; i++) + putchar('-'); + printf(" |\n"); + + for_each_string_list_item(item, &table->rows) { + struct stats_table_entry *entry = item->util; + const char *value = ""; + + if (entry) { + struct stats_table_entry *entry = item->util; + value = entry->value; + } + + printf("| %-*s | %*s |\n", name_col_width, item->string, + value_col_width, value); + } +} + +static void stats_table_clear(struct stats_table *table) +{ + struct stats_table_entry *entry; + struct string_list_item *item; + + for_each_string_list_item(item, &table->rows) { + entry = item->util; + if (entry) + free(entry->value); + } + + string_list_clear(&table->rows, 1); +} + +static int count_references(const char *refname, + const char *referent UNUSED, + const struct object_id *oid UNUSED, + int flags UNUSED, void *cb_data) +{ + struct ref_stats *stats = cb_data; + + switch (ref_kind_from_refname(refname)) { + case FILTER_REFS_BRANCHES: + stats->branches++; + break; + case FILTER_REFS_REMOTES: + stats->remotes++; + break; + case FILTER_REFS_TAGS: + stats->tags++; + break; + case FILTER_REFS_OTHERS: + stats->others++; + break; + default: + BUG("unexpected reference type"); + } + + return 0; +} + +static void structure_count_references(struct ref_stats *stats, + struct repository *repo) +{ + refs_for_each_ref(get_main_ref_store(repo), count_references, &stats); +} + +static int cmd_repo_structure(int argc, const char **argv, const char *prefix, + struct repository *repo) +{ + struct stats_table table = { + .rows = STRING_LIST_INIT_DUP, + }; + struct ref_stats stats = { 0 }; + struct option options[] = { 0 }; + + argc = parse_options(argc, argv, prefix, options, repo_usage, 0); + if (argc) + usage(_("too many arguments")); + + structure_count_references(&stats, repo); + + stats_table_setup_structure(&table, &stats); + stats_table_print_structure(&table); + + stats_table_clear(&table); + + return 0; +} + int cmd_repo(int argc, const char **argv, const char *prefix, struct repository *repo) { parse_opt_subcommand_fn *fn = NULL; struct option options[] = { OPT_SUBCOMMAND("info", &fn, cmd_repo_info), + OPT_SUBCOMMAND("structure", &fn, cmd_repo_structure), OPT_END() }; diff --git a/t/meson.build b/t/meson.build index 7974795fe4..9e426f8edc 100644 --- a/t/meson.build +++ b/t/meson.build @@ -236,6 +236,7 @@ integration_tests = [ 't1701-racy-split-index.sh', 't1800-hook.sh', 't1900-repo.sh', + 't1901-repo-structure.sh', 't2000-conflict-when-checking-files-out.sh', 't2002-checkout-cache-u.sh', 't2003-checkout-cache-mkdir.sh', diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh new file mode 100755 index 0000000000..e592eea0eb --- /dev/null +++ b/t/t1901-repo-structure.sh @@ -0,0 +1,61 @@ +#!/bin/sh + +test_description='test git repo structure' + +. ./test-lib.sh + +test_expect_success 'empty repository' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + cat >expect <<-\EOF && + | Repository structure | Value | + | -------------------- | ----- | + | * References | | + | * Count | 0 | + | * Branches | 0 | + | * Tags | 0 | + | * Remotes | 0 | + | * Others | 0 | + EOF + + git repo structure >out 2>err && + + test_cmp expect out && + test_line_count = 0 err + ) +' + +test_expect_success 'repository with references' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + git commit --allow-empty -m init && + git tag -a foo -m bar && + + oid="$(git rev-parse HEAD)" && + git update-ref refs/remotes/origin/foo "$oid" && + + git notes add -m foo && + + cat >expect <<-\EOF && + | Repository structure | Value | + | -------------------- | ----- | + | * References | | + | * Count | 4 | + | * Branches | 1 | + | * Tags | 1 | + | * Remotes | 1 | + | * Others | 1 | + EOF + + git repo structure >out 2>err && + + test_cmp expect out && + test_line_count = 0 err + ) +' + +test_done -- GitLab From eb5cf58ffcd4bb117c870d448b0df0193df52c82 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 21 Oct 2025 13:25:59 -0500 Subject: [PATCH 05/31] builtin/repo: add object counts in structure output The amount of objects in a repository can provide insight regarding its shape. To surface this information, use the path-walk API to count the number of reachable objects in the repository by object type. All regular references are used to determine the reachable set of objects. The object counts are appended to the same table containing the reference information. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- Documentation/git-repo.adoc | 1 + builtin/repo.c | 105 +++++++++++++++++++++++++++++++++--- t/t1901-repo-structure.sh | 19 ++++++- 3 files changed, 117 insertions(+), 8 deletions(-) diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc index 8193298dd5..ae62d2415f 100644 --- a/Documentation/git-repo.adoc +++ b/Documentation/git-repo.adoc @@ -49,6 +49,7 @@ supported: following kinds of information are reported: + * Reference counts categorized by type +* Reachable object counts categorized by type + The table output format may change and is not intended for machine parsing. diff --git a/builtin/repo.c b/builtin/repo.c index e77e8db563..f39f06ee8c 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -3,9 +3,11 @@ #include "builtin.h" #include "environment.h" #include "parse-options.h" +#include "path-walk.h" #include "quote.h" #include "ref-filter.h" #include "refs.h" +#include "revision.h" #include "strbuf.h" #include "string-list.h" #include "shallow.h" @@ -167,6 +169,18 @@ struct ref_stats { size_t others; }; +struct object_stats { + size_t tags; + size_t commits; + size_t trees; + size_t blobs; +}; + +struct repo_structure { + struct ref_stats refs; + struct object_stats objects; +}; + struct stats_table { struct string_list rows; @@ -234,9 +248,17 @@ static inline size_t get_total_reference_count(struct ref_stats *stats) return stats->branches + stats->remotes + stats->tags + stats->others; } +static inline size_t get_total_object_count(struct object_stats *stats) +{ + return stats->tags + stats->commits + stats->trees + stats->blobs; +} + static void stats_table_setup_structure(struct stats_table *table, - struct ref_stats *refs) + struct repo_structure *stats) { + struct object_stats *objects = &stats->objects; + struct ref_stats *refs = &stats->refs; + size_t object_total; size_t ref_total; ref_total = get_total_reference_count(refs); @@ -246,6 +268,15 @@ static void stats_table_setup_structure(struct stats_table *table, stats_table_count_addf(table, refs->tags, " * %s", _("Tags")); stats_table_count_addf(table, refs->remotes, " * %s", _("Remotes")); stats_table_count_addf(table, refs->others, " * %s", _("Others")); + + object_total = get_total_object_count(objects); + stats_table_addf(table, ""); + stats_table_addf(table, "* %s", _("Reachable objects")); + stats_table_count_addf(table, object_total, " * %s", _("Count")); + stats_table_count_addf(table, objects->commits, " * %s", _("Commits")); + stats_table_count_addf(table, objects->trees, " * %s", _("Trees")); + stats_table_count_addf(table, objects->blobs, " * %s", _("Blobs")); + stats_table_count_addf(table, objects->tags, " * %s", _("Tags")); } static void stats_table_print_structure(const struct stats_table *table) @@ -299,12 +330,18 @@ static void stats_table_clear(struct stats_table *table) string_list_clear(&table->rows, 1); } +struct count_references_data { + struct ref_stats *stats; + struct rev_info *revs; +}; + static int count_references(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, + const struct object_id *oid, int flags UNUSED, void *cb_data) { - struct ref_stats *stats = cb_data; + struct count_references_data *data = cb_data; + struct ref_stats *stats = data->stats; switch (ref_kind_from_refname(refname)) { case FILTER_REFS_BRANCHES: @@ -323,13 +360,64 @@ static int count_references(const char *refname, BUG("unexpected reference type"); } + /* + * While iterating through references for counting, also add OIDs in + * preparation for the path walk. + */ + add_pending_oid(data->revs, NULL, oid, 0); + return 0; } static void structure_count_references(struct ref_stats *stats, + struct rev_info *revs, struct repository *repo) { - refs_for_each_ref(get_main_ref_store(repo), count_references, &stats); + struct count_references_data data = { + .stats = stats, + .revs = revs, + }; + + refs_for_each_ref(get_main_ref_store(repo), count_references, &data); +} + + +static int count_objects(const char *path UNUSED, struct oid_array *oids, + enum object_type type, void *cb_data) +{ + struct object_stats *stats = cb_data; + + switch (type) { + case OBJ_TAG: + stats->tags += oids->nr; + break; + case OBJ_COMMIT: + stats->commits += oids->nr; + break; + case OBJ_TREE: + stats->trees += oids->nr; + break; + case OBJ_BLOB: + stats->blobs += oids->nr; + break; + default: + BUG("invalid object type"); + } + + return 0; +} + +static void structure_count_objects(struct object_stats *stats, + struct rev_info *revs) +{ + struct path_walk_info info = PATH_WALK_INFO_INIT; + + info.revs = revs; + info.path_fn = count_objects; + info.path_fn_data = stats; + + walk_objects_by_path(&info); + path_walk_info_clear(&info); } static int cmd_repo_structure(int argc, const char **argv, const char *prefix, @@ -338,19 +426,24 @@ static int cmd_repo_structure(int argc, const char **argv, const char *prefix, struct stats_table table = { .rows = STRING_LIST_INIT_DUP, }; - struct ref_stats stats = { 0 }; + struct repo_structure stats = { 0 }; + struct rev_info revs; struct option options[] = { 0 }; argc = parse_options(argc, argv, prefix, options, repo_usage, 0); if (argc) usage(_("too many arguments")); - structure_count_references(&stats, repo); + repo_init_revisions(repo, &revs, prefix); + + structure_count_references(&stats.refs, &revs, repo); + structure_count_objects(&stats.objects, &revs); stats_table_setup_structure(&table, &stats); stats_table_print_structure(&table); stats_table_clear(&table); + release_revisions(&revs); return 0; } diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh index e592eea0eb..c32cf4e239 100755 --- a/t/t1901-repo-structure.sh +++ b/t/t1901-repo-structure.sh @@ -18,6 +18,13 @@ test_expect_success 'empty repository' ' | * Tags | 0 | | * Remotes | 0 | | * Others | 0 | + | | | + | * Reachable objects | | + | * Count | 0 | + | * Commits | 0 | + | * Trees | 0 | + | * Blobs | 0 | + | * Tags | 0 | EOF git repo structure >out 2>err && @@ -27,17 +34,18 @@ test_expect_success 'empty repository' ' ) ' -test_expect_success 'repository with references' ' +test_expect_success 'repository with references and objects' ' test_when_finished "rm -rf repo" && git init repo && ( cd repo && - git commit --allow-empty -m init && + test_commit_bulk 42 && git tag -a foo -m bar && oid="$(git rev-parse HEAD)" && git update-ref refs/remotes/origin/foo "$oid" && + # Also creates a commit, tree, and blob. git notes add -m foo && cat >expect <<-\EOF && @@ -49,6 +57,13 @@ test_expect_success 'repository with references' ' | * Tags | 1 | | * Remotes | 1 | | * Others | 1 | + | | | + | * Reachable objects | | + | * Count | 130 | + | * Commits | 43 | + | * Trees | 43 | + | * Blobs | 43 | + | * Tags | 1 | EOF git repo structure >out 2>err && -- GitLab From 17215675b5a2c2eab54b295a7e92d953af2e8779 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 21 Oct 2025 13:26:00 -0500 Subject: [PATCH 06/31] builtin/repo: add keyvalue and nul format for structure stats All repository structure stats are outputted in a human-friendly table form. This format is not suitable for machine parsing. Add a --format option that supports three output modes: `table`, `keyvalue`, and `nul`. The `table` mode is the default format and prints the same table output as before. With the `keyvalue` mode, each line of output contains a key-value pair of a repository stat. The '=' character is used to delimit between keys and values. The `nul` mode is similar to `keyvalue`, but key-values are delimited by a NUL character instead of a newline. Also, instead of a '=' character to delimit between keys and values, a newline character is used. This allows stat values to support special characters without having to cquote them. These two new modes provides output that is more machine-friendly. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- Documentation/git-repo.adoc | 25 +++++++++++++++-- builtin/repo.c | 55 ++++++++++++++++++++++++++++++++++--- t/t1901-repo-structure.sh | 33 ++++++++++++++++++++++ 3 files changed, 106 insertions(+), 7 deletions(-) diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc index ae62d2415f..ce43cb19c8 100644 --- a/Documentation/git-repo.adoc +++ b/Documentation/git-repo.adoc @@ -9,7 +9,7 @@ SYNOPSIS -------- [synopsis] git repo info [--format=(keyvalue|nul)] [-z] [...] -git repo structure +git repo structure [--format=(table|keyvalue|nul)] DESCRIPTION ----------- @@ -44,7 +44,7 @@ supported: + `-z` is an alias for `--format=nul`. -`structure`:: +`structure [--format=(table|keyvalue|nul)]`:: Retrieve statistics about the current repository structure. The following kinds of information are reported: + @@ -52,7 +52,26 @@ supported: * Reachable object counts categorized by type + -The table output format may change and is not intended for machine parsing. +The output format can be chosen through the flag `--format`. Three formats are +supported: ++ +`table`::: + Outputs repository stats in a human-friendly table. This format may + change and is not intended for machine parsing. This is the default + format. + +`keyvalue`::: + Each line of output contains a key-value pair for a repository stat. + The '=' character is used to delimit between the key and the value. + Values containing "unusual" characters are quoted as explained for the + configuration variable `core.quotePath` (see linkgit:git-config[1]). + +`nul`::: + Similar to `keyvalue`, but uses a NUL character to delimit between + key-value pairs instead of a newline. Also uses a newline character as + the delimiter between the key and value instead of '='. Unlike the + `keyvalue` format, values containing "unusual" characters are never + quoted. INFO KEYS --------- diff --git a/builtin/repo.c b/builtin/repo.c index f39f06ee8c..1754cc7e5d 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -15,13 +15,14 @@ static const char *const repo_usage[] = { "git repo info [--format=(keyvalue|nul)] [-z] [...]", - "git repo structure", + "git repo structure [--format=(table|keyvalue|nul)]", NULL }; typedef int get_value_fn(struct repository *repo, struct strbuf *buf); enum output_format { + FORMAT_TABLE, FORMAT_KEYVALUE, FORMAT_NUL_TERMINATED, }; @@ -136,6 +137,8 @@ static int parse_format_cb(const struct option *opt, *format = FORMAT_NUL_TERMINATED; else if (!strcmp(arg, "keyvalue")) *format = FORMAT_KEYVALUE; + else if (!strcmp(arg, "table")) + *format = FORMAT_TABLE; else die(_("invalid format '%s'"), arg); @@ -158,6 +161,8 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix, }; argc = parse_options(argc, argv, prefix, options, repo_usage, 0); + if (format != FORMAT_KEYVALUE && format != FORMAT_NUL_TERMINATED) + die(_("unsupported output format")); return print_fields(argc, argv, repo, format); } @@ -330,6 +335,30 @@ static void stats_table_clear(struct stats_table *table) string_list_clear(&table->rows, 1); } +static void structure_keyvalue_print(struct repo_structure *stats, + char key_delim, char value_delim) +{ + printf("references.branches.count%c%" PRIuMAX "%c", key_delim, + (uintmax_t)stats->refs.branches, value_delim); + printf("references.tags.count%c%" PRIuMAX "%c", key_delim, + (uintmax_t)stats->refs.tags, value_delim); + printf("references.remotes.count%c%" PRIuMAX "%c", key_delim, + (uintmax_t)stats->refs.remotes, value_delim); + printf("references.others.count%c%" PRIuMAX "%c", key_delim, + (uintmax_t)stats->refs.others, value_delim); + + printf("objects.commits.count%c%" PRIuMAX "%c", key_delim, + (uintmax_t)stats->objects.commits, value_delim); + printf("objects.trees.count%c%" PRIuMAX "%c", key_delim, + (uintmax_t)stats->objects.trees, value_delim); + printf("objects.blobs.count%c%" PRIuMAX "%c", key_delim, + (uintmax_t)stats->objects.blobs, value_delim); + printf("objects.tags.count%c%" PRIuMAX "%c", key_delim, + (uintmax_t)stats->objects.tags, value_delim); + + fflush(stdout); +} + struct count_references_data { struct ref_stats *stats; struct rev_info *revs; @@ -426,9 +455,15 @@ static int cmd_repo_structure(int argc, const char **argv, const char *prefix, struct stats_table table = { .rows = STRING_LIST_INIT_DUP, }; + enum output_format format = FORMAT_TABLE; struct repo_structure stats = { 0 }; struct rev_info revs; - struct option options[] = { 0 }; + struct option options[] = { + OPT_CALLBACK_F(0, "format", &format, N_("format"), + N_("output format"), + PARSE_OPT_NONEG, parse_format_cb), + OPT_END() + }; argc = parse_options(argc, argv, prefix, options, repo_usage, 0); if (argc) @@ -439,8 +474,20 @@ static int cmd_repo_structure(int argc, const char **argv, const char *prefix, structure_count_references(&stats.refs, &revs, repo); structure_count_objects(&stats.objects, &revs); - stats_table_setup_structure(&table, &stats); - stats_table_print_structure(&table); + switch (format) { + case FORMAT_TABLE: + stats_table_setup_structure(&table, &stats); + stats_table_print_structure(&table); + break; + case FORMAT_KEYVALUE: + structure_keyvalue_print(&stats, '=', '\n'); + break; + case FORMAT_NUL_TERMINATED: + structure_keyvalue_print(&stats, '\n', '\0'); + break; + default: + BUG("invalid output format"); + } stats_table_clear(&table); release_revisions(&revs); diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh index c32cf4e239..14bd8aede5 100755 --- a/t/t1901-repo-structure.sh +++ b/t/t1901-repo-structure.sh @@ -73,4 +73,37 @@ test_expect_success 'repository with references and objects' ' ) ' +test_expect_success 'keyvalue and nul format' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit_bulk 42 && + git tag -a foo -m bar && + + cat >expect <<-\EOF && + references.branches.count=1 + references.tags.count=1 + references.remotes.count=0 + references.others.count=0 + objects.commits.count=42 + objects.trees.count=42 + objects.blobs.count=42 + objects.tags.count=1 + EOF + + git repo structure --format=keyvalue >out 2>err && + + test_cmp expect out && + test_line_count = 0 err && + + # Replace key and value delimiters for nul format. + tr "\n=" "\0\n" expect_nul && + git repo structure --format=nul >out 2>err && + + test_cmp expect_nul out && + test_line_count = 0 err + ) +' + test_done -- GitLab From 16a93c03c7824a40b034a6ee1cb1c68c8ef48682 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 21 Oct 2025 13:26:01 -0500 Subject: [PATCH 07/31] builtin/repo: add progress meter for structure stats When using the structure subcommand for git-repo(1), evaluating a repository may take some time depending on its shape. Add a progress meter to provide feedback to the user about what is happening. The progress meter is enabled by default when the command is executed from a tty. It can also be explicitly enabled/disabled via the --[no-]progress option. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- builtin/repo.c | 46 ++++++++++++++++++++++++++++++++++----- t/t1901-repo-structure.sh | 20 +++++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/builtin/repo.c b/builtin/repo.c index 1754cc7e5d..9d4749f79b 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -4,6 +4,7 @@ #include "environment.h" #include "parse-options.h" #include "path-walk.h" +#include "progress.h" #include "quote.h" #include "ref-filter.h" #include "refs.h" @@ -362,6 +363,7 @@ static void structure_keyvalue_print(struct repo_structure *stats, struct count_references_data { struct ref_stats *stats; struct rev_info *revs; + struct progress *progress; }; static int count_references(const char *refname, @@ -371,6 +373,7 @@ static int count_references(const char *refname, { struct count_references_data *data = cb_data; struct ref_stats *stats = data->stats; + size_t ref_count; switch (ref_kind_from_refname(refname)) { case FILTER_REFS_BRANCHES: @@ -395,26 +398,41 @@ static int count_references(const char *refname, */ add_pending_oid(data->revs, NULL, oid, 0); + ref_count = get_total_reference_count(stats); + display_progress(data->progress, ref_count); + return 0; } static void structure_count_references(struct ref_stats *stats, struct rev_info *revs, - struct repository *repo) + struct repository *repo, + int show_progress) { struct count_references_data data = { .stats = stats, .revs = revs, }; + if (show_progress) + data.progress = start_delayed_progress(repo, + _("Counting references"), 0); + refs_for_each_ref(get_main_ref_store(repo), count_references, &data); + stop_progress(&data.progress); } +struct count_objects_data { + struct object_stats *stats; + struct progress *progress; +}; static int count_objects(const char *path UNUSED, struct oid_array *oids, enum object_type type, void *cb_data) { - struct object_stats *stats = cb_data; + struct count_objects_data *data = cb_data; + struct object_stats *stats = data->stats; + size_t object_count; switch (type) { case OBJ_TAG: @@ -433,20 +451,31 @@ static int count_objects(const char *path UNUSED, struct oid_array *oids, BUG("invalid object type"); } + object_count = get_total_object_count(stats); + display_progress(data->progress, object_count); + return 0; } static void structure_count_objects(struct object_stats *stats, - struct rev_info *revs) + struct rev_info *revs, + struct repository *repo, int show_progress) { struct path_walk_info info = PATH_WALK_INFO_INIT; + struct count_objects_data data = { + .stats = stats, + }; info.revs = revs; info.path_fn = count_objects; - info.path_fn_data = stats; + info.path_fn_data = &data; + + if (show_progress) + data.progress = start_delayed_progress(repo, _("Counting objects"), 0); walk_objects_by_path(&info); path_walk_info_clear(&info); + stop_progress(&data.progress); } static int cmd_repo_structure(int argc, const char **argv, const char *prefix, @@ -458,10 +487,12 @@ static int cmd_repo_structure(int argc, const char **argv, const char *prefix, enum output_format format = FORMAT_TABLE; struct repo_structure stats = { 0 }; struct rev_info revs; + int show_progress = -1; struct option options[] = { OPT_CALLBACK_F(0, "format", &format, N_("format"), N_("output format"), PARSE_OPT_NONEG, parse_format_cb), + OPT_BOOL(0, "progress", &show_progress, N_("show progress")), OPT_END() }; @@ -471,8 +502,11 @@ static int cmd_repo_structure(int argc, const char **argv, const char *prefix, repo_init_revisions(repo, &revs, prefix); - structure_count_references(&stats.refs, &revs, repo); - structure_count_objects(&stats.objects, &revs); + if (show_progress < 0) + show_progress = isatty(2); + + structure_count_references(&stats.refs, &revs, repo, show_progress); + structure_count_objects(&stats.objects, &revs, repo, show_progress); switch (format) { case FORMAT_TABLE: diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh index 14bd8aede5..36a71a144e 100755 --- a/t/t1901-repo-structure.sh +++ b/t/t1901-repo-structure.sh @@ -106,4 +106,24 @@ test_expect_success 'keyvalue and nul format' ' ) ' +test_expect_success 'progress meter option' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit foo && + + GIT_PROGRESS_DELAY=0 git repo structure --progress >out 2>err && + + test_file_not_empty out && + test_grep "Counting references: 2, done." err && + test_grep "Counting objects: 3, done." err && + + GIT_PROGRESS_DELAY=0 git repo structure --no-progress >out 2>err && + + test_file_not_empty out && + test_line_count = 0 err + ) +' + test_done -- GitLab From ed4b1df14d3994625f156406cb525d692e4fd369 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 18 Sep 2025 12:08:33 +0200 Subject: [PATCH 08/31] refs: improvements and fixes for peeling tags Hi, originally, all I wanted to do was the last patch: a small performance optimization that stops parsing objects in git-for-each-ref(1) unless we really need to parse them. But that fix cause one specific test to fail, and only with the reftable backend. So this led me down the rabbit hole of tag peeling, ending up with this patch series. The series is structured like follows: - Patches 1 to 8 refactor our codebase so that we don't have the `peel_iterated_object()` hack anymore. I just found it hard to follow and thought it shouldn't be too hard to get rid of it. - Patches 9 and 10 remove infrastructure that we don't need anymore after the first couple of patches. - Patches 11 to 13 fix a couple of issues with peeled tags that I found. The underlying issue is that tags store both the tagged object and their type, but this information may not match. We never verify the actual object type though when allocating the tagged object, so this only blows up much later. - Patch 14 was my original motivation, a small performance optimization. I'm not particularly fond of the patches 11 to 13. It feels more like playing whack-a-mole, and I very much assume that there still are edge cases where we should properly verify the tagged object type. But changing it in `parse_tag_buffer()` itself causes a bunch of tests to fail where we intentionally create such corrupted tags. So I didn't really dare to touch that part, to be honest. If anybody has suggestions for an alternative approach I'd be very open to it. Changes in v4: - Improve `get_or_parse_object()` to have better ergonomics. - Link to v3: https://lore.kernel.org/r/20251022-b4-pks-ref-filter-skip-parsing-objects-v3-0-eb9f71985ef0@pks.im Changes in v3: - I've rebuilt the topic on 133d151831 (The twenty-first batch, 2025-10-20) with - tb/incremental-midx-part-3.1 at 935ab44a0a (builtin/repack.c: clean up unused `#include`s, 2025-10-15) - jt/repo-structure 16a93c03c7 at (builtin/repo: add progress meter for structure stats, 2025-10-21) merged into it. This is done to fix a couple of merge conflicts with "seen". Both of the topics are only in "seen" right now, but they are close to be merged. - Link to v2: https://lore.kernel.org/r/20251008-b4-pks-ref-filter-skip-parsing-objects-v2-0-76e30d5c9542@pks.im Changes in v2: - A couple of improvements to commit messages. - A new commit that ensures that `struct ref_iterator::ref` is always zeroed out to protect against stale state. - Link to v1: https://lore.kernel.org/r/20251007-b4-pks-ref-filter-skip-parsing-objects-v1-0-916cc7c6886b@pks.im Thanks! Patrick To: git@vger.kernel.org Cc: Kristoffer Haugsbakk Cc: Karthik Nayak Cc: Taylor Blau Cc: Junio C Hamano Cc: Justin Tobler --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 4, "change-id": "20250918-b4-pks-ref-filter-skip-parsing-objects-f0d1f6af4a9f", "prefixes": [], "history": { "v1": [ "20251007-b4-pks-ref-filter-skip-parsing-objects-v1-0-916cc7c6886b@pks.im" ], "v2": [ "20251008-b4-pks-ref-filter-skip-parsing-objects-v2-0-76e30d5c9542@pks.im" ], "v3": [ "20251022-b4-pks-ref-filter-skip-parsing-objects-v3-0-eb9f71985ef0@pks.im" ] } } } -- GitLab From a8aafa4392bf55a4dcfe38c72427146cc77c11ea Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 22 Oct 2025 07:08:47 +0200 Subject: [PATCH 09/31] refs: introduce wrapper struct for `each_ref_fn` The `each_ref_fn` callback function type is used across our code base for several different functions that iterate through reference. There's a bunch of callbacks implementing this type, which makes any changes to the callback signature extremely noisy. An example of the required churn is e8207717f1 (refs: add referent to each_ref_fn, 2024-08-09): adding a single argument required us to change 48 files. It was already proposed back then [1] that we might want to introduce a wrapper structure to alleviate the pain going forward. While this of course requires the same kind of global refactoring as just introducing a new parameter, it at least allows us to more change the callback type afterwards by just extending the wrapper structure. One counterargument to this refactoring is that it makes the structure more opaque. While it is obvious which callsites need to be fixed up when we change the function type, it's not obvious anymore once we use a structure. That being said, we only have a handful of sites that actually need to populate this wrapper structure: our ref backends, "refs/iterator.c" as well as very few sites that invoke the iterator callback functions directly. Introduce this wrapper structure so that we can adapt the iterator interfaces more readily. [1]: Signed-off-by: Patrick Steinhardt --- bisect.c | 24 ++++++------- builtin/bisect.c | 17 +++------- builtin/checkout.c | 6 ++-- builtin/describe.c | 18 +++++----- builtin/fetch.c | 13 +++---- builtin/fsck.c | 33 ++++++++++-------- builtin/gc.c | 15 ++++----- builtin/name-rev.c | 17 +++++----- builtin/pack-objects.c | 27 ++++++--------- builtin/receive-pack.c | 13 ++++--- builtin/remote.c | 44 +++++++++++------------- builtin/replace.c | 21 +++++------- builtin/repo.c | 9 ++--- builtin/rev-parse.c | 12 +++---- builtin/show-branch.c | 35 +++++++++---------- builtin/show-ref.c | 20 +++++------ builtin/submodule--helper.c | 10 ++---- builtin/worktree.c | 6 +--- commit-graph.c | 14 ++++---- delta-islands.c | 9 +++-- fetch-pack.c | 16 +++------ help.c | 10 +++--- http-backend.c | 20 +++++------ log-tree.c | 24 ++++++------- ls-refs.c | 36 ++++++++++++-------- midx-write.c | 17 +++++----- negotiator/default.c | 7 ++-- negotiator/skipping.c | 7 ++-- notes.c | 8 ++--- object-name.c | 10 +++--- pseudo-merge.c | 21 +++++------- reachable.c | 9 +++-- ref-filter.c | 24 +++++++------ reflog.c | 9 ++--- refs.c | 67 +++++++++++++++++++++---------------- refs.h | 26 +++++++++++--- refs/files-backend.c | 7 ++-- refs/iterator.c | 9 ++++- remote.c | 27 +++++++-------- repack-midx.c | 16 ++++----- replace-object.c | 16 ++++----- revision.c | 12 +++---- server-info.c | 12 +++---- shallow.c | 16 +++------ submodule.c | 12 ++----- t/helper/test-ref-store.c | 5 ++- upload-pack.c | 29 +++++++--------- walker.c | 8 ++--- worktree.c | 11 ++++-- 49 files changed, 392 insertions(+), 462 deletions(-) diff --git a/bisect.c b/bisect.c index a6dc76b15c..326b59c0dc 100644 --- a/bisect.c +++ b/bisect.c @@ -450,21 +450,20 @@ void find_bisection(struct commit_list **commit_list, int *reaches, clear_commit_weight(&commit_weight); } -static int register_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, void *cb_data UNUSED) +static int register_ref(const struct reference *ref, void *cb_data UNUSED) { struct strbuf good_prefix = STRBUF_INIT; strbuf_addstr(&good_prefix, term_good); strbuf_addstr(&good_prefix, "-"); - if (!strcmp(refname, term_bad)) { + if (!strcmp(ref->name, term_bad)) { free(current_bad_oid); current_bad_oid = xmalloc(sizeof(*current_bad_oid)); - oidcpy(current_bad_oid, oid); - } else if (starts_with(refname, good_prefix.buf)) { - oid_array_append(&good_revs, oid); - } else if (starts_with(refname, "skip-")) { - oid_array_append(&skipped_revs, oid); + oidcpy(current_bad_oid, ref->oid); + } else if (starts_with(ref->name, good_prefix.buf)) { + oid_array_append(&good_revs, ref->oid); + } else if (starts_with(ref->name, "skip-")) { + oid_array_append(&skipped_revs, ref->oid); } strbuf_release(&good_prefix); @@ -1178,14 +1177,11 @@ int estimate_bisect_steps(int all) return (e < 3 * x) ? n : n - 1; } -static int mark_for_removal(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag UNUSED, void *cb_data) +static int mark_for_removal(const struct reference *ref, void *cb_data) { struct string_list *refs = cb_data; - char *ref = xstrfmt("refs/bisect%s", refname); - string_list_append(refs, ref); + char *bisect_ref = xstrfmt("refs/bisect%s", ref->name); + string_list_append(refs, bisect_ref); return 0; } diff --git a/builtin/bisect.c b/builtin/bisect.c index 8b8d870cd1..5b2024be62 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -358,10 +358,7 @@ static int check_and_set_terms(struct bisect_terms *terms, const char *cmd) return 0; } -static int inc_nr(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag UNUSED, void *cb_data) +static int inc_nr(const struct reference *ref UNUSED, void *cb_data) { unsigned int *nr = (unsigned int *)cb_data; (*nr)++; @@ -549,12 +546,11 @@ static int bisect_append_log_quoted(const char **argv) return res; } -static int add_bisect_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, void *cb) +static int add_bisect_ref(const struct reference *ref, void *cb) { struct add_bisect_ref_data *data = cb; - add_pending_oid(data->revs, refname, oid, data->object_flags); + add_pending_oid(data->revs, ref->name, ref->oid, data->object_flags); return 0; } @@ -1165,12 +1161,9 @@ static int bisect_visualize(struct bisect_terms *terms, int argc, return run_command(&cmd); } -static int get_first_good(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int get_first_good(const struct reference *ref, void *cb_data) { - oidcpy(cb_data, oid); + oidcpy(cb_data, ref->oid); return 1; } diff --git a/builtin/checkout.c b/builtin/checkout.c index f9453473fe..66b69df6e6 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1063,11 +1063,9 @@ static void update_refs_for_switch(const struct checkout_opts *opts, report_tracking(new_branch_info); } -static int add_pending_uninteresting_ref(const char *refname, const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int add_pending_uninteresting_ref(const struct reference *ref, void *cb_data) { - add_pending_oid(cb_data, refname, oid, UNINTERESTING); + add_pending_oid(cb_data, ref->name, ref->oid, UNINTERESTING); return 0; } diff --git a/builtin/describe.c b/builtin/describe.c index ffaf8d9f0a..7954535044 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -154,20 +154,19 @@ static void add_to_known_names(const char *path, } } -static int get_name(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int get_name(const struct reference *ref, void *cb_data UNUSED) { int is_tag = 0; struct object_id peeled; int is_annotated, prio; const char *path_to_match = NULL; - if (skip_prefix(path, "refs/tags/", &path_to_match)) { + if (skip_prefix(ref->name, "refs/tags/", &path_to_match)) { is_tag = 1; } else if (all) { if ((exclude_patterns.nr || patterns.nr) && - !skip_prefix(path, "refs/heads/", &path_to_match) && - !skip_prefix(path, "refs/remotes/", &path_to_match)) { + !skip_prefix(ref->name, "refs/heads/", &path_to_match) && + !skip_prefix(ref->name, "refs/remotes/", &path_to_match)) { /* Only accept reference of known type if there are match/exclude patterns */ return 0; } @@ -209,10 +208,10 @@ static int get_name(const char *path, const char *referent UNUSED, const struct } /* Is it annotated? */ - if (!peel_iterated_oid(the_repository, oid, &peeled)) { - is_annotated = !oideq(oid, &peeled); + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) { + is_annotated = !oideq(ref->oid, &peeled); } else { - oidcpy(&peeled, oid); + oidcpy(&peeled, ref->oid); is_annotated = 0; } @@ -229,7 +228,8 @@ static int get_name(const char *path, const char *referent UNUSED, const struct else prio = 0; - add_to_known_names(all ? path + 5 : path + 10, &peeled, prio, oid); + add_to_known_names(all ? ref->name + 5 : ref->name + 10, + &peeled, prio, ref->oid); return 0; } diff --git a/builtin/fetch.c b/builtin/fetch.c index c7ff3480fb..7052e6ff21 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -289,13 +289,11 @@ static struct refname_hash_entry *refname_hash_add(struct hashmap *map, return ent; } -static int add_one_refname(const char *refname, const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, void *cbdata) +static int add_one_refname(const struct reference *ref, void *cbdata) { struct hashmap *refname_map = cbdata; - (void) refname_hash_add(refname_map, refname, oid); + (void) refname_hash_add(refname_map, ref->name, ref->oid); return 0; } @@ -1416,14 +1414,11 @@ static void set_option(struct transport *transport, const char *name, const char } -static int add_oid(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int add_oid(const struct reference *ref, void *cb_data) { struct oid_array *oids = cb_data; - oid_array_append(oids, oid); + oid_array_append(oids, ref->oid); return 0; } diff --git a/builtin/fsck.c b/builtin/fsck.c index 8ee95e0d67..ed4eea1680 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -530,14 +530,13 @@ static int fsck_handle_reflog(const char *logname, void *cb_data) return 0; } -static int fsck_handle_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int fsck_handle_ref(const struct reference *ref, void *cb_data UNUSED) { struct object *obj; - obj = parse_object(the_repository, oid); + obj = parse_object(the_repository, ref->oid); if (!obj) { - if (is_promisor_object(the_repository, oid)) { + if (is_promisor_object(the_repository, ref->oid)) { /* * Increment default_refs anyway, because this is a * valid ref. @@ -546,19 +545,19 @@ static int fsck_handle_ref(const char *refname, const char *referent UNUSED, con return 0; } error(_("%s: invalid sha1 pointer %s"), - refname, oid_to_hex(oid)); + ref->name, oid_to_hex(ref->oid)); errors_found |= ERROR_REACHABLE; /* We'll continue with the rest despite the error.. */ return 0; } - if (obj->type != OBJ_COMMIT && is_branch(refname)) { - error(_("%s: not a commit"), refname); + if (obj->type != OBJ_COMMIT && is_branch(ref->name)) { + error(_("%s: not a commit"), ref->name); errors_found |= ERROR_REFS; } default_refs++; obj->flags |= USED; fsck_put_object_name(&fsck_walk_options, - oid, "%s", refname); + ref->oid, "%s", ref->name); mark_object_reachable(obj); return 0; @@ -580,13 +579,19 @@ static void get_default_heads(void) worktrees = get_worktrees(); for (p = worktrees; *p; p++) { struct worktree *wt = *p; - struct strbuf ref = STRBUF_INIT; + struct strbuf refname = STRBUF_INIT; - strbuf_worktree_ref(wt, &ref, "HEAD"); - fsck_head_link(ref.buf, &head_points_at, &head_oid); - if (head_points_at && !is_null_oid(&head_oid)) - fsck_handle_ref(ref.buf, NULL, &head_oid, 0, NULL); - strbuf_release(&ref); + strbuf_worktree_ref(wt, &refname, "HEAD"); + fsck_head_link(refname.buf, &head_points_at, &head_oid); + if (head_points_at && !is_null_oid(&head_oid)) { + struct reference ref = { + .name = refname.buf, + .oid = &head_oid, + }; + + fsck_handle_ref(&ref, NULL); + } + strbuf_release(&refname); if (include_reflogs) refs_for_each_reflog(get_worktree_ref_store(wt), diff --git a/builtin/gc.c b/builtin/gc.c index e19e13d978..9de5de175f 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1100,24 +1100,21 @@ struct cg_auto_data { int limit; }; -static int dfs_on_ref(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *cb_data) +static int dfs_on_ref(const struct reference *ref, void *cb_data) { struct cg_auto_data *data = (struct cg_auto_data *)cb_data; int result = 0; + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct commit_list *stack = NULL; struct commit *commit; - if (!peel_iterated_oid(the_repository, oid, &peeled)) - oid = &peeled; - if (odb_read_object_info(the_repository->objects, oid, NULL) != OBJ_COMMIT) + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + maybe_peeled = &peeled; + if (odb_read_object_info(the_repository->objects, maybe_peeled, NULL) != OBJ_COMMIT) return 0; - commit = lookup_commit(the_repository, oid); + commit = lookup_commit(the_repository, maybe_peeled); if (!commit) return 0; if (repo_parse_commit(the_repository, commit) || diff --git a/builtin/name-rev.c b/builtin/name-rev.c index 74512e54a3..615f7d1aae 100644 --- a/builtin/name-rev.c +++ b/builtin/name-rev.c @@ -339,10 +339,9 @@ static int cmp_by_tag_and_age(const void *a_, const void *b_) return a->taggerdate != b->taggerdate; } -static int name_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int name_ref(const struct reference *ref, void *cb_data) { - struct object *o = parse_object(the_repository, oid); + struct object *o = parse_object(the_repository, ref->oid); struct name_ref_data *data = cb_data; int can_abbreviate_output = data->tags_only && data->name_only; int deref = 0; @@ -350,14 +349,14 @@ static int name_ref(const char *path, const char *referent UNUSED, const struct struct commit *commit = NULL; timestamp_t taggerdate = TIME_MAX; - if (data->tags_only && !starts_with(path, "refs/tags/")) + if (data->tags_only && !starts_with(ref->name, "refs/tags/")) return 0; if (data->exclude_filters.nr) { struct string_list_item *item; for_each_string_list_item(item, &data->exclude_filters) { - if (subpath_matches(path, item->string) >= 0) + if (subpath_matches(ref->name, item->string) >= 0) return 0; } } @@ -378,7 +377,7 @@ static int name_ref(const char *path, const char *referent UNUSED, const struct * shouldn't stop when seeing 'refs/tags/v1.4' matches * 'refs/tags/v*'. We should show it as 'v1.4'. */ - switch (subpath_matches(path, item->string)) { + switch (subpath_matches(ref->name, item->string)) { case -1: /* did not match */ break; case 0: /* matched fully */ @@ -406,13 +405,13 @@ static int name_ref(const char *path, const char *referent UNUSED, const struct } if (o && o->type == OBJ_COMMIT) { commit = (struct commit *)o; - from_tag = starts_with(path, "refs/tags/"); + from_tag = starts_with(ref->name, "refs/tags/"); if (taggerdate == TIME_MAX) taggerdate = commit->date; } - add_to_tip_table(oid, path, can_abbreviate_output, commit, taggerdate, - from_tag, deref); + add_to_tip_table(ref->oid, ref->name, can_abbreviate_output, + commit, taggerdate, from_tag, deref); return 0; } diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 5bdc44fb2d..39633a0158 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -831,15 +831,14 @@ static enum write_one_status write_one(struct hashfile *f, return WRITE_ONE_WRITTEN; } -static int mark_tagged(const char *path UNUSED, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int mark_tagged(const struct reference *ref, void *cb_data UNUSED) { struct object_id peeled; - struct object_entry *entry = packlist_find(&to_pack, oid); + struct object_entry *entry = packlist_find(&to_pack, ref->oid); if (entry) entry->tagged = 1; - if (!peel_iterated_oid(the_repository, oid, &peeled)) { + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) { entry = packlist_find(&to_pack, &peeled); if (entry) entry->tagged = 1; @@ -3306,13 +3305,12 @@ static void add_tag_chain(const struct object_id *oid) } } -static int add_ref_tag(const char *tag UNUSED, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int add_ref_tag(const struct reference *ref, void *cb_data UNUSED) { struct object_id peeled; - if (!peel_iterated_oid(the_repository, oid, &peeled) && obj_is_packed(&peeled)) - add_tag_chain(oid); + if (!peel_iterated_oid(the_repository, ref->oid, &peeled) && obj_is_packed(&peeled)) + add_tag_chain(ref->oid); return 0; } @@ -4533,19 +4531,16 @@ static void record_recent_commit(struct commit *commit, void *data UNUSED) oid_array_append(&recent_objects, &commit->object.oid); } -static int mark_bitmap_preferred_tip(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *data UNUSED) +static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNUSED) { + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct object *object; - if (!peel_iterated_oid(the_repository, oid, &peeled)) - oid = &peeled; + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + maybe_peeled = &peeled; - object = parse_object_or_die(the_repository, oid, refname); + object = parse_object_or_die(the_repository, maybe_peeled, ref->name); if (object->type == OBJ_COMMIT) object->flags |= NEEDS_BITMAP; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index c9288a9c7e..e8ee0e7321 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -305,13 +305,12 @@ static void show_ref(const char *path, const struct object_id *oid) } } -static int show_ref_cb(const char *path_full, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *data) +static int show_ref_cb(const struct reference *ref, void *data) { struct oidset *seen = data; - const char *path = strip_namespace(path_full); + const char *path = strip_namespace(ref->name); - if (ref_is_hidden(path, path_full, &hidden_refs)) + if (ref_is_hidden(path, ref->name, &hidden_refs)) return 0; /* @@ -320,13 +319,13 @@ static int show_ref_cb(const char *path_full, const char *referent UNUSED, const * transfer but will otherwise ignore them. */ if (!path) { - if (oidset_insert(seen, oid)) + if (oidset_insert(seen, ref->oid)) return 0; path = ".have"; } else { - oidset_insert(seen, oid); + oidset_insert(seen, ref->oid); } - show_ref(path, oid); + show_ref(path, ref->oid); return 0; } diff --git a/builtin/remote.c b/builtin/remote.c index 8a7ed4299a..7ffc14ba15 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -570,17 +570,14 @@ struct branches_for_remote { struct known_remotes *keep; }; -static int add_branch_for_removal(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, void *cb_data) +static int add_branch_for_removal(const struct reference *ref, void *cb_data) { struct branches_for_remote *branches = cb_data; struct refspec_item refspec; struct known_remote *kr; memset(&refspec, 0, sizeof(refspec)); - refspec.dst = (char *)refname; + refspec.dst = (char *)ref->name; if (remote_find_tracking(branches->remote, &refspec)) return 0; free(refspec.src); @@ -588,7 +585,7 @@ static int add_branch_for_removal(const char *refname, /* don't delete a branch if another remote also uses it */ for (kr = branches->keep->list; kr; kr = kr->next) { memset(&refspec, 0, sizeof(refspec)); - refspec.dst = (char *)refname; + refspec.dst = (char *)ref->name; if (!remote_find_tracking(kr->remote, &refspec)) { free(refspec.src); return 0; @@ -596,16 +593,16 @@ static int add_branch_for_removal(const char *refname, } /* don't delete non-remote-tracking refs */ - if (!starts_with(refname, "refs/remotes/")) { + if (!starts_with(ref->name, "refs/remotes/")) { /* advise user how to delete local branches */ - if (starts_with(refname, "refs/heads/")) + if (starts_with(ref->name, "refs/heads/")) string_list_append(branches->skipped, - abbrev_branch(refname)); + abbrev_branch(ref->name)); /* silently skip over other non-remote refs */ return 0; } - string_list_append(branches->branches, refname); + string_list_append(branches->branches, ref->name); return 0; } @@ -713,18 +710,18 @@ static int rename_one_reflog(const char *old_refname, return error; } -static int rename_one_ref(const char *old_refname, const char *referent, - const struct object_id *oid, - int flags, void *cb_data) +static int rename_one_ref(const struct reference *ref, void *cb_data) { struct strbuf new_referent = STRBUF_INIT; struct strbuf new_refname = STRBUF_INIT; struct rename_info *rename = cb_data; + const struct object_id *oid = ref->oid; + const char *referent = ref->target; int error; - compute_renamed_ref(rename, old_refname, &new_refname); + compute_renamed_ref(rename, ref->name, &new_refname); - if (flags & REF_ISSYMREF) { + if (ref->flags & REF_ISSYMREF) { /* * Stupidly enough `referent` is not pointing to the immediate * target of a symref, but it's the recursively resolved value. @@ -732,25 +729,25 @@ static int rename_one_ref(const char *old_refname, const char *referent, * unborn symrefs don't have any value for the `referent` at all. */ referent = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), - old_refname, RESOLVE_REF_NO_RECURSE, + ref->name, RESOLVE_REF_NO_RECURSE, NULL, NULL); compute_renamed_ref(rename, referent, &new_referent); oid = NULL; } - error = ref_transaction_delete(rename->transaction, old_refname, + error = ref_transaction_delete(rename->transaction, ref->name, oid, referent, REF_NO_DEREF, NULL, rename->err); if (error < 0) goto out; error = ref_transaction_update(rename->transaction, new_refname.buf, oid, null_oid(the_hash_algo), - (flags & REF_ISSYMREF) ? new_referent.buf : NULL, NULL, + (ref->flags & REF_ISSYMREF) ? new_referent.buf : NULL, NULL, REF_SKIP_CREATE_REFLOG | REF_NO_DEREF | REF_SKIP_OID_VERIFICATION, NULL, rename->err); if (error < 0) goto out; - error = rename_one_reflog(old_refname, oid, rename); + error = rename_one_reflog(ref->name, oid, rename); if (error < 0) goto out; @@ -1125,19 +1122,16 @@ static void free_remote_ref_states(struct ref_states *states) string_list_clear_func(&states->push, clear_push_info); } -static int append_ref_to_tracked_list(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags, void *cb_data) +static int append_ref_to_tracked_list(const struct reference *ref, void *cb_data) { struct ref_states *states = cb_data; struct refspec_item refspec; - if (flags & REF_ISSYMREF) + if (ref->flags & REF_ISSYMREF) return 0; memset(&refspec, 0, sizeof(refspec)); - refspec.dst = (char *)refname; + refspec.dst = (char *)ref->name; if (!remote_find_tracking(states->remote, &refspec)) { string_list_append(&states->tracked, abbrev_branch(refspec.src)); free(refspec.src); diff --git a/builtin/replace.c b/builtin/replace.c index 900b560a77..4c62c5ab58 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -47,30 +47,27 @@ struct show_data { enum replace_format format; }; -static int show_reference(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int show_reference(const struct reference *ref, void *cb_data) { struct show_data *data = cb_data; - if (!wildmatch(data->pattern, refname, 0)) { + if (!wildmatch(data->pattern, ref->name, 0)) { if (data->format == REPLACE_FORMAT_SHORT) - printf("%s\n", refname); + printf("%s\n", ref->name); else if (data->format == REPLACE_FORMAT_MEDIUM) - printf("%s -> %s\n", refname, oid_to_hex(oid)); + printf("%s -> %s\n", ref->name, oid_to_hex(ref->oid)); else { /* data->format == REPLACE_FORMAT_LONG */ struct object_id object; enum object_type obj_type, repl_type; - if (repo_get_oid(data->repo, refname, &object)) - return error(_("failed to resolve '%s' as a valid ref"), refname); + if (repo_get_oid(data->repo, ref->name, &object)) + return error(_("failed to resolve '%s' as a valid ref"), ref->name); obj_type = odb_read_object_info(data->repo->objects, &object, NULL); - repl_type = odb_read_object_info(data->repo->objects, oid, NULL); + repl_type = odb_read_object_info(data->repo->objects, ref->oid, NULL); - printf("%s (%s) -> %s (%s)\n", refname, type_name(obj_type), - oid_to_hex(oid), type_name(repl_type)); + printf("%s (%s) -> %s (%s)\n", ref->name, type_name(obj_type), + oid_to_hex(ref->oid), type_name(repl_type)); } } diff --git a/builtin/repo.c b/builtin/repo.c index 9d4749f79b..f26640bd6e 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -366,16 +366,13 @@ struct count_references_data { struct progress *progress; }; -static int count_references(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int count_references(const struct reference *ref, void *cb_data) { struct count_references_data *data = cb_data; struct ref_stats *stats = data->stats; size_t ref_count; - switch (ref_kind_from_refname(refname)) { + switch (ref_kind_from_refname(ref->name)) { case FILTER_REFS_BRANCHES: stats->branches++; break; @@ -396,7 +393,7 @@ static int count_references(const char *refname, * While iterating through references for counting, also add OIDs in * preparation for the path walk. */ - add_pending_oid(data->revs, NULL, oid, 0); + add_pending_oid(data->revs, NULL, ref->oid, 0); ref_count = get_total_reference_count(stats); display_progress(data->progress, ref_count); diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 9da92b990d..3578591b4f 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -217,19 +217,17 @@ static int show_default(void) return 0; } -static int show_reference(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int show_reference(const struct reference *ref, void *cb_data UNUSED) { - if (ref_excluded(&ref_excludes, refname)) + if (ref_excluded(&ref_excludes, ref->name)) return 0; - show_rev(NORMAL, oid, refname); + show_rev(NORMAL, ref->oid, ref->name); return 0; } -static int anti_reference(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int anti_reference(const struct reference *ref, void *cb_data UNUSED) { - show_rev(REVERSED, oid, refname); + show_rev(REVERSED, ref->oid, ref->name); return 0; } diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 441babf2e3..10475a6b5e 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -413,34 +413,32 @@ static int append_ref(const char *refname, const struct object_id *oid, return 0; } -static int append_head_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int append_head_ref(const struct reference *ref, void *cb_data UNUSED) { struct object_id tmp; int ofs = 11; - if (!starts_with(refname, "refs/heads/")) + if (!starts_with(ref->name, "refs/heads/")) return 0; /* If both heads/foo and tags/foo exists, get_sha1 would * get confused. */ - if (repo_get_oid(the_repository, refname + ofs, &tmp) || !oideq(&tmp, oid)) + if (repo_get_oid(the_repository, ref->name + ofs, &tmp) || !oideq(&tmp, ref->oid)) ofs = 5; - return append_ref(refname + ofs, oid, 0); + return append_ref(ref->name + ofs, ref->oid, 0); } -static int append_remote_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int append_remote_ref(const struct reference *ref, void *cb_data UNUSED) { struct object_id tmp; int ofs = 13; - if (!starts_with(refname, "refs/remotes/")) + if (!starts_with(ref->name, "refs/remotes/")) return 0; /* If both heads/foo and tags/foo exists, get_sha1 would * get confused. */ - if (repo_get_oid(the_repository, refname + ofs, &tmp) || !oideq(&tmp, oid)) + if (repo_get_oid(the_repository, ref->name + ofs, &tmp) || !oideq(&tmp, ref->oid)) ofs = 5; - return append_ref(refname + ofs, oid, 0); + return append_ref(ref->name + ofs, ref->oid, 0); } static int append_tag_ref(const char *refname, const struct object_id *oid, @@ -454,27 +452,26 @@ static int append_tag_ref(const char *refname, const struct object_id *oid, static const char *match_ref_pattern = NULL; static int match_ref_slash = 0; -static int append_matching_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data) +static int append_matching_ref(const struct reference *ref, void *cb_data) { /* we want to allow pattern hold/ to show all * branches under refs/heads/hold/, and v0.99.9? to show * refs/tags/v0.99.9a and friends. */ const char *tail; - int slash = count_slashes(refname); - for (tail = refname; *tail && match_ref_slash < slash; ) + int slash = count_slashes(ref->name); + for (tail = ref->name; *tail && match_ref_slash < slash; ) if (*tail++ == '/') slash--; if (!*tail) return 0; if (wildmatch(match_ref_pattern, tail, 0)) return 0; - if (starts_with(refname, "refs/heads/")) - return append_head_ref(refname, NULL, oid, flag, cb_data); - if (starts_with(refname, "refs/tags/")) - return append_tag_ref(refname, oid, flag, cb_data); - return append_ref(refname, oid, 0); + if (starts_with(ref->name, "refs/heads/")) + return append_head_ref(ref, cb_data); + if (starts_with(ref->name, "refs/tags/")) + return append_tag_ref(ref->name, ref->oid, ref->flags, cb_data); + return append_ref(ref->name, ref->oid, 0); } static void snarf_refs(int head, int remotes) diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 0b6f9edf86..4803b5e598 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -66,26 +66,25 @@ struct show_ref_data { int show_head; }; -static int show_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cbdata) +static int show_ref(const struct reference *ref, void *cbdata) { struct show_ref_data *data = cbdata; - if (data->show_head && !strcmp(refname, "HEAD")) + if (data->show_head && !strcmp(ref->name, "HEAD")) goto match; if (data->patterns) { - int reflen = strlen(refname); + int reflen = strlen(ref->name); const char **p = data->patterns, *m; while ((m = *p++) != NULL) { int len = strlen(m); if (len > reflen) continue; - if (memcmp(m, refname + reflen - len, len)) + if (memcmp(m, ref->name + reflen - len, len)) continue; if (len == reflen) goto match; - if (refname[reflen - len - 1] == '/') + if (ref->name[reflen - len - 1] == '/') goto match; } return 0; @@ -94,18 +93,15 @@ static int show_ref(const char *refname, const char *referent UNUSED, const stru match: data->found_match++; - show_one(data->show_one_opts, refname, oid); + show_one(data->show_one_opts, ref->name, ref->oid); return 0; } -static int add_existing(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag UNUSED, void *cbdata) +static int add_existing(const struct reference *ref, void *cbdata) { struct string_list *list = (struct string_list *)cbdata; - string_list_insert(list, refname); + string_list_insert(list, ref->name); return 0; } diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index fcd73abe53..35f6cf735e 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -593,16 +593,12 @@ static void print_status(unsigned int flags, char state, const char *path, printf("\n"); } -static int handle_submodule_head_ref(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *cb_data) +static int handle_submodule_head_ref(const struct reference *ref, void *cb_data) { struct object_id *output = cb_data; - if (oid) - oidcpy(output, oid); + if (ref->oid) + oidcpy(output, ref->oid); return 0; } diff --git a/builtin/worktree.c b/builtin/worktree.c index 812774a5ca..b7f323b5e4 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -635,11 +635,7 @@ static void print_preparing_worktree_line(int detach, * * Returns 0 on failure and non-zero on success. */ -static int first_valid_ref(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, - void *cb_data UNUSED) +static int first_valid_ref(const struct reference *ref UNUSED, void *cb_data UNUSED) { return 1; } diff --git a/commit-graph.c b/commit-graph.c index 474454db73..f91af41625 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1851,18 +1851,16 @@ struct refs_cb_data { struct progress *progress; }; -static int add_ref_to_set(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int add_ref_to_set(const struct reference *ref, void *cb_data) { + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct refs_cb_data *data = (struct refs_cb_data *)cb_data; - if (!peel_iterated_oid(data->repo, oid, &peeled)) - oid = &peeled; - if (odb_read_object_info(data->repo->objects, oid, NULL) == OBJ_COMMIT) - oidset_insert(data->commits, oid); + if (!peel_iterated_oid(data->repo, ref->oid, &peeled)) + maybe_peeled = &peeled; + if (odb_read_object_info(data->repo->objects, maybe_peeled, NULL) == OBJ_COMMIT) + oidset_insert(data->commits, maybe_peeled); display_progress(data->progress, oidset_size(data->commits)); diff --git a/delta-islands.c b/delta-islands.c index 36c94799d6..7cfebc4162 100644 --- a/delta-islands.c +++ b/delta-islands.c @@ -390,8 +390,7 @@ static void add_ref_to_island(kh_str_t *remote_islands, const char *island_name, rl->hash += sha_core; } -static int find_island_for_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, void *cb) +static int find_island_for_ref(const struct reference *ref, void *cb) { struct island_load_data *ild = cb; @@ -406,7 +405,7 @@ static int find_island_for_ref(const char *refname, const char *referent UNUSED, /* walk backwards to get last-one-wins ordering */ for (i = ild->nr - 1; i >= 0; i--) { - if (!regexec(&ild->rx[i], refname, + if (!regexec(&ild->rx[i], ref->name, ARRAY_SIZE(matches), matches, 0)) break; } @@ -428,10 +427,10 @@ static int find_island_for_ref(const char *refname, const char *referent UNUSED, if (island_name.len) strbuf_addch(&island_name, '-'); - strbuf_add(&island_name, refname + match->rm_so, match->rm_eo - match->rm_so); + strbuf_add(&island_name, ref->name + match->rm_so, match->rm_eo - match->rm_so); } - add_ref_to_island(ild->remote_islands, island_name.buf, oid); + add_ref_to_island(ild->remote_islands, island_name.buf, ref->oid); strbuf_release(&island_name); return 0; } diff --git a/fetch-pack.c b/fetch-pack.c index fe7a84bf2f..78c45d4a15 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -188,13 +188,9 @@ static int rev_list_insert_ref(struct fetch_negotiator *negotiator, return 0; } -static int rev_list_insert_ref_oid(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int rev_list_insert_ref_oid(const struct reference *ref, void *cb_data) { - return rev_list_insert_ref(cb_data, oid); + return rev_list_insert_ref(cb_data, ref->oid); } enum ack_type { @@ -616,13 +612,9 @@ static int mark_complete(const struct object_id *oid) return 0; } -static int mark_complete_oid(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, - void *cb_data UNUSED) +static int mark_complete_oid(const struct reference *ref, void *cb_data UNUSED) { - return mark_complete(oid); + return mark_complete(ref->oid); } static void mark_recent_complete_commits(struct fetch_pack_args *args, diff --git a/help.c b/help.c index 5854dd4a7e..20e114432d 100644 --- a/help.c +++ b/help.c @@ -851,18 +851,16 @@ struct similar_ref_cb { struct string_list *similar_refs; }; -static int append_similar_ref(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, void *cb_data) +static int append_similar_ref(const struct reference *ref, void *cb_data) { struct similar_ref_cb *cb = (struct similar_ref_cb *)(cb_data); - char *branch = strrchr(refname, '/') + 1; + char *branch = strrchr(ref->name, '/') + 1; /* A remote branch of the same name is deemed similar */ - if (starts_with(refname, "refs/remotes/") && + if (starts_with(ref->name, "refs/remotes/") && !strcmp(branch, cb->base_ref)) string_list_append_nodup(cb->similar_refs, - refs_shorten_unambiguous_ref(get_main_ref_store(the_repository), refname, 1)); + refs_shorten_unambiguous_ref(get_main_ref_store(the_repository), ref->name, 1)); return 0; } diff --git a/http-backend.c b/http-backend.c index 9084058f1e..92e1733f14 100644 --- a/http-backend.c +++ b/http-backend.c @@ -513,18 +513,17 @@ static void run_service(const char **argv, int buffer_input) exit(1); } -static int show_text_ref(const char *name, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int show_text_ref(const struct reference *ref, void *cb_data) { - const char *name_nons = strip_namespace(name); + const char *name_nons = strip_namespace(ref->name); struct strbuf *buf = cb_data; - struct object *o = parse_object(the_repository, oid); + struct object *o = parse_object(the_repository, ref->oid); if (!o) return 0; - strbuf_addf(buf, "%s\t%s\n", oid_to_hex(oid), name_nons); + strbuf_addf(buf, "%s\t%s\n", oid_to_hex(ref->oid), name_nons); if (o->type == OBJ_TAG) { - o = deref_tag(the_repository, o, name, 0); + o = deref_tag(the_repository, o, ref->name, 0); if (!o) return 0; strbuf_addf(buf, "%s\t%s^{}\n", oid_to_hex(&o->oid), @@ -569,21 +568,20 @@ static void get_info_refs(struct strbuf *hdr, char *arg UNUSED) strbuf_release(&buf); } -static int show_head_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data) +static int show_head_ref(const struct reference *ref, void *cb_data) { struct strbuf *buf = cb_data; - if (flag & REF_ISSYMREF) { + if (ref->flags & REF_ISSYMREF) { const char *target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), - refname, + ref->name, RESOLVE_REF_READING, NULL, NULL); if (target) strbuf_addf(buf, "ref: %s\n", strip_namespace(target)); } else { - strbuf_addf(buf, "%s\n", oid_to_hex(oid)); + strbuf_addf(buf, "%s\n", oid_to_hex(ref->oid)); } return 0; diff --git a/log-tree.c b/log-tree.c index 7d917f2a83..1729b0c201 100644 --- a/log-tree.c +++ b/log-tree.c @@ -147,9 +147,7 @@ static int ref_filter_match(const char *refname, return 1; } -static int add_ref_decoration(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, - void *cb_data) +static int add_ref_decoration(const struct reference *ref, void *cb_data) { int i; struct object *obj; @@ -158,16 +156,16 @@ static int add_ref_decoration(const char *refname, const char *referent UNUSED, struct decoration_filter *filter = (struct decoration_filter *)cb_data; const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref; - if (filter && !ref_filter_match(refname, filter)) + if (filter && !ref_filter_match(ref->name, filter)) return 0; - if (starts_with(refname, git_replace_ref_base)) { + if (starts_with(ref->name, git_replace_ref_base)) { struct object_id original_oid; if (!replace_refs_enabled(the_repository)) return 0; - if (get_oid_hex(refname + strlen(git_replace_ref_base), + if (get_oid_hex(ref->name + strlen(git_replace_ref_base), &original_oid)) { - warning("invalid replace ref %s", refname); + warning("invalid replace ref %s", ref->name); return 0; } obj = parse_object(the_repository, &original_oid); @@ -176,10 +174,10 @@ static int add_ref_decoration(const char *refname, const char *referent UNUSED, return 0; } - objtype = odb_read_object_info(the_repository->objects, oid, NULL); + objtype = odb_read_object_info(the_repository->objects, ref->oid, NULL); if (objtype < 0) return 0; - obj = lookup_object_by_type(the_repository, oid, objtype); + obj = lookup_object_by_type(the_repository, ref->oid, objtype); for (i = 0; i < ARRAY_SIZE(ref_namespace); i++) { struct ref_namespace_info *info = &ref_namespace[i]; @@ -187,24 +185,24 @@ static int add_ref_decoration(const char *refname, const char *referent UNUSED, if (!info->decoration) continue; if (info->exact) { - if (!strcmp(refname, info->ref)) { + if (!strcmp(ref->name, info->ref)) { deco_type = info->decoration; break; } - } else if (starts_with(refname, info->ref)) { + } else if (starts_with(ref->name, info->ref)) { deco_type = info->decoration; break; } } - add_name_decoration(deco_type, refname, obj); + add_name_decoration(deco_type, ref->name, obj); while (obj->type == OBJ_TAG) { if (!obj->parsed) parse_object(the_repository, &obj->oid); obj = ((struct tag *)obj)->tagged; if (!obj) break; - add_name_decoration(DECORATION_REF_TAG, refname, obj); + add_name_decoration(DECORATION_REF_TAG, ref->name, obj); } return 0; } diff --git a/ls-refs.c b/ls-refs.c index c47acde07f..64d0272369 100644 --- a/ls-refs.c +++ b/ls-refs.c @@ -75,42 +75,42 @@ struct ls_refs_data { unsigned unborn : 1; }; -static int send_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data) +static int send_ref(const struct reference *ref, void *cb_data) { struct ls_refs_data *data = cb_data; - const char *refname_nons = strip_namespace(refname); + const char *refname_nons = strip_namespace(ref->name); strbuf_reset(&data->buf); - if (ref_is_hidden(refname_nons, refname, &data->hidden_refs)) + if (ref_is_hidden(refname_nons, ref->name, &data->hidden_refs)) return 0; if (!ref_match(&data->prefixes, refname_nons)) return 0; - if (oid) - strbuf_addf(&data->buf, "%s %s", oid_to_hex(oid), refname_nons); + if (ref->oid) + strbuf_addf(&data->buf, "%s %s", oid_to_hex(ref->oid), refname_nons); else strbuf_addf(&data->buf, "unborn %s", refname_nons); - if (data->symrefs && flag & REF_ISSYMREF) { + if (data->symrefs && ref->flags & REF_ISSYMREF) { + int unused_flag; struct object_id unused; const char *symref_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), - refname, + ref->name, 0, &unused, - &flag); + &unused_flag); if (!symref_target) - die("'%s' is a symref but it is not?", refname); + die("'%s' is a symref but it is not?", ref->name); strbuf_addf(&data->buf, " symref-target:%s", strip_namespace(symref_target)); } - if (data->peel && oid) { + if (data->peel && ref->oid) { struct object_id peeled; - if (!peel_iterated_oid(the_repository, oid, &peeled)) + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) strbuf_addf(&data->buf, " peeled:%s", oid_to_hex(&peeled)); } @@ -131,9 +131,17 @@ static void send_possibly_unborn_head(struct ls_refs_data *data) if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), namespaced.buf, 0, &oid, &flag)) return; /* bad ref */ oid_is_null = is_null_oid(&oid); + if (!oid_is_null || - (data->unborn && data->symrefs && (flag & REF_ISSYMREF))) - send_ref(namespaced.buf, NULL, oid_is_null ? NULL : &oid, flag, data); + (data->unborn && data->symrefs && (flag & REF_ISSYMREF))) { + struct reference ref = { + .name = namespaced.buf, + .oid = oid_is_null ? NULL : &oid, + .flags = flag, + }; + + send_ref(&ref, data); + } strbuf_release(&namespaced); } diff --git a/midx-write.c b/midx-write.c index c73010df6d..f4dd875747 100644 --- a/midx-write.c +++ b/midx-write.c @@ -697,28 +697,27 @@ static void prepare_midx_packing_data(struct packing_data *pdata, trace2_region_leave("midx", "prepare_midx_packing_data", ctx->repo); } -static int add_ref_to_pending(const char *refname, const char *referent UNUSED, - const struct object_id *oid, - int flag, void *cb_data) +static int add_ref_to_pending(const struct reference *ref, void *cb_data) { struct rev_info *revs = (struct rev_info*)cb_data; + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct object *object; - if ((flag & REF_ISSYMREF) && (flag & REF_ISBROKEN)) { - warning("symbolic ref is dangling: %s", refname); + if ((ref->flags & REF_ISSYMREF) && (ref->flags & REF_ISBROKEN)) { + warning("symbolic ref is dangling: %s", ref->name); return 0; } - if (!peel_iterated_oid(revs->repo, oid, &peeled)) - oid = &peeled; + if (!peel_iterated_oid(revs->repo, ref->oid, &peeled)) + maybe_peeled = &peeled; - object = parse_object_or_die(revs->repo, oid, refname); + object = parse_object_or_die(revs->repo, maybe_peeled, ref->name); if (object->type != OBJ_COMMIT) return 0; add_pending_object(revs, object, ""); - if (bitmap_is_preferred_refname(revs->repo, refname)) + if (bitmap_is_preferred_refname(revs->repo, ref->name)) object->flags |= NEEDS_BITMAP; return 0; } diff --git a/negotiator/default.c b/negotiator/default.c index c479da9b09..116dedcf83 100644 --- a/negotiator/default.c +++ b/negotiator/default.c @@ -38,11 +38,10 @@ static void rev_list_push(struct negotiation_state *ns, } } -static int clear_marks(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data UNUSED) +static int clear_marks(const struct reference *ref, void *cb_data UNUSED) { - struct object *o = deref_tag(the_repository, parse_object(the_repository, oid), refname, 0); + struct object *o = deref_tag(the_repository, parse_object(the_repository, ref->oid), + ref->name, 0); if (o && o->type == OBJ_COMMIT) clear_commit_marks((struct commit *)o, diff --git a/negotiator/skipping.c b/negotiator/skipping.c index 616df6bf3a..0a272130fb 100644 --- a/negotiator/skipping.c +++ b/negotiator/skipping.c @@ -75,11 +75,10 @@ static struct entry *rev_list_push(struct data *data, struct commit *commit, int return entry; } -static int clear_marks(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data UNUSED) +static int clear_marks(const struct reference *ref, void *cb_data UNUSED) { - struct object *o = deref_tag(the_repository, parse_object(the_repository, oid), refname, 0); + struct object *o = deref_tag(the_repository, parse_object(the_repository, ref->oid), + ref->name, 0); if (o && o->type == OBJ_COMMIT) clear_commit_marks((struct commit *)o, diff --git a/notes.c b/notes.c index 9a2e9181fe..8e00fd8c47 100644 --- a/notes.c +++ b/notes.c @@ -938,13 +938,11 @@ int combine_notes_cat_sort_uniq(struct object_id *cur_oid, return ret; } -static int string_list_add_one_ref(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag UNUSED, void *cb) +static int string_list_add_one_ref(const struct reference *ref, void *cb) { struct string_list *refs = cb; - if (!unsorted_string_list_has_string(refs, refname)) - string_list_append(refs, refname); + if (!unsorted_string_list_has_string(refs, ref->name)) + string_list_append(refs, ref->name); return 0; } diff --git a/object-name.c b/object-name.c index f6902e140d..7e8109f25f 100644 --- a/object-name.c +++ b/object-name.c @@ -1444,18 +1444,16 @@ struct handle_one_ref_cb { struct commit_list **list; }; -static int handle_one_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int handle_one_ref(const struct reference *ref, void *cb_data) { struct handle_one_ref_cb *cb = cb_data; struct commit_list **list = cb->list; - struct object *object = parse_object(cb->repo, oid); + struct object *object = parse_object(cb->repo, ref->oid); if (!object) return 0; if (object->type == OBJ_TAG) { - object = deref_tag(cb->repo, object, path, - strlen(path)); + object = deref_tag(cb->repo, object, ref->name, + strlen(ref->name)); if (!object) return 0; } diff --git a/pseudo-merge.c b/pseudo-merge.c index 893b763fe4..0abd51b42c 100644 --- a/pseudo-merge.c +++ b/pseudo-merge.c @@ -221,28 +221,25 @@ void load_pseudo_merges_from_config(struct repository *r, } } -static int find_pseudo_merge_group_for_ref(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *_data) +static int find_pseudo_merge_group_for_ref(const struct reference *ref, void *_data) { struct bitmap_writer *writer = _data; + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct commit *c; uint32_t i; int has_bitmap; - if (!peel_iterated_oid(the_repository, oid, &peeled)) - oid = &peeled; + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + maybe_peeled = &peeled; - c = lookup_commit(the_repository, oid); + c = lookup_commit(the_repository, maybe_peeled); if (!c) return 0; - if (!packlist_find(writer->to_pack, oid)) + if (!packlist_find(writer->to_pack, maybe_peeled)) return 0; - has_bitmap = bitmap_writer_has_bitmapped_object_id(writer, oid); + has_bitmap = bitmap_writer_has_bitmapped_object_id(writer, maybe_peeled); for (i = 0; i < writer->pseudo_merge_groups.nr; i++) { struct pseudo_merge_group *group; @@ -252,7 +249,7 @@ static int find_pseudo_merge_group_for_ref(const char *refname, size_t j; group = writer->pseudo_merge_groups.items[i].util; - if (regexec(group->pattern, refname, ARRAY_SIZE(captures), + if (regexec(group->pattern, ref->name, ARRAY_SIZE(captures), captures, 0)) continue; @@ -269,7 +266,7 @@ static int find_pseudo_merge_group_for_ref(const char *refname, if (group_name.len) strbuf_addch(&group_name, '-'); - strbuf_add(&group_name, refname + match->rm_so, + strbuf_add(&group_name, ref->name + match->rm_so, match->rm_eo - match->rm_so); } diff --git a/reachable.c b/reachable.c index 22266db523..b753c39553 100644 --- a/reachable.c +++ b/reachable.c @@ -83,18 +83,17 @@ static void add_rebase_files(struct rev_info *revs) free_worktrees(worktrees); } -static int add_one_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data) +static int add_one_ref(const struct reference *ref, void *cb_data) { struct rev_info *revs = (struct rev_info *)cb_data; struct object *object; - if ((flag & REF_ISSYMREF) && (flag & REF_ISBROKEN)) { - warning("symbolic ref is dangling: %s", path); + if ((ref->flags & REF_ISSYMREF) && (ref->flags & REF_ISBROKEN)) { + warning("symbolic ref is dangling: %s", ref->name); return 0; } - object = parse_object_or_die(the_repository, oid, path); + object = parse_object_or_die(the_repository, ref->oid, ref->name); add_pending_object(revs, object, ""); return 0; diff --git a/ref-filter.c b/ref-filter.c index 30cc488d8a..6837fa60a9 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2954,14 +2954,15 @@ struct ref_filter_cbdata { * A call-back given to for_each_ref(). Filter refs and keep them for * later object processing. */ -static int filter_one(const char *refname, const char *referent, const struct object_id *oid, int flag, void *cb_data) +static int filter_one(const struct reference *ref, void *cb_data) { struct ref_filter_cbdata *ref_cbdata = cb_data; - struct ref_array_item *ref; + struct ref_array_item *item; - ref = apply_ref_filter(refname, referent, oid, flag, ref_cbdata->filter); - if (ref) - ref_array_append(ref_cbdata->array, ref); + item = apply_ref_filter(ref->name, ref->target, ref->oid, + ref->flags, ref_cbdata->filter); + if (item) + ref_array_append(ref_cbdata->array, item); return 0; } @@ -2990,17 +2991,18 @@ struct ref_filter_and_format_cbdata { } internal; }; -static int filter_and_format_one(const char *refname, const char *referent, const struct object_id *oid, int flag, void *cb_data) +static int filter_and_format_one(const struct reference *ref, void *cb_data) { struct ref_filter_and_format_cbdata *ref_cbdata = cb_data; - struct ref_array_item *ref; + struct ref_array_item *item; struct strbuf output = STRBUF_INIT, err = STRBUF_INIT; - ref = apply_ref_filter(refname, referent, oid, flag, ref_cbdata->filter); - if (!ref) + item = apply_ref_filter(ref->name, ref->target, ref->oid, + ref->flags, ref_cbdata->filter); + if (!item) return 0; - if (format_ref_array_item(ref, ref_cbdata->format, &output, &err)) + if (format_ref_array_item(item, ref_cbdata->format, &output, &err)) die("%s", err.buf); if (output.len || !ref_cbdata->format->array_opts.omit_empty) { @@ -3010,7 +3012,7 @@ static int filter_and_format_one(const char *refname, const char *referent, cons strbuf_release(&output); strbuf_release(&err); - free_array_item(ref); + free_array_item(item); /* * Increment the running count of refs that match the filter. If diff --git a/reflog.c b/reflog.c index 65ef259b4f..ac87e20c4f 100644 --- a/reflog.c +++ b/reflog.c @@ -423,16 +423,13 @@ int should_expire_reflog_ent_verbose(struct object_id *ooid, return expire; } -static int push_tip_to_list(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags, void *cb_data) +static int push_tip_to_list(const struct reference *ref, void *cb_data) { struct commit_list **list = cb_data; struct commit *tip_commit; - if (flags & REF_ISSYMREF) + if (ref->flags & REF_ISSYMREF) return 0; - tip_commit = lookup_commit_reference_gently(the_repository, oid, 1); + tip_commit = lookup_commit_reference_gently(the_repository, ref->oid, 1); if (!tip_commit) return 0; commit_list_insert(tip_commit, list); diff --git a/refs.c b/refs.c index 965381367e..25f0579d61 100644 --- a/refs.c +++ b/refs.c @@ -426,17 +426,19 @@ int refs_ref_exists(struct ref_store *refs, const char *refname) NULL, NULL); } -static int for_each_filter_refs(const char *refname, const char *referent, - const struct object_id *oid, - int flags, void *data) +static int for_each_filter_refs(const struct reference *ref, void *data) { struct for_each_ref_filter *filter = data; - if (wildmatch(filter->pattern, refname, 0)) + if (wildmatch(filter->pattern, ref->name, 0)) return 0; - if (filter->prefix) - skip_prefix(refname, filter->prefix, &refname); - return filter->fn(refname, referent, oid, flags, filter->cb_data); + if (filter->prefix) { + struct reference skipped = *ref; + skip_prefix(skipped.name, filter->prefix, &skipped.name); + return filter->fn(&skipped, filter->cb_data); + } else { + return filter->fn(ref, filter->cb_data); + } } struct warn_if_dangling_data { @@ -447,17 +449,15 @@ struct warn_if_dangling_data { int dry_run; }; -static int warn_if_dangling_symref(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags, void *cb_data) +static int warn_if_dangling_symref(const struct reference *ref, void *cb_data) { struct warn_if_dangling_data *d = cb_data; const char *resolves_to, *msg; - if (!(flags & REF_ISSYMREF)) + if (!(ref->flags & REF_ISSYMREF)) return 0; - resolves_to = refs_resolve_ref_unsafe(d->refs, refname, 0, NULL, NULL); + resolves_to = refs_resolve_ref_unsafe(d->refs, ref->name, 0, NULL, NULL); if (!resolves_to || !string_list_has_string(d->refnames, resolves_to)) { return 0; @@ -466,7 +466,7 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU msg = d->dry_run ? _("%s%s will become dangling after %s is deleted\n") : _("%s%s has become dangling after %s was deleted\n"); - fprintf(d->fp, msg, d->indent, refname, resolves_to); + fprintf(d->fp, msg, d->indent, ref->name, resolves_to); return 0; } @@ -507,8 +507,15 @@ int refs_head_ref_namespaced(struct ref_store *refs, each_ref_fn fn, void *cb_da int flag; strbuf_addf(&buf, "%sHEAD", get_git_namespace()); - if (!refs_read_ref_full(refs, buf.buf, RESOLVE_REF_READING, &oid, &flag)) - ret = fn(buf.buf, NULL, &oid, flag, cb_data); + if (!refs_read_ref_full(refs, buf.buf, RESOLVE_REF_READING, &oid, &flag)) { + struct reference ref = { + .name = buf.buf, + .oid = &oid, + .flags = flag, + }; + + ret = fn(&ref, cb_data); + } strbuf_release(&buf); return ret; @@ -1741,8 +1748,15 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) int flag; if (refs_resolve_ref_unsafe(refs, "HEAD", RESOLVE_REF_READING, - &oid, &flag)) - return fn("HEAD", NULL, &oid, flag, cb_data); + &oid, &flag)) { + struct reference ref = { + .name = "HEAD", + .oid = &oid, + .flags = flag, + }; + + return fn(&ref, cb_data); + } return 0; } @@ -2753,14 +2767,10 @@ struct do_for_each_reflog_help { void *cb_data; }; -static int do_for_each_reflog_helper(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, - void *cb_data) +static int do_for_each_reflog_helper(const struct reference *ref, void *cb_data) { struct do_for_each_reflog_help *hp = cb_data; - return hp->fn(refname, hp->cb_data); + return hp->fn(ref->name, hp->cb_data); } int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_data) @@ -2976,25 +2986,24 @@ struct migration_data { uint64_t index; }; -static int migrate_one_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags, void *cb_data) +static int migrate_one_ref(const struct reference *ref, void *cb_data) { struct migration_data *data = cb_data; struct strbuf symref_target = STRBUF_INIT; int ret; - if (flags & REF_ISSYMREF) { - ret = refs_read_symbolic_ref(data->old_refs, refname, &symref_target); + if (ref->flags & REF_ISSYMREF) { + ret = refs_read_symbolic_ref(data->old_refs, ref->name, &symref_target); if (ret < 0) goto done; - ret = ref_transaction_update(data->transaction, refname, NULL, null_oid(the_hash_algo), + ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo), symref_target.buf, NULL, REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf); if (ret < 0) goto done; } else { - ret = ref_transaction_create(data->transaction, refname, oid, NULL, + ret = ref_transaction_create(data->transaction, ref->name, ref->oid, NULL, REF_SKIP_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION, NULL, data->errbuf); if (ret < 0) diff --git a/refs.h b/refs.h index 4e6bd63aa8..68d235438c 100644 --- a/refs.h +++ b/refs.h @@ -355,14 +355,32 @@ struct ref_transaction; */ #define REF_BAD_NAME 0x08 +/* A reference passed to `for_each_ref()`-style callbacks. */ +struct reference { + /* The fully-qualified name of the reference. */ + const char *name; + + /* The target of a symbolic ref. `NULL` for direct references. */ + const char *target; + + /* + * The object ID of a reference. Either the direct object ID or the + * resolved object ID in the case of a symbolic ref. May be the zero + * object ID in case the symbolic ref cannot be resolved. + */ + const struct object_id *oid; + + /* A bitfield of `REF_` flags. */ + int flags; +}; + /* * The signature for the callback function for the for_each_*() - * functions below. The memory pointed to by the refname and oid - * arguments is only guaranteed to be valid for the duration of a + * functions below. The memory pointed to by the `struct reference` + * argument is only guaranteed to be valid for the duration of a * single callback invocation. */ -typedef int each_ref_fn(const char *refname, const char *referent, - const struct object_id *oid, int flags, void *cb_data); +typedef int each_ref_fn(const struct reference *ref, void *cb_data); /* * The following functions invoke the specified callback function for diff --git a/refs/files-backend.c b/refs/files-backend.c index 8d7007f4aa..eb3142f8f2 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3150,14 +3150,11 @@ static int parse_and_write_reflog(struct files_ref_store *refs, return 0; } -static int ref_present(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, - void *cb_data) +static int ref_present(const struct reference *ref, void *cb_data) { struct string_list *affected_refnames = cb_data; - return string_list_has_string(affected_refnames, refname); + return string_list_has_string(affected_refnames, ref->name); } static int files_transaction_finish_initial(struct files_ref_store *refs, diff --git a/refs/iterator.c b/refs/iterator.c index 17ef841d8a..7f2e718f1c 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -476,7 +476,14 @@ int do_for_each_ref_iterator(struct ref_iterator *iter, current_ref_iter = iter; while ((ok = ref_iterator_advance(iter)) == ITER_OK) { - retval = fn(iter->refname, iter->referent, iter->oid, iter->flags, cb_data); + struct reference ref = { + .name = iter->refname, + .target = iter->referent, + .oid = iter->oid, + .flags = iter->flags, + }; + + retval = fn(&ref, cb_data); if (retval) goto out; } diff --git a/remote.c b/remote.c index df9675cd33..59b3715120 100644 --- a/remote.c +++ b/remote.c @@ -2315,21 +2315,19 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb, return 1; } -static int one_local_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int one_local_ref(const struct reference *ref, void *cb_data) { struct ref ***local_tail = cb_data; - struct ref *ref; + struct ref *local_ref; /* we already know it starts with refs/ to get here */ - if (check_refname_format(refname + 5, 0)) + if (check_refname_format(ref->name + 5, 0)) return 0; - ref = alloc_ref(refname); - oidcpy(&ref->new_oid, oid); - **local_tail = ref; - *local_tail = &ref->next; + local_ref = alloc_ref(ref->name); + oidcpy(&local_ref->new_oid, ref->oid); + **local_tail = local_ref; + *local_tail = &local_ref->next; return 0; } @@ -2402,15 +2400,14 @@ struct stale_heads_info { struct refspec *rs; }; -static int get_stale_heads_cb(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags, void *cb_data) +static int get_stale_heads_cb(const struct reference *ref, void *cb_data) { struct stale_heads_info *info = cb_data; struct string_list matches = STRING_LIST_INIT_DUP; struct refspec_item query; int i, stale = 1; memset(&query, 0, sizeof(struct refspec_item)); - query.dst = (char *)refname; + query.dst = (char *)ref->name; refspec_find_all_matches(info->rs, &query, &matches); if (matches.nr == 0) @@ -2423,7 +2420,7 @@ static int get_stale_heads_cb(const char *refname, const char *referent UNUSED, * overlapping refspecs, we need to go over all of the * matching refs. */ - if (flags & REF_ISSYMREF) + if (ref->flags & REF_ISSYMREF) goto clean_exit; for (i = 0; stale && i < matches.nr; i++) @@ -2431,8 +2428,8 @@ static int get_stale_heads_cb(const char *refname, const char *referent UNUSED, stale = 0; if (stale) { - struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail); - oidcpy(&ref->new_oid, oid); + struct ref *linked_ref = make_linked_ref(ref->name, &info->stale_refs_tail); + oidcpy(&linked_ref->new_oid, ref->oid); } clean_exit: diff --git a/repack-midx.c b/repack-midx.c index 6f6202c5bc..349f7e20b5 100644 --- a/repack-midx.c +++ b/repack-midx.c @@ -16,25 +16,23 @@ struct midx_snapshot_ref_data { int preferred; }; -static int midx_snapshot_ref_one(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, void *_data) +static int midx_snapshot_ref_one(const struct reference *ref, void *_data) { struct midx_snapshot_ref_data *data = _data; + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; - if (!peel_iterated_oid(data->repo, oid, &peeled)) - oid = &peeled; + if (!peel_iterated_oid(data->repo, ref->oid, &peeled)) + maybe_peeled = &peeled; - if (oidset_insert(&data->seen, oid)) + if (oidset_insert(&data->seen, maybe_peeled)) return 0; /* already seen */ - if (odb_read_object_info(data->repo->objects, oid, NULL) != OBJ_COMMIT) + if (odb_read_object_info(data->repo->objects, maybe_peeled, NULL) != OBJ_COMMIT) return 0; fprintf(data->f->fp, "%s%s\n", data->preferred ? "+" : "", - oid_to_hex(oid)); + oid_to_hex(maybe_peeled)); return 0; } diff --git a/replace-object.c b/replace-object.c index 3eae051074..03d0f1f083 100644 --- a/replace-object.c +++ b/replace-object.c @@ -8,31 +8,27 @@ #include "repository.h" #include "commit.h" -static int register_replace_ref(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int register_replace_ref(const struct reference *ref, void *cb_data) { struct repository *r = cb_data; /* Get sha1 from refname */ - const char *slash = strrchr(refname, '/'); - const char *hash = slash ? slash + 1 : refname; + const char *slash = strrchr(ref->name, '/'); + const char *hash = slash ? slash + 1 : ref->name; struct replace_object *repl_obj = xmalloc(sizeof(*repl_obj)); if (get_oid_hex_algop(hash, &repl_obj->original.oid, r->hash_algo)) { free(repl_obj); - warning(_("bad replace ref name: %s"), refname); + warning(_("bad replace ref name: %s"), ref->name); return 0; } /* Copy sha1 from the read ref */ - oidcpy(&repl_obj->replacement, oid); + oidcpy(&repl_obj->replacement, ref->oid); /* Register new object */ if (oidmap_put(&r->objects->replace_map, repl_obj)) - die(_("duplicate replace ref: %s"), refname); + die(_("duplicate replace ref: %s"), ref->name); return 0; } diff --git a/revision.c b/revision.c index cf5e6c1ec9..5f0850ae5c 100644 --- a/revision.c +++ b/revision.c @@ -1644,19 +1644,17 @@ struct all_refs_cb { struct worktree *wt; }; -static int handle_one_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int handle_one_ref(const struct reference *ref, void *cb_data) { struct all_refs_cb *cb = cb_data; struct object *object; - if (ref_excluded(&cb->all_revs->ref_excludes, path)) + if (ref_excluded(&cb->all_revs->ref_excludes, ref->name)) return 0; - object = get_reference(cb->all_revs, path, oid, cb->all_flags); - add_rev_cmdline(cb->all_revs, object, path, REV_CMD_REF, cb->all_flags); - add_pending_object(cb->all_revs, object, path); + object = get_reference(cb->all_revs, ref->name, ref->oid, cb->all_flags); + add_rev_cmdline(cb->all_revs, object, ref->name, REV_CMD_REF, cb->all_flags); + add_pending_object(cb->all_revs, object, ref->name); return 0; } diff --git a/server-info.c b/server-info.c index 1d33de821e..0a07c722e8 100644 --- a/server-info.c +++ b/server-info.c @@ -148,23 +148,21 @@ static int update_info_file(struct repository *r, char *path, return ret; } -static int add_info_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int add_info_ref(const struct reference *ref, void *cb_data) { struct update_info_ctx *uic = cb_data; - struct object *o = parse_object(uic->repo, oid); + struct object *o = parse_object(uic->repo, ref->oid); if (!o) return -1; - if (uic_printf(uic, "%s %s\n", oid_to_hex(oid), path) < 0) + if (uic_printf(uic, "%s %s\n", oid_to_hex(ref->oid), ref->name) < 0) return -1; if (o->type == OBJ_TAG) { - o = deref_tag(uic->repo, o, path, 0); + o = deref_tag(uic->repo, o, ref->name, 0); if (o) if (uic_printf(uic, "%s %s^{}\n", - oid_to_hex(&o->oid), path) < 0) + oid_to_hex(&o->oid), ref->name) < 0) return -1; } return 0; diff --git a/shallow.c b/shallow.c index d9cd4e219c..55b9cd9d3f 100644 --- a/shallow.c +++ b/shallow.c @@ -626,14 +626,10 @@ static void paint_down(struct paint_info *info, const struct object_id *oid, free(tmp); } -static int mark_uninteresting(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *cb_data UNUSED) +static int mark_uninteresting(const struct reference *ref, void *cb_data UNUSED) { struct commit *commit = lookup_commit_reference_gently(the_repository, - oid, 1); + ref->oid, 1); if (!commit) return 0; commit->object.flags |= UNINTERESTING; @@ -742,16 +738,12 @@ struct commit_array { size_t nr, alloc; }; -static int add_ref(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *cb_data) +static int add_ref(const struct reference *ref, void *cb_data) { struct commit_array *ca = cb_data; ALLOC_GROW(ca->commits, ca->nr + 1, ca->alloc); ca->commits[ca->nr] = lookup_commit_reference_gently(the_repository, - oid, 1); + ref->oid, 1); if (ca->commits[ca->nr]) ca->nr++; return 0; diff --git a/submodule.c b/submodule.c index 35c55155f7..40a5c6fb9d 100644 --- a/submodule.c +++ b/submodule.c @@ -934,10 +934,7 @@ static void free_submodules_data(struct string_list *submodules) string_list_clear(submodules, 1); } -static int has_remote(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, void *cb_data UNUSED) +static int has_remote(const struct reference *ref UNUSED, void *cb_data UNUSED) { return 1; } @@ -1255,13 +1252,10 @@ int push_unpushed_submodules(struct repository *r, return ret; } -static int append_oid_to_array(const char *ref UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *data) +static int append_oid_to_array(const struct reference *ref, void *data) { struct oid_array *array = data; - oid_array_append(array, oid); + oid_array_append(array, ref->oid); return 0; } diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index 83b06d39a3..b1215947c5 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -154,10 +154,9 @@ static int cmd_rename_ref(struct ref_store *refs, const char **argv) return refs_rename_ref(refs, oldref, newref, logmsg); } -static int each_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags, void *cb_data UNUSED) +static int each_ref(const struct reference *ref, void *cb_data UNUSED) { - printf("%s %s 0x%x\n", oid_to_hex(oid), refname, flags); + printf("%s %s 0x%x\n", oid_to_hex(ref->oid), ref->name, ref->flags); return 0; } diff --git a/upload-pack.c b/upload-pack.c index 1e87ae9559..0d563ae74e 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -870,8 +870,8 @@ static void send_unshallow(struct upload_pack_data *data) } } -static int check_ref(const char *refname_full, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data); +static int check_ref(const struct reference *ref, void *cb_data); + static void deepen(struct upload_pack_data *data, int depth) { if (depth == INFINITE_DEPTH && !is_repository_shallow(the_repository)) { @@ -1224,13 +1224,12 @@ static int mark_our_ref(const char *refname, const char *refname_full, return 0; } -static int check_ref(const char *refname_full, const char *referent UNUSED,const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int check_ref(const struct reference *ref, void *cb_data) { - const char *refname = strip_namespace(refname_full); + const char *refname = strip_namespace(ref->name); struct upload_pack_data *data = cb_data; - mark_our_ref(refname, refname_full, oid, &data->hidden_refs); + mark_our_ref(refname, ref->name, ref->oid, &data->hidden_refs); return 0; } @@ -1292,27 +1291,25 @@ static void write_v0_ref(struct upload_pack_data *data, return; } -static int send_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int send_ref(const struct reference *ref, void *cb_data) { - write_v0_ref(cb_data, refname, strip_namespace(refname), oid); + write_v0_ref(cb_data, ref->name, strip_namespace(ref->name), ref->oid); return 0; } -static int find_symref(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag, void *cb_data) +static int find_symref(const struct reference *ref, void *cb_data) { const char *symref_target; struct string_list_item *item; + int flag; - if ((flag & REF_ISSYMREF) == 0) + if ((ref->flags & REF_ISSYMREF) == 0) return 0; symref_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), - refname, 0, NULL, &flag); + ref->name, 0, NULL, &flag); if (!symref_target || (flag & REF_ISSYMREF) == 0) - die("'%s' is a symref but it is not?", refname); - item = string_list_append(cb_data, strip_namespace(refname)); + die("'%s' is a symref but it is not?", ref->name); + item = string_list_append(cb_data, strip_namespace(ref->name)); item->util = xstrdup(strip_namespace(symref_target)); return 0; } diff --git a/walker.c b/walker.c index 8073754517..409b646578 100644 --- a/walker.c +++ b/walker.c @@ -226,14 +226,10 @@ static int interpret_target(struct walker *walker, char *target, struct object_i return -1; } -static int mark_complete(const char *path UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, - void *cb_data UNUSED) +static int mark_complete(const struct reference *ref, void *cb_data UNUSED) { struct commit *commit = lookup_commit_reference_gently(the_repository, - oid, 1); + ref->oid, 1); if (commit) { commit->object.flags |= COMPLETE; diff --git a/worktree.c b/worktree.c index a2a5f51f29..9308389cb6 100644 --- a/worktree.c +++ b/worktree.c @@ -595,8 +595,15 @@ int other_head_refs(each_ref_fn fn, void *cb_data) if (refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname.buf, RESOLVE_REF_READING, - &oid, &flag)) - ret = fn(refname.buf, NULL, &oid, flag, cb_data); + &oid, &flag)) { + struct reference ref = { + .name = refname.buf, + .oid = &oid, + .flags = flag, + }; + + ret = fn(&ref, cb_data); + } if (ret) break; } -- GitLab From cf5faae21bd4e981785f0dfb481a7c0afd37a984 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Sep 2025 11:47:54 +0200 Subject: [PATCH 10/31] refs: introduce `.ref` field for the base iterator The base iterator has a couple of fields that tracks the name, target, object ID and flags for the current reference. Due to this design we have to create a new `struct reference` whenever we want to hand over that reference to the callback function, which is tedious and not very efficient. Convert the structure to instead contain a `struct reference` as member. This member is expected to be populated by the implementations of the iterator and is handed over to the callback directly. While at it, simplify `should_pack_ref()` to take a `struct reference` directly instead of passing its respective fields. Signed-off-by: Patrick Steinhardt --- refs.c | 8 +++---- refs/debug.c | 8 +++---- refs/files-backend.c | 47 ++++++++++++++++++----------------------- refs/iterator.c | 39 +++++++++++----------------------- refs/packed-backend.c | 46 ++++++++++++++++++++-------------------- refs/ref-cache.c | 10 ++++----- refs/refs-internal.h | 5 +---- refs/reftable-backend.c | 12 +++++------ 8 files changed, 75 insertions(+), 100 deletions(-) diff --git a/refs.c b/refs.c index 25f0579d61..f96cf43b12 100644 --- a/refs.c +++ b/refs.c @@ -2327,8 +2327,8 @@ int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) int peel_iterated_oid(struct repository *r, const struct object_id *base, struct object_id *peeled) { if (current_ref_iter && - (current_ref_iter->oid == base || - oideq(current_ref_iter->oid, base))) + (current_ref_iter->ref.oid == base || + oideq(current_ref_iter->ref.oid, base))) return ref_iterator_peel(current_ref_iter, peeled); return peel_object(r, base, peeled) ? -1 : 0; @@ -2703,7 +2703,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs while ((ok = ref_iterator_advance(iter)) == ITER_OK) { if (skip && - string_list_has_string(skip, iter->refname)) + string_list_has_string(skip, iter->ref.name)) continue; if (transaction && ref_transaction_maybe_set_rejected( @@ -2712,7 +2712,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs continue; strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - iter->refname, refname); + iter->ref.name, refname); goto cleanup; } diff --git a/refs/debug.c b/refs/debug.c index 697adbd0dc..67718bd1f4 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -160,11 +160,9 @@ static int debug_ref_iterator_advance(struct ref_iterator *ref_iterator) trace_printf_key(&trace_refs, "iterator_advance: (%d)\n", res); else trace_printf_key(&trace_refs, "iterator_advance: %s (0)\n", - diter->iter->refname); + diter->iter->ref.name); - diter->base.refname = diter->iter->refname; - diter->base.oid = diter->iter->oid; - diter->base.flags = diter->iter->flags; + diter->base.ref = diter->iter->ref; return res; } @@ -185,7 +183,7 @@ static int debug_ref_iterator_peel(struct ref_iterator *ref_iterator, struct debug_ref_iterator *diter = (struct debug_ref_iterator *)ref_iterator; int res = diter->iter->vtable->peel(diter->iter, peeled); - trace_printf_key(&trace_refs, "iterator_peel: %s: %d\n", diter->iter->refname, res); + trace_printf_key(&trace_refs, "iterator_peel: %s: %d\n", diter->iter->ref.name, res); return res; } diff --git a/refs/files-backend.c b/refs/files-backend.c index eb3142f8f2..fac53fa052 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -961,26 +961,23 @@ static int files_ref_iterator_advance(struct ref_iterator *ref_iterator) while ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) { if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY && - parse_worktree_ref(iter->iter0->refname, NULL, NULL, + parse_worktree_ref(iter->iter0->ref.name, NULL, NULL, NULL) != REF_WORKTREE_CURRENT) continue; if ((iter->flags & DO_FOR_EACH_OMIT_DANGLING_SYMREFS) && - (iter->iter0->flags & REF_ISSYMREF) && - (iter->iter0->flags & REF_ISBROKEN)) + (iter->iter0->ref.flags & REF_ISSYMREF) && + (iter->iter0->ref.flags & REF_ISBROKEN)) continue; if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) && - !ref_resolves_to_object(iter->iter0->refname, + !ref_resolves_to_object(iter->iter0->ref.name, iter->repo, - iter->iter0->oid, - iter->iter0->flags)) + iter->iter0->ref.oid, + iter->iter0->ref.flags)) continue; - iter->base.refname = iter->iter0->refname; - iter->base.oid = iter->iter0->oid; - iter->base.flags = iter->iter0->flags; - iter->base.referent = iter->iter0->referent; + iter->base.ref = iter->iter0->ref; return ITER_OK; } @@ -1367,30 +1364,29 @@ static void prune_refs(struct files_ref_store *refs, struct ref_to_prune **refs_ * Return true if the specified reference should be packed. */ static int should_pack_ref(struct files_ref_store *refs, - const char *refname, - const struct object_id *oid, unsigned int ref_flags, + const struct reference *ref, struct pack_refs_opts *opts) { struct string_list_item *item; /* Do not pack per-worktree refs: */ - if (parse_worktree_ref(refname, NULL, NULL, NULL) != + if (parse_worktree_ref(ref->name, NULL, NULL, NULL) != REF_WORKTREE_SHARED) return 0; /* Do not pack symbolic refs: */ - if (ref_flags & REF_ISSYMREF) + if (ref->flags & REF_ISSYMREF) return 0; /* Do not pack broken refs: */ - if (!ref_resolves_to_object(refname, refs->base.repo, oid, ref_flags)) + if (!ref_resolves_to_object(ref->name, refs->base.repo, ref->oid, ref->flags)) return 0; - if (ref_excluded(opts->exclusions, refname)) + if (ref_excluded(opts->exclusions, ref->name)) return 0; for_each_string_list_item(item, opts->includes) - if (!wildmatch(item->string, refname, 0)) + if (!wildmatch(item->string, ref->name, 0)) return 1; return 0; @@ -1443,8 +1439,7 @@ static int should_pack_refs(struct files_ref_store *refs, iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, 0), NULL, refs->base.repo, 0); while ((ret = ref_iterator_advance(iter)) == ITER_OK) { - if (should_pack_ref(refs, iter->refname, iter->oid, - iter->flags, opts)) + if (should_pack_ref(refs, &iter->ref, opts)) refcount++; if (refcount >= limit) { ref_iterator_free(iter); @@ -1489,24 +1484,24 @@ static int files_pack_refs(struct ref_store *ref_store, * in the packed ref cache. If the reference should be * pruned, also add it to refs_to_prune. */ - if (!should_pack_ref(refs, iter->refname, iter->oid, iter->flags, opts)) + if (!should_pack_ref(refs, &iter->ref, opts)) continue; /* * Add a reference creation for this reference to the * packed-refs transaction: */ - if (ref_transaction_update(transaction, iter->refname, - iter->oid, NULL, NULL, NULL, + if (ref_transaction_update(transaction, iter->ref.name, + iter->ref.oid, NULL, NULL, NULL, REF_NO_DEREF, NULL, &err)) die("failure preparing to create packed reference %s: %s", - iter->refname, err.buf); + iter->ref.name, err.buf); /* Schedule the loose reference for pruning if requested. */ if ((opts->flags & PACK_REFS_PRUNE)) { struct ref_to_prune *n; - FLEX_ALLOC_STR(n, name, iter->refname); - oidcpy(&n->oid, iter->oid); + FLEX_ALLOC_STR(n, name, iter->ref.name); + oidcpy(&n->oid, iter->ref.oid); n->next = refs_to_prune; refs_to_prune = n; } @@ -2379,7 +2374,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator) REFNAME_ALLOW_ONELEVEL)) continue; - iter->base.refname = diter->relative_path; + iter->base.ref.name = diter->relative_path; return ITER_OK; } diff --git a/refs/iterator.c b/refs/iterator.c index 7f2e718f1c..fe5980e1b6 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -41,10 +41,7 @@ void base_ref_iterator_init(struct ref_iterator *iter, struct ref_iterator_vtable *vtable) { iter->vtable = vtable; - iter->refname = NULL; - iter->referent = NULL; - iter->oid = NULL; - iter->flags = 0; + memset(&iter->ref, 0, sizeof(iter->ref)); } struct empty_ref_iterator { @@ -127,8 +124,8 @@ enum iterator_selection ref_iterator_select(struct ref_iterator *iter_worktree, * latter. */ if (iter_worktree) { - int cmp = strcmp(iter_worktree->refname, - iter_common->refname); + int cmp = strcmp(iter_worktree->ref.name, + iter_common->ref.name); if (cmp < 0) return ITER_SELECT_0; else if (!cmp) @@ -139,7 +136,7 @@ enum iterator_selection ref_iterator_select(struct ref_iterator *iter_worktree, * We now know that the lexicographically-next ref is a common * ref. When the common ref is a shared one we return it. */ - if (parse_worktree_ref(iter_common->refname, NULL, NULL, + if (parse_worktree_ref(iter_common->ref.name, NULL, NULL, NULL) == REF_WORKTREE_SHARED) return ITER_SELECT_1; @@ -212,10 +209,7 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) } if (selection & ITER_YIELD_CURRENT) { - iter->base.referent = (*iter->current)->referent; - iter->base.refname = (*iter->current)->refname; - iter->base.oid = (*iter->current)->oid; - iter->base.flags = (*iter->current)->flags; + iter->base.ref = (*iter->current)->ref; return ITER_OK; } } @@ -313,7 +307,7 @@ static enum iterator_selection overlay_iterator_select( else if (!front) return ITER_SELECT_1; - cmp = strcmp(front->refname, back->refname); + cmp = strcmp(front->ref.name, back->ref.name); if (cmp < 0) return ITER_SELECT_0; @@ -371,7 +365,7 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator) int ok; while ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) { - int cmp = compare_prefix(iter->iter0->refname, iter->prefix); + int cmp = compare_prefix(iter->iter0->ref.name, iter->prefix); if (cmp < 0) continue; /* @@ -382,6 +376,8 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator) if (cmp > 0) return ITER_DONE; + iter->base.ref = iter->iter0->ref; + if (iter->trim) { /* * It is nonsense to trim off characters that @@ -392,15 +388,11 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator) * one character left in the refname after * trimming, report it as a bug: */ - if (strlen(iter->iter0->refname) <= iter->trim) + if (strlen(iter->base.ref.name) <= iter->trim) BUG("attempt to trim too many characters"); - iter->base.refname = iter->iter0->refname + iter->trim; - } else { - iter->base.refname = iter->iter0->refname; + iter->base.ref.name += iter->trim; } - iter->base.oid = iter->iter0->oid; - iter->base.flags = iter->iter0->flags; return ITER_OK; } @@ -476,14 +468,7 @@ int do_for_each_ref_iterator(struct ref_iterator *iter, current_ref_iter = iter; while ((ok = ref_iterator_advance(iter)) == ITER_OK) { - struct reference ref = { - .name = iter->refname, - .target = iter->referent, - .oid = iter->oid, - .flags = iter->flags, - }; - - retval = fn(&ref, cb_data); + retval = fn(&iter->ref, cb_data); if (retval) goto out; } diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a8c22a0a7f..7987acdc96 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -908,7 +908,7 @@ static int next_record(struct packed_ref_iterator *iter) if (iter->pos == iter->eof) return ITER_DONE; - iter->base.flags = REF_ISPACKED; + iter->base.ref.flags = REF_ISPACKED; p = iter->pos; if (iter->eof - p < snapshot_hexsz(iter->snapshot) + 2 || @@ -923,22 +923,22 @@ static int next_record(struct packed_ref_iterator *iter) iter->pos, iter->eof - iter->pos); strbuf_add(&iter->refname_buf, p, eol - p); - iter->base.refname = iter->refname_buf.buf; + iter->base.ref.name = iter->refname_buf.buf; if (refname_contains_nul(&iter->refname_buf)) - die("packed refname contains embedded NULL: %s", iter->base.refname); + die("packed refname contains embedded NULL: %s", iter->base.ref.name); - if (check_refname_format(iter->base.refname, REFNAME_ALLOW_ONELEVEL)) { - if (!refname_is_safe(iter->base.refname)) + if (check_refname_format(iter->base.ref.name, REFNAME_ALLOW_ONELEVEL)) { + if (!refname_is_safe(iter->base.ref.name)) die("packed refname is dangerous: %s", - iter->base.refname); + iter->base.ref.name); oidclr(&iter->oid, iter->repo->hash_algo); - iter->base.flags |= REF_BAD_NAME | REF_ISBROKEN; + iter->base.ref.flags |= REF_BAD_NAME | REF_ISBROKEN; } if (iter->snapshot->peeled == PEELED_FULLY || (iter->snapshot->peeled == PEELED_TAGS && - starts_with(iter->base.refname, "refs/tags/"))) - iter->base.flags |= REF_KNOWS_PEELED; + starts_with(iter->base.ref.name, "refs/tags/"))) + iter->base.ref.flags |= REF_KNOWS_PEELED; iter->pos = eol + 1; @@ -956,11 +956,11 @@ static int next_record(struct packed_ref_iterator *iter) * definitely know the value of *this* reference. But * we suppress it if the reference is broken: */ - if ((iter->base.flags & REF_ISBROKEN)) { + if ((iter->base.ref.flags & REF_ISBROKEN)) { oidclr(&iter->peeled, iter->repo->hash_algo); - iter->base.flags &= ~REF_KNOWS_PEELED; + iter->base.ref.flags &= ~REF_KNOWS_PEELED; } else { - iter->base.flags |= REF_KNOWS_PEELED; + iter->base.ref.flags |= REF_KNOWS_PEELED; } } else { oidclr(&iter->peeled, iter->repo->hash_algo); @@ -976,15 +976,15 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator) int ok; while ((ok = next_record(iter)) == ITER_OK) { - const char *refname = iter->base.refname; + const char *refname = iter->base.ref.name; const char *prefix = iter->prefix; if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY && - !is_per_worktree_ref(iter->base.refname)) + !is_per_worktree_ref(iter->base.ref.name)) continue; if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) && - !ref_resolves_to_object(iter->base.refname, iter->repo, + !ref_resolves_to_object(iter->base.ref.name, iter->repo, &iter->oid, iter->flags)) continue; @@ -1033,10 +1033,10 @@ static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator, struct packed_ref_iterator *iter = (struct packed_ref_iterator *)ref_iterator; - if ((iter->base.flags & REF_KNOWS_PEELED)) { + if ((iter->base.ref.flags & REF_KNOWS_PEELED)) { oidcpy(peeled, &iter->peeled); return is_null_oid(&iter->peeled) ? -1 : 0; - } else if ((iter->base.flags & (REF_ISBROKEN | REF_ISSYMREF))) { + } else if ((iter->base.ref.flags & (REF_ISBROKEN | REF_ISSYMREF))) { return -1; } else { return peel_object(iter->repo, &iter->oid, peeled) ? -1 : 0; @@ -1194,7 +1194,7 @@ static struct ref_iterator *packed_ref_iterator_begin( iter->snapshot = snapshot; acquire_snapshot(snapshot); strbuf_init(&iter->refname_buf, 0); - iter->base.oid = &iter->oid; + iter->base.ref.oid = &iter->oid; iter->repo = ref_store->repo; iter->flags = flags; @@ -1436,7 +1436,7 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re if (!iter) cmp = +1; else - cmp = strcmp(iter->refname, update->refname); + cmp = strcmp(iter->ref.name, update->refname); } if (!cmp) { @@ -1459,11 +1459,11 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re } goto error; - } else if (!oideq(&update->old_oid, iter->oid)) { + } else if (!oideq(&update->old_oid, iter->ref.oid)) { strbuf_addf(err, "cannot update ref '%s': " "is at %s but expected %s", update->refname, - oid_to_hex(iter->oid), + oid_to_hex(iter->ref.oid), oid_to_hex(&update->old_oid)); ret = REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE; @@ -1527,8 +1527,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re struct object_id peeled; int peel_error = ref_iterator_peel(iter, &peeled); - if (write_packed_entry(out, iter->refname, - iter->oid, + if (write_packed_entry(out, iter->ref.name, + iter->ref.oid, peel_error ? NULL : &peeled)) goto write_error; diff --git a/refs/ref-cache.c b/refs/ref-cache.c index e5e5df16d8..f1abc39624 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -425,10 +425,10 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator) level->prefix_state = entry_prefix_state; level->index = -1; } else { - iter->base.refname = entry->name; - iter->base.referent = entry->u.value.referent; - iter->base.oid = &entry->u.value.oid; - iter->base.flags = entry->flag; + iter->base.ref.name = entry->name; + iter->base.ref.target = entry->u.value.referent; + iter->base.ref.oid = &entry->u.value.oid; + iter->base.ref.flags = entry->flag; return ITER_OK; } } @@ -550,7 +550,7 @@ static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator, { struct cache_ref_iterator *iter = (struct cache_ref_iterator *)ref_iterator; - return peel_object(iter->repo, ref_iterator->oid, peeled) ? -1 : 0; + return peel_object(iter->repo, ref_iterator->ref.oid, peeled) ? -1 : 0; } static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 4ef3bd75c6..ed749d1657 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -249,10 +249,7 @@ const char *find_descendant_ref(const char *dirname, */ struct ref_iterator { struct ref_iterator_vtable *vtable; - const char *refname; - const char *referent; - const struct object_id *oid; - unsigned int flags; + struct reference ref; }; /* diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index d4b7928620..0e47986cb5 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -704,10 +704,10 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) &iter->oid, flags)) continue; - iter->base.refname = iter->ref.refname; - iter->base.referent = referent; - iter->base.oid = &iter->oid; - iter->base.flags = flags; + iter->base.ref.name = iter->ref.refname; + iter->base.ref.target = referent; + iter->base.ref.oid = &iter->oid; + iter->base.ref.flags = flags; break; } @@ -828,7 +828,7 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_ iter = xcalloc(1, sizeof(*iter)); base_ref_iterator_init(&iter->base, &reftable_ref_iterator_vtable); - iter->base.oid = &iter->oid; + iter->base.ref.oid = &iter->oid; iter->flags = flags; iter->refs = refs; iter->exclude_patterns = filter_exclude_patterns(exclude_patterns); @@ -2072,7 +2072,7 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator) strbuf_reset(&iter->last_name); strbuf_addstr(&iter->last_name, iter->log.refname); - iter->base.refname = iter->log.refname; + iter->base.ref.name = iter->log.refname; break; } -- GitLab From c0715cf44d887f34ad2aa66b137b3365d128e974 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 8 Oct 2025 15:57:57 +0200 Subject: [PATCH 11/31] refs: fully reset `struct ref_iterator::ref` on iteration With the introduction of the `struct ref_iterator::ref` field it now is a whole lot easier to introduce new fields that become accessible to the caller without having to adapt every single callsite. But there's a downside: when a new field is introduced we always have to adapt all backends to set that field. This isn't something we can avoid in the general case: when the new field is expected to be populated by all backends we of course cannot avoid doing so. But new fields may be entirely optional, in which case we'd still have such churn. And furthermore, it is very easy right now to leak state from a previous iteration into the next iteration. Address this issue by ensuring that the reference backends all fully reset the field on every single iteration. This ensures that no state from previous iterations can leak into the next one. And it ensures that any newly introduced fields will be zeroed out by default. Note that we don't have to explicitly adapt the "files" backend, as it uses the `cache_ref_iterator` internally. Furthermore, other "wrapping" iterators like for example the `prefix_ref_iterator` copy around the whole reference, so these don't need to be adapted either. Signed-off-by: Patrick Steinhardt --- refs/packed-backend.c | 3 ++- refs/ref-cache.c | 1 + refs/reftable-backend.c | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 7987acdc96..711e07f832 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -882,6 +882,7 @@ static int next_record(struct packed_ref_iterator *iter) { const char *p, *eol; + memset(&iter->base.ref, 0, sizeof(iter->base.ref)); strbuf_reset(&iter->refname_buf); /* @@ -916,6 +917,7 @@ static int next_record(struct packed_ref_iterator *iter) !isspace(*p++)) die_invalid_line(iter->snapshot->refs->path, iter->pos, iter->eof - iter->pos); + iter->base.ref.oid = &iter->oid; eol = memchr(p, '\n', iter->eof - p); if (!eol) @@ -1194,7 +1196,6 @@ static struct ref_iterator *packed_ref_iterator_begin( iter->snapshot = snapshot; acquire_snapshot(snapshot); strbuf_init(&iter->refname_buf, 0); - iter->base.ref.oid = &iter->oid; iter->repo = ref_store->repo; iter->flags = flags; diff --git a/refs/ref-cache.c b/refs/ref-cache.c index f1abc39624..e427848879 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -425,6 +425,7 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator) level->prefix_state = entry_prefix_state; level->index = -1; } else { + memset(&iter->base.ref, 0, sizeof(iter->base.ref)); iter->base.ref.name = entry->name; iter->base.ref.target = entry->u.value.referent; iter->base.ref.oid = &entry->u.value.oid; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 0e47986cb5..728886eafd 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -704,6 +704,7 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) &iter->oid, flags)) continue; + memset(&iter->base.ref, 0, sizeof(iter->base.ref)); iter->base.ref.name = iter->ref.refname; iter->base.ref.target = referent; iter->base.ref.oid = &iter->oid; -- GitLab From 52f65b25e8958fd553cbd59c60062a933cbdbb22 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Sep 2025 10:47:39 +0200 Subject: [PATCH 12/31] refs: refactor reference status flags The reference flags encode information like whether or not a reference is a symbolic reference or whether it may be broken. This information is stored in a `int flags` bitfield, which is in conflict with our modern best practices; we tend to use an unsigned integer to store flags. Change the type of the field to be `unsigned`. While at it, refactor the individual flags to be part of an `enum` instead of using preprocessor defines. Signed-off-by: Patrick Steinhardt --- refs.h | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/refs.h b/refs.h index 68d235438c..4f0a685714 100644 --- a/refs.h +++ b/refs.h @@ -333,27 +333,28 @@ struct ref_transaction; * stored in ref_iterator::flags. Other bits are for internal use * only: */ +enum reference_status { + /* Reference is a symbolic reference. */ + REF_ISSYMREF = (1 << 0), -/* Reference is a symbolic reference. */ -#define REF_ISSYMREF 0x01 + /* Reference is a packed reference. */ + REF_ISPACKED = (1 << 1), -/* Reference is a packed reference. */ -#define REF_ISPACKED 0x02 - -/* - * Reference cannot be resolved to an object name: dangling symbolic - * reference (directly or indirectly), corrupt reference file, - * reference exists but name is bad, or symbolic reference refers to - * ill-formatted reference name. - */ -#define REF_ISBROKEN 0x04 + /* + * Reference cannot be resolved to an object name: dangling symbolic + * reference (directly or indirectly), corrupt reference file, + * reference exists but name is bad, or symbolic reference refers to + * ill-formatted reference name. + */ + REF_ISBROKEN = (1 << 2), -/* - * Reference name is not well formed. - * - * See git-check-ref-format(1) for the definition of well formed ref names. - */ -#define REF_BAD_NAME 0x08 + /* + * Reference name is not well formed. + * + * See git-check-ref-format(1) for the definition of well formed ref names. + */ + REF_BAD_NAME = (1 << 3), +}; /* A reference passed to `for_each_ref()`-style callbacks. */ struct reference { @@ -370,8 +371,8 @@ struct reference { */ const struct object_id *oid; - /* A bitfield of `REF_` flags. */ - int flags; + /* A bitfield of `enum reference_status` flags. */ + unsigned flags; }; /* -- GitLab From 230cc4e7431cbe6f43452ad72a39fdb47f9fadfd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 22 Oct 2025 07:09:50 +0200 Subject: [PATCH 13/31] refs: expose peeled object ID via the iterator Both the "files" and "reftable" backend are able to store peeled values for tags in the respective formats. This allows for a more efficient lookup of the target object of such a tag without having to manually peel via the object database. The infrastructure to access these peeled object IDs is somewhat funky though. When iterating through objects, we store a pointer reference to the current iterator in a global variable. The callbacks invoked by that iterator are then expected to call `peel_iterated_oid()`, which checks whether the globally-stored iterator's current reference refers to the one handed into that function. If so, we ask the iterator to peel the object, otherwise we manually peel the object via the object database. Depending on global state like this is somewhat weird and also quite fragile. Introduce a new `struct reference::peeled_oid` field that can be populated by the reference backends. This field can be accessed via a new function `reference_get_peeled_oid()` that either uses that value, if set, or alternatively peels via the ODB. With this change we don't have to rely on global state anymore, but make the peeled object ID available to the callback functions directly. Adjust trivial callers that already have a `struct reference` available. Remaining callers will be adjusted in subsequent commits. Signed-off-by: Patrick Steinhardt --- builtin/describe.c | 2 +- builtin/gc.c | 2 +- builtin/pack-objects.c | 7 ++++--- commit-graph.c | 2 +- ls-refs.c | 2 +- midx-write.c | 2 +- pseudo-merge.c | 2 +- refs.c | 12 ++++++++++++ refs.h | 19 +++++++++++++++++++ refs/packed-backend.c | 1 + refs/reftable-backend.c | 5 +++++ repack-midx.c | 2 +- 12 files changed, 48 insertions(+), 10 deletions(-) diff --git a/builtin/describe.c b/builtin/describe.c index 7954535044..443546aaac 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -208,7 +208,7 @@ static int get_name(const struct reference *ref, void *cb_data UNUSED) } /* Is it annotated? */ - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) { + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) { is_annotated = !oideq(ref->oid, &peeled); } else { oidcpy(&peeled, ref->oid); diff --git a/builtin/gc.c b/builtin/gc.c index 9de5de175f..f0cf20d423 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1109,7 +1109,7 @@ static int dfs_on_ref(const struct reference *ref, void *cb_data) struct commit_list *stack = NULL; struct commit *commit; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) maybe_peeled = &peeled; if (odb_read_object_info(the_repository->objects, maybe_peeled, NULL) != OBJ_COMMIT) return 0; diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 39633a0158..1613fecb66 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -838,7 +838,7 @@ static int mark_tagged(const struct reference *ref, void *cb_data UNUSED) if (entry) entry->tagged = 1; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) { + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) { entry = packlist_find(&to_pack, &peeled); if (entry) entry->tagged = 1; @@ -3309,7 +3309,8 @@ static int add_ref_tag(const struct reference *ref, void *cb_data UNUSED) { struct object_id peeled; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled) && obj_is_packed(&peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled) && + obj_is_packed(&peeled)) add_tag_chain(ref->oid); return 0; } @@ -4537,7 +4538,7 @@ static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNU struct object_id peeled; struct object *object; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) maybe_peeled = &peeled; object = parse_object_or_die(the_repository, maybe_peeled, ref->name); diff --git a/commit-graph.c b/commit-graph.c index f91af41625..80be2ff2c3 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1857,7 +1857,7 @@ static int add_ref_to_set(const struct reference *ref, void *cb_data) struct object_id peeled; struct refs_cb_data *data = (struct refs_cb_data *)cb_data; - if (!peel_iterated_oid(data->repo, ref->oid, &peeled)) + if (!reference_get_peeled_oid(data->repo, ref, &peeled)) maybe_peeled = &peeled; if (odb_read_object_info(data->repo->objects, maybe_peeled, NULL) == OBJ_COMMIT) oidset_insert(data->commits, maybe_peeled); diff --git a/ls-refs.c b/ls-refs.c index 64d0272369..8641281b86 100644 --- a/ls-refs.c +++ b/ls-refs.c @@ -110,7 +110,7 @@ static int send_ref(const struct reference *ref, void *cb_data) if (data->peel && ref->oid) { struct object_id peeled; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) strbuf_addf(&data->buf, " peeled:%s", oid_to_hex(&peeled)); } diff --git a/midx-write.c b/midx-write.c index f4dd875747..23e61cb000 100644 --- a/midx-write.c +++ b/midx-write.c @@ -709,7 +709,7 @@ static int add_ref_to_pending(const struct reference *ref, void *cb_data) return 0; } - if (!peel_iterated_oid(revs->repo, ref->oid, &peeled)) + if (!reference_get_peeled_oid(revs->repo, ref, &peeled)) maybe_peeled = &peeled; object = parse_object_or_die(revs->repo, maybe_peeled, ref->name); diff --git a/pseudo-merge.c b/pseudo-merge.c index 0abd51b42c..a2d5bd85f9 100644 --- a/pseudo-merge.c +++ b/pseudo-merge.c @@ -230,7 +230,7 @@ static int find_pseudo_merge_group_for_ref(const struct reference *ref, void *_d uint32_t i; int has_bitmap; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) maybe_peeled = &peeled; c = lookup_commit(the_repository, maybe_peeled); diff --git a/refs.c b/refs.c index f96cf43b12..1b1551f981 100644 --- a/refs.c +++ b/refs.c @@ -2334,6 +2334,18 @@ int peel_iterated_oid(struct repository *r, const struct object_id *base, struct return peel_object(r, base, peeled) ? -1 : 0; } +int reference_get_peeled_oid(struct repository *repo, + const struct reference *ref, + struct object_id *peeled_oid) +{ + if (ref->peeled_oid) { + oidcpy(peeled_oid, ref->peeled_oid); + return 0; + } + + return peel_object(repo, ref->oid, peeled_oid) ? -1 : 0; +} + int refs_update_symref(struct ref_store *refs, const char *ref, const char *target, const char *logmsg) { diff --git a/refs.h b/refs.h index 4f0a685714..886ed2c0f4 100644 --- a/refs.h +++ b/refs.h @@ -371,10 +371,29 @@ struct reference { */ const struct object_id *oid; + /* + * An optional peeled object ID. This field _may_ be set for tags in + * case the peeled value is present in the backend. Please refer to + * `reference_get_peeled_oid()`. + */ + const struct object_id *peeled_oid; + /* A bitfield of `enum reference_status` flags. */ unsigned flags; }; +/* + * Peel the tag to a non-tag commit. If present, this uses the peeled object ID + * exposed by the reference backend. Otherwise, the object is peeled via the + * object database, which is less efficient. + * + * Return `0` if the reference could be peeled, a negative error code + * otherwise. + */ +int reference_get_peeled_oid(struct repository *repo, + const struct reference *ref, + struct object_id *peeled_oid); + /* * The signature for the callback function for the for_each_*() * functions below. The memory pointed to by the `struct reference` diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 711e07f832..1fefefd54e 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -963,6 +963,7 @@ static int next_record(struct packed_ref_iterator *iter) iter->base.ref.flags &= ~REF_KNOWS_PEELED; } else { iter->base.ref.flags |= REF_KNOWS_PEELED; + iter->base.ref.peeled_oid = &iter->peeled; } } else { oidclr(&iter->peeled, iter->repo->hash_algo); diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 728886eafd..e214e120d7 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -547,6 +547,7 @@ struct reftable_ref_iterator { struct reftable_iterator iter; struct reftable_ref_record ref; struct object_id oid; + struct object_id peeled_oid; char *prefix; size_t prefix_len; @@ -671,6 +672,8 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) case REFTABLE_REF_VAL2: oidread(&iter->oid, iter->ref.value.val2.value, refs->base.repo->hash_algo); + oidread(&iter->peeled_oid, iter->ref.value.val2.target_value, + refs->base.repo->hash_algo); break; case REFTABLE_REF_SYMREF: referent = refs_resolve_ref_unsafe(&iter->refs->base, @@ -708,6 +711,8 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) iter->base.ref.name = iter->ref.refname; iter->base.ref.target = referent; iter->base.ref.oid = &iter->oid; + if (iter->ref.value_type == REFTABLE_REF_VAL2) + iter->base.ref.peeled_oid = &iter->peeled_oid; iter->base.ref.flags = flags; break; diff --git a/repack-midx.c b/repack-midx.c index 349f7e20b5..74bdfa3a6e 100644 --- a/repack-midx.c +++ b/repack-midx.c @@ -22,7 +22,7 @@ static int midx_snapshot_ref_one(const struct reference *ref, void *_data) const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; - if (!peel_iterated_oid(data->repo, ref->oid, &peeled)) + if (!reference_get_peeled_oid(data->repo, ref, &peeled)) maybe_peeled = &peeled; if (oidset_insert(&data->seen, maybe_peeled)) -- GitLab From bca033ba86d35e5a72654332000d59ddcfe0b941 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Sep 2025 12:38:31 +0200 Subject: [PATCH 14/31] upload-pack: convert to use `reference_get_peeled_oid()` The `write_v0_ref()` callback is invoked from two callsites: - Once via `send_ref()` which is a callback passed to `for_each_namespaced_ref_1()` and `refs_head_ref_namespaced()`. - Once manually to announce capabilities. When sending references to the client we also send the peeled value of tags. As we don't have a `struct reference` available in the second case, we cannot easily peel by calling `reference_get_peeled_oid()`, but we instead have to depend on on global state via `peel_iterated_oid()`. We do have a reference available though in the first case, it's only the second case that keeps us from using `reference_get_peeled_oid()`. But that second case only announces capabilities anyway, so we're not really handling a reference at all here. Adapt that case to construct a reference manually and pass that to `write_v0_ref()`. Start to use `reference_get_peeled_oid()` now that we always have a `struct reference` available. Signed-off-by: Patrick Steinhardt --- upload-pack.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/upload-pack.c b/upload-pack.c index 0d563ae74e..2d2b70cbf2 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -1249,15 +1249,15 @@ static void format_session_id(struct strbuf *buf, struct upload_pack_data *d) { } static void write_v0_ref(struct upload_pack_data *data, - const char *refname, const char *refname_nons, - const struct object_id *oid) + const struct reference *ref, + const char *refname_nons) { static const char *capabilities = "multi_ack thin-pack side-band" " side-band-64k ofs-delta shallow deepen-since deepen-not" " deepen-relative no-progress include-tag multi_ack_detailed"; struct object_id peeled; - if (mark_our_ref(refname_nons, refname, oid, &data->hidden_refs)) + if (mark_our_ref(refname_nons, ref->name, ref->oid, &data->hidden_refs)) return; if (capabilities) { @@ -1267,7 +1267,7 @@ static void write_v0_ref(struct upload_pack_data *data, format_symref_info(&symref_info, &data->symref); format_session_id(&session_id, data); packet_fwrite_fmt(stdout, "%s %s%c%s%s%s%s%s%s%s object-format=%s agent=%s\n", - oid_to_hex(oid), refname_nons, + oid_to_hex(ref->oid), refname_nons, 0, capabilities, (data->allow_uor & ALLOW_TIP_SHA1) ? " allow-tip-sha1-in-want" : "", @@ -1283,17 +1283,17 @@ static void write_v0_ref(struct upload_pack_data *data, strbuf_release(&session_id); data->sent_capabilities = 1; } else { - packet_fwrite_fmt(stdout, "%s %s\n", oid_to_hex(oid), refname_nons); + packet_fwrite_fmt(stdout, "%s %s\n", oid_to_hex(ref->oid), refname_nons); } capabilities = NULL; - if (!peel_iterated_oid(the_repository, oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) packet_fwrite_fmt(stdout, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons); return; } static int send_ref(const struct reference *ref, void *cb_data) { - write_v0_ref(cb_data, ref->name, strip_namespace(ref->name), ref->oid); + write_v0_ref(cb_data, ref, strip_namespace(ref->name)); return 0; } @@ -1442,8 +1442,12 @@ void upload_pack(const int advertise_refs, const int stateless_rpc, send_ref, &data); for_each_namespaced_ref_1(send_ref, &data); if (!data.sent_capabilities) { - const char *refname = "capabilities^{}"; - write_v0_ref(&data, refname, refname, null_oid(the_hash_algo)); + struct reference ref = { + .name = "capabilities^{}", + .oid = null_oid(the_hash_algo), + }; + + write_v0_ref(&data, &ref, ref.name); } /* * fflush stdout before calling advertise_shallow_grafts because send_ref -- GitLab From fa914f4cede9500de0d76987fb935705926d798d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Sep 2025 12:54:42 +0200 Subject: [PATCH 15/31] ref-filter: propagate peeled object ID When queueing a reference in the "ref-filter" subsystem we end up creating a new ref array item that contains the reference's info. One bit of info that we always discard though is the peeled object ID, and because of that we are forced to use `peel_iterated_oid()`. Refactor the code to propagate the peeled object ID via the ref array, if available. This allows us to manually peel tags without having to go through the object database. Signed-off-by: Patrick Steinhardt --- builtin/ls-remote.c | 2 +- builtin/tag.c | 2 +- builtin/verify-tag.c | 2 +- ref-filter.c | 66 +++++++++++++++++++++++++------------------- ref-filter.h | 5 +++- 5 files changed, 45 insertions(+), 32 deletions(-) diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index df09000b30..fe77829557 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -156,7 +156,7 @@ int cmd_ls_remote(int argc, continue; if (!tail_match(&pattern, ref->name)) continue; - item = ref_array_push(&ref_array, ref->name, &ref->old_oid); + item = ref_array_push(&ref_array, ref->name, &ref->old_oid, NULL); item->symref = xstrdup_or_null(ref->symref); } diff --git a/builtin/tag.c b/builtin/tag.c index f0665af3ac..01eba90c5c 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -153,7 +153,7 @@ static int verify_tag(const char *name, const char *ref UNUSED, return -1; if (format->format) - pretty_print_ref(name, oid, format); + pretty_print_ref(name, oid, NULL, format); return 0; } diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index cd6bc11095..558121eaa1 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -67,7 +67,7 @@ int cmd_verify_tag(int argc, } if (format.format) - pretty_print_ref(name, &oid, &format); + pretty_print_ref(name, &oid, NULL, &format); } return had_error; } diff --git a/ref-filter.c b/ref-filter.c index 6837fa60a9..7fd8babec8 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2578,8 +2578,15 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) * If it is a tag object, see if we use the peeled value. If we do, * grab the peeled OID. */ - if (need_tagged && peel_iterated_oid(the_repository, &obj->oid, &oi_deref.oid)) - die("bad tag"); + if (need_tagged) { + if (!is_null_oid(&ref->peeled_oid)) { + oidcpy(&oi_deref.oid, &ref->peeled_oid); + } else if (!peel_object(the_repository, &obj->oid, &oi_deref.oid)) { + /* We managed to peel the object ourselves. */ + } else { + die("bad tag"); + } + } return get_object(ref, 1, &obj, &oi_deref, err); } @@ -2807,12 +2814,15 @@ static int match_points_at(struct oid_array *points_at, * Callers can then fill in other struct members at their leisure. */ static struct ref_array_item *new_ref_array_item(const char *refname, - const struct object_id *oid) + const struct object_id *oid, + const struct object_id *peeled_oid) { struct ref_array_item *ref; FLEX_ALLOC_STR(ref, refname, refname); oidcpy(&ref->objectname, oid); + if (peeled_oid) + oidcpy(&ref->peeled_oid, peeled_oid); ref->rest = NULL; return ref; @@ -2826,9 +2836,10 @@ static void ref_array_append(struct ref_array *array, struct ref_array_item *ref struct ref_array_item *ref_array_push(struct ref_array *array, const char *refname, - const struct object_id *oid) + const struct object_id *oid, + const struct object_id *peeled_oid) { - struct ref_array_item *ref = new_ref_array_item(refname, oid); + struct ref_array_item *ref = new_ref_array_item(refname, oid, peeled_oid); ref_array_append(array, ref); return ref; } @@ -2871,25 +2882,25 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname) return ref_kind_from_refname(refname); } -static struct ref_array_item *apply_ref_filter(const char *refname, const char *referent, const struct object_id *oid, - int flag, struct ref_filter *filter) +static struct ref_array_item *apply_ref_filter(const struct reference *ref, + struct ref_filter *filter) { - struct ref_array_item *ref; + struct ref_array_item *item; struct commit *commit = NULL; unsigned int kind; - if (flag & REF_BAD_NAME) { - warning(_("ignoring ref with broken name %s"), refname); + if (ref->flags & REF_BAD_NAME) { + warning(_("ignoring ref with broken name %s"), ref->name); return NULL; } - if (flag & REF_ISBROKEN) { - warning(_("ignoring broken ref %s"), refname); + if (ref->flags & REF_ISBROKEN) { + warning(_("ignoring broken ref %s"), ref->name); return NULL; } /* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */ - kind = filter_ref_kind(filter, refname); + kind = filter_ref_kind(filter, ref->name); /* * Generally HEAD refs are printed with special description denoting a rebase, @@ -2902,13 +2913,13 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const char * else if (!(kind & filter->kind)) return NULL; - if (!filter_pattern_match(filter, refname)) + if (!filter_pattern_match(filter, ref->name)) return NULL; - if (filter_exclude_match(filter, refname)) + if (filter_exclude_match(filter, ref->name)) return NULL; - if (filter->points_at.nr && !match_points_at(&filter->points_at, oid, refname)) + if (filter->points_at.nr && !match_points_at(&filter->points_at, ref->oid, ref->name)) return NULL; /* @@ -2918,7 +2929,7 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const char * */ if (filter->reachable_from || filter->unreachable_from || filter->with_commit || filter->no_commit || filter->verbose) { - commit = lookup_commit_reference_gently(the_repository, oid, 1); + commit = lookup_commit_reference_gently(the_repository, ref->oid, 1); if (!commit) return NULL; /* We perform the filtering for the '--contains' option... */ @@ -2936,13 +2947,13 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const char * * to do its job and the resulting list may yet to be pruned * by maxcount logic. */ - ref = new_ref_array_item(refname, oid); - ref->commit = commit; - ref->flag = flag; - ref->kind = kind; - ref->symref = xstrdup_or_null(referent); + item = new_ref_array_item(ref->name, ref->oid, ref->peeled_oid); + item->commit = commit; + item->flag = ref->flags; + item->kind = kind; + item->symref = xstrdup_or_null(ref->target); - return ref; + return item; } struct ref_filter_cbdata { @@ -2959,8 +2970,7 @@ static int filter_one(const struct reference *ref, void *cb_data) struct ref_filter_cbdata *ref_cbdata = cb_data; struct ref_array_item *item; - item = apply_ref_filter(ref->name, ref->target, ref->oid, - ref->flags, ref_cbdata->filter); + item = apply_ref_filter(ref, ref_cbdata->filter); if (item) ref_array_append(ref_cbdata->array, item); @@ -2997,8 +3007,7 @@ static int filter_and_format_one(const struct reference *ref, void *cb_data) struct ref_array_item *item; struct strbuf output = STRBUF_INIT, err = STRBUF_INIT; - item = apply_ref_filter(ref->name, ref->target, ref->oid, - ref->flags, ref_cbdata->filter); + item = apply_ref_filter(ref, ref_cbdata->filter); if (!item) return 0; @@ -3585,13 +3594,14 @@ void print_formatted_ref_array(struct ref_array *array, struct ref_format *forma } void pretty_print_ref(const char *name, const struct object_id *oid, + const struct object_id *peeled_oid, struct ref_format *format) { struct ref_array_item *ref_item; struct strbuf output = STRBUF_INIT; struct strbuf err = STRBUF_INIT; - ref_item = new_ref_array_item(name, oid); + ref_item = new_ref_array_item(name, oid, peeled_oid); ref_item->kind = ref_kind_from_refname(name); if (format_ref_array_item(ref_item, format, &output, &err)) die("%s", err.buf); diff --git a/ref-filter.h b/ref-filter.h index 235c60f79c..120221b47f 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -41,6 +41,7 @@ enum ref_sorting_order { struct ref_array_item { struct object_id objectname; + struct object_id peeled_oid; const char *rest; int flag; unsigned int kind; @@ -187,6 +188,7 @@ void print_formatted_ref_array(struct ref_array *array, struct ref_format *forma * name must be a fully qualified refname. */ void pretty_print_ref(const char *name, const struct object_id *oid, + const struct object_id *peeled_oid, struct ref_format *format); /* @@ -195,7 +197,8 @@ void pretty_print_ref(const char *name, const struct object_id *oid, */ struct ref_array_item *ref_array_push(struct ref_array *array, const char *refname, - const struct object_id *oid); + const struct object_id *oid, + const struct object_id *peeled_oid); /* * If the provided format includes ahead-behind atoms, then compute the -- GitLab From 1d9ba79bcf71284dab1bad516e5e231289a1d39b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Sep 2025 12:40:11 +0200 Subject: [PATCH 16/31] builtin/show-ref: convert to use `reference_get_peeled_oid()` The git-show-ref(1) command has multiple different modes: - It knows to show all references matching a pattern. - It knows to list all references that are an exact match to whatever the user has provided. - It knows to check for reference existence. The first two commands use mostly the same infrastructure to print the references via `show_one()`. But while the former mode uses a proper iterator and thus has a `struct reference` available in its context, the latter calls `refs_read_ref()` and thus doesn't. Consequently, we cannot easily use `reference_get_peeled_oid()` to print the peeled value. Adapt the code so that we manually construct a `struct reference` when verifying refs. We wouldn't ever have the peeled value available anyway as we're not using an iterator here, so we can simply plug in the values we _do_ have. With this change we now have a `struct reference` available at both callsites of `show_one()` and can thus pass it, which allows us to use `reference_get_peeled_oid()` instead of `peel_iterated_oid()`. Signed-off-by: Patrick Steinhardt --- builtin/show-ref.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 4803b5e598..4d4984e4e0 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -31,31 +31,31 @@ struct show_one_options { }; static void show_one(const struct show_one_options *opts, - const char *refname, const struct object_id *oid) + const struct reference *ref) { const char *hex; struct object_id peeled; - if (!odb_has_object(the_repository->objects, oid, + if (!odb_has_object(the_repository->objects, ref->oid, HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) - die("git show-ref: bad ref %s (%s)", refname, - oid_to_hex(oid)); + die("git show-ref: bad ref %s (%s)", ref->name, + oid_to_hex(ref->oid)); if (opts->quiet) return; - hex = repo_find_unique_abbrev(the_repository, oid, opts->abbrev); + hex = repo_find_unique_abbrev(the_repository, ref->oid, opts->abbrev); if (opts->hash_only) printf("%s\n", hex); else - printf("%s %s\n", hex, refname); + printf("%s %s\n", hex, ref->name); if (!opts->deref_tags) return; - if (!peel_iterated_oid(the_repository, oid, &peeled)) { + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) { hex = repo_find_unique_abbrev(the_repository, &peeled, opts->abbrev); - printf("%s %s^{}\n", hex, refname); + printf("%s %s^{}\n", hex, ref->name); } } @@ -93,7 +93,7 @@ static int show_ref(const struct reference *ref, void *cbdata) match: data->found_match++; - show_one(data->show_one_opts, ref->name, ref->oid); + show_one(data->show_one_opts, ref); return 0; } @@ -175,12 +175,18 @@ static int cmd_show_ref__verify(const struct show_one_options *show_one_opts, if ((starts_with(*refs, "refs/") || refname_is_safe(*refs)) && !refs_read_ref(get_main_ref_store(the_repository), *refs, &oid)) { - show_one(show_one_opts, *refs, &oid); - } - else if (!show_one_opts->quiet) + struct reference ref = { + .name = *refs, + .oid = &oid, + }; + + show_one(show_one_opts, &ref); + } else if (!show_one_opts->quiet) { die("'%s' - not a valid ref", *refs); - else + } else { return 1; + } + refs++; } -- GitLab From 8451059caff6d122d8431e5909fac2edff90e2d7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Oct 2025 10:57:52 +0200 Subject: [PATCH 17/31] refs: drop `current_ref_iter` hack In preceding commits we have refactored all callers of `peel_iterated_oid()` to instead use `reference_get_peeled_oid()`. This allows us to thus get rid of the former function. Getting rid of that function is nice, but even nicer is that this also allows us to get rid of the `current_ref_iter` hack. This global variable tracked the currently-active ref iterator so that we can use it to peel an object ID. Now that the peeled object ID is propagated via `struct reference` though we don't have to depend on this hack anymore, which makes for a more robust and easier-to-understand infrastructure. Signed-off-by: Patrick Steinhardt --- refs.c | 10 ---------- refs/iterator.c | 5 ----- refs/refs-internal.h | 13 ------------- 3 files changed, 28 deletions(-) diff --git a/refs.c b/refs.c index 1b1551f981..9d8f0a9ca4 100644 --- a/refs.c +++ b/refs.c @@ -2324,16 +2324,6 @@ int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) return refs->be->optimize(refs, opts); } -int peel_iterated_oid(struct repository *r, const struct object_id *base, struct object_id *peeled) -{ - if (current_ref_iter && - (current_ref_iter->ref.oid == base || - oideq(current_ref_iter->ref.oid, base))) - return ref_iterator_peel(current_ref_iter, peeled); - - return peel_object(r, base, peeled) ? -1 : 0; -} - int reference_get_peeled_oid(struct repository *repo, const struct reference *ref, struct object_id *peeled_oid) diff --git a/refs/iterator.c b/refs/iterator.c index fe5980e1b6..072c6aacdb 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -458,15 +458,11 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0, return ref_iterator; } -struct ref_iterator *current_ref_iter = NULL; - int do_for_each_ref_iterator(struct ref_iterator *iter, each_ref_fn fn, void *cb_data) { int retval = 0, ok; - struct ref_iterator *old_ref_iter = current_ref_iter; - current_ref_iter = iter; while ((ok = ref_iterator_advance(iter)) == ITER_OK) { retval = fn(&iter->ref, cb_data); if (retval) @@ -474,7 +470,6 @@ int do_for_each_ref_iterator(struct ref_iterator *iter, } out: - current_ref_iter = old_ref_iter; if (ok == ITER_ERROR) retval = -1; ref_iterator_free(iter); diff --git a/refs/refs-internal.h b/refs/refs-internal.h index ed749d1657..f4f845bbea 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -376,19 +376,6 @@ struct ref_iterator_vtable { ref_iterator_release_fn *release; }; -/* - * current_ref_iter is a performance hack: when iterating over - * references using the for_each_ref*() functions, current_ref_iter is - * set to the reference iterator before calling the callback function. - * If the callback function calls peel_ref(), then peel_ref() first - * checks whether the reference to be peeled is the one referred to by - * the iterator (it usually is) and if so, asks the iterator for the - * peeled version of the reference if it is available. This avoids a - * refname lookup in a common case. current_ref_iter is set to NULL - * when the iteration is over. - */ -extern struct ref_iterator *current_ref_iter; - struct ref_store; /* refs backends */ -- GitLab From de3e67c31d25c33162ada67c09cd119ff8917046 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Sep 2025 14:13:17 +0200 Subject: [PATCH 18/31] refs: drop infrastructure to peel via iterators Now that the peeled object ID gets propagated via the `struct reference` there is no need anymore to call into the reference iterator itself to dereference an object. Remove this infrastructure. Most of the changes are straight-forward deletions of code. There is one exception though in `refs/packed-backend.c::write_with_updates()`. Here we stop peeling the iterator and instead just pass the peeled object ID of that iterator directly. Signed-off-by: Patrick Steinhardt --- refs.h | 14 -------------- refs/debug.c | 11 ----------- refs/files-backend.c | 17 ----------------- refs/iterator.c | 36 ------------------------------------ refs/packed-backend.c | 24 +----------------------- refs/ref-cache.c | 9 --------- refs/refs-internal.h | 7 ------- refs/reftable-backend.c | 24 ------------------------ 8 files changed, 1 insertion(+), 141 deletions(-) diff --git a/refs.h b/refs.h index 886ed2c0f4..2dd7ac1a16 100644 --- a/refs.h +++ b/refs.h @@ -1289,10 +1289,6 @@ int repo_migrate_ref_storage_format(struct repository *repo, * to the next entry, ref_iterator_advance() aborts the iteration, * frees the ref_iterator, and returns ITER_ERROR. * - * The reference currently being looked at can be peeled by calling - * ref_iterator_peel(). This function is often faster than peel_ref(), - * so it should be preferred when iterating over references. - * * Putting it all together, a typical iteration looks like this: * * int ok; @@ -1307,9 +1303,6 @@ int repo_migrate_ref_storage_format(struct repository *repo, * // Access information about the current reference: * if (!(iter->flags & REF_ISSYMREF)) * printf("%s is %s\n", iter->refname, oid_to_hex(iter->oid)); - * - * // If you need to peel the reference: - * ref_iterator_peel(iter, &oid); * } * * if (ok != ITER_DONE) @@ -1400,13 +1393,6 @@ enum ref_iterator_seek_flag { int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *refname, unsigned int flags); -/* - * If possible, peel the reference currently being viewed by the - * iterator. Return 0 on success. - */ -int ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled); - /* Free the reference iterator and any associated resources. */ void ref_iterator_free(struct ref_iterator *ref_iterator); diff --git a/refs/debug.c b/refs/debug.c index 67718bd1f4..01499b9033 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -177,16 +177,6 @@ static int debug_ref_iterator_seek(struct ref_iterator *ref_iterator, return res; } -static int debug_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct debug_ref_iterator *diter = - (struct debug_ref_iterator *)ref_iterator; - int res = diter->iter->vtable->peel(diter->iter, peeled); - trace_printf_key(&trace_refs, "iterator_peel: %s: %d\n", diter->iter->ref.name, res); - return res; -} - static void debug_ref_iterator_release(struct ref_iterator *ref_iterator) { struct debug_ref_iterator *diter = @@ -198,7 +188,6 @@ static void debug_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable debug_ref_iterator_vtable = { .advance = debug_ref_iterator_advance, .seek = debug_ref_iterator_seek, - .peel = debug_ref_iterator_peel, .release = debug_ref_iterator_release, }; diff --git a/refs/files-backend.c b/refs/files-backend.c index fac53fa052..5aeb454fb4 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -993,15 +993,6 @@ static int files_ref_iterator_seek(struct ref_iterator *ref_iterator, return ref_iterator_seek(iter->iter0, refname, flags); } -static int files_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct files_ref_iterator *iter = - (struct files_ref_iterator *)ref_iterator; - - return ref_iterator_peel(iter->iter0, peeled); -} - static void files_ref_iterator_release(struct ref_iterator *ref_iterator) { struct files_ref_iterator *iter = @@ -1012,7 +1003,6 @@ static void files_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable files_ref_iterator_vtable = { .advance = files_ref_iterator_advance, .seek = files_ref_iterator_seek, - .peel = files_ref_iterator_peel, .release = files_ref_iterator_release, }; @@ -2388,12 +2378,6 @@ static int files_reflog_iterator_seek(struct ref_iterator *ref_iterator UNUSED, BUG("ref_iterator_seek() called for reflog_iterator"); } -static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED, - struct object_id *peeled UNUSED) -{ - BUG("ref_iterator_peel() called for reflog_iterator"); -} - static void files_reflog_iterator_release(struct ref_iterator *ref_iterator) { struct files_reflog_iterator *iter = @@ -2404,7 +2388,6 @@ static void files_reflog_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable files_reflog_iterator_vtable = { .advance = files_reflog_iterator_advance, .seek = files_reflog_iterator_seek, - .peel = files_reflog_iterator_peel, .release = files_reflog_iterator_release, }; diff --git a/refs/iterator.c b/refs/iterator.c index 072c6aacdb..d79aa5ec82 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -21,12 +21,6 @@ int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *refname, return ref_iterator->vtable->seek(ref_iterator, refname, flags); } -int ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - return ref_iterator->vtable->peel(ref_iterator, peeled); -} - void ref_iterator_free(struct ref_iterator *ref_iterator) { if (ref_iterator) { @@ -60,12 +54,6 @@ static int empty_ref_iterator_seek(struct ref_iterator *ref_iterator UNUSED, return 0; } -static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator UNUSED, - struct object_id *peeled UNUSED) -{ - BUG("peel called for empty iterator"); -} - static void empty_ref_iterator_release(struct ref_iterator *ref_iterator UNUSED) { } @@ -73,7 +61,6 @@ static void empty_ref_iterator_release(struct ref_iterator *ref_iterator UNUSED) static struct ref_iterator_vtable empty_ref_iterator_vtable = { .advance = empty_ref_iterator_advance, .seek = empty_ref_iterator_seek, - .peel = empty_ref_iterator_peel, .release = empty_ref_iterator_release, }; @@ -240,18 +227,6 @@ static int merge_ref_iterator_seek(struct ref_iterator *ref_iterator, return 0; } -static int merge_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct merge_ref_iterator *iter = - (struct merge_ref_iterator *)ref_iterator; - - if (!iter->current) { - BUG("peel called before advance for merge iterator"); - } - return ref_iterator_peel(*iter->current, peeled); -} - static void merge_ref_iterator_release(struct ref_iterator *ref_iterator) { struct merge_ref_iterator *iter = @@ -263,7 +238,6 @@ static void merge_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable merge_ref_iterator_vtable = { .advance = merge_ref_iterator_advance, .seek = merge_ref_iterator_seek, - .peel = merge_ref_iterator_peel, .release = merge_ref_iterator_release, }; @@ -412,15 +386,6 @@ static int prefix_ref_iterator_seek(struct ref_iterator *ref_iterator, return ref_iterator_seek(iter->iter0, refname, flags); } -static int prefix_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct prefix_ref_iterator *iter = - (struct prefix_ref_iterator *)ref_iterator; - - return ref_iterator_peel(iter->iter0, peeled); -} - static void prefix_ref_iterator_release(struct ref_iterator *ref_iterator) { struct prefix_ref_iterator *iter = @@ -432,7 +397,6 @@ static void prefix_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable prefix_ref_iterator_vtable = { .advance = prefix_ref_iterator_advance, .seek = prefix_ref_iterator_seek, - .peel = prefix_ref_iterator_peel, .release = prefix_ref_iterator_release, }; diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 1fefefd54e..6fa229edd0 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1030,22 +1030,6 @@ static int packed_ref_iterator_seek(struct ref_iterator *ref_iterator, return 0; } -static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct packed_ref_iterator *iter = - (struct packed_ref_iterator *)ref_iterator; - - if ((iter->base.ref.flags & REF_KNOWS_PEELED)) { - oidcpy(peeled, &iter->peeled); - return is_null_oid(&iter->peeled) ? -1 : 0; - } else if ((iter->base.ref.flags & (REF_ISBROKEN | REF_ISSYMREF))) { - return -1; - } else { - return peel_object(iter->repo, &iter->oid, peeled) ? -1 : 0; - } -} - static void packed_ref_iterator_release(struct ref_iterator *ref_iterator) { struct packed_ref_iterator *iter = @@ -1059,7 +1043,6 @@ static void packed_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable packed_ref_iterator_vtable = { .advance = packed_ref_iterator_advance, .seek = packed_ref_iterator_seek, - .peel = packed_ref_iterator_peel, .release = packed_ref_iterator_release, }; @@ -1525,13 +1508,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re if (cmp < 0) { /* Pass the old reference through. */ - - struct object_id peeled; - int peel_error = ref_iterator_peel(iter, &peeled); - if (write_packed_entry(out, iter->ref.name, - iter->ref.oid, - peel_error ? NULL : &peeled)) + iter->ref.oid, iter->ref.peeled_oid)) goto write_error; if ((ok = ref_iterator_advance(iter)) != ITER_OK) { diff --git a/refs/ref-cache.c b/refs/ref-cache.c index e427848879..ffef01a597 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -546,14 +546,6 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator, return 0; } -static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct cache_ref_iterator *iter = - (struct cache_ref_iterator *)ref_iterator; - return peel_object(iter->repo, ref_iterator->ref.oid, peeled) ? -1 : 0; -} - static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) { struct cache_ref_iterator *iter = @@ -565,7 +557,6 @@ static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable cache_ref_iterator_vtable = { .advance = cache_ref_iterator_advance, .seek = cache_ref_iterator_seek, - .peel = cache_ref_iterator_peel, .release = cache_ref_iterator_release, }; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index f4f845bbea..4671517dad 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -357,12 +357,6 @@ typedef int ref_iterator_advance_fn(struct ref_iterator *ref_iterator); typedef int ref_iterator_seek_fn(struct ref_iterator *ref_iterator, const char *refname, unsigned int flags); -/* - * Peels the current ref, returning 0 for success or -1 for failure. - */ -typedef int ref_iterator_peel_fn(struct ref_iterator *ref_iterator, - struct object_id *peeled); - /* * Implementations of this function should free any resources specific * to the derived class. @@ -372,7 +366,6 @@ typedef void ref_iterator_release_fn(struct ref_iterator *ref_iterator); struct ref_iterator_vtable { ref_iterator_advance_fn *advance; ref_iterator_seek_fn *seek; - ref_iterator_peel_fn *peel; ref_iterator_release_fn *release; }; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index e214e120d7..e329d4a423 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -744,21 +744,6 @@ static int reftable_ref_iterator_seek(struct ref_iterator *ref_iterator, return iter->err; } -static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct reftable_ref_iterator *iter = - (struct reftable_ref_iterator *)ref_iterator; - - if (iter->ref.value_type == REFTABLE_REF_VAL2) { - oidread(peeled, iter->ref.value.val2.target_value, - iter->refs->base.repo->hash_algo); - return 0; - } - - return -1; -} - static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator) { struct reftable_ref_iterator *iter = @@ -776,7 +761,6 @@ static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable reftable_ref_iterator_vtable = { .advance = reftable_ref_iterator_advance, .seek = reftable_ref_iterator_seek, - .peel = reftable_ref_iterator_peel, .release = reftable_ref_iterator_release, }; @@ -2098,13 +2082,6 @@ static int reftable_reflog_iterator_seek(struct ref_iterator *ref_iterator UNUSE return -1; } -static int reftable_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED, - struct object_id *peeled UNUSED) -{ - BUG("reftable reflog iterator cannot be peeled"); - return -1; -} - static void reftable_reflog_iterator_release(struct ref_iterator *ref_iterator) { struct reftable_reflog_iterator *iter = @@ -2117,7 +2094,6 @@ static void reftable_reflog_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable reftable_reflog_iterator_vtable = { .advance = reftable_reflog_iterator_advance, .seek = reftable_reflog_iterator_seek, - .peel = reftable_reflog_iterator_peel, .release = reftable_reflog_iterator_release, }; -- GitLab From 8782d6729df4197598c6dc1eebc0f7fe6b05ba44 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Sep 2025 15:55:03 +0200 Subject: [PATCH 19/31] object: add flag to `peel_object()` to verify object type When peeling a tag to a non-tag object we repeatedly call `parse_object()` on the tagged object until we find the first object that isn't a tag. While this feels sensible at first, there is a big catch here: `parse_object()` doesn't actually verify the type of the tagged object. The relevant code path here eventually ends up in `parse_tag_buffer()`. Here, we parse the various fields of the tag, including the "type". Once we've figured out the type and the tagged object ID, we call one of the `lookup_${type}()` functions for whatever type we have found. There is two possible outcomes in the successful case: 1. The object is already part of our cached objects. In that case we double-check whether the type we're trying to look up matches the type that was cached. 2. The object is _not_ part of our cached objects. In that case, we simply create a new object with the expected type, but we don't parse that object. In the first case we might notice type mismatches, but only in the case where our cache has the object with the correct type. In the second case, we'll blindly assume that the type is correct and then go with it. We'll only notice that the type might be wrong when we try to parse the object at a later point. Now arguably, we could change `parse_tag_buffer()` to verify the tagged object's type for us. But that would have the effect that such a tag cannot be parsed at all anymore, and we have a small bunch of tests for exactly this case that assert we still can open such tags. So this change does not feel like something we can retroactively tighten, even though one shouldn't ever hit such corrupted tags. Instead, add a new `flags` field to `peel_object()` that allows the caller to opt in to strict object verification. This will be wired up at a subset of callsites over the next few commits. Note that this change also inlines `deref_tag_noverify()`. There's only been two callsites of that function, the one we're changing and one in our test helpers. The latter callsite can trivially use `deref_tag()` instead, so by inlining the function we avoid having to pass down the flag. Signed-off-by: Patrick Steinhardt --- object.c | 20 +++++++++++++++++--- object.h | 15 ++++++++++++++- ref-filter.c | 2 +- refs.c | 2 +- refs/packed-backend.c | 5 ++--- refs/reftable-backend.c | 4 ++-- t/helper/test-reach.c | 2 +- tag.c | 12 ------------ tag.h | 1 - 9 files changed, 38 insertions(+), 25 deletions(-) diff --git a/object.c b/object.c index 986114a6db..e72b0ed436 100644 --- a/object.c +++ b/object.c @@ -209,11 +209,12 @@ struct object *lookup_object_by_type(struct repository *r, enum peel_status peel_object(struct repository *r, const struct object_id *name, - struct object_id *oid) + struct object_id *oid, + unsigned flags) { struct object *o = lookup_unknown_object(r, name); - if (o->type == OBJ_NONE) { + if (o->type == OBJ_NONE || flags & PEEL_OBJECT_VERIFY_OBJECT_TYPE) { int type = odb_read_object_info(r->objects, name, NULL); if (type < 0 || !object_as_type(o, type, 0)) return PEEL_INVALID; @@ -222,7 +223,20 @@ enum peel_status peel_object(struct repository *r, if (o->type != OBJ_TAG) return PEEL_NON_TAG; - o = deref_tag_noverify(r, o); + while (o && o->type == OBJ_TAG) { + o = parse_object(r, &o->oid); + if (o && o->type == OBJ_TAG && ((struct tag *)o)->tagged) { + o = ((struct tag *)o)->tagged; + + if (flags & PEEL_OBJECT_VERIFY_OBJECT_TYPE) { + int type = odb_read_object_info(r->objects, &o->oid, NULL); + if (type < 0 || !object_as_type(o, type, 0)) + return PEEL_INVALID; + } + } else { + o = NULL; + } + } if (!o) return PEEL_INVALID; diff --git a/object.h b/object.h index 8c3c1c46e1..1499f63d50 100644 --- a/object.h +++ b/object.h @@ -287,6 +287,17 @@ enum peel_status { PEEL_BROKEN = -4 }; +enum peel_object_flags { + /* + * Always verify the object type, even in the case where the looked-up + * object already has an object type. This can be useful when the + * stored object type may be invalid. One such case is when looking up + * objects via tags, where we blindly trust the object type declared by + * the tag. + */ + PEEL_OBJECT_VERIFY_OBJECT_TYPE = (1 << 0), +}; + /* * Peel the named object; i.e., if the object is a tag, resolve the * tag recursively until a non-tag is found. If successful, store the @@ -295,7 +306,9 @@ enum peel_status { * and leave oid unchanged. */ enum peel_status peel_object(struct repository *r, - const struct object_id *name, struct object_id *oid); + const struct object_id *name, + struct object_id *oid, + unsigned flags); struct object_list *object_list_insert(struct object *item, struct object_list **list_p); diff --git a/ref-filter.c b/ref-filter.c index 7fd8babec8..9a8ed8c8fc 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2581,7 +2581,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) if (need_tagged) { if (!is_null_oid(&ref->peeled_oid)) { oidcpy(&oi_deref.oid, &ref->peeled_oid); - } else if (!peel_object(the_repository, &obj->oid, &oi_deref.oid)) { + } else if (!peel_object(the_repository, &oi.oid, &oi_deref.oid, 0)) { /* We managed to peel the object ourselves. */ } else { die("bad tag"); diff --git a/refs.c b/refs.c index 9d8f0a9ca4..a41a94ae55 100644 --- a/refs.c +++ b/refs.c @@ -2333,7 +2333,7 @@ int reference_get_peeled_oid(struct repository *repo, return 0; } - return peel_object(repo, ref->oid, peeled_oid) ? -1 : 0; + return peel_object(repo, ref->oid, peeled_oid, 0) ? -1 : 0; } int refs_update_symref(struct ref_store *refs, const char *ref, diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 6fa229edd0..4752d3f398 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1527,9 +1527,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re i++; } else { struct object_id peeled; - int peel_error = peel_object(refs->base.repo, - &update->new_oid, - &peeled); + int peel_error = peel_object(refs->base.repo, &update->new_oid, + &peeled, 0); if (write_packed_entry(out, update->refname, &update->new_oid, diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index e329d4a423..9febb2322c 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1632,7 +1632,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data ref.refname = (char *)u->refname; ref.update_index = ts; - peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled); + peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled, 0); if (!peel_error) { ref.value_type = REFTABLE_REF_VAL2; memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ); @@ -2497,7 +2497,7 @@ static int write_reflog_expiry_table(struct reftable_writer *writer, void *cb_da ref.refname = (char *)arg->refname; ref.update_index = ts; - if (!peel_object(arg->refs->base.repo, &arg->update_oid, &peeled)) { + if (!peel_object(arg->refs->base.repo, &arg->update_oid, &peeled, 0)) { ref.value_type = REFTABLE_REF_VAL2; memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ); memcpy(ref.value.val2.value, arg->update_oid.hash, GIT_MAX_RAWSZ); diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c index 028ec00306..c58c93800f 100644 --- a/t/helper/test-reach.c +++ b/t/helper/test-reach.c @@ -63,7 +63,7 @@ int cmd__reach(int ac, const char **av) die("failed to resolve %s", buf.buf + 2); orig = parse_object(r, &oid); - peeled = deref_tag_noverify(the_repository, orig); + peeled = deref_tag(the_repository, orig, NULL, 0); if (!peeled) die("failed to load commit for input %s resulting in oid %s", diff --git a/tag.c b/tag.c index 1d52686ee1..f5c232d2f1 100644 --- a/tag.c +++ b/tag.c @@ -94,18 +94,6 @@ struct object *deref_tag(struct repository *r, struct object *o, const char *war return o; } -struct object *deref_tag_noverify(struct repository *r, struct object *o) -{ - while (o && o->type == OBJ_TAG) { - o = parse_object(r, &o->oid); - if (o && o->type == OBJ_TAG && ((struct tag *)o)->tagged) - o = ((struct tag *)o)->tagged; - else - o = NULL; - } - return o; -} - struct tag *lookup_tag(struct repository *r, const struct object_id *oid) { struct object *obj = lookup_object(r, oid); diff --git a/tag.h b/tag.h index c49d7c19ad..ef12a61037 100644 --- a/tag.h +++ b/tag.h @@ -16,7 +16,6 @@ int parse_tag_buffer(struct repository *r, struct tag *item, const void *data, u int parse_tag(struct tag *item); void release_tag_memory(struct tag *t); struct object *deref_tag(struct repository *r, struct object *, const char *, int); -struct object *deref_tag_noverify(struct repository *r, struct object *); int gpg_verify_tag(const struct object_id *oid, const char *name_to_report, unsigned flags); struct object_id *get_tagged_oid(struct tag *tag); -- GitLab From ba7f85c8fefd3e93604de0f8ecfdf0a58fdf6432 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Oct 2025 10:58:50 +0200 Subject: [PATCH 20/31] refs: don't store peeled object IDs for invalid tags Both the "files" and "reftable" backend store peeled object IDs for references that point to tags: - The "files" backend stores the value when packing refs, where each peeled object ID is prefixed with "^". - The "reftable" backend stores the value whenever writing a new reference that points to a tag via a special ref record type. Both of these backends use `peel_object()` to find the peeled object ID. But as explained in the preceding commit, that function does not detect the case where the tag's tagged object and its claimed type mismatch. The consequence of storing these bogus peeled object IDs is that we're less likely to detect such corruption in other parts of Git. git-for-each-ref(1) for example does not notice anymore that the tag is broken when using "--format=%(*objectname)" to dereference tags. One could claim that this is good, because it still allows us to mostly use the tag as intended. But the biggest problem here is that we now have different behaviour for such a broken tag depending on whether or not we have its peeled value in the refdb. Fix the issue by verifying the object type when peeling the object. If that verification fails we simply skip storing the peeled value in either of the reference formats. Signed-off-by: Patrick Steinhardt --- refs/packed-backend.c | 2 +- refs/reftable-backend.c | 3 ++- t/pack-refs-tests.sh | 32 ++++++++++++++++++++++++++++++++ t/t0610-reftable-basics.sh | 28 ++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 2 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 4752d3f398..1ab0c50393 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1528,7 +1528,7 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re } else { struct object_id peeled; int peel_error = peel_object(refs->base.repo, &update->new_oid, - &peeled, 0); + &peeled, PEEL_OBJECT_VERIFY_OBJECT_TYPE); if (write_packed_entry(out, update->refname, &update->new_oid, diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 9febb2322c..6bbfd5618d 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1632,7 +1632,8 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data ref.refname = (char *)u->refname; ref.update_index = ts; - peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled, 0); + peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled, + PEEL_OBJECT_VERIFY_OBJECT_TYPE); if (!peel_error) { ref.value_type = REFTABLE_REF_VAL2; memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ); diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh index 3dbcc01718..095823d915 100644 --- a/t/pack-refs-tests.sh +++ b/t/pack-refs-tests.sh @@ -428,4 +428,36 @@ do ' done +test_expect_success 'pack-refs does not store invalid peeled tag value' ' + test_when_finished rm -rf repo && + git init repo && + ( + cd repo && + git commit --allow-empty --message initial && + + echo garbage >blob-content && + blob_id=$(git hash-object -w -t blob blob-content) && + + # Write an invalid tag into the object database. The tag itself + # is well-formed, but the tagged object is a blob while we + # claim that it is a commit. + cat >tag-content <<-EOF && + object $blob_id + type commit + tag bad-tag + tagger C O Mitter 1112354055 +0200 + + annotated + EOF + tag_id=$(git hash-object -w -t tag tag-content) && + git update-ref refs/tags/bad-tag "$tag_id" && + + # The packed-refs file should not contain the peeled object ID. + # If it did this would cause commands that use the peeled value + # to not notice this corrupted tag. + git pack-refs --all && + test_grep ! "^\^" .git/packed-refs + ) +' + test_done diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh index 3ea5d51532..6575528f21 100755 --- a/t/t0610-reftable-basics.sh +++ b/t/t0610-reftable-basics.sh @@ -1135,4 +1135,32 @@ test_expect_success 'fetch: accessing FETCH_HEAD special ref works' ' test_cmp expect actual ' +test_expect_success 'writes do not persist peeled value for invalid tags' ' + test_when_finished rm -rf repo && + git init repo && + ( + cd repo && + git commit --allow-empty --message initial && + + # We cannot easily verify that the peeled value is not stored + # in the tables. Instead, we test this indirectly: we create + # two tags that both point to the same object, but they claim + # different object types. If we parse both tags we notice that + # the parsed tagged object has a mismatch between the two tags + # and bail out. + # + # If we instead use the persisted peeled value we would not + # even parse the tags. As such, we would not notice the + # discrepancy either and thus listing these tags would succeed. + git tag tag-1 -m "tag 1" && + git cat-file tag tag-1 >raw-tag && + sed "s/^type commit$/type blob/" broken-tag && + broken_tag_id=$(git hash-object -w -t tag broken-tag) && + git update-ref refs/tags/tag-2 $broken_tag_id && + + test_must_fail git for-each-ref --format="%(*objectname)" refs/tags/ 2>err && + test_grep "bad tag pointer" err + ) +' + test_done -- GitLab From 74ad4413535e614155f93d8d8010e7dae8da2ed3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Sep 2025 16:00:31 +0200 Subject: [PATCH 21/31] ref-filter: detect broken tags when dereferencing them Users can ask git-for-each-ref(1) to peel tags and return information of the tagged object by adding an asterisk to the format, like for example "%(*$objectname)". If so, git-for-each-ref(1) peels that object to the first non-tag object and then returns its values. As mentioned in preceding commits, it can happen that the tagged object type and the claimed object type differ, effectively resulting in a corrupt tag. git-for-each-ref(1) would notice this mismatch, print an error and then bail out when trying to peel the tag. But we only notice this corruption in some very specific edge cases! While we have a test in "t/for-each-ref-tests.sh" that verifies the above scenario, this test is specifically crafted to detect the issue at hand. Namely, we create two tags: - One tag points to a specific object with the correct type. - The other tag points to the *same* object with a different type. The fact that both tags point to the same object is important here: `peel_object()` wouldn't notice the corruption if the tagged objects were different. The root cause is that `peel_object()` calls `lookup_${type}()` eventually, where the type is the same type declared in the tag object. Consequently, when we have two tags pointing to the same object but with different declared types we'll call two different lookup functions. The first lookup will store the object with an unverified type A, whereas the second lookup will try to look up the object with a different unverified type B. And it is only now that we notice the discrepancy in object types, even though type A could've already been the wrong type. Fix the issue by verifying the object type in `populate_value()`. With this change we'll also notice type mismatches when only dereferencing a tag once. Signed-off-by: Patrick Steinhardt --- ref-filter.c | 3 ++- t/for-each-ref-tests.sh | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 9a8ed8c8fc..c54025d6b4 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2581,7 +2581,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) if (need_tagged) { if (!is_null_oid(&ref->peeled_oid)) { oidcpy(&oi_deref.oid, &ref->peeled_oid); - } else if (!peel_object(the_repository, &oi.oid, &oi_deref.oid, 0)) { + } else if (!peel_object(the_repository, &oi.oid, &oi_deref.oid, + PEEL_OBJECT_VERIFY_OBJECT_TYPE)) { /* We managed to peel the object ourselves. */ } else { die("bad tag"); diff --git a/t/for-each-ref-tests.sh b/t/for-each-ref-tests.sh index e3ad19298a..4593be5fd5 100644 --- a/t/for-each-ref-tests.sh +++ b/t/for-each-ref-tests.sh @@ -1809,7 +1809,9 @@ test_expect_success "${git_for_each_ref} reports broken tags" ' bad=$(git hash-object -w -t tag bad) && git update-ref refs/tags/broken-tag-bad $bad && test_must_fail ${git_for_each_ref} --format="%(*objectname)" \ - refs/tags/broken-tag-* + refs/tags/broken-tag-* && + test_must_fail ${git_for_each_ref} --format="%(*objectname)" \ + refs/tags/broken-tag-bad ' test_expect_success 'set up tag with signature and no blank lines' ' -- GitLab From e833e9943e2745004cd92841cbc836e6d79663c7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 17 Sep 2025 15:55:35 +0200 Subject: [PATCH 22/31] ref-filter: parse objects on demand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When formatting an arbitrary object we parse that object regardless of whether or not we actually need any parsed data. In fact, many of the atoms we have don't require any. Refactor the code so that we parse the data on demand when we see an atom that wants to access the objects. This leads to a small speedup, for example in the Chromium repository with around 40000 refs: Benchmark 1: for-each-ref --format='%(raw)' (HEAD~) Time (mean ± σ): 388.7 ms ± 1.1 ms [User: 322.2 ms, System: 65.0 ms] Range (min … max): 387.3 ms … 390.8 ms 10 runs Benchmark 2: for-each-ref --format='%(raw)' (HEAD) Time (mean ± σ): 344.7 ms ± 0.7 ms [User: 287.8 ms, System: 55.1 ms] Range (min … max): 343.9 ms … 345.7 ms 10 runs Summary for-each-ref --format='%(raw)' (HEAD) ran 1.13 ± 0.00 times faster than for-each-ref --format='%(raw)' (HEAD~) With this change, we now spend ~90% of the time decompressing objects, which is almost as good as it gets regarding git-for-each-ref(1)'s own infrastructure. Signed-off-by: Patrick Steinhardt --- ref-filter.c | 142 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 106 insertions(+), 36 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index c54025d6b4..7cfcd5c355 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -91,6 +91,7 @@ static struct expand_data { struct object_id delta_base_oid; void *content; + struct object *maybe_object; struct object_info info; } oi, oi_deref; @@ -1475,11 +1476,29 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ } } +static struct object *get_or_parse_object(struct expand_data *data, const char *refname, + struct strbuf *err, int *eaten) +{ + if (!data->maybe_object) { + data->maybe_object = parse_object_buffer(the_repository, &data->oid, data->type, + data->size, data->content, eaten); + if (!data->maybe_object) { + strbuf_addf(err, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(&data->oid), refname); + return NULL; + } + } + + return data->maybe_object; +} + /* See grab_values */ -static void grab_tag_values(struct atom_value *val, int deref, struct object *obj) +static int grab_tag_values(struct atom_value *val, int deref, + struct expand_data *data, const char *refname, + struct strbuf *err, int *eaten) { + struct tag *tag = NULL; int i; - struct tag *tag = (struct tag *) obj; for (i = 0; i < used_atom_cnt; i++) { const char *name = used_atom[i].name; @@ -1487,6 +1506,14 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob struct atom_value *v = &val[i]; if (!!deref != (*name == '*')) continue; + + if (!tag) { + tag = (struct tag *) get_or_parse_object(data, refname, + err, eaten); + if (!tag) + return -1; + } + if (deref) name++; if (atom_type == ATOM_TAG) @@ -1496,22 +1523,35 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob else if (atom_type == ATOM_OBJECT && tag->tagged) v->s = xstrdup(oid_to_hex(&tag->tagged->oid)); } + + return 0; } /* See grab_values */ -static void grab_commit_values(struct atom_value *val, int deref, struct object *obj) +static int grab_commit_values(struct atom_value *val, int deref, + struct expand_data *data, const char *refname, + struct strbuf *err, int *eaten) { int i; - struct commit *commit = (struct commit *) obj; + struct commit *commit = NULL; for (i = 0; i < used_atom_cnt; i++) { const char *name = used_atom[i].name; enum atom_type atom_type = used_atom[i].atom_type; struct atom_value *v = &val[i]; + if (!!deref != (*name == '*')) continue; if (deref) name++; + + if (!commit) { + commit = (struct commit *) get_or_parse_object(data, refname, + err, eaten); + if (!commit) + return -1; + } + if (atom_type == ATOM_TREE && grab_oid(name, "tree", get_commit_tree_oid(commit), v, &used_atom[i])) continue; @@ -1531,6 +1571,8 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object v->s = strbuf_detach(&s, NULL); } } + + return 0; } static const char *find_wholine(const char *who, int wholen, const char *buf) @@ -1759,10 +1801,12 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void } } -static void grab_signature(struct atom_value *val, int deref, struct object *obj) +static int grab_signature(struct atom_value *val, int deref, + struct expand_data *data, const char *refname, + struct strbuf *err, int *eaten) { int i; - struct commit *commit = (struct commit *) obj; + struct commit *commit = NULL; struct signature_check sigc = { 0 }; int signature_checked = 0; @@ -1790,6 +1834,13 @@ static void grab_signature(struct atom_value *val, int deref, struct object *obj continue; if (!signature_checked) { + if (!commit) { + commit = (struct commit *) get_or_parse_object(data, refname, + err, eaten); + if (!commit) + return -1; + } + check_commit_signature(commit, &sigc); signature_checked = 1; } @@ -1843,6 +1894,8 @@ static void grab_signature(struct atom_value *val, int deref, struct object *obj if (signature_checked) signature_check_clear(&sigc); + + return 0; } static void find_subpos(const char *buf, @@ -1920,9 +1973,8 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size } static void grab_describe_values(struct atom_value *val, int deref, - struct object *obj) + struct expand_data *data) { - struct commit *commit = (struct commit *)obj; int i; for (i = 0; i < used_atom_cnt; i++) { @@ -1944,7 +1996,7 @@ static void grab_describe_values(struct atom_value *val, int deref, cmd.git_cmd = 1; strvec_push(&cmd.args, "describe"); strvec_pushv(&cmd.args, atom->u.describe_args.v); - strvec_push(&cmd.args, oid_to_hex(&commit->object.oid)); + strvec_push(&cmd.args, oid_to_hex(&data->oid)); if (pipe_command(&cmd, NULL, 0, &out, 0, &err, 0) < 0) { error(_("failed to run 'describe'")); v->s = xstrdup(""); @@ -2066,24 +2118,36 @@ static void fill_missing_values(struct atom_value *val) * pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data) +static int grab_values(struct atom_value *val, int deref, struct expand_data *data, + const char *refname, struct strbuf *err, int *eaten) { void *buf = data->content; + int ret; - switch (obj->type) { + switch (data->type) { case OBJ_TAG: - grab_tag_values(val, deref, obj); + ret = grab_tag_values(val, deref, data, refname, err, eaten); + if (ret < 0) + goto out; + grab_sub_body_contents(val, deref, data); grab_person("tagger", val, deref, buf); - grab_describe_values(val, deref, obj); + grab_describe_values(val, deref, data); break; case OBJ_COMMIT: - grab_commit_values(val, deref, obj); + ret = grab_commit_values(val, deref, data, refname, err, eaten); + if (ret < 0) + goto out; + grab_sub_body_contents(val, deref, data); grab_person("author", val, deref, buf); grab_person("committer", val, deref, buf); - grab_signature(val, deref, obj); - grab_describe_values(val, deref, obj); + + ret = grab_signature(val, deref, data, refname, err, eaten); + if (ret < 0) + goto out; + + grab_describe_values(val, deref, data); break; case OBJ_TREE: /* grab_tree_values(val, deref, obj, buf, sz); */ @@ -2094,8 +2158,12 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s grab_sub_body_contents(val, deref, data); break; default: - die("Eh? Object of type %d?", obj->type); + die("Eh? Object of type %d?", data->type); } + + ret = 0; +out: + return ret; } static inline char *copy_advance(char *dst, const char *src) @@ -2292,38 +2360,41 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, ref->refname); } -static int get_object(struct ref_array_item *ref, int deref, struct object **obj, +static int get_object(struct ref_array_item *ref, int deref, struct expand_data *oi, struct strbuf *err) { - /* parse_object_buffer() will set eaten to 0 if free() will be needed */ - int eaten = 1; + /* parse_object_buffer() will set eaten to 1 if free() will be needed */ + int eaten = 0; + int ret; + if (oi->info.contentp) { /* We need to know that to use parse_object_buffer properly */ oi->info.sizep = &oi->size; oi->info.typep = &oi->type; } + if (odb_read_object_info_extended(the_repository->objects, &oi->oid, &oi->info, - OBJECT_INFO_LOOKUP_REPLACE)) - return strbuf_addf_ret(err, -1, _("missing object %s for %s"), - oid_to_hex(&oi->oid), ref->refname); + OBJECT_INFO_LOOKUP_REPLACE)) { + ret = strbuf_addf_ret(err, -1, _("missing object %s for %s"), + oid_to_hex(&oi->oid), ref->refname); + goto out; + } if (oi->info.disk_sizep && oi->disk_size < 0) BUG("Object size is less than zero."); if (oi->info.contentp) { - *obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten); - if (!*obj) { - if (!eaten) - free(oi->content); - return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), - oid_to_hex(&oi->oid), ref->refname); - } - grab_values(ref->value, deref, *obj, oi); + ret = grab_values(ref->value, deref, oi, ref->refname, err, &eaten); + if (ret < 0) + goto out; } grab_common_values(ref->value, deref, oi); + ret = 0; + +out: if (!eaten) free(oi->content); - return 0; + return ret; } static void populate_worktree_map(struct hashmap *map, struct worktree **worktrees) @@ -2376,7 +2447,6 @@ static char *get_worktree_path(const struct ref_array_item *ref) */ static int populate_value(struct ref_array_item *ref, struct strbuf *err) { - struct object *obj; int i; struct object_info empty = OBJECT_INFO_INIT; int ahead_behind_atoms = 0; @@ -2564,14 +2634,14 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) oi.oid = ref->objectname; - if (get_object(ref, 0, &obj, &oi, err)) + if (get_object(ref, 0, &oi, err)) return -1; /* * If there is no atom that wants to know about tagged * object, we are done. */ - if (!need_tagged || (obj->type != OBJ_TAG)) + if (!need_tagged || (oi.type != OBJ_TAG)) return 0; /* @@ -2589,7 +2659,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) } } - return get_object(ref, 1, &obj, &oi_deref, err); + return get_object(ref, 1, &oi_deref, err); } /* -- GitLab From 27a736a54c4290634c13b5fe4e318256d23b6d08 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 20 Oct 2025 10:18:29 +0200 Subject: [PATCH 23/31] refs: move to using the '.optimize' functions The `struct ref_store` variable exposes two ways to optimize a reftable backend: 1. pack_refs 2. optimize The former was specific to the 'files' + 'packed' refs backend. The latter is more generic and covers all backends. While the naming is different, both of these functions perform the same functionality. Consolidate this code to only maintain the 'optimize' functions. Do this by modifying the backends so that they exclusively implement the `optimize` callback, only. All users of the refs subsystem already use the 'optimize' function so there is no changes needed on the callee side. Finally, cleanup all references to the 'pack_refs' field of the structure and code around it. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- refs.c | 6 ------ refs.h | 6 ------ refs/debug.c | 8 ++++---- refs/files-backend.c | 14 ++------------ refs/packed-backend.c | 6 +++--- refs/refs-internal.h | 3 --- refs/reftable-backend.c | 13 +++---------- 7 files changed, 12 insertions(+), 44 deletions(-) diff --git a/refs.c b/refs.c index a41a94ae55..b9a4a60646 100644 --- a/refs.c +++ b/refs.c @@ -2313,12 +2313,6 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo, refs->gitdir = xstrdup(path); } -/* backend functions */ -int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts) -{ - return refs->be->pack_refs(refs, opts); -} - int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) { return refs->be->optimize(refs, opts); diff --git a/refs.h b/refs.h index 2dd7ac1a16..8ff591ea95 100644 --- a/refs.h +++ b/refs.h @@ -514,12 +514,6 @@ struct pack_refs_opts { struct string_list *includes; }; -/* - * Write a packed-refs file for the current repository. - * flags: Combination of the above PACK_REFS_* flags. - */ -int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts); - /* * Optimize the ref store. The exact behavior is up to the backend. * For the files backend, this is equivalent to packing refs. diff --git a/refs/debug.c b/refs/debug.c index 01499b9033..40cd1d9c15 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -116,11 +116,11 @@ static int debug_transaction_abort(struct ref_store *refs, return res; } -static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts) +static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts) { struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; - int res = drefs->refs->be->pack_refs(drefs->refs, opts); - trace_printf_key(&trace_refs, "pack_refs: %d\n", res); + int res = drefs->refs->be->optimize(drefs->refs, opts); + trace_printf_key(&trace_refs, "optimize: %d\n", res); return res; } @@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = { .transaction_finish = debug_transaction_finish, .transaction_abort = debug_transaction_abort, - .pack_refs = debug_pack_refs, + .optimize = debug_optimize, .rename_ref = debug_rename_ref, .copy_ref = debug_copy_ref, diff --git a/refs/files-backend.c b/refs/files-backend.c index f4809edda8..bfa79a4a84 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1444,8 +1444,8 @@ static int should_pack_refs(struct files_ref_store *refs, return 0; } -static int files_pack_refs(struct ref_store *ref_store, - struct pack_refs_opts *opts) +static int files_optimize(struct ref_store *ref_store, + struct pack_refs_opts *opts) { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, @@ -1512,15 +1512,6 @@ static int files_pack_refs(struct ref_store *ref_store, return 0; } -static int files_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts) -{ - /* - * For the "files" backend, "optimizing" is the same as "packing". - * So, we just call the existing worker function for packing. - */ - return files_pack_refs(ref_store, opts); -} - /* * People using contrib's git-new-workdir have .git/logs/refs -> * /some/other/path/.git/logs/refs, and that may live on another device. @@ -3990,7 +3981,6 @@ struct ref_storage_be refs_be_files = { .transaction_finish = files_transaction_finish, .transaction_abort = files_transaction_abort, - .pack_refs = files_pack_refs, .optimize = files_optimize, .rename_ref = files_rename_ref, .copy_ref = files_copy_ref, diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 1ab0c50393..20cf9fab18 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1773,8 +1773,8 @@ static int packed_transaction_finish(struct ref_store *ref_store, return ret; } -static int packed_pack_refs(struct ref_store *ref_store UNUSED, - struct pack_refs_opts *pack_opts UNUSED) +static int packed_optimize(struct ref_store *ref_store UNUSED, + struct pack_refs_opts *pack_opts UNUSED) { /* * Packed refs are already packed. It might be that loose refs @@ -2129,7 +2129,7 @@ struct ref_storage_be refs_be_packed = { .transaction_finish = packed_transaction_finish, .transaction_abort = packed_transaction_abort, - .pack_refs = packed_pack_refs, + .optimize = packed_optimize, .rename_ref = NULL, .copy_ref = NULL, diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 4671517dad..fc5149df5b 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -422,8 +422,6 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs, struct ref_transaction *transaction, struct strbuf *err); -typedef int pack_refs_fn(struct ref_store *ref_store, - struct pack_refs_opts *opts); typedef int optimize_fn(struct ref_store *ref_store, struct pack_refs_opts *opts); typedef int rename_ref_fn(struct ref_store *ref_store, @@ -550,7 +548,6 @@ struct ref_storage_be { ref_transaction_finish_fn *transaction_finish; ref_transaction_abort_fn *transaction_abort; - pack_refs_fn *pack_refs; optimize_fn *optimize; rename_ref_fn *rename_ref; copy_ref_fn *copy_ref; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 6bbfd5618d..43cc66a48e 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1700,11 +1700,11 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED, return ret; } -static int reftable_be_pack_refs(struct ref_store *ref_store, - struct pack_refs_opts *opts) +static int reftable_be_optimize(struct ref_store *ref_store, + struct pack_refs_opts *opts) { struct reftable_ref_store *refs = - reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "pack_refs"); + reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "optimize_refs"); struct reftable_stack *stack; int ret; @@ -1733,12 +1733,6 @@ static int reftable_be_pack_refs(struct ref_store *ref_store, return ret; } -static int reftable_be_optimize(struct ref_store *ref_store, - struct pack_refs_opts *opts) -{ - return reftable_be_pack_refs(ref_store, opts); -} - struct write_create_symref_arg { struct reftable_ref_store *refs; struct reftable_stack *stack; @@ -2761,7 +2755,6 @@ struct ref_storage_be refs_be_reftable = { .transaction_finish = reftable_be_transaction_finish, .transaction_abort = reftable_be_transaction_abort, - .pack_refs = reftable_be_pack_refs, .optimize = reftable_be_optimize, .rename_ref = reftable_be_rename_ref, .copy_ref = reftable_be_copy_ref, -- GitLab From 03645cb0c461cf8e4f7c4dca394084e99653c5f2 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 20 Oct 2025 10:18:30 +0200 Subject: [PATCH 24/31] refs: rename 'pack_refs_opts' to 'refs_optimize_opts' The previous commit removed all references to 'pack_refs()' within the refs subsystem. Continue this cleanup by also renaming 'pack_refs_opts' to 'refs_optimize_opts' and the respective flags accordingly. Keeping the naming consistent will make the code easier to maintain. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- pack-refs.c | 20 ++++++++++---------- refs.c | 2 +- refs.h | 18 +++++++++--------- refs/debug.c | 2 +- refs/files-backend.c | 10 +++++----- refs/packed-backend.c | 2 +- refs/refs-internal.h | 2 +- refs/reftable-backend.c | 4 ++-- 8 files changed, 30 insertions(+), 30 deletions(-) diff --git a/pack-refs.c b/pack-refs.c index 1a5e07d8b8..eb6b2ba2c2 100644 --- a/pack-refs.c +++ b/pack-refs.c @@ -14,10 +14,10 @@ int pack_refs_core(int argc, { struct ref_exclusions excludes = REF_EXCLUSIONS_INIT; struct string_list included_refs = STRING_LIST_INIT_NODUP; - struct pack_refs_opts pack_refs_opts = { + struct refs_optimize_opts optimize_opts = { .exclusions = &excludes, .includes = &included_refs, - .flags = PACK_REFS_PRUNE, + .flags = REFS_OPTIMIZE_PRUNE, }; struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP; struct string_list_item *item; @@ -26,9 +26,9 @@ int pack_refs_core(int argc, struct option opts[] = { OPT_BOOL(0, "all", &pack_all, N_("pack everything")), - OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE), - OPT_BIT(0, "auto", &pack_refs_opts.flags, N_("auto-pack refs as needed"), PACK_REFS_AUTO), - OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"), + OPT_BIT(0, "prune", &optimize_opts.flags, N_("prune loose refs (default)"), REFS_OPTIMIZE_PRUNE), + OPT_BIT(0, "auto", &optimize_opts.flags, N_("auto-pack refs as needed"), REFS_OPTIMIZE_AUTO), + OPT_STRING_LIST(0, "include", optimize_opts.includes, N_("pattern"), N_("references to include")), OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"), N_("references to exclude")), @@ -39,15 +39,15 @@ int pack_refs_core(int argc, usage_with_options(usage_opts, opts); for_each_string_list_item(item, &option_excluded_refs) - add_ref_exclusion(pack_refs_opts.exclusions, item->string); + add_ref_exclusion(optimize_opts.exclusions, item->string); if (pack_all) - string_list_append(pack_refs_opts.includes, "*"); + string_list_append(optimize_opts.includes, "*"); - if (!pack_refs_opts.includes->nr) - string_list_append(pack_refs_opts.includes, "refs/tags/*"); + if (!optimize_opts.includes->nr) + string_list_append(optimize_opts.includes, "refs/tags/*"); - ret = refs_optimize(get_main_ref_store(repo), &pack_refs_opts); + ret = refs_optimize(get_main_ref_store(repo), &optimize_opts); clear_ref_exclusions(&excludes); string_list_clear(&included_refs, 0); diff --git a/refs.c b/refs.c index b9a4a60646..0d0831f29b 100644 --- a/refs.c +++ b/refs.c @@ -2313,7 +2313,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo, refs->gitdir = xstrdup(path); } -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts) { return refs->be->optimize(refs, opts); } diff --git a/refs.h b/refs.h index 8ff591ea95..6b05bba527 100644 --- a/refs.h +++ b/refs.h @@ -499,16 +499,16 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, const struct string_list *refnames); /* - * Flags for controlling behaviour of pack_refs() - * PACK_REFS_PRUNE: Prune loose refs after packing - * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end - * result are decided by the ref backend. Backends may ignore - * this flag and fall back to a normal repack. + * Flags for controlling behaviour of refs_optimize() + * REFS_OPTIMIZE_PRUNE: Prune loose refs after packing + * REFS_OPTIMIZE_AUTO: Pack refs on a best effort basis. The heuristics and end + * result are decided by the ref backend. Backends may ignore + * this flag and fall back to a normal repack. */ -#define PACK_REFS_PRUNE (1 << 0) -#define PACK_REFS_AUTO (1 << 1) +#define REFS_OPTIMIZE_PRUNE (1 << 0) +#define REFS_OPTIMIZE_AUTO (1 << 1) -struct pack_refs_opts { +struct refs_optimize_opts { unsigned int flags; struct ref_exclusions *exclusions; struct string_list *includes; @@ -518,7 +518,7 @@ struct pack_refs_opts { * Optimize the ref store. The exact behavior is up to the backend. * For the files backend, this is equivalent to packing refs. */ -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts); +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts); /* * Setup reflog before using. Fill in err and return -1 on failure. diff --git a/refs/debug.c b/refs/debug.c index 40cd1d9c15..2defd2d465 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -116,7 +116,7 @@ static int debug_transaction_abort(struct ref_store *refs, return res; } -static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts) +static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts *opts) { struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; int res = drefs->refs->be->optimize(drefs->refs, opts); diff --git a/refs/files-backend.c b/refs/files-backend.c index bfa79a4a84..a1e70b1c10 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1355,7 +1355,7 @@ static void prune_refs(struct files_ref_store *refs, struct ref_to_prune **refs_ */ static int should_pack_ref(struct files_ref_store *refs, const struct reference *ref, - struct pack_refs_opts *opts) + struct refs_optimize_opts *opts) { struct string_list_item *item; @@ -1383,7 +1383,7 @@ static int should_pack_ref(struct files_ref_store *refs, } static int should_pack_refs(struct files_ref_store *refs, - struct pack_refs_opts *opts) + struct refs_optimize_opts *opts) { struct ref_iterator *iter; size_t packed_size; @@ -1391,7 +1391,7 @@ static int should_pack_refs(struct files_ref_store *refs, size_t limit; int ret; - if (!(opts->flags & PACK_REFS_AUTO)) + if (!(opts->flags & REFS_OPTIMIZE_AUTO)) return 1; ret = packed_refs_size(refs->packed_ref_store, &packed_size); @@ -1445,7 +1445,7 @@ static int should_pack_refs(struct files_ref_store *refs, } static int files_optimize(struct ref_store *ref_store, - struct pack_refs_opts *opts) + struct refs_optimize_opts *opts) { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, @@ -1488,7 +1488,7 @@ static int files_optimize(struct ref_store *ref_store, iter->ref.name, err.buf); /* Schedule the loose reference for pruning if requested. */ - if ((opts->flags & PACK_REFS_PRUNE)) { + if ((opts->flags & REFS_OPTIMIZE_PRUNE)) { struct ref_to_prune *n; FLEX_ALLOC_STR(n, name, iter->ref.name); oidcpy(&n->oid, iter->ref.oid); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 20cf9fab18..10062fd8b6 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1774,7 +1774,7 @@ static int packed_transaction_finish(struct ref_store *ref_store, } static int packed_optimize(struct ref_store *ref_store UNUSED, - struct pack_refs_opts *pack_opts UNUSED) + struct refs_optimize_opts *opts UNUSED) { /* * Packed refs are already packed. It might be that loose refs diff --git a/refs/refs-internal.h b/refs/refs-internal.h index fc5149df5b..dee42f231d 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -423,7 +423,7 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs, struct strbuf *err); typedef int optimize_fn(struct ref_store *ref_store, - struct pack_refs_opts *opts); + struct refs_optimize_opts *opts); typedef int rename_ref_fn(struct ref_store *ref_store, const char *oldref, const char *newref, const char *logmsg); diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 43cc66a48e..c23c45f3bf 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1701,7 +1701,7 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED, } static int reftable_be_optimize(struct ref_store *ref_store, - struct pack_refs_opts *opts) + struct refs_optimize_opts *opts) { struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "optimize_refs"); @@ -1715,7 +1715,7 @@ static int reftable_be_optimize(struct ref_store *ref_store, if (!stack) stack = refs->main_backend.stack; - if (opts->flags & PACK_REFS_AUTO) + if (opts->flags & REFS_OPTIMIZE_AUTO) ret = reftable_stack_auto_compact(stack); else ret = reftable_stack_compact_all(stack, NULL); -- GitLab From edd2018f5db39d68d55a7a4af42375b1a06b9406 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 20 Oct 2025 10:18:31 +0200 Subject: [PATCH 25/31] t/pack-refs-tests: move the 'test_done' to callees In ac0bad0af4 (t0601: refactor tests to be shareable, 2025-09-19), we refactored 't/t0601-reffiles-pack-refs.sh' to move all of the tests to 't/pack-refs-tests.sh', which became a common test suite which was also used by 't/t1463-refs-optimize.sh'. This also moved the 'test_done' directive to 't/pack-refs-tests.sh'. Which inhibits additional tests from being added to either of the tests. Let's move the directive out to both the tests, so that we can add additional specific tests to them. Also the test flow logic shouldn't be part of tests which can be embedded in other test scripts. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/pack-refs-tests.sh | 2 -- t/t0601-reffiles-pack-refs.sh | 2 ++ t/t1463-refs-optimize.sh | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh index 095823d915..81086c3690 100644 --- a/t/pack-refs-tests.sh +++ b/t/pack-refs-tests.sh @@ -459,5 +459,3 @@ test_expect_success 'pack-refs does not store invalid peeled tag value' ' test_grep ! "^\^" .git/packed-refs ) ' - -test_done diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh index 12cf5d1dcb..3c706978ef 100755 --- a/t/t0601-reffiles-pack-refs.sh +++ b/t/t0601-reffiles-pack-refs.sh @@ -18,3 +18,5 @@ export GIT_TEST_DEFAULT_REF_FORMAT . ./test-lib.sh . "$TEST_DIRECTORY"/pack-refs-tests.sh + +test_done diff --git a/t/t1463-refs-optimize.sh b/t/t1463-refs-optimize.sh index c11c905d79..9afe3c1ed7 100755 --- a/t/t1463-refs-optimize.sh +++ b/t/t1463-refs-optimize.sh @@ -15,3 +15,5 @@ export GIT_TEST_DEFAULT_REF_FORMAT pack_refs='refs optimize' . "$TEST_DIRECTORY"/pack-refs-tests.sh + +test_done -- GitLab From 581eed5c4f441c7d369eb6c93c185725f9fca91b Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Tue, 21 Oct 2025 10:51:39 +0200 Subject: [PATCH 26/31] maintenance: add an 'is-needed' subcommand Hello, I recently raised a patch series [1] to add 'git refs optimize --required' which checks if the reference backend can be optimized, without actually performing the optimization. Back then, we had decided [2] that it would be a better to broaden the approach and add a 'is-needed' subcommand to 'git-maintenance(1)'. This would allow users to check if maintenance was required for the repository and users could also provide a task via the '--task' to check if maintenance was needed for a particular task. Ideally the subcommand will be used with the '--auto' flag which can check the same heuristics as that used with 'git maintenance run --auto'. Future patches can also add support for the '--schedule' flag which can be used to check required schedule it met. However that flag isn't added as part of this series. This series implements that. Commits 1-3 add the required functionality in the refs subsystem to expose an 'optimize_required' field which can be used to check if backends need to be optimized. Commit 4 utilizes this within the 'git-maintenance(1)' code. Commit 5 adds the 'is-needed' subcommand to 'git-maintenance(1)'. This is based on top of master a99f379adf (The 27th batch, 2025-10-30) and is dependent on the following series: - kn/refs-optim-cleanup - ps/ref-peeled-tags Merges cleanly with `next`. I think those two topics are close to being merged to `next` so hopefully this dependency tree doesn't get too complicated. I'll rebase as needed to resolve conflicts. [1]: https://lore.kernel.org/git/20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com/ [2]: https://lore.kernel.org/git/CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com/ --- Changes in v4: - In `update_segment_if_compaction_required()` change the argument name from `use_heuristics` to `use_geometric` since we only have one heuristic currently and this is much clearer to understand. - Swap out `required == true` with `!!required`. - Add a TODO for improvements to the flow when running `git maintenance is-needed` without the `--auto` flag. - Link to v3: https://patch.msgid.link/20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com Changes in v3: - In patch 2/5 extract out code for deciding if compaction is required into a static function. This removes duplication of logic for deciding if compaction is needed. - Link to v2: https://patch.msgid.link/20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com Changes in v2: - Added more documentation for `reftable_stack_compaction_required()`. - Fixed some typos and grammar mistakes in commit messages. - Clarify which tasks will be run when '--task' is not used. - Move the call to 'usage_with_options()' to be with 'parse_options()'. - Link to v1: https://patch.msgid.link/20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com --- b4-submit-tracking --- { "series": { "revision": 4, "change-id": "20251021-562-add-sub-command-to-check-if-maintenance-is-needed-01cae01b4606", "prefixes": [], "history": { "v1": [ "20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com" ], "v2": [ "20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com" ], "v3": [ "20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com" ] } } } -- GitLab From 1f583392eedb830a0e0eebfdecbbe817472223ab Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 1 Oct 2025 14:29:22 +0200 Subject: [PATCH 27/31] reftable/stack: return stack segments directly The `stack_table_sizes_for_compaction()` function returns individual sizes of each reftable table. This function is only called by `reftable_stack_auto_compact()` to decide which tables need to be compacted, if any. Modify the function to directly return the segments, which avoids the extra step of receiving the sizes only to pass it to `suggest_compaction_segment()`. A future commit will also add functionality for checking whether auto-compaction is necessary without performing it. This change allows code re-usability in that context. Signed-off-by: Karthik Nayak --- reftable/stack.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/reftable/stack.c b/reftable/stack.c index 65d89820bd..49387f9344 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n, return seg; } -static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st) +static int stack_segments_for_compaction(struct reftable_stack *st, + struct segment *seg) { int version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2; int overhead = header_size(version) - 1; @@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st) REFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len); if (!sizes) - return NULL; + return REFTABLE_OUT_OF_MEMORY_ERROR; for (size_t i = 0; i < st->merged->tables_len; i++) sizes[i] = st->tables[i]->size - overhead; - return sizes; + *seg = suggest_compaction_segment(sizes, st->merged->tables_len, + st->opts.auto_compaction_factor); + reftable_free(sizes); + + return 0; } int reftable_stack_auto_compact(struct reftable_stack *st) { struct segment seg; - uint64_t *sizes; + int err; if (st->merged->tables_len < 2) return 0; - sizes = stack_table_sizes_for_compaction(st); - if (!sizes) - return REFTABLE_OUT_OF_MEMORY_ERROR; - - seg = suggest_compaction_segment(sizes, st->merged->tables_len, - st->opts.auto_compaction_factor); - reftable_free(sizes); + err = stack_segments_for_compaction(st, &seg); + if (err) + return err; if (segment_size(&seg) > 0) return stack_compact_range(st, seg.start, seg.end - 1, -- GitLab From b5743b3baf04f04aa9a415c3de09b62bf656072e Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 13 Oct 2025 11:37:17 +0200 Subject: [PATCH 28/31] reftable/stack: add function to check if optimization is required The reftable backend performs auto-compaction as part of its regular flow, which is required to keep the number of tables part of a stack at bay. This allows it to stay optimized. Compaction can also be triggered voluntarily by the user via the 'git pack-refs' or the 'git refs optimize' command. However, currently there is no way for the user to check if optimization is required without actually performing it. Extract out the heuristics logic from 'reftable_stack_auto_compact()' into an internal function 'update_segment_if_compaction_required()'. Then use this to add and expose `reftable_stack_compaction_required()` which will allow users to check if the reftable backend can be optimized. Signed-off-by: Karthik Nayak --- reftable/reftable-stack.h | 11 +++++++++ reftable/stack.c | 42 +++++++++++++++++++++++++++++---- t/unit-tests/u-reftable-stack.c | 12 ++++++++-- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h index d70fcb705d..c2415cbc6e 100644 --- a/reftable/reftable-stack.h +++ b/reftable/reftable-stack.h @@ -123,6 +123,17 @@ struct reftable_log_expiry_config { int reftable_stack_compact_all(struct reftable_stack *st, struct reftable_log_expiry_config *config); +/* + * Check if compaction is required. + * + * When `use_heuristics` is false, check if all tables can be compacted to a + * single table. If true, use heuristics to determine if the tables need to be + * compacted to maintain geometric progression. + */ +int reftable_stack_compaction_required(struct reftable_stack *st, + bool use_heuristics, + bool *required); + /* heuristically compact unbalanced table stack. */ int reftable_stack_auto_compact(struct reftable_stack *st); diff --git a/reftable/stack.c b/reftable/stack.c index 49387f9344..1c9f21dfe1 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st, return 0; } -int reftable_stack_auto_compact(struct reftable_stack *st) +static int update_segment_if_compaction_required(struct reftable_stack *st, + struct segment *seg, + bool use_geometric, + bool *required) { - struct segment seg; int err; - if (st->merged->tables_len < 2) + if (st->merged->tables_len < 2) { + *required = false; + return 0; + } + + if (!use_geometric) { + *required = true; return 0; + } + + err = stack_segments_for_compaction(st, seg); + if (err) + return err; + + *required = segment_size(seg) > 0; + return 0; +} + +int reftable_stack_compaction_required(struct reftable_stack *st, + bool use_heuristics, + bool *required) +{ + struct segment seg; + return update_segment_if_compaction_required(st, &seg, use_heuristics, + required); +} + +int reftable_stack_auto_compact(struct reftable_stack *st) +{ + struct segment seg; + bool required; + int err; - err = stack_segments_for_compaction(st, &seg); + err = update_segment_if_compaction_required(st, &seg, true, &required); if (err) return err; - if (segment_size(&seg) > 0) + if (required) return stack_compact_range(st, seg.start, seg.end - 1, NULL, STACK_COMPACT_RANGE_BEST_EFFORT); diff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c index a8b91812e8..b8110cdeee 100644 --- a/t/unit-tests/u-reftable-stack.c +++ b/t/unit-tests/u-reftable-stack.c @@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void) .value_type = REFTABLE_REF_SYMREF, .value.symref = (char *) "master", }; + bool required = false; char buf[128]; /* @@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void) * auto compaction is disabled. When enabled, we should merge * all tables in the stack. */ - if (i != n) + cl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0); + if (i != n) { cl_assert_equal_i(st->merged->tables_len, i + 1); - else + if (i < 1) + cl_assert_equal_b(required, false); + else + cl_assert_equal_b(required, true); + } else { cl_assert_equal_i(st->merged->tables_len, 1); + cl_assert_equal_b(required, false); + } } reftable_stack_destroy(st); -- GitLab From db38a40ed1aba3c80481d1f55d817f744ac76f24 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 13 Oct 2025 11:37:47 +0200 Subject: [PATCH 29/31] refs: add a `optimize_required` field to `struct ref_storage_be` To allow users of the refs namespace to check if the reference backend requires optimization, add a new field `optimize_required` field to `struct ref_storage_be`. This field is of type `optimize_required_fn` which is also introduced in this commit. Modify the debug, files, packed and reftable backend to implement this field. A following commit will expose this via 'git pack-refs' and 'git refs optimize'. Signed-off-by: Karthik Nayak --- refs.c | 7 +++++++ refs.h | 7 +++++++ refs/debug.c | 13 +++++++++++++ refs/files-backend.c | 11 +++++++++++ refs/packed-backend.c | 13 +++++++++++++ refs/refs-internal.h | 6 ++++++ refs/reftable-backend.c | 25 +++++++++++++++++++++++++ 7 files changed, 82 insertions(+) diff --git a/refs.c b/refs.c index 0d0831f29b..5583f6e09d 100644 --- a/refs.c +++ b/refs.c @@ -2318,6 +2318,13 @@ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts) return refs->be->optimize(refs, opts); } +int refs_optimize_required(struct ref_store *refs, + struct refs_optimize_opts *opts, + bool *required) +{ + return refs->be->optimize_required(refs, opts, required); +} + int reference_get_peeled_oid(struct repository *repo, const struct reference *ref, struct object_id *peeled_oid) diff --git a/refs.h b/refs.h index 6b05bba527..d9051bbb04 100644 --- a/refs.h +++ b/refs.h @@ -520,6 +520,13 @@ struct refs_optimize_opts { */ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts); +/* + * Check if refs backend can be optimized by calling 'refs_optimize'. + */ +int refs_optimize_required(struct ref_store *ref_store, + struct refs_optimize_opts *opts, + bool *required); + /* * Setup reflog before using. Fill in err and return -1 on failure. */ diff --git a/refs/debug.c b/refs/debug.c index 2defd2d465..36f8c58b6c 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -124,6 +124,17 @@ static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts return res; } +static int debug_optimize_required(struct ref_store *ref_store, + struct refs_optimize_opts *opts, + bool *required) +{ + struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; + int res = drefs->refs->be->optimize_required(drefs->refs, opts, required); + trace_printf_key(&trace_refs, "optimize_required: %s, res: %d\n", + required ? "yes" : "no", res); + return res; +} + static int debug_rename_ref(struct ref_store *ref_store, const char *oldref, const char *newref, const char *logmsg) { @@ -431,6 +442,8 @@ struct ref_storage_be refs_be_debug = { .transaction_abort = debug_transaction_abort, .optimize = debug_optimize, + .optimize_required = debug_optimize_required, + .rename_ref = debug_rename_ref, .copy_ref = debug_copy_ref, diff --git a/refs/files-backend.c b/refs/files-backend.c index a1e70b1c10..6e0c9b340a 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1512,6 +1512,16 @@ static int files_optimize(struct ref_store *ref_store, return 0; } +static int files_optimize_required(struct ref_store *ref_store, + struct refs_optimize_opts *opts, + bool *required) +{ + struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ, + "optimize_required"); + *required = should_pack_refs(refs, opts); + return 0; +} + /* * People using contrib's git-new-workdir have .git/logs/refs -> * /some/other/path/.git/logs/refs, and that may live on another device. @@ -3982,6 +3992,7 @@ struct ref_storage_be refs_be_files = { .transaction_abort = files_transaction_abort, .optimize = files_optimize, + .optimize_required = files_optimize_required, .rename_ref = files_rename_ref, .copy_ref = files_copy_ref, diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 10062fd8b6..19ce4d5872 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1784,6 +1784,17 @@ static int packed_optimize(struct ref_store *ref_store UNUSED, return 0; } +static int packed_optimize_required(struct ref_store *ref_store UNUSED, + struct refs_optimize_opts *opts UNUSED, + bool *required) +{ + /* + * Packed refs are already optimized. + */ + *required = false; + return 0; +} + static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store UNUSED) { return empty_ref_iterator_begin(); @@ -2130,6 +2141,8 @@ struct ref_storage_be refs_be_packed = { .transaction_abort = packed_transaction_abort, .optimize = packed_optimize, + .optimize_required = packed_optimize_required, + .rename_ref = NULL, .copy_ref = NULL, diff --git a/refs/refs-internal.h b/refs/refs-internal.h index dee42f231d..c7d2a6e50b 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -424,6 +424,11 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs, typedef int optimize_fn(struct ref_store *ref_store, struct refs_optimize_opts *opts); + +typedef int optimize_required_fn(struct ref_store *ref_store, + struct refs_optimize_opts *opts, + bool *required); + typedef int rename_ref_fn(struct ref_store *ref_store, const char *oldref, const char *newref, const char *logmsg); @@ -549,6 +554,7 @@ struct ref_storage_be { ref_transaction_abort_fn *transaction_abort; optimize_fn *optimize; + optimize_required_fn *optimize_required; rename_ref_fn *rename_ref; copy_ref_fn *copy_ref; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index c23c45f3bf..a3ae0cf74a 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1733,6 +1733,29 @@ static int reftable_be_optimize(struct ref_store *ref_store, return ret; } +static int reftable_be_optimize_required(struct ref_store *ref_store, + struct refs_optimize_opts *opts, + bool *required) +{ + struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ, + "optimize_refs_required"); + struct reftable_stack *stack; + bool use_heuristics = false; + + if (refs->err) + return refs->err; + + stack = refs->worktree_backend.stack; + if (!stack) + stack = refs->main_backend.stack; + + if (opts->flags & REFS_OPTIMIZE_AUTO) + use_heuristics = true; + + return reftable_stack_compaction_required(stack, use_heuristics, + required); +} + struct write_create_symref_arg { struct reftable_ref_store *refs; struct reftable_stack *stack; @@ -2756,6 +2779,8 @@ struct ref_storage_be refs_be_reftable = { .transaction_abort = reftable_be_transaction_abort, .optimize = reftable_be_optimize, + .optimize_required = reftable_be_optimize_required, + .rename_ref = reftable_be_rename_ref, .copy_ref = reftable_be_copy_ref, -- GitLab From 3c0bfa92a3668c7568acef3756c762b70542d10a Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 22 Oct 2025 17:03:57 +0200 Subject: [PATCH 30/31] maintenance: add checking logic in `pack_refs_condition()` The 'git-maintenance(1)' command supports an '--auto' flag. Usage of the flag ensures to run maintenance tasks only if certain thresholds are met. The heuristic is defined on a task level, wherein each task defines an 'auto_condition', which states if the task should be run. The 'pack-refs' task is hard-coded to return 1 as: 1. There was never a way to check if the reference backend needs to be optimized without actually performing the optimization. 2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would optimize based on heuristics. The previous commit added a `refs_optimize_required()` function, which can be used to check if a reference backend required optimization. Use this within `pack_refs_condition()`. This allows us to add a 'git maintenance is-needed' subcommand which can notify the user if maintenance is needed without actually performing the optimization. Without this change, the reference backend would always state that optimization is needed. Since we import 'revision.h', we need to remove the definition for 'SEEN' which is duplicated in the included header. Signed-off-by: Karthik Nayak --- builtin/gc.c | 30 +++++++++++++++++++++--------- object.h | 1 - 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/builtin/gc.c b/builtin/gc.c index c6d62c74a7..b62dcc4da6 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -35,6 +35,7 @@ #include "path.h" #include "reflog.h" #include "rerere.h" +#include "revision.h" #include "blob.h" #include "tree.h" #include "promisor-remote.h" @@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts) static int pack_refs_condition(UNUSED struct gc_config *cfg) { - /* - * The auto-repacking logic for refs is handled by the ref backends and - * exposed via `git pack-refs --auto`. We thus always return truish - * here and let the backend decide for us. - */ - return 1; + struct string_list included_refs = STRING_LIST_INIT_NODUP; + struct ref_exclusions excludes = REF_EXCLUSIONS_INIT; + struct refs_optimize_opts optimize_opts = { + .exclusions = &excludes, + .includes = &included_refs, + .flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO, + }; + bool required; + + /* Check for all refs, similar to 'git refs optimize --all'. */ + string_list_append(optimize_opts.includes, "*"); + + if (refs_optimize_required(get_main_ref_store(the_repository), + &optimize_opts, &required)) + return 0; + + clear_ref_exclusions(&excludes); + string_list_clear(&included_refs, 0); + + return !!required; } static int maintenance_task_pack_refs(struct maintenance_run_opts *opts, @@ -1090,9 +1105,6 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg, return 0; } -/* Remember to update object flag allocation in object.h */ -#define SEEN (1u<<0) - struct cg_auto_data { int num_not_in_graph; int limit; diff --git a/object.h b/object.h index 1499f63d50..832299e763 100644 --- a/object.h +++ b/object.h @@ -79,7 +79,6 @@ void object_array_init(struct object_array *array); * list-objects-filter.c: 21 * bloom.c: 2122 * builtin/fsck.c: 0--3 - * builtin/gc.c: 0 * builtin/index-pack.c: 2021 * reflog.c: 10--12 * builtin/show-branch.c: 0-------------------------------------------26 -- GitLab From ff67e1b81fc898a4e239be7c7829882bbd0f0d7c Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 22 Oct 2025 16:40:21 +0200 Subject: [PATCH 31/31] maintenance: add 'is-needed' subcommand The 'git-maintenance(1)' command provides tooling to run maintenance tasks over Git repositories. The 'run' subcommand, as the name suggests, runs the maintenance tasks. When used with the '--auto' flag, it uses heuristics to determine if the required thresholds are met for running said maintenance tasks. There is however a lack of insight into these heuristics. Meaning, the checks are linked to the execution. Add a new 'is-needed' subcommand to 'git-maintenance(1)' which allows users to simply check if it is needed to run maintenance without performing it. This subcommand can check if it is needed to run maintenance without actually running it. Ideally it should be used with the '--auto' flag, which would allow users to check if the thresholds required are met. The subcommand also supports the '--task' flag which can be used to check specific maintenance tasks. While adding the respective tests in 't/t7900-maintenance.sh', remove a duplicate of the test: 'worktree-prune task with --auto honors maintenance.worktree-prune.auto'. Signed-off-by: Karthik Nayak --- Documentation/git-maintenance.adoc | 13 ++++++ builtin/gc.c | 63 +++++++++++++++++++++++++++++- t/t7900-maintenance.sh | 54 +++++++++++++++++-------- 3 files changed, 113 insertions(+), 17 deletions(-) diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc index 540b5cf68b..bda616f14c 100644 --- a/Documentation/git-maintenance.adoc +++ b/Documentation/git-maintenance.adoc @@ -12,6 +12,7 @@ SYNOPSIS 'git maintenance' run [] 'git maintenance' start [--scheduler=] 'git maintenance' (stop|register|unregister) [] +'git maintenance' is-needed [] DESCRIPTION @@ -84,6 +85,16 @@ The `unregister` subcommand will report an error if the current repository is not already registered. Use the `--force` option to return success even when the current repository is not registered. +is-needed:: + Check whether maintenance needs to be run without actually running it. + Exits with a 0 status code if maintenance needs to be run, 1 otherwise. + Ideally used with the '--auto' flag. ++ +If one or more `--task` options are specified, then those tasks are checked +in that order. Otherwise, the tasks are determined by which +`maintenance..enabled` config options are true. By default, only +`maintenance.gc.enabled` is true. + TASKS ----- @@ -183,6 +194,8 @@ OPTIONS in the `gc.auto` config setting, or when the number of pack-files exceeds the `gc.autoPackLimit` config setting. Not compatible with the `--schedule` option. + When combined with the `is-needed` subcommand, check if the required + thresholds are met without actually running maintenance. --schedule:: When combined with the `run` subcommand, run maintenance tasks diff --git a/builtin/gc.c b/builtin/gc.c index b62dcc4da6..8ca28eab90 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -3253,7 +3253,67 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix, return update_background_schedule(NULL, 0); } -static const char * const builtin_maintenance_usage[] = { +static const char *const builtin_maintenance_is_needed_usage[] = { + "git maintenance is-needed [--task=] [--schedule]", + NULL +}; + +static int maintenance_is_needed(int argc, const char **argv, const char *prefix, + struct repository *repo UNUSED) +{ + struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT; + struct string_list selected_tasks = STRING_LIST_INIT_DUP; + struct gc_config cfg = GC_CONFIG_INIT; + struct option options[] = { + OPT_BOOL(0, "auto", &opts.auto_flag, + N_("run tasks based on the state of the repository")), + OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"), + N_("check a specific task"), + PARSE_OPT_NONEG, task_option_parse), + OPT_END() + }; + bool is_needed = false; + + argc = parse_options(argc, argv, prefix, options, + builtin_maintenance_is_needed_usage, + PARSE_OPT_STOP_AT_NON_OPTION); + if (argc) + usage_with_options(builtin_maintenance_is_needed_usage, options); + + gc_config(&cfg); + initialize_task_config(&opts, &selected_tasks); + + if (opts.auto_flag) { + for (size_t i = 0; i < opts.tasks_nr; i++) { + if (tasks[opts.tasks[i]].auto_condition && + tasks[opts.tasks[i]].auto_condition(&cfg)) { + is_needed = true; + break; + } + } + } else { + /* + * When not using --auto we always require maintenance right now. + * + * TODO: this certainly is too eager, as some maintenance tasks may + * decide to not do anything because the data structures are already + * fully optimized. We may eventually want to extend the auto + * condition to also cover non-auto runs so that we can detect such + * cases. + */ + is_needed = true; + } + + string_list_clear(&selected_tasks, 0); + maintenance_run_opts_release(&opts); + gc_config_release(&cfg); + + if (is_needed) + return 0; + return 1; +} + +static const char *const builtin_maintenance_usage[] = { N_("git maintenance []"), NULL, }; @@ -3270,6 +3330,7 @@ int cmd_maintenance(int argc, OPT_SUBCOMMAND("stop", &fn, maintenance_stop), OPT_SUBCOMMAND("register", &fn, maintenance_register), OPT_SUBCOMMAND("unregister", &fn, maintenance_unregister), + OPT_SUBCOMMAND("is-needed", &fn, maintenance_is_needed), OPT_END(), }; diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index ddd273d8dc..a17e2091c2 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -49,7 +49,9 @@ test_expect_success 'run [--auto|--quiet]' ' git maintenance run --auto 2>/dev/null && GIT_TRACE2_EVENT="$(pwd)/run-no-quiet.txt" \ git maintenance run --no-quiet 2>/dev/null && + git maintenance is-needed && test_subcommand git gc --quiet --no-detach --skip-foreground-tasks /dev/null && test_subcommand ! git prune-packed --quiet /dev/null && test_subcommand ! git prune-packed --quiet /dev/null && test_subcommand git prune-packed --quiet /dev/null && @@ -421,10 +435,13 @@ run_incremental_repack_and_verify () { test_commit A && git repack -adk && git multi-pack-index write && + ! git -c maintenance.incremental-repack.auto=1 \ + maintenance is-needed --auto --task=incremental-repack && GIT_TRACE2_EVENT="$(pwd)/midx-init.txt" git \ -c maintenance.incremental-repack.auto=1 \ maintenance run --auto --task=incremental-repack 2>/dev/null && test_subcommand ! git multi-pack-index write --no-progress /dev/null && test_subcommand ! git multi-pack-index write --no-progress /dev/null && @@ -485,9 +505,15 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits' git reflog expire --all --expire=now && test_commit reflog-one && test_commit reflog-two && + + ! git -c maintenance.reflog-expire.auto=3 \ + maintenance is-needed --auto --task=reflog-expire && GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \ git -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire && test_subcommand ! git reflog expire --all .git/worktrees/abc && + git maintenance is-needed --auto --task=worktree-prune && test_expect_worktree_prune git maintenance run --auto --task=worktree-prune ' @@ -530,22 +557,7 @@ test_expect_success 'worktree-prune task with --auto honors maintenance.worktree test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune && # A positive value should require at least this many prunable worktrees. test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune && - test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune -' - -test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' ' - # A negative value should always prune. - test_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune && - - mkdir .git/worktrees && - : >.git/worktrees/first && - : >.git/worktrees/second && - : >.git/worktrees/third && - - # Zero should never prune. - test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune && - # A positive value should require at least this many prunable worktrees. - test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune && + git -c maintenance.worktree-prune.auto=3 maintenance is-needed --auto --task=worktree-prune && test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune ' @@ -554,11 +566,13 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' ' rm -rf worktree && rm -f worktree-prune.txt && + ! git -c gc.worktreePruneExpire=1.week.ago maintenance is-needed --auto --task=worktree-prune && GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune && test_subcommand ! git worktree prune --expire 1.week.ago .git/rr-cache/entry && + git maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc git maintenance run --auto --task=rerere-gc ' @@ -594,17 +611,22 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut test_when_finished "rm -rf .git/rr-cache" && # A negative value should always prune. + git -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc && # A positive value prunes when there is at least one entry. + ! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc && mkdir .git/rr-cache && + ! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc && : >.git/rr-cache/entry-1 && + git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc && # Zero should never prune. : >.git/rr-cache/entry-1 && + ! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc && test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc ' -- GitLab