* [PATCH v5 3/4] environment: move trust_executable_bit into repo_config_values
From: Tian Yuchen @ 2026-07-15 3:55 UTC (permalink / raw)
To: git
Cc: ps, cirnovskyv, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260715035501.48271-1-cat@malon.dev>
Move the global 'trust_executable_bit' configuration
into the repository-specific 'repo_config_values'
struct.
To ensure code readability, the getter function
'repo_trust_executable_bit()' has been introduced.
Callers access this configuration by passing in 'repo'
when possible, and explicitly fall back to 'the_repository'
the rest of time.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
apply.c | 2 +-
environment.c | 11 +++++++++--
environment.h | 4 +++-
read-cache.c | 8 ++++----
4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/apply.c b/apply.c
index 249248d4f2..47b6ae5904 100644
--- a/apply.c
+++ b/apply.c
@@ -3893,7 +3893,7 @@ static int check_preimage(struct apply_state *state,
if (*ce && !(*ce)->ce_mode)
BUG("ce_mode == 0 for path '%s'", old_name);
- if (trust_executable_bit || !S_ISREG(st->st_mode))
+ if (repo_trust_executable_bit(state->repo) || !S_ISREG(st->st_mode))
st_mode = ce_mode_from_stat(*ce, st->st_mode);
else if (*ce)
st_mode = (*ce)->ce_mode;
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..75069a884d 100644
--- a/environment.c
+++ b/environment.c
@@ -41,7 +41,6 @@
static int pack_compression_seen;
static int zlib_compression_seen;
-int trust_executable_bit = 1;
int trust_ctime = 1;
int check_stat = 1;
int has_symlinks = 1;
@@ -142,6 +141,13 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
}
+int repo_trust_executable_bit(struct repository *repo)
+{
+ return repo->gitdir?
+ repo_config_values(repo)->trust_executable_bit :
+ 1;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -305,7 +311,7 @@ int git_default_core_config(const char *var, const char *value,
/* This needs a better name */
if (!strcmp(var, "core.filemode")) {
- trust_executable_bit = git_config_bool(var, value);
+ cfg->trust_executable_bit = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "core.trustctime")) {
@@ -720,5 +726,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
+ cfg->trust_executable_bit = 1;
cfg->branch_track = BRANCH_TRACK_REMOTE;
}
diff --git a/environment.h b/environment.h
index 123a71cdc8..72b59fd89c 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,7 @@ struct repo_config_values {
/* section "core" config values */
char *attributes_file;
int apply_sparse_checkout;
+ int trust_executable_bit;
/* section "branch" config values */
enum branch_track branch_track;
@@ -123,6 +124,8 @@ int git_default_config(const char *, const char *,
int git_default_core_config(const char *var, const char *value,
const struct config_context *ctx, void *cb);
+int repo_trust_executable_bit(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -160,7 +163,6 @@ int is_bare_repository(void);
extern char *git_work_tree_cfg;
/* Environment bits from configuration mechanism */
-extern int trust_executable_bit;
extern int trust_ctime;
extern int check_stat;
extern int has_symlinks;
diff --git a/read-cache.c b/read-cache.c
index cb4f4878c8..a9c11a3346 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -214,7 +214,7 @@ unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
if (!has_symlinks && S_ISREG(mode) &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
- if (!trust_executable_bit && S_ISREG(mode)) {
+ if (!repo_trust_executable_bit(the_repository) && S_ISREG(mode)) {
if (ce && S_ISREG(ce->ce_mode))
return ce->ce_mode;
return create_ce_mode(0666);
@@ -228,7 +228,7 @@ static unsigned int st_mode_from_ce(const struct cache_entry *ce)
case S_IFLNK:
return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
case S_IFREG:
- return (ce->ce_mode & (trust_executable_bit ? 0755 : 0644)) | S_IFREG;
+ return (ce->ce_mode & (repo_trust_executable_bit(the_repository) ? 0755 : 0644)) | S_IFREG;
case S_IFGITLINK:
return S_IFDIR | 0755;
case S_IFDIR:
@@ -338,7 +338,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
/* We consider only the owner x bit to be relevant for
* "mode changes"
*/
- if (trust_executable_bit &&
+ if (repo_trust_executable_bit(the_repository) &&
(0100 & (ce->ce_mode ^ st->st_mode)))
changed |= MODE_CHANGED;
break;
@@ -759,7 +759,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_flags |= CE_INTENT_TO_ADD;
- if (trust_executable_bit && has_symlinks) {
+ if (repo_trust_executable_bit(istate->repo) && has_symlinks) {
ce->ce_mode = create_ce_mode(st_mode);
} else {
/* If there is an existing entry, pick the mode bits and type
--
2.43.0
^ permalink raw reply related
* [PATCH v5 4/4] environment: move has_symlinks into repo_config_values
From: Tian Yuchen @ 2026-07-15 3:55 UTC (permalink / raw)
To: git
Cc: ps, cirnovskyv, Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260715035501.48271-1-cat@malon.dev>
Move the global 'has_symlinks' configuration into the
repository-specific 'repo_config_values' struct.
To ensure code readability, the getter function
'repo_has_symlinks()' has been introduced. Callers access
this configuration by passing in 'repo' when possible,
and explicitly fall back to 'the_repository' the rest
of the time.
Note:
To support early platform-specific (MinGW) overrides
before repository initialization, a global variable
'default_has_symlinks' fallback is introduced as a fallback
in environment.h. The *writer* in compat/mingw.c can only
access this variable.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
apply.c | 2 +-
combine-diff.c | 2 +-
compat/mingw.c | 7 ++++---
entry.c | 2 +-
environment.c | 17 +++++++++++++++--
environment.h | 5 ++++-
read-cache.c | 9 +++++----
7 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/apply.c b/apply.c
index 47b6ae5904..4ce4160b48 100644
--- a/apply.c
+++ b/apply.c
@@ -4511,7 +4511,7 @@ static int try_create_file(struct apply_state *state, const char *path,
return !!mkdir(path, 0777);
}
- if (has_symlinks && S_ISLNK(mode))
+ if (repo_has_symlinks(state->repo) && S_ISLNK(mode))
/* Although buf:size is counted string, it also is NUL
* terminated.
*/
diff --git a/combine-diff.c b/combine-diff.c
index b799862068..80e5c46e9b 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1078,7 +1078,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
/* if symlinks don't work, assume symlink if all parents
* are symlinks
*/
- is_file = has_symlinks;
+ is_file = repo_has_symlinks(rev->repo);
for (i = 0; !is_file && i < num_parent; i++)
is_file = !S_ISLNK(elem->parent[i].mode);
if (!is_file)
diff --git a/compat/mingw.c b/compat/mingw.c
index aa7525f419..b38cd9a1ec 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -7,6 +7,7 @@
#include "config.h"
#include "dir.h"
#include "environment.h"
+#include "repository.h"
#include "gettext.h"
#include "run-command.h"
#include "strbuf.h"
@@ -1043,7 +1044,7 @@ int mingw_chdir(const char *dirname)
if (xutftowcs_path(wdirname, dirname) < 0)
return -1;
- if (has_symlinks) {
+ if (repo_has_symlinks(the_repository)) {
HANDLE hnd = CreateFileW(wdirname, 0,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
@@ -2903,7 +2904,7 @@ int symlink(const char *target, const char *link)
int len;
/* fail if symlinks are disabled or API is not supported (WinXP) */
- if (!has_symlinks) {
+ if (!repo_has_symlinks(the_repository)) {
errno = ENOSYS;
return -1;
}
@@ -3181,7 +3182,7 @@ static void setup_windows_environment(void)
* symlink support.
*/
if (!(tmp = getenv("MSYS")) || !strstr(tmp, "winsymlinks:nativestrict"))
- has_symlinks = 0;
+ default_has_symlinks = 0;
}
static void get_current_user_sid(PSID *sid, HANDLE *linked_token)
diff --git a/entry.c b/entry.c
index 7817aee362..f2854b4cd8 100644
--- a/entry.c
+++ b/entry.c
@@ -321,7 +321,7 @@ static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca
* We can't make a real symlink; write out a regular file entry
* with the symlink destination as its contents.
*/
- if (!has_symlinks || to_tempfile)
+ if (!repo_has_symlinks(state->istate ? state->istate->repo : NULL) || to_tempfile)
goto write_file_entry;
ret = symlink(new_blob, path);
diff --git a/environment.c b/environment.c
index 75069a884d..c7cebe18fd 100644
--- a/environment.c
+++ b/environment.c
@@ -43,7 +43,7 @@ static int zlib_compression_seen;
int trust_ctime = 1;
int check_stat = 1;
-int has_symlinks = 1;
+int default_has_symlinks = 1;
int minimum_abbrev = 4, default_abbrev = -1;
int ignore_case;
int assume_unchanged;
@@ -148,6 +148,17 @@ int repo_trust_executable_bit(struct repository *repo)
1;
}
+int repo_has_symlinks(struct repository *repo)
+{
+ if (!repo)
+ repo = the_repository;
+
+ if (!repo->gitdir)
+ return default_has_symlinks;
+
+ return repo_config_values(repo)->has_symlinks;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -336,7 +347,8 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.symlinks")) {
- has_symlinks = git_config_bool(var, value);
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+ cfg->has_symlinks = git_config_bool(var, value);
return 0;
}
@@ -727,5 +739,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
cfg->trust_executable_bit = 1;
+ cfg->has_symlinks = default_has_symlinks;
cfg->branch_track = BRANCH_TRACK_REMOTE;
}
diff --git a/environment.h b/environment.h
index 72b59fd89c..e590225c86 100644
--- a/environment.h
+++ b/environment.h
@@ -92,6 +92,7 @@ struct repo_config_values {
char *attributes_file;
int apply_sparse_checkout;
int trust_executable_bit;
+ int has_symlinks;
/* section "branch" config values */
enum branch_track branch_track;
@@ -126,6 +127,8 @@ int git_default_core_config(const char *var, const char *value,
int repo_trust_executable_bit(struct repository *repo);
+int repo_has_symlinks(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -165,7 +168,7 @@ extern char *git_work_tree_cfg;
/* Environment bits from configuration mechanism */
extern int trust_ctime;
extern int check_stat;
-extern int has_symlinks;
+extern int default_has_symlinks;
extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
diff --git a/read-cache.c b/read-cache.c
index a9c11a3346..5a40ffa061 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -211,7 +211,7 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
*/
unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
{
- if (!has_symlinks && S_ISREG(mode) &&
+ if (!repo_has_symlinks(the_repository) && S_ISREG(mode) &&
ce && S_ISLNK(ce->ce_mode))
return ce->ce_mode;
if (!repo_trust_executable_bit(the_repository) && S_ISREG(mode)) {
@@ -226,7 +226,7 @@ static unsigned int st_mode_from_ce(const struct cache_entry *ce)
{
switch (ce->ce_mode & S_IFMT) {
case S_IFLNK:
- return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
+ return repo_has_symlinks(the_repository) ? S_IFLNK : (S_IFREG | 0644);
case S_IFREG:
return (ce->ce_mode & (repo_trust_executable_bit(the_repository) ? 0755 : 0644)) | S_IFREG;
case S_IFGITLINK:
@@ -344,7 +344,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
break;
case S_IFLNK:
if (!S_ISLNK(st->st_mode) &&
- (has_symlinks || !S_ISREG(st->st_mode)))
+ (repo_has_symlinks(the_repository) || !S_ISREG(st->st_mode)))
changed |= TYPE_CHANGED;
break;
case S_IFGITLINK:
@@ -759,7 +759,8 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
ce->ce_flags |= CE_INTENT_TO_ADD;
- if (repo_trust_executable_bit(istate->repo) && has_symlinks) {
+ if (repo_trust_executable_bit(istate->repo) &&
+ repo_has_symlinks(istate->repo)) {
ce->ce_mode = create_ce_mode(st_mode);
} else {
/* If there is an existing entry, pick the mode bits and type
--
2.43.0
^ permalink raw reply related
* [PATCH v4] show-branch: convert per-branch flags to commit-slab
From: Gatla Vishweshwar Reddy @ 2026-07-15 4:18 UTC (permalink / raw)
To: gitster; +Cc: git, Gatla Vishweshwar Reddy
In-Reply-To: <xmqq1pd5q632.fsf@gitster.g>
show-branch uses commit->object.flags to store per-branch
reachability bits, one bit per branch starting at REV_SHIFT.
The flags word has only a fixed number of available bits, limiting
the number of branches that can be shown simultaneously to MAX_REVS.
Convert the per-branch bits to a dedicated commit-slab using uint64_t
as the element type, initialized with a stride via
init_commit_rev_flags_with_stride(). Keep the UNINTERESTING bit in
object.flags where it belongs, as it is used for revision walking and
does not need to be in the per-branch slab. With UNINTERESTING removed
from the slab, REV_SHIFT becomes 0 and all 64 bits of uint64_t are
available for branch tracking, lifting MAX_REVS from 27 to 64 branches.
Add helper functions get_rev_flags_ptr(), peek_rev_flags_ptr(),
has_any_rev_flags(), or_rev_flag_bit(), test_rev_flag_bit(),
has_all_rev_flags(), and has_only_rev_flag_bit() to encapsulate
per-bit slab access cleanly. Use has_only_rev_flag_bit() in
show_independent() to preserve the original semantics: a commit is
independent only if reachable from exactly one tip, not merely if
the i-th bit happens to be set. Update all bit operations to use
UINT64_C(1) for correct 64-bit shifts. Initialize and clear the slab
in cmd_show_branch().
Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
---
Changes in v4:
- Fix show_independent() to use has_only_rev_flag_bit() instead of
test_rev_flag_bit(), preserving the original semantics: a commit is
independent only if reachable from exactly one tip, not merely if
the i-th bit is set.
builtin/show-branch.c | 159 ++++++++++++++++++++++++++----------------
1 file changed, 99 insertions(+), 60 deletions(-)
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index f02831b085..a6598541b9 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -34,16 +34,9 @@ static enum git_colorbool showbranch_use_color = GIT_COLOR_UNKNOWN;
static struct strvec default_args = STRVEC_INIT;
-/*
- * TODO: convert this use of commit->object.flags to commit-slab
- * instead to store a pointer to ref name directly. Then use the same
- * UNINTERESTING definition from revision.h here.
- */
#define UNINTERESTING 01
-
-#define REV_SHIFT 2
-#define MAX_REVS (FLAG_BITS - REV_SHIFT) /* should not exceed bits_per_int - REV_SHIFT */
-
+#define REV_SHIFT 0
+#define MAX_REVS (sizeof(uint64_t) * 8)
#define DEFAULT_REFLOG 4
static const char *get_color_code(int idx)
@@ -79,11 +72,72 @@ struct commit_name {
define_commit_slab(commit_name_slab, struct commit_name *);
static struct commit_name_slab name_slab;
+define_commit_slab(commit_rev_flags, uint64_t);
+static struct commit_rev_flags rev_flags_slab;
+static int flags_stride; /* number of uint64_t words per commit */
+
static struct commit_name *commit_to_name(struct commit *commit)
{
return *commit_name_slab_at(&name_slab, commit);
}
+static uint64_t *get_rev_flags_ptr(struct commit *commit)
+{
+ return commit_rev_flags_at(&rev_flags_slab, commit);
+}
+
+static uint64_t *peek_rev_flags_ptr(struct commit *commit)
+{
+ return commit_rev_flags_peek(&rev_flags_slab, commit);
+}
+
+static int has_any_rev_flags(struct commit *commit)
+{
+ uint64_t *f = peek_rev_flags_ptr(commit);
+ int i;
+ if (!f)
+ return 0;
+ for (i = 0; i < flags_stride; i++)
+ if (f[i])
+ return 1;
+ return 0;
+}
+
+static void or_rev_flag_bit(struct commit *commit, int branch)
+{
+ get_rev_flags_ptr(commit)[branch / 64] |= UINT64_C(1) << (branch % 64);
+}
+
+static int test_rev_flag_bit(struct commit *commit, int branch)
+{
+ uint64_t *f = peek_rev_flags_ptr(commit);
+ return f && !!(f[branch / 64] & (UINT64_C(1) << (branch % 64)));
+}
+
+static int has_all_rev_flags(struct commit *commit, int num_rev)
+{
+ int i;
+ for (i = 0; i < num_rev; i++)
+ if (!test_rev_flag_bit(commit, i))
+ return 0;
+ return 1;
+}
+
+static int has_only_rev_flag_bit(struct commit *commit, int branch)
+{
+ uint64_t *f = peek_rev_flags_ptr(commit);
+ int i;
+ if (!f)
+ return 0;
+ for (i = 0; i < flags_stride; i++) {
+ uint64_t expected = (i == branch / 64)
+ ? (UINT64_C(1) << (branch % 64))
+ : 0;
+ if (f[i] != expected)
+ return 0;
+ }
+ return 1;
+}
/* Name the commit as nth generation ancestor of head_name;
* we count only the first-parent relationship for naming purposes.
@@ -215,7 +269,7 @@ static void name_commits(struct commit_list *list,
static int mark_seen(struct commit *commit, struct commit_list **seen_p)
{
- if (!commit->object.flags) {
+ if (!has_any_rev_flags(commit)) {
commit_list_insert(commit, seen_p);
return 1;
}
@@ -226,34 +280,34 @@ static void join_revs(struct prio_queue *queue,
struct commit_list **seen_p,
int num_rev, int extra)
{
- int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
- int all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
while (queue->nr) {
struct commit_list *parents;
int still_interesting = !!interesting(queue);
struct commit *commit = prio_queue_peek(queue);
bool get_pending = true;
- int flags = commit->object.flags & all_mask;
if (!still_interesting && extra <= 0)
break;
mark_seen(commit, seen_p);
- if ((flags & all_revs) == all_revs)
- flags |= UNINTERESTING;
+ if (has_all_rev_flags(commit, num_rev))
+ commit->object.flags |= UNINTERESTING;
parents = commit->parents;
while (parents) {
struct commit *p = parents->item;
- int this_flag = p->object.flags;
parents = parents->next;
- if ((this_flag & flags) == flags)
+ if (has_all_rev_flags(p, num_rev))
continue;
repo_parse_commit(the_repository, p);
if (mark_seen(p, seen_p) && !still_interesting)
extra--;
- p->object.flags |= flags;
+ {
+ int _b;
+ for (_b = 0; _b < num_rev; _b++)
+ if (test_rev_flag_bit(commit, _b))
+ or_rev_flag_bit(p, _b);
+ }
if (get_pending)
prio_queue_replace(queue, p);
else
@@ -263,7 +317,6 @@ static void join_revs(struct prio_queue *queue,
if (get_pending)
prio_queue_get(queue);
}
-
/*
* Postprocess to complete well-poisoning.
*
@@ -278,7 +331,7 @@ static void join_revs(struct prio_queue *queue,
struct commit *c = s->item;
struct commit_list *parents;
- if (((c->object.flags & all_revs) != all_revs) &&
+ if (!has_all_rev_flags(c, num_rev) &&
!(c->object.flags & UNINTERESTING))
continue;
@@ -410,8 +463,8 @@ static int append_ref(const char *refname, const struct object_id *oid,
return 0;
}
if (MAX_REVS <= ref_name_cnt) {
- warning(Q_("ignoring %s; cannot handle more than %d ref",
- "ignoring %s; cannot handle more than %d refs",
+ warning(Q_("ignoring %s; cannot handle more than %zu ref",
+ "ignoring %s; cannot handle more than %zu refs",
MAX_REVS), refname, MAX_REVS);
return 0;
}
@@ -511,15 +564,12 @@ static int rev_is_head(const char *head, const char *name)
static int show_merge_base(const struct commit_list *seen, int num_rev)
{
- int all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
- int all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
int exit_status = 1;
for (const struct commit_list *s = seen; s; s = s->next) {
struct commit *commit = s->item;
- int flags = commit->object.flags & all_mask;
- if (!(flags & UNINTERESTING) &&
- ((flags & all_revs) == all_revs)) {
+ if (!(commit->object.flags & UNINTERESTING) &&
+ has_all_rev_flags(commit, num_rev)) {
puts(oid_to_hex(&commit->object.oid));
exit_status = 0;
commit->object.flags |= UNINTERESTING;
@@ -528,17 +578,13 @@ static int show_merge_base(const struct commit_list *seen, int num_rev)
return exit_status;
}
-static int show_independent(struct commit **rev,
- int num_rev,
- unsigned int *rev_mask)
+static int show_independent(struct commit **rev, int num_rev)
{
int i;
for (i = 0; i < num_rev; i++) {
struct commit *commit = rev[i];
- unsigned int flag = rev_mask[i];
-
- if (commit->object.flags == flag)
+ if (has_only_rev_flag_bit(commit, i))
puts(oid_to_hex(&commit->object.oid));
commit->object.flags |= UNINTERESTING;
}
@@ -603,13 +649,12 @@ static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
* Otherwise, if it is a merge that is reachable from only one
* tip, it is not that interesting.
*/
- int i, flag, count;
+ int i, count;
for (i = 0; i < n; i++)
if (rev[i] == commit)
return 0;
- flag = commit->object.flags;
for (i = count = 0; i < n; i++) {
- if (flag & (1u << (i + REV_SHIFT)))
+ if (test_rev_flag_bit(commit, i))
count++;
}
if (count == 1)
@@ -648,10 +693,8 @@ int cmd_show_branch(int ac,
char *reflog_msg[MAX_REVS] = {0};
struct commit_list *seen = NULL;
struct prio_queue queue = { compare_commits_by_commit_date };
- unsigned int rev_mask[MAX_REVS];
int num_rev, i, extra = 0;
int all_heads = 0, all_remotes = 0;
- int all_mask, all_revs;
enum rev_sort_order sort_order = REV_SORT_IN_GRAPH_ORDER;
char *head;
struct object_id head_oid;
@@ -713,7 +756,8 @@ int cmd_show_branch(int ac,
const char **args_copy = NULL;
int ret;
- init_commit_name_slab(&name_slab);
+ flags_stride = (MAX_REVS + 63) / 64;
+ init_commit_rev_flags_with_stride(&rev_flags_slab, flags_stride);
repo_config(the_repository, git_show_branch_config, NULL);
@@ -779,8 +823,8 @@ int cmd_show_branch(int ac,
die(_("--reflog option needs one branch name"));
if (MAX_REVS < reflog)
- die(Q_("only %d entry can be shown at one time.",
- "only %d entries can be shown at one time.",
+ die(Q_("only %zu entry can be shown at one time.",
+ "only %zu entries can be shown at one time.",
MAX_REVS), MAX_REVS);
if (!repo_dwim_ref(the_repository, *av, strlen(*av), &oid,
&ref, 0))
@@ -870,11 +914,11 @@ int cmd_show_branch(int ac,
for (num_rev = 0; ref_name[num_rev]; num_rev++) {
struct object_id revkey;
- unsigned int flag = 1u << (num_rev + REV_SHIFT);
+ int first_seen;
if (MAX_REVS <= num_rev)
- die(Q_("cannot handle more than %d rev.",
- "cannot handle more than %d revs.",
+ die(Q_("cannot handle more than %zu rev.",
+ "cannot handle more than %zu revs.",
MAX_REVS), MAX_REVS);
if (repo_get_oid(the_repository, ref_name[num_rev], &revkey))
die(_("'%s' is not a valid ref."), ref_name[num_rev]);
@@ -885,17 +929,15 @@ int cmd_show_branch(int ac,
repo_parse_commit(the_repository, commit);
mark_seen(commit, &seen);
- /* rev#0 uses bit REV_SHIFT, rev#1 uses bit REV_SHIFT+1,
- * and so on. REV_SHIFT bits from bit 0 are used for
- * internal bookkeeping.
+ /* rev#0 uses bit 0, rev#1 uses bit 1,
+ * and so on. All bits are available for branch tracking.
*/
- commit->object.flags |= flag;
- if (commit->object.flags == flag)
+ first_seen = !has_any_rev_flags(commit);
+ or_rev_flag_bit(commit, num_rev);
+ if (first_seen)
prio_queue_put(&queue, commit);
rev[num_rev] = commit;
}
- for (i = 0; i < num_rev; i++)
- rev_mask[i] = rev[i]->object.flags;
if (0 <= extra)
join_revs(&queue, &seen, num_rev, extra);
@@ -908,7 +950,7 @@ int cmd_show_branch(int ac,
}
if (independent) {
- ret = show_independent(rev, num_rev, rev_mask);
+ ret = show_independent(rev, num_rev);
goto out;
}
@@ -958,13 +1000,9 @@ int cmd_show_branch(int ac,
if (!sha1_name && !no_name)
name_commits(seen, rev, ref_name, num_rev);
- all_mask = ((1u << (REV_SHIFT + num_rev)) - 1);
- all_revs = all_mask & ~((1u << REV_SHIFT) - 1);
-
for (struct commit_list *l = seen; l; l = l->next) {
struct commit *commit = l->item;
- int this_flag = commit->object.flags;
- int is_merge_point = ((this_flag & all_revs) == all_revs);
+ int is_merge_point = has_all_rev_flags(commit, num_rev);
shown_merge_point |= is_merge_point;
@@ -973,14 +1011,14 @@ int cmd_show_branch(int ac,
commit->parents->next);
if (topics &&
!is_merge_point &&
- (this_flag & (1u << REV_SHIFT)))
+ test_rev_flag_bit(commit, 0))
continue;
if (!sparse && is_merge &&
omit_in_dense(commit, rev, num_rev))
continue;
for (i = 0; i < num_rev; i++) {
int mark;
- if (!(this_flag & (1u << (i + REV_SHIFT))))
+ if (!test_rev_flag_bit(commit, i))
mark = ' ';
else if (is_merge)
mark = '-';
@@ -1010,6 +1048,7 @@ int cmd_show_branch(int ac,
free(reflog_msg[i]);
commit_list_free(seen);
clear_prio_queue(&queue);
+ clear_commit_rev_flags(&rev_flags_slab);
free(args_copy);
free(head);
return ret;
--
2.54.0
^ permalink raw reply related
* [PATCH] remote-curl: simplify passing of push specs
From: René Scharfe @ 2026-07-15 4:41 UTC (permalink / raw)
To: Git List
The push specs are kept in a strvec, whose array is NULL-terminated.
Pass only that to the protocol handlers, which avoids dealing with item
counts and their conversions from size_t to int, slightly simplifying
the code.
Signed-off-by: René Scharfe <l.s.r@web.de>
---
remote-curl.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 9e614c5567..2c35dd5240 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1340,10 +1340,9 @@ static void parse_get(const char *arg)
fflush(stdout);
}
-static int push_dav(int nr_spec, const char **specs)
+static int push_dav(const char **specs)
{
struct child_process child = CHILD_PROCESS_INIT;
- size_t i;
child.git_cmd = 1;
strvec_push(&child.args, "http-push");
@@ -1353,15 +1352,14 @@ static int push_dav(int nr_spec, const char **specs)
if (options.verbosity > 1)
strvec_push(&child.args, "--verbose");
strvec_push(&child.args, url.buf);
- for (i = 0; i < nr_spec; i++)
- strvec_push(&child.args, specs[i]);
+ strvec_pushv(&child.args, specs);
if (run_command(&child))
die(_("git-http-push failed"));
return 0;
}
-static int push_git(struct discovery *heads, int nr_spec, const char **specs)
+static int push_git(struct discovery *heads, const char **specs)
{
struct rpc_state rpc = RPC_STATE_INIT;
int i, err;
@@ -1400,8 +1398,8 @@ static int push_git(struct discovery *heads, int nr_spec, const char **specs)
strvec_push(&args, "--force-if-includes");
strvec_push(&args, "--stdin");
- for (i = 0; i < nr_spec; i++)
- packet_buf_write(&preamble, "%s\n", specs[i]);
+ for (; *specs; specs++)
+ packet_buf_write(&preamble, "%s\n", *specs);
packet_buf_flush(&preamble);
memset(&rpc, 0, sizeof(rpc));
@@ -1416,15 +1414,15 @@ static int push_git(struct discovery *heads, int nr_spec, const char **specs)
return err;
}
-static int push(int nr_spec, const char **specs)
+static int push(const char **specs)
{
struct discovery *heads = discover_refs("git-receive-pack", 1);
int ret;
if (heads->proto_git)
- ret = push_git(heads, nr_spec, specs);
+ ret = push_git(heads, specs);
else
- ret = push_dav(nr_spec, specs);
+ ret = push_dav(specs);
free_discovery(heads);
return ret;
}
@@ -1448,7 +1446,7 @@ static void parse_push(struct strbuf *buf)
break;
} while (1);
- ret = push(specs.nr, specs.v);
+ ret = push(specs.v);
printf("\n");
fflush(stdout);
--
2.55.0
^ permalink raw reply related
* Re: [PATCH v1] repository: move fetch_if_missing into struct repository
From: Tian Yuchen @ 2026-07-15 4:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, ps, five231003, hariom18599, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <xmqq5x2hq6eb.fsf@gitster.g>
On 7/15/26 11:27, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
>
>> The global variable 'fetch_if_missing' controls whether a missing
>> object check should prompt a lazy fetch from a promisor remote.
>> In order to continue the libification effort, move it into
>> 'struct repository' and initialize it to 1 by default to keep the
>> previous behavior.
>> ...
>> diff --git a/setup.c b/setup.c
>> index b4652651df..ce2a80ac31 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -1064,7 +1064,7 @@ static void setup_git_env_internal(struct repository *repo,
>> set_alternate_shallow_file(repo, shallow_file, 0);
>>
>> if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
>> - fetch_if_missing = 0;
>> + the_repository->fetch_if_missing = 0;
>> }
>
> Could a caller pass a "repo" that is not the_repository? In other
> words, shouldn't this be
>
> repo->fetch_if_missing = 0;
>
> instead?
Thanks, will change in the next reroll!
Regards, yuchen
^ permalink raw reply
* Re: [PATCH 6/7] odb: introduce object filters to `odb_for_each_object()`
From: Toon Claes @ 2026-07-15 5:09 UTC (permalink / raw)
To: Patrick Steinhardt, Justin Tobler; +Cc: git
In-Reply-To: <alCakwxxOc4FEEAv@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> An alternative going forward could be to perform filtering of yielded
> objects inside `odb_for_each_object()` itself so that it will filter out
> any objects that the backends themselves couldn't filter efficiently.
> But I'm not sure I want to go there as part of this series -- we only
> have a single caller anyway that iterates with a filter, and that caller
> already knows to manually filter references.
Ah, that's a valid point. I didn't think of that.
If only we were using a language that has lazy iterators ;)
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v3 7/9] pack-bitmap: introduce function to open bitmap for a single source
From: Toon Claes @ 2026-07-15 5:25 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau
In-Reply-To: <20260713-pks-odb-for-each-object-filter-v3-7-b3c65c641073@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> The function `prepare_bitmap_git()` opens the first bitmap it can find
> in any of the object sources connected to the repository. In a
> subsequent commit, the "packed" object database backend will learn to
> use bitmaps to answer object filters when enumerating objects. That
> backend operates on a single object source though, so using a bitmap
> that potentially belongs to a different source would be wrong:
>
> - The source would yield objects that are not part of the source
> itself.
>
> - The object source info would be attributed to the wrong source.
>
> - With multiple sources, each source would enumerate the same bitmap
> another time.
>
> Introduce a new function `prepare_source_bitmap_git()` that only opens
> bitmaps belonging to the given object source.
Tinies nit: this should be `prepare_bitmap_git_for_source()`
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> pack-bitmap.c | 12 ++++++++++++
> pack-bitmap.h | 2 ++
> 2 files changed, 14 insertions(+)
>
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 72c8ae3228..09ba15d26b 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -753,6 +753,18 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
> return NULL;
> }
>
> +struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source)
> +{
> + struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
> +
> + if (!open_bitmap_for_source(source, bitmap_git) &&
> + !load_bitmap(source->base.odb->repo, bitmap_git, 0))
> + return bitmap_git;
> +
> + free_bitmap_index(bitmap_git);
> + return NULL;
> +}
> +
> int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)
> {
> for (; bitmap; bitmap = bitmap->base) {
> diff --git a/pack-bitmap.h b/pack-bitmap.h
> index ae8dc491ac..9f20fb6e56 100644
> --- a/pack-bitmap.h
> +++ b/pack-bitmap.h
> @@ -9,6 +9,7 @@
> #include "string-list.h"
>
> struct commit;
> +struct odb_source_packed;
> struct repository;
> struct rev_info;
>
> @@ -68,6 +69,7 @@ struct bitmapped_pack {
>
> struct bitmap_index *prepare_bitmap_git(struct repository *r);
> struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx);
> +struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source);
>
> /*
> * Given a bitmap index, determine whether it contains the pack either directly
>
> --
> 2.55.0.313.g8d093f411d.dirty
>
>
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v3 8/9] odb: introduce object filters to `odb_for_each_object()`
From: Toon Claes @ 2026-07-15 5:27 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau
In-Reply-To: <20260713-pks-odb-for-each-object-filter-v3-8-b3c65c641073@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> The function `for_each_bitmapped_object()` can be used to iterate
> through all objects covered by a bitmap. The benefit of this function is
> that it allows the caller to efficiently handle some object filters. For
> example, this can be used to filter out objects of a specific type with
> some simple bitmap operations. But callers are currently required to
> manually wire up the use of bitmaps though, and to do so they have to
> reach into internals of a given object database source.
>
> Introduce a new `struct odb_for_each_object_options::filter` field so
> that the interface becomes generic. When set, then a backend may
> optionally use the filter to skip some objects that it would have
> otherwise yielded.
>
> Note that the respective backends are free to ignore this field if they
> cannot meaningfully optimize for a given filter, and consequently
> callers need to verify whether they actually want the returned objects.
> While annoying, we cannot easily lift this restriction anyway as the
> object filter infrastructure supports some filters that cannot be
> answered by the object database alone.
>
> An alternative might be to limit the filters to only those that _can_ be
> answered by backends. But ultimately, the filters that can be answered
> efficiently by the "packed" backend are completely disjunct from those
> that can be answered by the "loose" backend, and consequently the set of
> filters supported by all backends would be empty. Furthermore, it would
> require us to make assumptions about capabilities of future backends,
> which may be able to efficiently handle more filters than current ones.
> So in the end, this alternative would only limit us artificially.
>
> Implement the logic for the "packed" source. Note that we use the new
> function `prepare_source_bitmap_git()` to open the bitmap: as the
Also here: `prepare_bitmap_git_for_source()`
> backend operates on a single object source, we must only use bitmaps
> that belong to that specific source. Otherwise we might yield objects
> that are not part of the source at all, and with multiple sources we
> would enumerate the same bitmap once per source.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v3 9/9] builtin/cat-file: filter objects via object database
From: Toon Claes @ 2026-07-15 5:41 UTC (permalink / raw)
To: Patrick Steinhardt, git
Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau
In-Reply-To: <20260713-pks-odb-for-each-object-filter-v3-9-b3c65c641073@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> When batching all objects, git-cat-file(1) reaches into the internals of
> the object database and manually manages bitmaps to apply object
> filters. This creates coupling between the command and the internals of
> the respective backend.
>
> Refactor git-cat-file(1) to use the new object filter option when
> batching all objects. This significantly simplifies the logic and
> ensures that we don't have to reach into internals of the "files" source
> anymore.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> builtin/cat-file.c | 76 +++++-------------------------------------------------
> 1 file changed, 7 insertions(+), 69 deletions(-)
>
> diff --git a/builtin/cat-file.c b/builtin/cat-file.c
> index b4b99a73da..1458dd76d6 100644
> --- a/builtin/cat-file.c
> +++ b/builtin/cat-file.c
> @@ -20,7 +20,6 @@
> #include "userdiff.h"
> #include "oid-array.h"
> #include "packfile.h"
> -#include "pack-bitmap.h"
> #include "object-file.h"
> #include "object-name.h"
> #include "odb.h"
> @@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
> return payload->callback(oid, NULL, 0, payload->payload);
> }
>
> -static int batch_one_object_packed(const struct object_id *oid,
> - struct packed_git *pack,
> - uint32_t pos,
> - void *_payload)
> -{
> - struct for_each_object_payload *payload = _payload;
> - return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
> - payload->payload);
> -}
> -
> -static int batch_one_object_bitmapped(const struct object_id *oid,
> - enum object_type type UNUSED,
> - int flags UNUSED,
> - uint32_t hash UNUSED,
> - struct packed_git *pack,
> - off_t offset,
> - void *_payload)
> -{
> - struct for_each_object_payload *payload = _payload;
> - return payload->callback(oid, pack, offset, payload->payload);
> -}
> -
> static void batch_each_object(struct batch_options *opt,
> for_each_object_fn callback,
> unsigned flags,
> @@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
> .callback = callback,
> .payload = _payload,
> };
> + struct odb_source_info source_info;
> + struct object_info oi = {
> + .source_infop = &source_info,
> + };
> struct odb_for_each_object_options opts = {
> .flags = flags,
> + .filter = &opt->objects_filter,
Ahha, so here we pass the filter down.
> };
> - struct bitmap_index *bitmap = NULL;
> - struct odb_source *source;
> -
> - /*
> - * TODO: we still need to tap into implementation details of the object
> - * database sources. Ideally, we should extend `odb_for_each_object()`
> - * to handle object filters itself so that we can move the filtering
> - * logic into the individual sources.
> - */
> - odb_prepare_alternates(the_repository->objects);
> - for (source = the_repository->objects->sources; source; source = source->next) {
> - struct odb_source_files *files = odb_source_files_downcast(source);
> - int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
> - &payload, &opts);
> - if (ret)
> - break;
> - }
> -
> - if (opt->objects_filter.choice != LOFC_DISABLED &&
> - (bitmap = prepare_bitmap_git(the_repository)) &&
> - !for_each_bitmapped_object(bitmap, &opt->objects_filter,
> - batch_one_object_bitmapped, &payload)) {
Similar to the new code, the filter is used if possible. It is
batch_object_write() which ensures the filter is applied. That didn't
change.
> - struct packed_git *pack;
> -
> - repo_for_each_pack(the_repository, pack) {
> - if (bitmap_index_contains_pack(bitmap, pack) ||
> - open_pack_index(pack))
> - continue;
> - for_each_object_in_pack(pack, batch_one_object_packed,
> - &payload, flags);
> - }
> - } else {
> - struct odb_source_info source_info;
> - struct object_info oi = {
> - .source_infop = &source_info,
> - };
> -
> - for (source = the_repository->objects->sources; source; source = source->next) {
> - struct odb_source_files *files = odb_source_files_downcast(source);
> - int ret = odb_source_for_each_object(&files->packed->base, &oi,
> - batch_one_object_oi, &payload, &opts);
> - if (ret)
> - break;
> - }
> - }
>
> - free_bitmap_index(bitmap);
> + odb_for_each_object_ext(the_repository->objects, &oi,
> + batch_one_object_oi, &payload, &opts);
Nice to see it abstracted out like this!
> }
>
> static int batch_objects(struct batch_options *opt)
>
> --
> 2.55.0.313.g8d093f411d.dirty
>
>
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v3 0/9] odb: introduce object filters to `odb_for_each_object()`
From: Toon Claes @ 2026-07-15 5:43 UTC (permalink / raw)
To: Patrick Steinhardt, Taylor Blau
Cc: git, Justin Tobler, Junio C Hamano, Jeff King
In-Reply-To: <alXKzb-GHodV6uGj@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> On Mon, Jul 13, 2026 at 08:59:39PM -0700, Taylor Blau wrote:
>> On Mon, Jul 13, 2026 at 04:41:24PM +0200, Patrick Steinhardt wrote:
>> > Range-diff versus v2:
>> >
>> > 1: baf2adb012 = 1: 7c0dc1be0d odb/source-packed: improve lookup when enumerating objects
>> > 2: 57eecf3031 = 2: 2e5908c9c3 pack-bitmap: mark object filter as `const`
>> > -: ---------- > 3: f4d66ccfc6 pack-objects: drop unused return value from add_object_entry()
>> > 3: 92dd6a6f6e = 4: af475654b8 pack-bitmap: allow aborting iteration of bitmapped objects
>> > 4: 92fe41577d = 5: 6ca42587c9 pack-bitmap: iterate object sources when opening bitmaps
>> > 5: e5d59959e3 = 6: f62c3bbc81 pack-bitmap: drop `_1` suffix from functions that open bitmaps
>> > 6: ab3547ac2b = 7: b2d25b6e9b pack-bitmap: introduce function to open bitmap for a single source
>> > 7: 026f21f522 = 8: a5bf309bec odb: introduce object filters to `odb_for_each_object()`
>> > 8: 534b25c817 = 9: 600b15a907 builtin/cat-file: filter objects via object database
>>
>> Thanks, this version looks good to me.
I only posted a nit about function name in the commit messages 7 & 8
isn't correct, but other than that I'm happy with these patches too.
--
Cheers,
Toon
^ permalink raw reply
* [PATCH] diff: ignore unmerged paths outside prefix with --relative --cached
From: Jeff King @ 2026-07-15 6:05 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
A diff using --relative ignores entries outside the current directory.
This results in a segfault when we try to process an unmerged entry
that's outside of our prefix, since we end up with a NULL diff_filepair
and use it without checking that it's valid.
I think this bug goes back to 76399c0195 (diff.c: return filepair from
diff_unmerge(), 2011-04-22). Prior to that, diff_unmerge() knew to skip
entries outside of our prefix, due to cd676a5136 (diff --relative:
output paths as relative to the current subdirectory, 2008-02-12). Back
then the caller didn't care that we hadn't added anything to the queue.
In 76399c0195 that changed; we now returned the pair (or NULL), and the
caller in do_oneway_diff() was then called fill_filespec() itself. And
it does so without checking for NULL, causing a segfault.
The obvious fix is to skip the fill_filespec() call (after which we just
return), which this patch does.
There's another call to diff_unmerge() in run_diff_files(). That case
was already fixed by 8174627b3d (diff-lib: ignore paths that are outside
$cwd if --relative asked, 2021-08-22), but of course it didn't help us
for --cached.
That commit also claims that checking the result of diff_unmerge() is
not enough, as we'd want other code paths to skip the entry, too (even
if they wouldn't segfault). But as far as I can tell, that is not true
for --cached. We eventually end up in diff_queue_addremove() or in
diff_queue_change(), both of which know to return early when we're
outside of the prefix.
Arguably we could be checking at the top of oneway_diff() whether the
path is interesting at all. That would not only avoid this code path
entirely, but would also possibly save a small amount of work. But since
everything else appears to work OK, I went for the smallest fix here to
avoid any regression.
Specifically, a comment in oneway_diff() claims we're supposed to
advance o->pos, which we might fail to do if we return early. Though
that "advance" seems to have gone away in da165f470e (unpack-trees.c:
prepare for looking ahead in the index, 2010-01-07), so it is possible
the comment is simply out of date. We can explore that separately;
checking for a NULL return from diff_unmerge() seems like a sensible
thing to do regardless.
We can piggy-back on the tests added by 8174627b3d; we're just checking
the --cached variant.
Signed-off-by: Jeff King <peff@peff.net>
---
+cc Junio, as you may have some wisdom on that further exploration.
diff-lib.c | 2 +-
t/t4045-diff-relative.sh | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/diff-lib.c b/diff-lib.c
index ae91027a02..a23119b852 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -467,7 +467,7 @@ static void do_oneway_diff(struct unpack_trees_options *o,
if (cached && idx && ce_stage(idx)) {
struct diff_filepair *pair;
pair = diff_unmerge(&revs->diffopt, idx->name);
- if (tree)
+ if (pair && tree)
fill_filespec(pair->one, &tree->oid, 1,
tree->ce_mode);
return;
diff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh
index 2c8493fe66..167be0bdcc 100755
--- a/t/t4045-diff-relative.sh
+++ b/t/t4045-diff-relative.sh
@@ -245,4 +245,13 @@ test_expect_failure 'diff --relative with change in subdir' '
test_cmp expected out
'
+test_expect_success 'diff --relative --cached with change in subdir' '
+ git switch br3 &&
+ test_when_finished "git merge --abort" &&
+ test_must_fail git merge sub1 &&
+ echo file0 >expected &&
+ git -C subdir diff --relative --name-only --cached >out &&
+ test_cmp expected out
+'
+
test_done
--
2.55.0.612.g68473ef936
^ permalink raw reply related
* Re: [PATCH v5 4/4] environment: move has_symlinks into repo_config_values
From: Christian Couder @ 2026-07-15 6:21 UTC (permalink / raw)
To: Tian Yuchen; +Cc: git, ps, cirnovskyv, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260715035501.48271-5-cat@malon.dev>
On Wed, Jul 15, 2026 at 5:55 AM Tian Yuchen <cat@malon.dev> wrote:
>
> Move the global 'has_symlinks' configuration into the
> repository-specific 'repo_config_values' struct.
>
> To ensure code readability, the getter function
> 'repo_has_symlinks()' has been introduced. Callers access
> this configuration by passing in 'repo' when possible,
> and explicitly fall back to 'the_repository' the rest
> of the time.
>
> Note:
> To support early platform-specific (MinGW) overrides
> before repository initialization, a global variable
> 'default_has_symlinks' fallback is introduced as a fallback
It seems a bit redundant to use "fallback" twice in the above sentence.
> in environment.h. The *writer* in compat/mingw.c can only
> access this variable.
Otherwise this series looks good to me.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/9] odb: introduce object filters to `odb_for_each_object()`
From: Patrick Steinhardt @ 2026-07-15 6:21 UTC (permalink / raw)
To: Toon Claes; +Cc: Taylor Blau, git, Justin Tobler, Junio C Hamano, Jeff King
In-Reply-To: <87wluwlsdf.fsf@emacs.iotcl.com>
On Wed, Jul 15, 2026 at 07:43:56AM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > On Mon, Jul 13, 2026 at 08:59:39PM -0700, Taylor Blau wrote:
> >> On Mon, Jul 13, 2026 at 04:41:24PM +0200, Patrick Steinhardt wrote:
> >> > Range-diff versus v2:
> >> >
> >> > 1: baf2adb012 = 1: 7c0dc1be0d odb/source-packed: improve lookup when enumerating objects
> >> > 2: 57eecf3031 = 2: 2e5908c9c3 pack-bitmap: mark object filter as `const`
> >> > -: ---------- > 3: f4d66ccfc6 pack-objects: drop unused return value from add_object_entry()
> >> > 3: 92dd6a6f6e = 4: af475654b8 pack-bitmap: allow aborting iteration of bitmapped objects
> >> > 4: 92fe41577d = 5: 6ca42587c9 pack-bitmap: iterate object sources when opening bitmaps
> >> > 5: e5d59959e3 = 6: f62c3bbc81 pack-bitmap: drop `_1` suffix from functions that open bitmaps
> >> > 6: ab3547ac2b = 7: b2d25b6e9b pack-bitmap: introduce function to open bitmap for a single source
> >> > 7: 026f21f522 = 8: a5bf309bec odb: introduce object filters to `odb_for_each_object()`
> >> > 8: 534b25c817 = 9: 600b15a907 builtin/cat-file: filter objects via object database
> >>
> >> Thanks, this version looks good to me.
>
> I only posted a nit about function name in the commit messages 7 & 8
> isn't correct, but other than that I'm happy with these patches too.
Thanks for your review! I'll send one more iteration that fixes these
nits.
Patrick
^ permalink raw reply
* Re: [PATCH v3 7/9] pack-bitmap: introduce function to open bitmap for a single source
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau
In-Reply-To: <875x2gn7rx.fsf@emacs.iotcl.com>
On Wed, Jul 15, 2026 at 07:25:54AM +0200, Toon Claes wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > The function `prepare_bitmap_git()` opens the first bitmap it can find
> > in any of the object sources connected to the repository. In a
> > subsequent commit, the "packed" object database backend will learn to
> > use bitmaps to answer object filters when enumerating objects. That
> > backend operates on a single object source though, so using a bitmap
> > that potentially belongs to a different source would be wrong:
> >
> > - The source would yield objects that are not part of the source
> > itself.
> >
> > - The object source info would be attributed to the wrong source.
> >
> > - With multiple sources, each source would enumerate the same bitmap
> > another time.
> >
> > Introduce a new function `prepare_source_bitmap_git()` that only opens
> > bitmaps belonging to the given object source.
>
> Tinies nit: this should be `prepare_bitmap_git_for_source()`
Indeed it should be, good catch. Shows that I've been iterating on the
name a bit :)
Patrick
^ permalink raw reply
* [PATCH v4 0/9] odb: introduce object filters to `odb_for_each_object()`
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260709-pks-odb-for-each-object-filter-v1-0-82fe014b12b3@pks.im>
Hi,
this patch series introduces object filters to `odb_for_each_object()`.
The intent of this is to make `git cat-file --batch-all-objects` work
with pluggable object databases. Right now it doesn't because it reaches
into internals of the "packed" backend to efficiently handle bitmapped
objects.
The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
2026-07-06) with ps/odb-drop-whence at 8a7ad23e11 (odb: document object
info fields, 2026-07-02) merged into it.
Changes in v4:
- Fix references to an old function name in commit messages.
- Link to v3: https://patch.msgid.link/20260713-pks-odb-for-each-object-filter-v3-0-b3c65c641073@pks.im
Changes in v3:
- Weave Peff's patch into the patch series.
- Link to v2: https://patch.msgid.link/20260710-pks-odb-for-each-object-filter-v2-0-3710a9cc165a@pks.im
Changes in v2:
- Add another patch to drop the `_1()` prefixes that aren't required
anymore.
- Change the approach in `open_bitmap_for_source()` to also use a
`found` boolean instead of a confusing integer.
- Add some more explanations to commit messages.
- Link to v1: https://patch.msgid.link/20260709-pks-odb-for-each-object-filter-v1-0-82fe014b12b3@pks.im
Thanks!
Patrick
---
Jeff King (1):
pack-objects: drop unused return value from add_object_entry()
Patrick Steinhardt (8):
odb/source-packed: improve lookup when enumerating objects
pack-bitmap: mark object filter as `const`
pack-bitmap: allow aborting iteration of bitmapped objects
pack-bitmap: iterate object sources when opening bitmaps
pack-bitmap: drop `_1` suffix from functions that open bitmaps
pack-bitmap: introduce function to open bitmap for a single source
odb: introduce object filters to `odb_for_each_object()`
builtin/cat-file: filter objects via object database
builtin/cat-file.c | 76 +++--------------------------
builtin/pack-objects.c | 11 ++---
builtin/rev-list.c | 2 +-
odb.h | 12 +++++
odb/source-packed.c | 77 ++++++++++++++++++++++++++---
pack-bitmap.c | 129 +++++++++++++++++++++++++++----------------------
pack-bitmap.h | 10 +++-
7 files changed, 175 insertions(+), 142 deletions(-)
Range-diff versus v3:
1: 2d4bc229ac = 1: 30cf75ce12 odb/source-packed: improve lookup when enumerating objects
2: dfa942f6da = 2: 735eb8ad5c pack-bitmap: mark object filter as `const`
3: 9bfba42b27 = 3: a1f32c5ea6 pack-objects: drop unused return value from add_object_entry()
4: 405c77bad3 = 4: acb0aad580 pack-bitmap: allow aborting iteration of bitmapped objects
5: 70e68f10aa = 5: 199db72f4c pack-bitmap: iterate object sources when opening bitmaps
6: 08ba2c1db3 = 6: 69d5882b57 pack-bitmap: drop `_1` suffix from functions that open bitmaps
7: d410cfa4e2 ! 7: a1eabafebd pack-bitmap: introduce function to open bitmap for a single source
@@ Commit message
- With multiple sources, each source would enumerate the same bitmap
another time.
- Introduce a new function `prepare_source_bitmap_git()` that only opens
- bitmaps belonging to the given object source.
+ Introduce a new function `prepare_bitmap_git_for_source()` that only
+ opens bitmaps belonging to the given object source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
8: e0df97e318 ! 8: 6ef9885006 odb: introduce object filters to `odb_for_each_object()`
@@ Commit message
So in the end, this alternative would only limit us artificially.
Implement the logic for the "packed" source. Note that we use the new
- function `prepare_source_bitmap_git()` to open the bitmap: as the
+ function `prepare_bitmap_git_for_source()` to open the bitmap: as the
backend operates on a single object source, we must only use bitmaps
that belong to that specific source. Otherwise we might yield objects
that are not part of the source at all, and with multiple sources we
9: e866b1f4b8 = 9: 104da65906 builtin/cat-file: filter objects via object database
---
base-commit: 3c8e2790f2ce15e8b5d4b4e6ced711b12649f32a
change-id: 20260708-pks-odb-for-each-object-filter-13286fa3523d
^ permalink raw reply
* [PATCH v4 1/9] odb/source-packed: improve lookup when enumerating objects
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
When iterating through objects of a packed source that have a specific
prefix we do so via two different methods:
- When a multi-pack index is available we use that one to efficiently
loop through all objects.
- We then loop through all packfiles that aren't covered by a
multi-pack index.
Regardless of which mechanism we use, we then iterate through all the
objects indexed by the respective data structure. Curiously though,
while we use the indices for enumerating the objects, we completely
ignore it for the actual object lookup. Instead, we call into the
generic `odb_source_read_object_info()` function, which will itself
consult the indices to figure out where the object in question even
lives.
This has two consequences:
- It's inefficient, as we basically have to figure out the position of
the object a second time.
- It's subtly wrong, as it may now happen that a specific object will
be looked up via a different pack in case it exists multiple times.
This is unlikely to have any real-world consequences, but it's still
the wrong thing to do.
Fix the issue by using `packed_object_info()` directly. While at it,
rename the `store` variable to `source`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb/source-packed.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 0edea5356d..9cfa02b7a2 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -143,7 +143,7 @@ static bool should_exclude_pack(struct packed_git *p, enum odb_for_each_object_f
}
static int for_each_prefixed_object_in_midx(
- struct odb_source_packed *store,
+ struct odb_source_packed *source,
struct multi_pack_index *m,
const struct odb_for_each_object_options *opts,
struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -170,6 +170,7 @@ static int for_each_prefixed_object_in_midx(
*/
for (i = first; i < num; i++) {
const struct object_id *current = NULL;
+ struct packed_git *pack;
struct object_id oid;
current = nth_midxed_object_oid(&oid, m, i);
@@ -177,9 +178,8 @@ static int for_each_prefixed_object_in_midx(
if (!match_hash(len, opts->prefix->hash, current->hash))
break;
- if (opts->flags) {
+ if (opts->flags || data->request) {
uint32_t pack_id = nth_midxed_pack_int_id(m, i);
- struct packed_git *pack;
if (prepare_midx_pack(m, pack_id)) {
pack_errors = true;
@@ -193,9 +193,9 @@ static int for_each_prefixed_object_in_midx(
if (data->request) {
struct object_info oi = *data->request;
+ off_t offset = nth_midxed_offset(m, i);
- ret = odb_source_read_object_info(&store->base, current,
- &oi, 0);
+ ret = packed_object_info(source, pack, offset, &oi);
if (ret)
goto out;
@@ -219,7 +219,7 @@ static int for_each_prefixed_object_in_midx(
}
static int for_each_prefixed_object_in_pack(
- struct odb_source_packed *store,
+ struct odb_source_packed *source,
struct packed_git *p,
const struct odb_for_each_object_options *opts,
struct odb_source_packed_for_each_object_wrapper_data *data)
@@ -246,8 +246,9 @@ static int for_each_prefixed_object_in_pack(
if (data->request) {
struct object_info oi = *data->request;
+ off_t offset = nth_packed_object_offset(p, i);
- ret = odb_source_read_object_info(&store->base, &oid, &oi, 0);
+ ret = packed_object_info(source, p, offset, &oi);
if (ret)
goto out;
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH v4 2/9] pack-bitmap: mark object filter as `const`
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
The function `for_each_bitmapped_object()` accepts an optional object
filter. This filter is never modified by the function, but is not
declared as `const`. Fix this.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 6 +++---
pack-bitmap.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 35774b6f0c..a47c231632 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1976,7 +1976,7 @@ static void filter_bitmap_object_type(struct bitmap_index *bitmap_git,
static int filter_bitmap(struct bitmap_index *bitmap_git,
struct object_list *tip_objects,
struct bitmap *to_filter,
- struct list_objects_filter_options *filter)
+ const struct list_objects_filter_options *filter)
{
if (!filter || filter->choice == LOFC_DISABLED)
return 0;
@@ -2027,7 +2027,7 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return -1;
}
-static int can_filter_bitmap(struct list_objects_filter_options *filter)
+static int can_filter_bitmap(const struct list_objects_filter_options *filter)
{
return !filter_bitmap(NULL, NULL, NULL, filter);
}
@@ -2058,7 +2058,7 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
}
int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
- struct list_objects_filter_options *filter,
+ const struct list_objects_filter_options *filter,
show_reachable_fn show_reach,
void *payload)
{
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 19a8655457..47935eb24e 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -96,7 +96,7 @@ struct list_objects_filter_options;
* not supported, `0` otherwise.
*/
int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
- struct list_objects_filter_options *filter,
+ const struct list_objects_filter_options *filter,
show_reachable_fn show_reach,
void *payload);
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH v4 3/9] pack-objects: drop unused return value from add_object_entry()
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
From: Jeff King <peff@peff.net>
This function returns 0/1 to its caller to tell them whether we actually
added a new entry (or if we considered it redundant). But nobody has
relied on that behavior since 5379a5c5ee (Thin pack generation:
optimization., 2006-04-05).
The extra return does not hurt much, but it is a bit confusing. We have
a sister function, add_object_entry_from_bitmap(), which has the same
return value semantics. That function is about to change to always return
0 (not void, because it must conform to a callback function interface).
So with that change, we'd have two related functions which both return
an "int" but with different semantics.
Let's drop the unused "int" return from add_object_entry() entirely,
which makes it more clear that the two functions have diverged.
Signed-off-by: Jeff King <peff@peff.net>
[ps: slightly massaged the commit message]
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/pack-objects.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ea5eab4cf8..188c4f6d4b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1867,8 +1867,8 @@ static const char no_closure_warning[] = N_(
"disabling bitmap writing, as some objects are not being packed"
);
-static int add_object_entry(const struct object_id *oid, enum object_type type,
- const char *name, int exclude)
+static void add_object_entry(const struct object_id *oid, enum object_type type,
+ const char *name, int exclude)
{
struct packed_git *found_pack = NULL;
off_t found_offset = 0;
@@ -1876,7 +1876,7 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
display_progress(progress_state, ++nr_seen);
if (have_duplicate_entry(oid, exclude))
- return 0;
+ return;
if (!want_object_in_pack(oid, exclude, &found_pack, &found_offset)) {
/* The pack is missing an object, so it will not have closure */
@@ -1885,13 +1885,12 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,
warning(_(no_closure_warning));
write_bitmap_index = 0;
}
- return 0;
+ return;
}
create_object_entry(oid, type, pack_name_hash_fn(name),
exclude, name && no_try_delta(name),
found_pack, found_offset);
- return 1;
}
static int add_object_entry_from_bitmap(const struct object_id *oid,
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH v4 4/9] pack-bitmap: allow aborting iteration of bitmapped objects
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
In a subsequent commit we'll lift iteration of bitmapped objects into
the "packed" backend and make it accessible via `odb_for_each_object()`.
The calling convention for that function is that the callback may return
a non-zero exit code, and if so we'll abort iteration. This is currently
impossible to realize though, as `for_each_bitmapped_object()` will
ignore any return value and just churn through all objects completely.
This doesn't matter to the callers of `for_each_bitmapped_object()`, as
there's only one of them in git-cat-file(1), and the callbacks we pass
always return zero. But once we move the logic into the generic
infrastructure it becomes a latent bug waiting to happen.
Refactor the code so that the return value of the `show_reach` callback
is not ignored anymore. Instead, returning a non-zero value will cause
us to abort iteration in both `show_objects_for_type()` and in
`for_each_bitmapped_object()`.
Note though that there's a second user of `show_objects_for_type()` with
`traverse_bitmap_commit_list()`, and that function does indeed invoke
callbacks that may return non-zero. This non-zero return value never had
any effect at all though, and the callbacks that return non-zero values
are only ever invoked via `traverse_bitmap_commit_list()`. Consequently,
we adapt them to always return 0.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/pack-objects.c | 2 +-
builtin/rev-list.c | 2 +-
pack-bitmap.c | 31 +++++++++++++++++++++----------
pack-bitmap.h | 3 ++-
4 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 188c4f6d4b..3673b14b89 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1908,7 +1908,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
return 0;
create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
- return 1;
+ return 0;
}
struct pbase_tree_cache {
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709..02818b81c6 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -486,7 +486,7 @@ static int show_object_fast(
void *payload UNUSED)
{
fprintf(stdout, "%s\n", oid_to_hex(oid));
- return 1;
+ return 0;
}
static void print_disk_usage(off_t size)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index a47c231632..eda38a5433 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1695,7 +1695,7 @@ static void init_type_iterator(struct ewah_or_iterator *it,
}
}
-static void show_objects_for_type(
+static int show_objects_for_type(
struct bitmap_index *bitmap_git,
struct bitmap *objects,
enum object_type object_type,
@@ -1704,6 +1704,7 @@ static void show_objects_for_type(
{
size_t i = 0;
uint32_t offset;
+ int ret;
struct ewah_or_iterator it;
eword_t filter;
@@ -1749,11 +1750,17 @@ static void show_objects_for_type(
hash = bitmap_name_hash(bitmap_git, index_pos);
- show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+ ret = show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+ if (ret)
+ goto out;
}
}
+ ret = 0;
+
+out:
ewah_or_iterator_release(&it);
+ return ret;
}
static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
@@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
show_reachable_fn show_reach,
void *payload)
{
+ const enum object_type types[] = {
+ OBJ_COMMIT,
+ OBJ_TREE,
+ OBJ_BLOB,
+ OBJ_TAG,
+ };
struct bitmap *filtered_bitmap = NULL;
uint32_t objects_nr;
size_t full_word_count;
@@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
goto out;
}
- show_objects_for_type(bitmap_git, filtered_bitmap,
- OBJ_COMMIT, show_reach, payload);
- show_objects_for_type(bitmap_git, filtered_bitmap,
- OBJ_TREE, show_reach, payload);
- show_objects_for_type(bitmap_git, filtered_bitmap,
- OBJ_BLOB, show_reach, payload);
- show_objects_for_type(bitmap_git, filtered_bitmap,
- OBJ_TAG, show_reach, payload);
+ for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
+ ret = show_objects_for_type(bitmap_git, filtered_bitmap,
+ types[i], show_reach, payload);
+ if (ret)
+ goto out;
+ }
ret = 0;
out:
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 47935eb24e..ae8dc491ac 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -93,7 +93,8 @@ struct list_objects_filter_options;
/*
* Filter bitmapped objects and iterate through all resulting objects,
* executing `show_reach` for each of them. Returns `-1` in case the filter is
- * not supported, `0` otherwise.
+ * not supported, `0` otherwise. Aborts iteration and bubbles up the return
+ * value in case `show_reach()` returns non-zero.
*/
int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
const struct list_objects_filter_options *filter,
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH v4 5/9] pack-bitmap: iterate object sources when opening bitmaps
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
When opening a bitmap for a repository we perform two steps:
- We first look for a multi-pack index bitmap in any of the object
sources connected to the repository.
- We then look for a packfile bitmap in any of the packfiles of any of
the object sources.
Both of these steps thus iterate through object sources themselves, one
via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This
layout makes it hard to introduce a way to open the bitmap of one
specific object source, which is functionality that we'll require in a
subsequent commit.
Reverse the loop so that we instead loop through all sources in the
outer loop, and then for each source we try to load its bitmap via
either the multi-pack index or via a packfile.
Note that this changes the precedence of bitmaps in one specific edge
case: when an earlier object source only has a packfile bitmap, but a
later source has a multi-pack index bitmap, we now pick the packfile
bitmap of the earlier source. Previously, a multi-pack index bitmap from
any source would have taken precedence over all packfile bitmaps. Given
that object sources are ordered such that the local source comes first,
this arguably is an improvement, as we now prefer local bitmaps over
bitmaps in alternates. Furthermore, we already warn about repositories
that have multiple bitmaps, so this setup is broken and thus arguably
not worth worrying about too much.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 69 +++++++++++++++++++++++++++--------------------------------
1 file changed, 31 insertions(+), 38 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index eda38a5433..e32795a595 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -680,60 +680,53 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git,
return 0;
}
-static int open_pack_bitmap(struct repository *r,
- struct bitmap_index *bitmap_git)
+static int open_bitmap_for_source(struct odb_source_packed *source,
+ struct bitmap_index *bitmap_git)
{
- struct packed_git *p;
- int ret = -1;
+ struct multi_pack_index *midx = get_multi_pack_index(source);
+ struct packfile_list_entry *e;
+ bool found = false;
- repo_for_each_pack(r, p) {
- if (open_pack_bitmap_1(bitmap_git, p) == 0) {
- ret = 0;
- /*
- * The only reason to keep looking is to report
- * duplicates.
- */
- if (!trace2_is_enabled())
- break;
- }
+ if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+ found = true;
+
+ for (e = packfile_store_get_packs(source); e; e = e->next) {
+ /*
+ * When tracing is enabled we want to keep looking to report
+ * duplicates even if we have already found a bitmap.
+ */
+ if (found && !trace2_is_enabled())
+ break;
+
+ if (!open_pack_bitmap_1(bitmap_git, e->pack))
+ found = true;
}
- return ret;
+ return found ? 0 : -1;
}
-static int open_midx_bitmap(struct repository *r,
- struct bitmap_index *bitmap_git)
+static int open_bitmap(struct repository *r,
+ struct bitmap_index *bitmap_git)
{
struct odb_source *source;
- int ret = -1;
+ bool found = false;
assert(!bitmap_git->map);
odb_prepare_alternates(r->objects);
for (source = r->objects->sources; source; source = source->next) {
struct odb_source_files *files = odb_source_files_downcast(source);
- struct multi_pack_index *midx = get_multi_pack_index(files->packed);
- if (midx && !open_midx_bitmap_1(bitmap_git, midx))
- ret = 0;
- }
- return ret;
-}
-
-static int open_bitmap(struct repository *r,
- struct bitmap_index *bitmap_git)
-{
- int found;
- assert(!bitmap_git->map);
+ if (!open_bitmap_for_source(files->packed, bitmap_git))
+ found = true;
- found = !open_midx_bitmap(r, bitmap_git);
-
- /*
- * these will all be skipped if we opened a midx bitmap; but run it
- * anyway if tracing is enabled to report the duplicates
- */
- if (!found || trace2_is_enabled())
- found |= !open_pack_bitmap(r, bitmap_git);
+ /*
+ * The only reason to keep looking after having found a bitmap
+ * is to report duplicates.
+ */
+ if (found && !trace2_is_enabled())
+ break;
+ }
return found ? 0 : -1;
}
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH v4 6/9] pack-bitmap: drop `_1` suffix from functions that open bitmaps
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
In the preceding commit we've refactored how we open bitmaps. As part of
the refactoring we have consolidated `open_pack_bitmap()` as well as
`open_midx_bitmap()` into `open_bitmap_for_source()`. Consequently, we
only have their `open_pack_bitmap_1()` and `open_midx_bitmap_1()`
variants left over, where the `_1` suffix doesn't really make much sense
anymore.
Drop the suffix.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index e32795a595..72c8ae3228 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -460,8 +460,8 @@ char *pack_bitmap_filename(struct packed_git *p)
return xstrfmt("%.*s.bitmap", (int)len, p->pack_name);
}
-static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
- struct multi_pack_index *midx)
+static int open_midx_bitmap(struct bitmap_index *bitmap_git,
+ struct multi_pack_index *midx)
{
struct stat st;
char *bitmap_name = midx_bitmap_filename(midx);
@@ -539,7 +539,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
return -1;
}
-static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git *packfile)
+static int open_pack_bitmap(struct bitmap_index *bitmap_git, struct packed_git *packfile)
{
int fd;
struct stat st;
@@ -603,7 +603,7 @@ static int load_reverse_index(struct repository *r, struct bitmap_index *bitmap_
/*
* The multi-pack-index's .rev file is already loaded via
- * open_pack_bitmap_1().
+ * open_pack_bitmap().
*
* But we still need to open the individual pack .rev files,
* since we will need to make use of them in pack-objects.
@@ -687,7 +687,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source,
struct packfile_list_entry *e;
bool found = false;
- if (midx && !open_midx_bitmap_1(bitmap_git, midx))
+ if (midx && !open_midx_bitmap(bitmap_git, midx))
found = true;
for (e = packfile_store_get_packs(source); e; e = e->next) {
@@ -698,7 +698,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source,
if (found && !trace2_is_enabled())
break;
- if (!open_pack_bitmap_1(bitmap_git, e->pack))
+ if (!open_pack_bitmap(bitmap_git, e->pack))
found = true;
}
@@ -746,7 +746,7 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
{
struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
- if (!open_midx_bitmap_1(bitmap_git, midx))
+ if (!open_midx_bitmap(bitmap_git, midx))
return bitmap_git;
free_bitmap_index(bitmap_git);
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH v4 7/9] pack-bitmap: introduce function to open bitmap for a single source
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
The function `prepare_bitmap_git()` opens the first bitmap it can find
in any of the object sources connected to the repository. In a
subsequent commit, the "packed" object database backend will learn to
use bitmaps to answer object filters when enumerating objects. That
backend operates on a single object source though, so using a bitmap
that potentially belongs to a different source would be wrong:
- The source would yield objects that are not part of the source
itself.
- The object source info would be attributed to the wrong source.
- With multiple sources, each source would enumerate the same bitmap
another time.
Introduce a new function `prepare_bitmap_git_for_source()` that only
opens bitmaps belonging to the given object source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
pack-bitmap.c | 12 ++++++++++++
pack-bitmap.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 72c8ae3228..09ba15d26b 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -753,6 +753,18 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx)
return NULL;
}
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source)
+{
+ struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
+
+ if (!open_bitmap_for_source(source, bitmap_git) &&
+ !load_bitmap(source->base.odb->repo, bitmap_git, 0))
+ return bitmap_git;
+
+ free_bitmap_index(bitmap_git);
+ return NULL;
+}
+
int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack)
{
for (; bitmap; bitmap = bitmap->base) {
diff --git a/pack-bitmap.h b/pack-bitmap.h
index ae8dc491ac..9f20fb6e56 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -9,6 +9,7 @@
#include "string-list.h"
struct commit;
+struct odb_source_packed;
struct repository;
struct rev_info;
@@ -68,6 +69,7 @@ struct bitmapped_pack {
struct bitmap_index *prepare_bitmap_git(struct repository *r);
struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx);
+struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source);
/*
* Given a bitmap index, determine whether it contains the pack either directly
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH v4 8/9] odb: introduce object filters to `odb_for_each_object()`
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
The function `for_each_bitmapped_object()` can be used to iterate
through all objects covered by a bitmap. The benefit of this function is
that it allows the caller to efficiently handle some object filters. For
example, this can be used to filter out objects of a specific type with
some simple bitmap operations. But callers are currently required to
manually wire up the use of bitmaps though, and to do so they have to
reach into internals of a given object database source.
Introduce a new `struct odb_for_each_object_options::filter` field so
that the interface becomes generic. When set, then a backend may
optionally use the filter to skip some objects that it would have
otherwise yielded.
Note that the respective backends are free to ignore this field if they
cannot meaningfully optimize for a given filter, and consequently
callers need to verify whether they actually want the returned objects.
While annoying, we cannot easily lift this restriction anyway as the
object filter infrastructure supports some filters that cannot be
answered by the object database alone.
An alternative might be to limit the filters to only those that _can_ be
answered by backends. But ultimately, the filters that can be answered
efficiently by the "packed" backend are completely disjunct from those
that can be answered by the "loose" backend, and consequently the set of
filters supported by all backends would be empty. Furthermore, it would
require us to make assumptions about capabilities of future backends,
which may be able to efficiently handle more filters than current ones.
So in the end, this alternative would only limit us artificially.
Implement the logic for the "packed" source. Note that we use the new
function `prepare_bitmap_git_for_source()` to open the bitmap: as the
backend operates on a single object source, we must only use bitmaps
that belong to that specific source. Otherwise we might yield objects
that are not part of the source at all, and with multiple sources we
would enumerate the same bitmap once per source.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
odb.h | 12 +++++++++++
odb/source-packed.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
pack-bitmap.c | 3 +--
pack-bitmap.h | 3 +++
4 files changed, 78 insertions(+), 2 deletions(-)
diff --git a/odb.h b/odb.h
index a1e222f605..67d0b34942 100644
--- a/odb.h
+++ b/odb.h
@@ -8,6 +8,7 @@
#include "thread-utils.h"
struct cached_object_entry;
+struct list_objects_filter_options;
struct odb_source_inmemory;
struct packed_git;
struct repository;
@@ -490,6 +491,17 @@ struct odb_for_each_object_options {
*/
const struct object_id *prefix;
size_t prefix_hex_len;
+
+ /*
+ * Optional object filter that allows backends to skip yielding
+ * objects that are excluded by the filter as an optimization. The
+ * filter is a best-effort hint: backends may use it to skip
+ * excluded objects (e.g. by consulting a reachability bitmap), but
+ * are also free to ignore it entirely and yield every object. As a
+ * consequence, callers must re-apply the filter on yielded objects
+ * if they require strict filtering semantics.
+ */
+ const struct list_objects_filter_options *filter;
};
/*
diff --git a/odb/source-packed.c b/odb/source-packed.c
index 9cfa02b7a2..4777395053 100644
--- a/odb/source-packed.c
+++ b/odb/source-packed.c
@@ -3,11 +3,13 @@
#include "chdir-notify.h"
#include "dir.h"
#include "git-zlib.h"
+#include "list-objects-filter-options.h"
#include "mergesort.h"
#include "midx.h"
#include "odb/source-packed.h"
#include "odb/streaming.h"
#include "packfile.h"
+#include "pack-bitmap.h"
static int find_pack_entry(struct odb_source_packed *store,
const struct object_id *oid,
@@ -315,6 +317,37 @@ static int odb_source_packed_for_each_prefixed_object(
return ret;
}
+struct bitmapped_for_each_object_data {
+ struct odb_source_packed *packed;
+ const struct object_info *request;
+ const struct odb_for_each_object_options *opts;
+ odb_for_each_object_cb cb;
+ void *cb_data;
+};
+
+static int bitmapped_for_each_object(const struct object_id *oid,
+ enum object_type type UNUSED,
+ int flags UNUSED,
+ uint32_t hash UNUSED,
+ struct packed_git *pack,
+ off_t offset,
+ void *cb_data)
+{
+ struct bitmapped_for_each_object_data *data = cb_data;
+
+ if (should_exclude_pack(pack, data->opts->flags))
+ return 0;
+
+ if (data->request) {
+ struct object_info oi = *data->request;
+ if (packed_object_info(data->packed, pack, offset, &oi) < 0)
+ return -1;
+ return data->cb(oid, &oi, data->cb_data);
+ }
+
+ return data->cb(oid, NULL, data->cb_data);
+}
+
static int odb_source_packed_for_each_object(struct odb_source *source,
const struct object_info *request,
odb_for_each_object_cb cb,
@@ -328,12 +361,33 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
.cb = cb,
.cb_data = cb_data,
};
+ struct bitmap_index *bitmap = NULL;
struct packfile_list_entry *e;
int pack_errors = 0, ret;
if (opts->prefix)
return odb_source_packed_for_each_prefixed_object(packed, opts, &data);
+ if (opts->filter &&
+ opts->filter->choice != LOFC_DISABLED &&
+ can_filter_bitmap(opts->filter))
+ bitmap = prepare_bitmap_git_for_source(packed);
+ if (bitmap) {
+ struct bitmapped_for_each_object_data bitmap_data = {
+ .packed = packed,
+ .request = request,
+ .opts = opts,
+ .cb = cb,
+ .cb_data = cb_data,
+ };
+
+ ret = for_each_bitmapped_object(bitmap, opts->filter,
+ bitmapped_for_each_object,
+ &bitmap_data);
+ if (ret)
+ goto out;
+ }
+
packed->skip_mru_updates = true;
for (e = packfile_store_get_packs(packed); e; e = e->next) {
@@ -342,6 +396,13 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
if (should_exclude_pack(p, opts->flags))
continue;
+ /*
+ * Objects covered by the bitmap have already been yielded
+ * above; skip them here to avoid duplicates.
+ */
+ if (bitmap && bitmap_index_contains_pack(bitmap, p))
+ continue;
+
if (open_pack_index(p)) {
pack_errors = 1;
continue;
@@ -357,6 +418,7 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
out:
packed->skip_mru_updates = false;
+ free_bitmap_index(bitmap);
if (!ret && pack_errors)
ret = -1;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 09ba15d26b..f55a0859ea 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2039,12 +2039,11 @@ static int filter_bitmap(struct bitmap_index *bitmap_git,
return -1;
}
-static int can_filter_bitmap(const struct list_objects_filter_options *filter)
+bool can_filter_bitmap(const struct list_objects_filter_options *filter)
{
return !filter_bitmap(NULL, NULL, NULL, filter);
}
-
static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git,
struct bitmap *result)
{
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 9f20fb6e56..1385027c1f 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -92,6 +92,9 @@ int test_bitmap_pseudo_merge_objects(struct repository *r, uint32_t n);
struct list_objects_filter_options;
+/* Check whether the filter can be computed via the bitmap. */
+bool can_filter_bitmap(const struct list_objects_filter_options *filter);
+
/*
* Filter bitmapped objects and iterate through all resulting objects,
* executing `show_reach` for each of them. Returns `-1` in case the filter is
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* [PATCH v4 9/9] builtin/cat-file: filter objects via object database
From: Patrick Steinhardt @ 2026-07-15 6:22 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Junio C Hamano, Jeff King, Taylor Blau, Toon Claes
In-Reply-To: <20260715-pks-odb-for-each-object-filter-v4-0-616d7adf7fb7@pks.im>
When batching all objects, git-cat-file(1) reaches into the internals of
the object database and manually manages bitmaps to apply object
filters. This creates coupling between the command and the internals of
the respective backend.
Refactor git-cat-file(1) to use the new object filter option when
batching all objects. This significantly simplifies the logic and
ensures that we don't have to reach into internals of the "files" source
anymore.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/cat-file.c | 76 +++++-------------------------------------------------
1 file changed, 7 insertions(+), 69 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index b4b99a73da..1458dd76d6 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,7 +20,6 @@
#include "userdiff.h"
#include "oid-array.h"
#include "packfile.h"
-#include "pack-bitmap.h"
#include "object-file.h"
#include "object-name.h"
#include "odb.h"
@@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid,
return payload->callback(oid, NULL, 0, payload->payload);
}
-static int batch_one_object_packed(const struct object_id *oid,
- struct packed_git *pack,
- uint32_t pos,
- void *_payload)
-{
- struct for_each_object_payload *payload = _payload;
- return payload->callback(oid, pack, nth_packed_object_offset(pack, pos),
- payload->payload);
-}
-
-static int batch_one_object_bitmapped(const struct object_id *oid,
- enum object_type type UNUSED,
- int flags UNUSED,
- uint32_t hash UNUSED,
- struct packed_git *pack,
- off_t offset,
- void *_payload)
-{
- struct for_each_object_payload *payload = _payload;
- return payload->callback(oid, pack, offset, payload->payload);
-}
-
static void batch_each_object(struct batch_options *opt,
for_each_object_fn callback,
unsigned flags,
@@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt,
.callback = callback,
.payload = _payload,
};
+ struct odb_source_info source_info;
+ struct object_info oi = {
+ .source_infop = &source_info,
+ };
struct odb_for_each_object_options opts = {
.flags = flags,
+ .filter = &opt->objects_filter,
};
- struct bitmap_index *bitmap = NULL;
- struct odb_source *source;
-
- /*
- * TODO: we still need to tap into implementation details of the object
- * database sources. Ideally, we should extend `odb_for_each_object()`
- * to handle object filters itself so that we can move the filtering
- * logic into the individual sources.
- */
- odb_prepare_alternates(the_repository->objects);
- for (source = the_repository->objects->sources; source; source = source->next) {
- struct odb_source_files *files = odb_source_files_downcast(source);
- int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi,
- &payload, &opts);
- if (ret)
- break;
- }
-
- if (opt->objects_filter.choice != LOFC_DISABLED &&
- (bitmap = prepare_bitmap_git(the_repository)) &&
- !for_each_bitmapped_object(bitmap, &opt->objects_filter,
- batch_one_object_bitmapped, &payload)) {
- struct packed_git *pack;
-
- repo_for_each_pack(the_repository, pack) {
- if (bitmap_index_contains_pack(bitmap, pack) ||
- open_pack_index(pack))
- continue;
- for_each_object_in_pack(pack, batch_one_object_packed,
- &payload, flags);
- }
- } else {
- struct odb_source_info source_info;
- struct object_info oi = {
- .source_infop = &source_info,
- };
-
- for (source = the_repository->objects->sources; source; source = source->next) {
- struct odb_source_files *files = odb_source_files_downcast(source);
- int ret = odb_source_for_each_object(&files->packed->base, &oi,
- batch_one_object_oi, &payload, &opts);
- if (ret)
- break;
- }
- }
- free_bitmap_index(bitmap);
+ odb_for_each_object_ext(the_repository->objects, &oi,
+ batch_one_object_oi, &payload, &opts);
}
static int batch_objects(struct batch_options *opt)
--
2.55.0.313.g8d093f411d.dirty
^ permalink raw reply related
* Re: [PATCH v1] repository: move fetch_if_missing into struct repository
From: Patrick Steinhardt @ 2026-07-15 6:35 UTC (permalink / raw)
To: Tian Yuchen
Cc: git, five231003, hariom18599, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260715011850.3181131-1-cat@malon.dev>
On Wed, Jul 15, 2026 at 09:18:50AM +0800, Tian Yuchen wrote:
> The global variable 'fetch_if_missing' controls whether a missing
> object check should prompt a lazy fetch from a promisor remote.
> In order to continue the libification effort, move it into
> 'struct repository' and initialize it to 1 by default to keep the
> previous behavior.
Right. I was also thinking about moving this into a non-global scope
multiple times. I was approaching this a bit differently though: it's
ultimately a property of the object database whether or not we want to
accept missing objects, so I moved it in there instead.
I don't really think there's a downside with your version, though. Quite
on the contrary: we can really only perform the backfill fetches with a
whole repository at hand anyway. So conceptually your version might even
be more sensible.
> Subsystems that already pass around a repository pointer, are
> updated to read this flag directly from their respective 'repo'
> instances. For the rest, we access 'the_repository'.
>
> Note that in builtin/fsck.c and builtin/index-pack.c, when running
> related commands with the '-h' parameter, the 'repo' pointer is not
> passed in. To prevent null pointer dereferences, we defer
> operations on the repo in until after parameter parsing is complete.
s/on the repo in/on the repo/
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 0793dc595c..721d576938 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1898,15 +1898,16 @@ int cmd_index_pack(int argc,
> int report_end_of_input = 0;
> int hash_algo = 0;
>
> + show_usage_if_asked(argc, argv, index_pack_usage);
> +
> /*
> * index-pack never needs to fetch missing objects except when
> * REF_DELTA bases are missing (which are explicitly handled). It only
> * accesses the repo to do hash collision checks and to check which
> * REF_DELTA bases need to be fetched.
> */
> - fetch_if_missing = 0;
> -
> - show_usage_if_asked(argc, argv, index_pack_usage);
> + if (repo)
> + repo->fetch_if_missing = 0;
>
> disable_replace_refs();
>
Okay. This command can run without a repository, in which case we'll end
up just indexing the pack. My assumption is that we'll probably end up
using `the_repository` if so, as we still use `the_repository` in this
file. So could this here cause a change in behaviour?
If the answer is "maybe" I'd propose that we simply continue to use
`the_repository` here.
> diff --git a/revision.c b/revision.c
> index e91d7e1f11..bb645654c3 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2714,7 +2714,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
> revs->ignore_missing = 1;
> } else if (opt && opt->allow_exclude_promisor_objects &&
> !strcmp(arg, "--exclude-promisor-objects")) {
> - if (fetch_if_missing)
> + if (revs->repo->fetch_if_missing)
> BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
> revs->exclude_promisor_objects = 1;
> } else {
This one here also makes me wonder whether it could cause weird
interactions in case a caller passes a repository other than
`the_repository`. It ideally _shouldn't_, but it's hard to tell because
we still use `the_repository` in lots of places here.
Thanks!
Patrick
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox