* [PATCH v8 8/9] environment: move autorebase into repo_config_values
From: Tian Yuchen @ 2026-07-08 16:02 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260708160300.8852-1-cat@malon.dev>
The global variable 'autorebase' dictates whether a newly created
branch should be configured to automatically rebase by default.
Move it into 'struct repo_config_values' to continue the
libification effort.
The 'enum rebase_setup_type' definition is moved higher up in
'environment.h' so that it is visible to the repository-specific
structure. The default state AUTOREBASE_NEVER is now correctly
initialized in 'repo_config_values_init()'.
Configuration parsing in 'git_default_branch_config()' is updated to
write directly to the repository's configuration instance.
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>
---
branch.c | 2 +-
environment.c | 10 +++++-----
environment.h | 16 ++++++++--------
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/branch.c b/branch.c
index 243db7d0fc..e1c1f8c89d 100644
--- a/branch.c
+++ b/branch.c
@@ -61,7 +61,7 @@ static int find_tracked_branch(struct remote *remote, void *priv)
static int should_setup_rebase(const char *origin)
{
- switch (autorebase) {
+ switch (repo_config_values(the_repository)->autorebase) {
case AUTOREBASE_NEVER:
return 0;
case AUTOREBASE_LOCAL:
diff --git a/environment.c b/environment.c
index 09de2fee87..7701aa3bc0 100644
--- a/environment.c
+++ b/environment.c
@@ -58,7 +58,6 @@ enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
enum eol core_eol = EOL_UNSET;
int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
char *check_roundtrip_encoding;
-enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
#ifndef OBJECT_CREATION_MODE
#define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
#endif
@@ -600,13 +599,13 @@ static int git_default_branch_config(const char *var, const char *value)
if (!value)
return config_error_nonbool(var);
else if (!strcmp(value, "never"))
- autorebase = AUTOREBASE_NEVER;
+ cfg->autorebase = AUTOREBASE_NEVER;
else if (!strcmp(value, "local"))
- autorebase = AUTOREBASE_LOCAL;
+ cfg->autorebase = AUTOREBASE_LOCAL;
else if (!strcmp(value, "remote"))
- autorebase = AUTOREBASE_REMOTE;
+ cfg->autorebase = AUTOREBASE_REMOTE;
else if (!strcmp(value, "always"))
- autorebase = AUTOREBASE_ALWAYS;
+ cfg->autorebase = AUTOREBASE_ALWAYS;
else
return error(_("malformed value for %s"), var);
return 0;
@@ -727,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->apply_default_whitespace = NULL;
cfg->apply_default_ignorewhitespace = NULL;
cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
+ cfg->autorebase = AUTOREBASE_NEVER;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 72859b5d76..464ff73136 100644
--- a/environment.h
+++ b/environment.h
@@ -102,6 +102,13 @@ enum push_default_type {
PUSH_DEFAULT_UNSPECIFIED
};
+enum rebase_setup_type {
+ AUTOREBASE_NEVER = 0,
+ AUTOREBASE_LOCAL,
+ AUTOREBASE_REMOTE,
+ AUTOREBASE_ALWAYS
+};
+
struct repo_config_values {
/* section "core" config values */
char *attributes_file;
@@ -112,6 +119,7 @@ struct repo_config_values {
char *apply_default_whitespace;
char *apply_default_ignorewhitespace;
enum push_default_type push_default;
+ enum rebase_setup_type autorebase;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -205,14 +213,6 @@ extern unsigned long pack_size_limit_cfg;
extern int protect_hfs;
extern int protect_ntfs;
-enum rebase_setup_type {
- AUTOREBASE_NEVER = 0,
- AUTOREBASE_LOCAL,
- AUTOREBASE_REMOTE,
- AUTOREBASE_ALWAYS
-};
-extern enum rebase_setup_type autorebase;
-
enum object_creation_mode {
OBJECT_CREATION_USES_HARDLINKS = 0,
OBJECT_CREATION_USES_RENAMES = 1
--
2.43.0
^ permalink raw reply related
* [PATCH v8 7/9] environment: move push_default into repo_config_values
From: Tian Yuchen @ 2026-07-08 16:02 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260708160300.8852-1-cat@malon.dev>
The global variable 'push_default' specifies the default behavior of
'git push' when no explicit refspec is provided. Move 'push_default'
into 'struct repo_config_values' to continue the libification effort.
While 'enum push_default_type' ideally belongs in 'remote.h', moving it
there introduces a circular dependency chain:
remote.h -> hash.h -> repository.h -> environment.h.
Therefore, the enum definition is kept in 'environment.h' just above
'struct repo_config_values' with a NEEDSWORK comment for future cleanup.
Modify the configuration parsing in environment.c to update the
per-repository structure directly, and update caller across the
codebase to access the value via 'repo_config_values()'.
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>
---
builtin/push.c | 8 ++++----
environment.c | 16 +++++++++-------
environment.h | 26 ++++++++++++++++----------
remote.c | 2 +-
4 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..6dc3224b60 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -88,7 +88,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,
}
}
- if (push_default == PUSH_DEFAULT_UPSTREAM &&
+ if (repo_config_values(the_repository)->push_default == PUSH_DEFAULT_UPSTREAM &&
skip_prefix(matched->name, "refs/heads/", &branch_name)) {
struct branch *branch = branch_get(branch_name);
if (branch->merge_nr == 1 && branch->merge[0]->src) {
@@ -160,7 +160,7 @@ static NORETURN void die_push_simple(struct branch *branch,
* Don't show advice for people who explicitly set
* push.default.
*/
- if (push_default == PUSH_DEFAULT_UNSPECIFIED)
+ if (cfg->push_default == PUSH_DEFAULT_UNSPECIFIED)
advice_pushdefault_maybe = _("\n"
"To choose either option permanently, "
"see push.default in 'git help config'.\n");
@@ -232,7 +232,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
const char *dst;
int same_remote;
- switch (push_default) {
+ switch (repo_config_values(the_repository)->push_default) {
case PUSH_DEFAULT_MATCHING:
refspec_append(&rs, ":");
return;
@@ -252,7 +252,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
dst = branch->refname;
same_remote = !strcmp(remote->name, remote_for_branch(branch, NULL));
- switch (push_default) {
+ switch (repo_config_values(the_repository)->push_default) {
default:
case PUSH_DEFAULT_UNSPECIFIED:
case PUSH_DEFAULT_SIMPLE:
diff --git a/environment.c b/environment.c
index 8744790219..09de2fee87 100644
--- a/environment.c
+++ b/environment.c
@@ -59,7 +59,6 @@ enum eol core_eol = EOL_UNSET;
int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
char *check_roundtrip_encoding;
enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
-enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
#ifndef OBJECT_CREATION_MODE
#define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
#endif
@@ -619,21 +618,23 @@ static int git_default_branch_config(const char *var, const char *value)
static int git_default_push_config(const char *var, const char *value)
{
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+
if (!strcmp(var, "push.default")) {
if (!value)
return config_error_nonbool(var);
else if (!strcmp(value, "nothing"))
- push_default = PUSH_DEFAULT_NOTHING;
+ cfg->push_default = PUSH_DEFAULT_NOTHING;
else if (!strcmp(value, "matching"))
- push_default = PUSH_DEFAULT_MATCHING;
+ cfg->push_default = PUSH_DEFAULT_MATCHING;
else if (!strcmp(value, "simple"))
- push_default = PUSH_DEFAULT_SIMPLE;
+ cfg->push_default = PUSH_DEFAULT_SIMPLE;
else if (!strcmp(value, "upstream"))
- push_default = PUSH_DEFAULT_UPSTREAM;
+ cfg->push_default = PUSH_DEFAULT_UPSTREAM;
else if (!strcmp(value, "tracking")) /* deprecated */
- push_default = PUSH_DEFAULT_UPSTREAM;
+ cfg->push_default = PUSH_DEFAULT_UPSTREAM;
else if (!strcmp(value, "current"))
- push_default = PUSH_DEFAULT_CURRENT;
+ cfg->push_default = PUSH_DEFAULT_CURRENT;
else {
error(_("malformed value for %s: %s"), var, value);
return error(_("must be one of nothing, matching, simple, "
@@ -725,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->askpass_program = NULL;
cfg->apply_default_whitespace = NULL;
cfg->apply_default_ignorewhitespace = NULL;
+ cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 9aecd64152..72859b5d76 100644
--- a/environment.h
+++ b/environment.h
@@ -87,6 +87,21 @@ extern const char * const local_repo_env[];
struct strvec;
struct repository;
+
+/*
+ * NEEDSWORK: It would be better if these definitions could be moved to
+ * other more specific files, but care is needed to avoid circular
+ * inclusion issues.
+ */
+enum push_default_type {
+ PUSH_DEFAULT_NOTHING = 0,
+ PUSH_DEFAULT_MATCHING,
+ PUSH_DEFAULT_SIMPLE,
+ PUSH_DEFAULT_UPSTREAM,
+ PUSH_DEFAULT_CURRENT,
+ PUSH_DEFAULT_UNSPECIFIED
+};
+
struct repo_config_values {
/* section "core" config values */
char *attributes_file;
@@ -96,6 +111,7 @@ struct repo_config_values {
char *askpass_program;
char *apply_default_whitespace;
char *apply_default_ignorewhitespace;
+ enum push_default_type push_default;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -197,16 +213,6 @@ enum rebase_setup_type {
};
extern enum rebase_setup_type autorebase;
-enum push_default_type {
- PUSH_DEFAULT_NOTHING = 0,
- PUSH_DEFAULT_MATCHING,
- PUSH_DEFAULT_SIMPLE,
- PUSH_DEFAULT_UPSTREAM,
- PUSH_DEFAULT_CURRENT,
- PUSH_DEFAULT_UNSPECIFIED
-};
-extern enum push_default_type push_default;
-
enum object_creation_mode {
OBJECT_CREATION_USES_HARDLINKS = 0,
OBJECT_CREATION_USES_RENAMES = 1
diff --git a/remote.c b/remote.c
index 00723b385e..d48c01d375 100644
--- a/remote.c
+++ b/remote.c
@@ -1933,7 +1933,7 @@ static char *branch_get_push_1(struct repository *repo,
if (remote->mirror)
return tracking_for_push_dest(remote, branch->refname, err);
- switch (push_default) {
+ switch (repo_config_values(repo)->push_default) {
case PUSH_DEFAULT_NOTHING:
return error_buf(err, _("push has no destination (push.default is 'nothing')"));
--
2.43.0
^ permalink raw reply related
* [PATCH v8 6/9] environment: migrate apply_default_whitespace and apply_default_ignorewhitespace
From: Tian Yuchen @ 2026-07-08 16:02 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260708160300.8852-1-cat@malon.dev>
The global variables 'apply_default_whitespace' and
'apply_default_ignorewhitespace' are used to store the default
whitespace configuration for 'git apply'. Move these variables
into 'struct repo_config_values' to continue the libification
effort.
Dynamically allocated strings fetched via 'repo_config_get_string()'
are now tracked per-repository and safely freed in
'repo_config_values_clear()'.
As part of this transition, update 'git_apply_config()' to accept a
'struct repository *' argument rather than relying on the
'the_repository' global.
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 | 20 ++++++++++++--------
environment.c | 6 ++++--
environment.h | 4 ++--
3 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/apply.c b/apply.c
index 249248d4f2..66db9b7678 100644
--- a/apply.c
+++ b/apply.c
@@ -47,11 +47,13 @@ struct gitdiff_data {
int p_value;
};
-static void git_apply_config(void)
+static void git_apply_config(struct repository *repo)
{
- repo_config_get_string(the_repository, "apply.whitespace", &apply_default_whitespace);
- repo_config_get_string(the_repository, "apply.ignorewhitespace", &apply_default_ignorewhitespace);
- repo_config(the_repository, git_xmerge_config, NULL);
+ repo_config_get_string(repo, "apply.whitespace",
+ &repo_config_values(repo)->apply_default_whitespace);
+ repo_config_get_string(repo, "apply.ignorewhitespace",
+ &repo_config_values(repo)->apply_default_ignorewhitespace);
+ repo_config(repo, git_xmerge_config, NULL);
}
static int parse_whitespace_option(struct apply_state *state, const char *option)
@@ -126,10 +128,12 @@ int init_apply_state(struct apply_state *state,
strset_init(&state->kept_symlinks);
strbuf_init(&state->root, 0);
- git_apply_config();
- if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
+ git_apply_config(repo);
+ if (repo_config_values(repo)->apply_default_whitespace &&
+ parse_whitespace_option(state, repo_config_values(repo)->apply_default_whitespace))
return -1;
- if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+ if (repo_config_values(repo)->apply_default_ignorewhitespace &&
+ parse_ignorewhitespace_option(state, repo_config_values(repo)->apply_default_ignorewhitespace))
return -1;
return 0;
}
@@ -192,7 +196,7 @@ int check_apply_state(struct apply_state *state, int force_apply)
static void set_default_whitespace_mode(struct apply_state *state)
{
- if (!state->whitespace_option && !apply_default_whitespace)
+ if (!state->whitespace_option && !repo_config_values(state->repo)->apply_default_whitespace)
state->ws_error_action = (state->apply ? warn_on_ws_error : nowarn_ws_error);
}
diff --git a/environment.c b/environment.c
index 3782bf68aa..8744790219 100644
--- a/environment.c
+++ b/environment.c
@@ -49,8 +49,6 @@ int assume_unchanged;
int is_bare_repository_cfg = -1; /* unspecified */
char *git_commit_encoding;
char *git_log_output_encoding;
-char *apply_default_whitespace;
-char *apply_default_ignorewhitespace;
int fsync_object_files = -1;
int use_fsync = -1;
enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
@@ -725,6 +723,8 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->editor_program = NULL;
cfg->pager_program = NULL;
cfg->askpass_program = NULL;
+ cfg->apply_default_whitespace = NULL;
+ cfg->apply_default_ignorewhitespace = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -758,4 +758,6 @@ void repo_config_values_clear(struct repository *repo)
FREE_AND_NULL(cfg->editor_program);
FREE_AND_NULL(cfg->pager_program);
FREE_AND_NULL(cfg->askpass_program);
+ FREE_AND_NULL(cfg->apply_default_whitespace);
+ FREE_AND_NULL(cfg->apply_default_ignorewhitespace);
}
diff --git a/environment.h b/environment.h
index d55b1ba073..9aecd64152 100644
--- a/environment.h
+++ b/environment.h
@@ -94,6 +94,8 @@ struct repo_config_values {
char *editor_program;
char *pager_program;
char *askpass_program;
+ char *apply_default_whitespace;
+ char *apply_default_ignorewhitespace;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -182,8 +184,6 @@ extern int has_symlinks;
extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
-extern char *apply_default_whitespace;
-extern char *apply_default_ignorewhitespace;
extern unsigned long pack_size_limit_cfg;
extern int protect_hfs;
--
2.43.0
^ permalink raw reply related
* [PATCH v8 5/9] environment: move askpass_program into repo_config_values
From: Tian Yuchen @ 2026-07-08 16:02 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260708160300.8852-1-cat@malon.dev>
The global variable 'askpass_program' stores the path to the program
used to prompt the user for credentials. Move it into repo_config_values
to continue the libification effort.
While it is uncommon for a single process to require different askpass
programs for different repositories, maintaining this value as a mutable
global string is a blocker for libification. Global heap-allocated
strings introduce thread-safety issues in a multi-repo environment.
Move 'askpass_program' into 'struct repo_config_values' to eliminate
this global state. The memory is now safely managed and freed via
'repo_config_values_clear()'.
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>
---
environment.c | 6 ++++--
environment.h | 1 +
prompt.c | 3 ++-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/environment.c b/environment.c
index a1204fdcb2..3782bf68aa 100644
--- a/environment.c
+++ b/environment.c
@@ -462,8 +462,8 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.askpass")) {
- FREE_AND_NULL(askpass_program);
- return git_config_string(&askpass_program, var, value);
+ FREE_AND_NULL(cfg->askpass_program);
+ return git_config_string(&cfg->askpass_program, var, value);
}
if (!strcmp(var, "core.excludesfile")) {
@@ -724,6 +724,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->excludes_file = NULL;
cfg->editor_program = NULL;
cfg->pager_program = NULL;
+ cfg->askpass_program = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -756,4 +757,5 @@ void repo_config_values_clear(struct repository *repo)
FREE_AND_NULL(cfg->excludes_file);
FREE_AND_NULL(cfg->editor_program);
FREE_AND_NULL(cfg->pager_program);
+ FREE_AND_NULL(cfg->askpass_program);
}
diff --git a/environment.h b/environment.h
index 22f6697c52..d55b1ba073 100644
--- a/environment.h
+++ b/environment.h
@@ -93,6 +93,7 @@ struct repo_config_values {
char *excludes_file;
char *editor_program;
char *pager_program;
+ char *askpass_program;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
diff --git a/prompt.c b/prompt.c
index 706fba2a50..d8d74c7e37 100644
--- a/prompt.c
+++ b/prompt.c
@@ -3,6 +3,7 @@
#include "git-compat-util.h"
#include "parse.h"
#include "environment.h"
+#include "repository.h"
#include "run-command.h"
#include "strbuf.h"
#include "prompt.h"
@@ -51,7 +52,7 @@ char *git_prompt(const char *prompt, int flags)
askpass = getenv("GIT_ASKPASS");
if (!askpass)
- askpass = askpass_program;
+ askpass = repo_config_values(the_repository)->askpass_program;
if (!askpass)
askpass = getenv("SSH_ASKPASS");
if (askpass && *askpass)
--
2.43.0
^ permalink raw reply related
* [PATCH v8 4/9] environment: move pager_program into repo_config_values
From: Tian Yuchen @ 2026-07-08 16:02 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260708160300.8852-1-cat@malon.dev>
The 'pager_program' variable is currently defined as a file-scoped
static string in pager.c. Move it into 'struct repo_config_values'.
The configuration parsing logic remains strictly within pager.c to
respect subsystem boundaries. The read/write operations are simply
redirected to the repository-specific structure using
'repo_config_values()'.
Similar to the recent editor_program migration, no standalone getter
is introduced to keep the code minimal. The dynamically allocated
memory is now managed by 'repo_config_values_clear()'.
On top of that, fix a memory leak in pager.c while we are at it.
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>
---
environment.c | 2 ++
environment.h | 1 +
| 21 +++++++++++++--------
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/environment.c b/environment.c
index 0a01f4761a..a1204fdcb2 100644
--- a/environment.c
+++ b/environment.c
@@ -723,6 +723,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->attributes_file = NULL;
cfg->excludes_file = NULL;
cfg->editor_program = NULL;
+ cfg->pager_program = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -754,4 +755,5 @@ void repo_config_values_clear(struct repository *repo)
FREE_AND_NULL(cfg->attributes_file);
FREE_AND_NULL(cfg->excludes_file);
FREE_AND_NULL(cfg->editor_program);
+ FREE_AND_NULL(cfg->pager_program);
}
diff --git a/environment.h b/environment.h
index 1ec19149cb..22f6697c52 100644
--- a/environment.h
+++ b/environment.h
@@ -92,6 +92,7 @@ struct repo_config_values {
char *attributes_file;
char *excludes_file;
char *editor_program;
+ char *pager_program;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
--git a/pager.c b/pager.c
index 35b210e048..c8ebdd4b31 100644
--- a/pager.c
+++ b/pager.c
@@ -5,6 +5,8 @@
#include "run-command.h"
#include "sigchain.h"
#include "alias.h"
+#include "repository.h"
+#include "environment.h"
int pager_use_color = 1;
@@ -13,7 +15,6 @@ int pager_use_color = 1;
#endif
static struct child_process pager_process;
-static char *pager_program;
static int old_fd1 = -1, old_fd2 = -1;
/* Is the value coming back from term_columns() just a guess? */
@@ -75,10 +76,12 @@ static void wait_for_pager_signal(int signo)
static int core_pager_config(const char *var, const char *value,
const struct config_context *ctx UNUSED,
- void *data UNUSED)
+ void *data)
{
+ struct repository *r = data;
+
if (!strcmp(var, "core.pager"))
- return git_config_string(&pager_program, var, value);
+ return git_config_string(&repo_config_values(r)->pager_program, var, value);
return 0;
}
@@ -91,10 +94,10 @@ const char *git_pager(struct repository *r, int stdout_is_tty)
pager = getenv("GIT_PAGER");
if (!pager) {
- if (!pager_program)
+ if (!repo_config_values(r)->pager_program)
read_early_config(r,
- core_pager_config, NULL);
- pager = pager_program;
+ core_pager_config, r);
+ pager = repo_config_values(r)->pager_program;
}
if (!pager)
pager = getenv("PAGER");
@@ -302,7 +305,9 @@ int check_pager_config(struct repository *r, const char *cmd)
read_early_config(r, pager_command_config, &data);
- if (data.value)
- pager_program = data.value;
+ if (data.value) {
+ free(repo_config_values(r)->pager_program);
+ repo_config_values(r)->pager_program = data.value;
+ }
return data.want;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v8 3/9] environment: move editor_program into repo_config_values
From: Tian Yuchen @ 2026-07-08 16:02 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260708160300.8852-1-cat@malon.dev>
The global variable 'editor_program' holds the path to the user's
preferred editor. Move 'editor_program' into
'struct repo_config_values' to continue the libification effort.
There have been discussions on whether external programs like
editors truly need to be configured on a per-repository basis within
the same process. While a single process might rarely invoke
different editors, this migration is necessary for two reasons:
1. Developers frequently use different toolchains for different
projects. Per-repo configuration respects this.
2. Moving this string into 'repo_config_values' eliminates mutable
global state. As the codebase moves toward becoming a long-running
processes managing multiple repositories concurrently must
not overwrite each other's program configurations.
No standalone getter function is introduced. Callers directly access
the field via 'repo_config_values()'. Heap memory is safely reclaimed
in 'repo_config_values_clear()'.
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>
---
editor.c | 4 ++--
environment.c | 7 ++++---
environment.h | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/editor.c b/editor.c
index fd174e6a03..07d264cba0 100644
--- a/editor.c
+++ b/editor.c
@@ -29,8 +29,8 @@ const char *git_editor(void)
const char *editor = getenv("GIT_EDITOR");
int terminal_is_dumb = is_terminal_dumb();
- if (!editor && editor_program)
- editor = editor_program;
+ if (!editor && repo_config_values(the_repository)->editor_program)
+ editor = repo_config_values(the_repository)->editor_program;
if (!editor && !terminal_is_dumb)
editor = getenv("VISUAL");
if (!editor)
diff --git a/environment.c b/environment.c
index 5950592d63..0a01f4761a 100644
--- a/environment.c
+++ b/environment.c
@@ -55,7 +55,6 @@ int fsync_object_files = -1;
int use_fsync = -1;
enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
-char *editor_program;
char *askpass_program;
enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
enum eol core_eol = EOL_UNSET;
@@ -435,8 +434,8 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.editor")) {
- FREE_AND_NULL(editor_program);
- return git_config_string(&editor_program, var, value);
+ FREE_AND_NULL(cfg->editor_program);
+ return git_config_string(&cfg->editor_program, var, value);
}
if (!strcmp(var, "core.commentchar") ||
@@ -723,6 +722,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
cfg->excludes_file = NULL;
+ cfg->editor_program = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -753,4 +753,5 @@ void repo_config_values_clear(struct repository *repo)
FREE_AND_NULL(cfg->attributes_file);
FREE_AND_NULL(cfg->excludes_file);
+ FREE_AND_NULL(cfg->editor_program);
}
diff --git a/environment.h b/environment.h
index 2e8352de7f..1ec19149cb 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,7 @@ struct repo_config_values {
/* section "core" config values */
char *attributes_file;
char *excludes_file;
+ char *editor_program;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -218,7 +219,6 @@ const char *get_commit_output_encoding(void);
extern char *git_commit_encoding;
extern char *git_log_output_encoding;
-extern char *editor_program;
extern char *askpass_program;
/*
--
2.43.0
^ permalink raw reply related
* [PATCH v8 1/9] repository: introduce repo_config_values_clear()
From: Tian Yuchen @ 2026-07-08 16:02 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260708160300.8852-1-cat@malon.dev>
As part of the ongoing libification effort, dynamically allocated
global configuration variables are being moved into
'struct repo_config_values'. To prevent memory leaks, we need a
destructor to free these heap-allocated variables when a repository
instance is torn down.
Introduce 'repo_config_values_clear()' in environment.c and invoke it
from 'repo_clear()' in repository.c. As a starting point, update this
new function to handle the cleanup of 'attributes_file'.
Note:
Submodules are currently not supported by repo_config_values(), which
explicitly BUG()s out if 'repo != the_repository'. Since repo_clear()
cleans up all repository instances, we must bypass them to prevent
crashing.
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>
---
environment.c | 19 +++++++++++++++++++
environment.h | 9 +++++++++
repository.c | 1 +
3 files changed, 29 insertions(+)
diff --git a/environment.c b/environment.c
index ba2c60103f..13677484de 100644
--- a/environment.c
+++ b/environment.c
@@ -726,3 +726,22 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->sparse_expect_files_outside_of_patterns = 0;
cfg->warn_on_object_refname_ambiguity = 1;
}
+
+void repo_config_values_clear(struct repository *repo)
+{
+ struct repo_config_values *cfg;
+
+ /*
+ * NEEDSWORK: Submodules are currently not supported by
+ * repo_config_values(), which explicitly BUG()s out if
+ * repo != the_repository. Since repo_clear() cleans up all
+ * repository instances, we must bypass them here to prevent
+ * crashing.
+ */
+ if (repo != the_repository)
+ return;
+
+ cfg = repo_config_values(repo);
+
+ FREE_AND_NULL(cfg->attributes_file);
+}
diff --git a/environment.h b/environment.h
index 6f18286955..c4a6a45704 100644
--- a/environment.h
+++ b/environment.h
@@ -135,6 +135,15 @@ int git_default_core_config(const char *var, const char *value,
void repo_config_values_init(struct repo_config_values *cfg);
+/*
+ * Frees memory allocated for dynamically loaded configuration values
+ * inside `repo_config_values`.
+ *
+ * As dynamically allocated variables are migrated into this struct,
+ * their FREE_AND_NULL() calls should be appended here.
+ */
+void repo_config_values_clear(struct repository *repo);
+
/*
* TODO: All the below state either explicitly or implicitly relies on
* `the_repository`. We should eventually get rid of these and make the
diff --git a/repository.c b/repository.c
index 187dd471c4..b31f1b7852 100644
--- a/repository.c
+++ b/repository.c
@@ -388,6 +388,7 @@ void repo_clear(struct repository *repo)
FREE_AND_NULL(repo->parsed_objects);
repo_settings_clear(repo);
+ repo_config_values_clear(repo);
if (repo->config) {
git_configset_clear(repo->config);
--
2.43.0
^ permalink raw reply related
* [PATCH v8 2/9] environment: move excludes_file into repo_config_values
From: Tian Yuchen @ 2026-07-08 16:02 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260708160300.8852-1-cat@malon.dev>
The global variable 'excludes_file' is used to track the path to the
global ignore file. If this variable is NULL,
'setup_standard_excludes()'
in 'dir.c' forcefully evaluates and assigns the XDG default path to it.
Continue the libification effort by encapsulating this lazy-loading
fallback logic into a proper getter and moving the variable into
'struct repo_config_values'.
Since 'excludes_file' is a dynamically allocated string, it requires
proper heap memory management. It is safely freed using the newly
introduced `repo_config_values_clear()` function when the repository
is torn down.
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>
---
dir.c | 4 ++--
environment.c | 15 ++++++++++++---
environment.h | 4 +++-
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index 7a73690fbc..4f87a52b3c 100644
--- a/dir.c
+++ b/dir.c
@@ -3481,11 +3481,11 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")
void setup_standard_excludes(struct dir_struct *dir)
{
+ const char *excludes_file = repo_excludes_file(the_repository);
+
dir->exclude_per_dir = ".gitignore";
/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */
- if (!excludes_file)
- excludes_file = xdg_config_home("ignore");
if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
add_patterns_from_file_1(dir, excludes_file,
dir->untracked ? &dir->internal.ss_excludes_file : NULL);
diff --git a/environment.c b/environment.c
index 13677484de..5950592d63 100644
--- a/environment.c
+++ b/environment.c
@@ -57,7 +57,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
char *editor_program;
char *askpass_program;
-char *excludes_file;
enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
enum eol core_eol = EOL_UNSET;
int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
@@ -134,6 +133,14 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
}
+const char *repo_excludes_file(struct repository *repo)
+{
+ if (!repo_config_values(repo)->excludes_file)
+ repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
+
+ return repo_config_values(repo)->excludes_file;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -461,8 +468,8 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.excludesfile")) {
- FREE_AND_NULL(excludes_file);
- return git_config_pathname(&excludes_file, var, value);
+ FREE_AND_NULL(cfg->excludes_file);
+ return git_config_pathname(&cfg->excludes_file, var, value);
}
if (!strcmp(var, "core.whitespace")) {
@@ -715,6 +722,7 @@ int git_default_config(const char *var, const char *value,
void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
+ cfg->excludes_file = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -744,4 +752,5 @@ void repo_config_values_clear(struct repository *repo)
cfg = repo_config_values(repo);
FREE_AND_NULL(cfg->attributes_file);
+ FREE_AND_NULL(cfg->excludes_file);
}
diff --git a/environment.h b/environment.h
index c4a6a45704..2e8352de7f 100644
--- a/environment.h
+++ b/environment.h
@@ -90,6 +90,7 @@ struct repository;
struct repo_config_values {
/* section "core" config values */
char *attributes_file;
+ char *excludes_file;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -133,6 +134,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);
+const char *repo_excludes_file(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -217,7 +220,6 @@ extern char *git_log_output_encoding;
extern char *editor_program;
extern char *askpass_program;
-extern char *excludes_file;
/*
* The character that begins a commented line in user-editable file
--
2.43.0
^ permalink raw reply related
* [PATCH v8 0/9] migrate more variables into repo_config_values
From: Tian Yuchen @ 2026-07-08 16:02 UTC (permalink / raw)
To: git; +Cc: cirnovskyv, szeder.dev, Tian Yuchen
In-Reply-To: <20260706142530.3681520-1-cat@malon.dev>
Hi everyone,
This patch series continues the ongoing libification effort by migrating
a batch of global configuration variables into struct repo_config_values.
What does this series do:
infrastructure & strings (commits 1-6):
Introduce 'repo_config_values_clear()' to manage the lifecycle
of heap-allocated configuration strings. This infrastructure is utilized
to migrate string variables, including 'excludes_file', 'apply' whitespace
configs, and external programs including 'editor', 'pager', 'askpass'.
enums (commits 7-9):
Migrate enumerations 'push_default', 'autorebase', and
'object_creation_mode'. Care was taken to make these types available
to the configuration structure without triggering circular header
dependencies.
RFC:
Commit 3~5. Is it really necessary to migrate _program variables?
https://lore.kernel.org/git/8e657184-ee0b-453a-9f2d-a98080d3582e@gmail.com/
Commit 6~9. Previous related discussions on 'git_branch_track'.
https://lore.kernel.org/git/CAD=f0L-mPX+KECUjXk-WBzEbTP7wCa8sB56GySQT0yh9mfUOWw@mail.gmail.com/
Note:
Since a new getter 'repo_excludes_file()' is introduced, as previously
promised, once it is finally merged into 'master', there will be a patch to
update and squash the comments.
Similarly, I've noticed that the classification and sorting of variables in
'repo_config_values' don't seem to be correct. There will also be a patch
to fix this, and I think it will form a commit series along with the comment
patch?
Change since v7:
Fixed a memory leak in pager.c.
Thanks!
Tian Yuchen (9):
repository: introduce repo_config_values_clear()
environment: move excludes_file into repo_config_values
environment: move editor_program into repo_config_values
environment: move pager_program into repo_config_values
environment: move askpass_program into repo_config_values
environment: migrate apply_default_whitespace and
apply_default_ignorewhitespace
environment: move push_default into repo_config_values
environment: move autorebase into repo_config_values
environment: move object_creation_mode into repo_config_values
apply.c | 20 +++++++-----
branch.c | 2 +-
builtin/push.c | 8 ++---
dir.c | 4 +--
editor.c | 4 +--
environment.c | 87 +++++++++++++++++++++++++++++++++++---------------
environment.h | 75 +++++++++++++++++++++++++++----------------
object-file.c | 2 +-
pager.c | 21 +++++++-----
prompt.c | 3 +-
remote.c | 2 +-
repository.c | 1 +
12 files changed, 148 insertions(+), 81 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v2 2/2] bundle-uri: stop sending invalid bundle configuration
From: Toon Claes @ 2026-07-08 15:03 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Toon Claes
In-Reply-To: <20260708-toon-bundle-uri-no-uri-v2-0-09a03d8db556@iotcl.com>
From: Justin Tobler <jltobler@gmail.com>
When bundle-URI info is requested by the client, the server responds
with all "bundle.*" config lines as key=value packet lines. On the
client-side, the received bundle config packet lines are always expected
to contain both a key and a value otherwise the client errors out during
parsing. The server performs no validation of the read bundle
configuration though which results in any misconfiguration on the
server-side, such as bundle configuration with an empty value, being
blindly sent to the client.
To avoid having the server transmit invalid configuration to clients,
only send bundle configuration that has non-empty values.
This change makes bundle-URI information sent by the server
syntactically correct, but semantically it still can be invalid. For
example the server may end up sending `bundle.bundle-1.creationToken`,
but be lacking a `bundle.bundle-1.uri` for that bundle. The `uri` is
mandatory, thus the client cannot process this bundle and will error
with the message:
error: bundle 'bundle-1' has no uri
Fixing this would require a more complex solution, because bundles need
to be validated as a whole and not line-by-line. This is considered
outside the scope of this change.
Co-authored-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Justin Tobler <jltobler@gmail.com>
Signed-off-by: Toon Claes <toon@iotcl.com>
---
bundle-uri.c | 8 ++++++--
t/lib-bundle-uri-protocol.sh | 23 +++++++++++++++++++++++
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/bundle-uri.c b/bundle-uri.c
index 3b2e347288..f956d3db7b 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -946,8 +946,12 @@ static int config_to_packet_line(const char *key, const char *value,
{
struct packet_reader *writer = data;
- if (starts_with(key, "bundle."))
- packet_write_fmt(writer->fd, "%s=%s", key, value);
+ if (starts_with(key, "bundle.")) {
+ if (value && *value)
+ packet_write_fmt(writer->fd, "%s=%s", key, value);
+ else
+ warning(_("config '%s' has no value"), key);
+ }
return 0;
}
diff --git a/t/lib-bundle-uri-protocol.sh b/t/lib-bundle-uri-protocol.sh
index de09b6b02e..e0e19715cd 100644
--- a/t/lib-bundle-uri-protocol.sh
+++ b/t/lib-bundle-uri-protocol.sh
@@ -214,3 +214,26 @@ test_expect_success "test bundle-uri with $BUNDLE_URI_PROTOCOL:// using protocol
>actual &&
test_cmp_config_output expect actual
'
+
+test_expect_success "test bundle-uri with $BUNDLE_URI_PROTOCOL:// using protocol v2 with empty value" '
+ test_config -C "$BUNDLE_URI_PARENT" \
+ bundle.bundle1.uri "$BUNDLE_URI_BUNDLE_URI_ESCAPED-1.bdl" &&
+ test_config -C "$BUNDLE_URI_PARENT" \
+ bundle.bundle2.uri "" &&
+
+ # The empty bundle.bundle2.uri value is invalid configuration and the
+ # server must not advertise it to the client.
+ cat >expect <<-EOF &&
+ [bundle]
+ version = 1
+ mode = all
+ [bundle "bundle1"]
+ uri = $BUNDLE_URI_BUNDLE_URI_ESCAPED-1.bdl
+ EOF
+
+ test-tool bundle-uri \
+ ls-remote \
+ "$BUNDLE_URI_REPO_URI" \
+ >actual &&
+ test_cmp_config_output expect actual
+'
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
* [PATCH v2 1/2] bundle-uri: drain remaining response on invalid bundle-uri lines
From: Toon Claes @ 2026-07-08 15:03 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Toon Claes
In-Reply-To: <20260708-toon-bundle-uri-no-uri-v2-0-09a03d8db556@iotcl.com>
On clone, when the client sends the `bundle-uri` command, the server
might respond with invalid data. For example if it sends information
about a bundle where the 'uri' is empty, it produces the following
error:
Cloning into 'foo'...
error: bundle-uri: line has empty key or value
error: error on bundle-uri response line 4: bundle.bundle-1.uri=
error: could not retrieve server-advertised bundle-uri list
This error is bubbled up to `transport_get_remote_bundle_uri()`, which
is called by `cmd_clone()` in builtin/clone.c. Over here, the return
value is ignored, so clone continues.
Despite this, it still dies with this error:
fatal: expected 'packfile'
This happens because `get_remote_bundle_uri()` exited early, leaving
some unprocessed packet data behind in the read buffer. This is
misleading to the user, because it suggests a problem with the packfile
exchange, when in reality it's caused by a misconfigured bundle-URI on
the server-side.
Fix this by continuing to read packets when an error was encountered,
but without processing the remaining lines. This drains the protocol
stream so no stale data is left behind and the caller can use it if they
like.
With this, clone now continues successfully if invalid bundle-URI data
was sent by the server. This is intentional, because since the inception
of `transport_get_remote_bundle_uri()` in 0cfde740f0 (clone: request the
'bundle-uri' command when available, 2022-12-22) the return value of
that function is ignored in `cmd_clone()` so the clone can continue
without bundles.
Signed-off-by: Toon Claes <toon@iotcl.com>
---
connect.c | 15 ++++++++++++---
t/t5558-clone-bundle-uri.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/connect.c b/connect.c
index 47e39d2a73..1d74c1eda2 100644
--- a/connect.c
+++ b/connect.c
@@ -517,7 +517,7 @@ static void send_capabilities(int fd_out, struct packet_reader *reader)
int get_remote_bundle_uri(int fd_out, struct packet_reader *reader,
struct bundle_list *bundles, int stateless_rpc)
{
- int line_nr = 1;
+ int line_nr = 1, err = 0;
/* Assert bundle-uri support */
ensure_server_supports_v2("bundle-uri");
@@ -536,10 +536,19 @@ int get_remote_bundle_uri(int fd_out, struct packet_reader *reader,
const char *line = reader->line;
line_nr++;
+ /*
+ * Do not parse if an error was encountered, but
+ * continue draining the response so no stale data
+ * is left in the reader for subsequent protocol
+ * exchanges.
+ */
+ if (err)
+ continue;
+
if (!bundle_uri_parse_line(bundles, line))
continue;
- return error(_("error on bundle-uri response line %d: %s"),
+ err = error(_("error on bundle-uri response line %d: %s"),
line_nr, line);
}
@@ -554,7 +563,7 @@ int get_remote_bundle_uri(int fd_out, struct packet_reader *reader,
check_stateless_delimiter(stateless_rpc, reader,
_("expected response end packet after ref listing"));
- return 0;
+ return err;
}
struct ref **get_remote_refs(int fd_out, struct packet_reader *reader,
diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh
index 7a0943bd36..7cc8627e17 100755
--- a/t/t5558-clone-bundle-uri.sh
+++ b/t/t5558-clone-bundle-uri.sh
@@ -1302,6 +1302,35 @@ test_expect_success 'bundles with newline in target path are rejected' '
test_path_is_missing escape
'
+test_expect_success 'bundles advertised with missing URI' '
+ git clone --no-local --mirror clone-from \
+ "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" config uploadpack.advertiseBundleURIs true &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" config bundle.version 1 &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" config bundle.mode all &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" config bundle.bundle-1.creationToken 1 &&
+
+ git -c transfer.bundleURI=true clone \
+ "$HTTPD_URL/smart/no-uri.git" target-no-uri 2>err &&
+ test_grep "bundle ${SQ}bundle-1${SQ} has no uri" err &&
+ test_grep ! "expected packfile" err
+'
+
+test_expect_success 'bundles advertised with empty URI' '
+ git clone --no-local --mirror clone-from \
+ "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config uploadpack.advertiseBundleURIs true &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config bundle.version 1 &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config bundle.mode all &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config bundle.bundle-1.uri "" &&
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config bundle.bundle-1.creationToken 1 &&
+
+ git -c transfer.bundleURI=true clone \
+ "$HTTPD_URL/smart/empty-uri.git" target-empty-uri 2>err &&
+ test_grep "bundle ${SQ}bundle-1${SQ} has no uri" err &&
+ test_grep ! "expected packfile" err
+'
+
# Do not add tests here unless they use the HTTP server, as they will
# not run unless the HTTP dependencies exist.
--
2.53.0.1323.g189a785ab5
^ permalink raw reply related
* [PATCH v2 0/2] Fix fatal error in git-clone(1) when reading empty bundle-URI
From: Toon Claes @ 2026-07-08 15:03 UTC (permalink / raw)
To: git; +Cc: Justin Tobler, Toon Claes
In-Reply-To: <20260408-toon-bundle-uri-no-uri-v1-1-d4a0e3937eba@iotcl.com>
This patch is a leftover from [1]. In that series I submitted two
patches. Because that series was submitted a long time ago, I'm
submitting this as a new series.
The first patch is in meantime superseded by [2], and thus is dropped
from this series.
The second patch fixes a misleading "fatal: expected 'packfile'" error
that occurs when cloning over HTTP from a server with misconfigured
bundle-URIs. It is modified to address Junio's concerns[3]:
> I tend to agree. Instead of papering over a misconfiguration, it
> would be better to let the users know, so they have a chance to
> report and/or correct such a misconfiguration.
To reiterate, in the previous series I changed the error() to a
warning() and Justin and Junio both didn't like this. In this series I
didn't remove the error(), but instead I'm ensuring the read buffer is
flushed before get_remote_bundle_uri() exits. This leaves a clean state
behind and clone can continue. (more details in the commit message).
In reply to that other series, Justin also insisted to implement a
server-side fix when bundles are misconfigured, and thus he provided the
second patch. This patch fixes bundles with an empty `uri`, but not with
a missing `uri`, that would require a substantial change which is
outside the scope of this series.
Because bundle-URIs are optional by design, I believe the changes in
this series are sufficient. Also, the series [2] takes a similar
approach: have the client gracefully continue in case of misconfigured
bundles.
[1]: <20250912-b4-toon-bundle-uri-no-uri-v1-0-f4525a406df8@iotcl.com>
[2]: <pull.2134.v2.git.git.1766160106521.gitgitgadget@gmail.com>
[3]: <xmqqbjnfmvwo.fsf@gitster.g>
Greets,
Toon
---
Changes in v2:
- Add second patch provided by Justin that fixes empty bundle `uri` on
the server-side.
- Extend inline code comments about continuing the loop in
get_remote_bundle_uri().
- Extend tests to check error message presented to the user.
- Link to v1: https://patch.msgid.link/20260408-toon-bundle-uri-no-uri-v1-1-d4a0e3937eba@iotcl.com
---
Justin Tobler (1):
bundle-uri: stop sending invalid bundle configuration
Toon Claes (1):
bundle-uri: drain remaining response on invalid bundle-uri lines
bundle-uri.c | 8 ++++++--
connect.c | 15 ++++++++++++---
t/lib-bundle-uri-protocol.sh | 23 +++++++++++++++++++++++
t/t5558-clone-bundle-uri.sh | 29 +++++++++++++++++++++++++++++
4 files changed, 70 insertions(+), 5 deletions(-)
Range-diff versus v1:
1: b2e52ca7fc ! 1: 22a9017826 bundle-uri: drain remaining response on invalid bundle-uri lines
@@ Commit message
This error is bubbled up to `transport_get_remote_bundle_uri()`, which
is called by `cmd_clone()` in builtin/clone.c. Over here, the return
- value of is ignored, so clone continues.
+ value is ignored, so clone continues.
Despite this, it still dies with this error:
@@ connect.c: int get_remote_bundle_uri(int fd_out, struct packet_reader *reader,
const char *line = reader->line;
line_nr++;
-+ /* Do not parse if an error was encountered */
++ /*
++ * Do not parse if an error was encountered, but
++ * continue draining the response so no stale data
++ * is left in the reader for subsequent protocol
++ * exchanges.
++ */
+ if (err)
+ continue;
+
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'bundles with newline in target
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/no-uri.git" config bundle.bundle-1.creationToken 1 &&
+
+ git -c transfer.bundleURI=true clone \
-+ "$HTTPD_URL/smart/no-uri.git" target-no-uri
++ "$HTTPD_URL/smart/no-uri.git" target-no-uri 2>err &&
++ test_grep "bundle ${SQ}bundle-1${SQ} has no uri" err &&
++ test_grep ! "expected packfile" err
+'
+
+test_expect_success 'bundles advertised with empty URI' '
@@ t/t5558-clone-bundle-uri.sh: test_expect_success 'bundles with newline in target
+ git -C "$HTTPD_DOCUMENT_ROOT_PATH/empty-uri.git" config bundle.bundle-1.creationToken 1 &&
+
+ git -c transfer.bundleURI=true clone \
-+ "$HTTPD_URL/smart/empty-uri.git" target-empty-uri
++ "$HTTPD_URL/smart/empty-uri.git" target-empty-uri 2>err &&
++ test_grep "bundle ${SQ}bundle-1${SQ} has no uri" err &&
++ test_grep ! "expected packfile" err
+'
+
# Do not add tests here unless they use the HTTP server, as they will
-: ---------- > 2: 5d31c12afb bundle-uri: stop sending invalid bundle configuration
---
base-commit: f85a7e662054a7b0d9070e432508831afa214b47
change-id: 20260408-toon-bundle-uri-no-uri-24f661a498aa
^ permalink raw reply
* Re: [PATCH v2] config: retry acquiring config.lock, configurable via core.configLockTimeout
From: Junio C Hamano @ 2026-07-08 14:49 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Patrick Steinhardt, Joerg Thalheim, git
In-Reply-To: <b5c80d76-5ef4-cf1f-f4e1-78e63cfea81b@gmx.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> ... doubt whether the cost of that cache was worth blocking this patch for
> over a month. Lacking such a cozy setting and at this time also lacking
> the leisure to enjoy said beverages, I'd rather go forward with the
> proposed version and move on to more exciting things.
>
> In other words: I consider this patch fine as-is, and in the event that I
> would consider highly unlikely where the cache _really_ bothers anyone, it
> will be an easy patch to remove it.
And until now, we had over a month to see such an add-on patch, or
hear argument like the above that such a patch is not needed. That
is what I find disturbing the most here.
^ permalink raw reply
* Re: [PATCH] merge --abort: don't delete autostash before reset succeeds
From: Phillip Wood @ 2026-07-08 13:35 UTC (permalink / raw)
To: Kris Point, git@vger.kernel.org; +Cc: gitster@pobox.com
In-Reply-To: <SI1PPF1BAF45F0FA46A6EED57B732BB04D7ABFF2@SI1PPF1BAF45F0F.apcprd02.prod.outlook.com>
Hi Kris
On 08/07/2026 02:51, Kris Point wrote:
> From bf4b12438a83d81f2c8df6e39f6114ddd5002430 Mon Sep 17 00:00:00 2001
> From: KrisPointCSGO <KrisPointCSGO@outlook.com>
> Date: Tue, 7 Jul 2026 20:10:00 +0800
> Subject: [PATCH] merge --abort: don't delete autostash before reset succeeds
> To: git@vger.kernel.org
> Cc: gitster@pobox.com
>
> In cmd_merge()'s --abort path, MERGE_AUTOSTASH was deleted before
> cmd_reset() was called. If cmd_reset() failed (e.g. due to a locked
> index), the autostash was permanently lost.
That's bad
> Instead, read the MERGE_AUTOSTASH OID without deleting the ref, run
> cmd_reset() (which itself calls remove_branch_state() ->
> save_autostash_ref() to persist the stash), and only apply the
> autostash on success.
I'm afraid I don't think this is the right solution. We only want to
save the stash if there are conflicts when we apply it - that is why
MERGE_AUTOSTASH is deleted before we do the reset - we want to prevent
remove_branch_state() from saving it. If the stash applies cleanly then
we should not save it. If the reset fails then we should keep
MERGE_AUTOSTASH along with the other merge state files rather than
saving the stash (which is actually what happens after this patch
because cmd_reset() dies before it calls remove_branch_state()).
I think the solution is probably to stop calling
builtin/reset.c:cmd_reset() and instead extend
reset.c:reset_working_tree()[1] to do a "merge" reset by adding a
"RESET_WORKING_TREE_MERGE" flag (or possibly we want to remove
RESET_WORKTING_TREE_HARD from the flags and add a reset_mode member).
Then we can call
struct reset_working_tree opts = {
.flags = RESET_WORKING_TREE_MERGE;
};
if (reset_working_tree(the_repository, &opts))
die(_("could not reset index and working tree"));
apply_autostash_ref(...); /* apply the stash */
remove_branch_state(...); /* remove merge state */
So we only delete MERGE_AUTOSTASH after a successful reset and we only
save the stash if it applies with conflicts. That's all a bit more
involved than the patch here - please do give me a shout if you want
some more information.
Thanks
Phillip
[1] Note that in the master branch this function is called reset_head(),
you should base the fix on top of the "ps/history-drop" branch which
is in "seen" (currently the tip is d11b348f784 (builtin/history:
implement "drop" subcommand, 2026-07-01) but that might change when
Junio rebuilds "seen".
> Reported-by: KrisPoint
> Signed-off-by: KrisPoint <KrisPointCSGO@outlook.com>
> ---
> builtin/merge.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 5b46a596f0..5d9a242027 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1427,15 +1427,14 @@ int cmd_merge(int argc,
> if (!file_exists(git_path_merge_head(the_repository)))
> die(_("There is no merge to abort (MERGE_HEAD missing)."));
>
> - if (!refs_read_ref(get_main_ref_store(the_repository), "MERGE_AUTOSTASH", &stash_oid))
> - refs_delete_ref(get_main_ref_store(the_repository),
> - "", "MERGE_AUTOSTASH", &stash_oid,
> - REF_NO_DEREF);
> + refs_read_ref(get_main_ref_store(the_repository), "MERGE_AUTOSTASH", &stash_oid);
>
> - /* Invoke 'git reset --merge' */
> + /* Invoke 'git reset --merge' (which also cleans up merge state,
> + * including saving the autostash to the stash list).
> + */
> ret = cmd_reset(nargc, nargv, prefix, the_repository);
>
> - if (!is_null_oid(&stash_oid)) {
> + if (!ret && !is_null_oid(&stash_oid)) {
> oid_to_hex_r(stash_oid_hex, &stash_oid);
> apply_autostash_oid(stash_oid_hex);
> }
^ permalink raw reply
* Re: [PATCH v3 00/12] reftable: harden against corrupted tables
From: Toon Claes @ 2026-07-08 13:19 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: oxsignal, Christian Couder
In-Reply-To: <20260703-pks-reftable-hardening-v3-0-b87c555b9920@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Hi,
>
> this patch series addresses a bunch of errors that may happen when
> trying to read corrupted tables. These errors include out-of-bounds
> writes, out-of-bounds reads and the ability to hit abort(3p) calls.
>
> The out-of-bounds write was originally reported by awo on the security
> mailing list. As we never transfer reftables over the protocol it would
> require local disk access to create such corrupted reftables, so there
> isn't really an easy way to exploit these.
>
> In any case, I took that chance and wrote a fuzzer for parsing the
> tables, which surfaced a bunch of issues. At the end of this series
> though the fuzzer can now run for an extended amount of time (2hrs+)
> without surfacing any new issues.
Great work on providing the fixes with their corresponding unit tests.
I've manually verified if each test, and they all fail correctly without
the code changes.
All looks good to me. The only thing I'm not sure about is whether it's
useful to have the Asan output in the commit messages.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
From: Kristofer Karlsson @ 2026-07-08 12:44 UTC (permalink / raw)
To: René Scharfe
Cc: Junio C Hamano, Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <15fa1b16-b911-47b1-a843-400e320d7e4f@web.de>
On Wed, 8 Jul 2026 at 13:55, René Scharfe <l.s.r@web.de> wrote:
>
> I meant that I didn't find this optimization in other priority queue
> implementations or papers, but admittedly I didn't do an exhaustive
> search. Given it's benefits I would have expected to find prior art
> on it pretty easily, though.
Aha! Got it, I misunderstood you first.
It's actually described here[1]:
> Bottom-up heapsort conceptually replaces the root with a value of −∞
> and sifts it down using only one comparison per level
> (since no child can possibly be less than −∞)
> until the leaves are reached,
> then replaces the −∞ with the correct value and sifts it up
> (again, using one comparison per level) until the correct position
> is found.
It's for heapsort, not an interactive heap, but the algorithm
still matches.
Also, your idea to split out sift up/down into helper functions did
work, and was quite clean - will share patch shortly once I have
cleaned up the commits and benchmark data.
Thanks again,
Kristofer
[1] https://en.wikipedia.org/wiki/Heapsort#Bottom-up_heapsort
^ permalink raw reply
* Re: [PATCH v3 4/4] doc: replay: move “default” to the right-hand side
From: Toon Claes @ 2026-07-08 12:09 UTC (permalink / raw)
To: kristofferhaugsbakk, Junio C Hamano
Cc: Kristoffer Haugsbakk, Siddharth Asthana, git, Patrick Steinhardt
In-Reply-To: <V3_default_RHS.784@msgid.xyz>
kristofferhaugsbakk@fastmail.com writes:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> This is now a description list (see previous commit) and parentheticals
> like this do not go on the left-hand side. Moving it to the other side
> makes it stand out just as much and is also more consistent with the
> rest of the documentation.
>
> Let’s also do the same for the `replay.refAction` description list.
> That makes the two desc. lists identical in the first sentence. Let’s
> add a comment about that for future editors.
Ah, these quad slashes are comments? Well, that's a thing I've learned
today.
Makes sense.
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
>
> Notes (series):
> v2:
> • It’s “description list”, not “definition list”
> • (Same mistake I have done for “line continuation” (it’s “list”))
> • It’s e.g. “right-hand side” (drop “-side” hyphen)
> • Change `replay.refAction` “default” placement
> • Now that these two description lists are so similar, add an
> AsciiDoc comment about it for future editors. Note that I
> outright deleted this list in the previous version because I
> didn’t want to keep them in synch. But we can remain aware of
> these with two comments.
>
> ---
>
> v1:
> > do not go on the left-hand-side.
>
> At least I haven’t seen it.
>
> Documentation/config/replay.adoc | 5 ++++-
> Documentation/git-replay.adoc | 5 ++++-
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config/replay.adoc b/Documentation/config/replay.adoc
> index 7328da9537d..40d1695782a 100644
> --- a/Documentation/config/replay.adoc
> +++ b/Documentation/config/replay.adoc
> @@ -3,7 +3,10 @@ replay.refAction::
> The value can be:
> +
> --
> -`update`;; Update refs directly using an atomic transaction (default behavior).
> +////
> +These use the first sentences from the description list in git-replay(1).
> +////
> +`update`;; (default) Update refs directly using an atomic transaction.
> `print`;; Output update-ref commands for pipeline use.
> --
> +
> diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc
> index b4fe43ec687..ea4d14baddb 100644
> --- a/Documentation/git-replay.adoc
> +++ b/Documentation/git-replay.adoc
> @@ -80,7 +80,10 @@ incompatible with `--contained` (which is a modifier for `--onto` only).
> Control how references are updated. The mode can be:
> +
> --
> -`update` (default);; Update refs directly using an atomic transaction.
> +////
> +Expanded description list compared to 'replay.refAction'.
> +////
> +`update`;; (default) Update refs directly using an atomic transaction.
> All refs are updated or none are (all-or-nothing behavior).
> `print`;; Output update-ref commands for pipeline use. This is the
> traditional behavior where output can be piped to `git update-ref --stdin`.
> --
> 2.54.0.22.g9e26862b904
>
>
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH RFC 2/2] builtin/history: print feedback after successful reword
From: Patrick Steinhardt @ 2026-07-08 12:04 UTC (permalink / raw)
To: D. Ben Knoble
Cc: Dominique Martinet, Pablo Sabater, Junio C Hamano, git,
Kaartic Sivaraam
In-Reply-To: <CALnO6CAjZfK3hPWn1vOxgw=4=cjRYEHabYJmJrpVVDU8yyQn_g@mail.gmail.com>
On Tue, Jul 07, 2026 at 12:10:12PM -0400, D. Ben Knoble wrote:
> On Tue, Jul 7, 2026 at 1:09 AM Dominique Martinet
> <asmadeus@codewreck.org> wrote:
[snip]
> > So I agree with Pablo's suggestion: printing old/new short hash on
> > success would help visualy confirming something worked.
>
> I think we have the machinery for this (see --update-refs=print for
> git-replay, for example), but I'm surprised to learn that we don't
> accept --update-refs=print for history.
>
> In any case, I second the "we should emit something"—I wonder what, though.
>
> - In the case of rewritten refs, we might like to emit the list of
> rewrites, a bit like a fetch or push will do: "+ $old...$new $ref
> (forced update)" or something
> - For new objects that aren't pointed to… maybe silence is a better
> indicator that "we didn't do what you intended"? Or we could just
> print the new commit objects "$new [unreferenced object]" or something
That's exactly my issue, as well. I'm slightly in favor of not writing
anything, but if we're able to figure out how exactly to represent
results to users in a nice and consistent way then I'm very happy to
change my opinion.
But that definitely needs to account not only for the case where the
current HEAD gets rewritten, but it needs to account for any reference
(including detached HEAD) that may be updated along the way.
> > ... But it might be worth to ensure that the commit has any ref we can
> > handle (if --update-refs is set then the commit we edit is ancestor to
> > some branch, if not set then it must be an ancestor of HEAD)
> >
> > What do you think?
>
> I don't think it's worth restricting the operation (I can imagine a
> use case where someone creates an unpointed-to object and later makes
> the ref, even if that's a bit weird), but
>
> - we could have a "strict" mode that ensured inputs are pointed to
> - we could warn when only unreferenced objects are rewritten
>
> ? I see git-history as very "porcelain"/user-focused, so I think it's
> feasible to add output niceties (and optionally a quiet mode to
> suppress the messages).
Yeah, I don't see any issue with having such a "strict" mode, either.
But I definitely don't want to enforce "arbitrary" restrictions that
require the user to work around them. It's intentional that you can
rewrite history of commits that aren't even reachable from HEAD.
It might be sensible to even make the strict mode the default, where you
need to pass a switch to rewrite commits that are not reachable from
HEAD. But if so, we need to have a switch that disables this mode.
Patrick
^ permalink raw reply
* Re: [PATCH v3 2/4] doc: replay: improve config description
From: Toon Claes @ 2026-07-08 12:04 UTC (permalink / raw)
To: kristofferhaugsbakk, Junio C Hamano
Cc: Kristoffer Haugsbakk, Siddharth Asthana, git, Patrick Steinhardt
In-Reply-To: <V3_doc_replay_improve_config.782@msgid.xyz>
kristofferhaugsbakk@fastmail.com writes:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> First of all, this unordered list for `replay.refAction` introduces
> a term with a colon. This is exactly what a description list is,
> structurally. Let’s be stylistically consistent and use the desc.
> list markup construct. Let’s also drop the harmless but unneeded
> indentation.
>
> We can reuse the `::` delimiter since we use an open block.
> But for consistency use the typical nested description list
> delimiter, namely `;;`.
Yeah, looking at some other docs (for example
Documenation/config/branch.adoc) it makes sense to do it like this.
> Second, let’s replace the inline-verbatim `git replay` with a link
> to git-replay(1), since we are naming the command. But make that
> conditional so that we avoid a self-link inside git-replay(1).[1]
>
> † 1: See e.g. e7b3a768 (doc: git-init: rework config item
> init.templateDir, 2024-03-10) for another example of
> avoiding self-linking
>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
>
> Notes (series):
> v3:
> • Msg:[1] typo, fix to “stylistically”
> • Msg: Move the paragraph about delimiters (;;) from the *next*
> patch over here instead. This is the first place we do it. In the
> next patch we can just say that we are doing the same trans-
> formation as here.
> • Msg: Remove double-space to separate two sentences. That’s
> inconsitent for me. I moved away from that because two-space
> separation takes up too much space when linewrapping is set to 72.
> • Msg: This isn’t the option, it is `replay.refAction`
> • Copy–paste mistake? We don’t have to ask
> • Msg: ... and it’s better to call it an unordered list rather than
> bullet points
>
> † 1: Commit message
>
> ---
>
> v2:
> • Keep the description list for `replay.refAction` (Junio)
> • Now rewrite the description list like in patch 1/3 (it’s
> technically an unordered list)
> • Msg: mention a previous commit which also avoided self-linking.
> This helps establish a bit more context for why we do this.
>
> Documentation/config/replay.adoc | 16 ++++++++++------
> Documentation/git-replay.adoc | 1 +
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/config/replay.adoc b/Documentation/config/replay.adoc
> index 7d549d2f0e5..7328da9537d 100644
> --- a/Documentation/config/replay.adoc
> +++ b/Documentation/config/replay.adoc
> @@ -1,11 +1,15 @@
> replay.refAction::
> - Specifies the default mode for handling reference updates in
> - `git replay`. The value can be:
> + Specifies the default mode for handling reference updates.
> + The value can be:
> +
> --
> - * `update`: Update refs directly using an atomic transaction (default behavior).
> - * `print`: Output update-ref commands for pipeline use.
> +`update`;; Update refs directly using an atomic transaction (default behavior).
> +`print`;; Output update-ref commands for pipeline use.
> --
> +
> -This setting can be overridden with the `--ref-action` command-line option.
> -When not configured, `git replay` defaults to `update` mode.
> +ifdef::git-replay[]
> +See `--ref-action`.
> +endif::git-replay[]
> +ifndef::git-replay[]
> +See `--ref-action` for linkgit:git-replay[1] for details.
I'm not sure about using "for" twice, how about:
See `--ref-action` in linkgit:git-replay[1] for details.
> +endif::git-replay[]
> diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc
> index f9ca2db2833..4de85088d6c 100644
> --- a/Documentation/git-replay.adoc
> +++ b/Documentation/git-replay.adoc
> @@ -211,6 +211,7 @@ to use bare commit IDs instead of branch names.
>
> CONFIGURATION
> -------------
> +:git-replay: 1
> include::config/replay.adoc[]
>
> GIT
> --
> 2.54.0.22.g9e26862b904
>
>
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
From: René Scharfe @ 2026-07-08 11:55 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: Junio C Hamano, Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <CAL71e4NiSSRgxO_L7vb5=ohnchOCvuhEZwMc0Ls+Xu-Q+YytDg@mail.gmail.com>
On 7/8/26 12:59 PM, Kristofer Karlsson wrote:
> On Wed, 8 Jul 2026 at 12:44, René Scharfe <l.s.r@web.de> wrote:
>>
>> I didn't
>> find this method used anywhere else, which is a warning sign, but I
>> can't find any catch.
>
> I am not sure why it's a warning sign to have no other usages,
> especially when it's a file local static function.
I meant that I didn't find this optimization in other priority queue
implementations or papers, but admittedly I didn't do an exhaustive
search. Given it's benefits I would have expected to find prior art
on it pretty easily, though.
René
^ permalink raw reply
* Re: What's cooking in git.git (Jul 2026, #03)
From: Kristofer Karlsson @ 2026-07-08 11:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqeche67lr.fsf@gitster.g>
On Tue, 7 Jul 2026 at 19:19, Junio C Hamano <gitster@pobox.com> wrote:
>
> * kk/commit-reach-find-all-fix (2026-06-29) 2 commits
> - commit-reach: guard !FIND_ALL early exit with generation ordering check
> - t6600: add test for merge-base early exit with clock skew
>
> The early-exit optimization in 'paint_down_to_common()' has been gated
> on the queue being generation-ordered, fixing a bug where 'git merge-
> base' (without '--all') could return incorrect results on repositories
> with v1 commit graphs and clock skew.
>
> Comments?
> cf. <xmqqa4sdw55v.fsf@gitster.g>
> source: <pull.2162.git.1782739162.gitgitgadget@gmail.com>
[snip]
> * kk/merge-base-exhaustion (2026-07-01) 10 commits
> . commit-reach: remove commit-date ordering fallback
> . commit-reach: move min_generation check into paint_queue_get()
> . commit-reach: terminate merge-base walk when one paint side is exhausted
> . commit-reach: introduce struct paint_state with per-side counters
> . t6600: add clock-skew topologies and step counts for edge cases
> . commit-reach: add trace2 instrumentation to paint_down_to_common()
> . t6099, t6600: add side-exhaustion regression tests
> . t6600: add test cases for side-exhaustion edge cases
> . test-lib-functions: improve diagnostic output for trace2 data assertions
> . Documentation/technical: add paint-down-to-common doc
>
> The merge-base computation has been optimized by stopping the walk
> early when one side's exclusive commits in the queue are exhausted,
> yielding significant speedups for queries with one-sided histories.
>
> Expecting a reroll.
> cf. <CAL71e4PgcZDK-gJziJa_yjEqX9TE+PFMwZn0xbjAUzuUDDDBYA@mail.gmail.com>
> source: <pull.2149.v5.git.1782923832.gitgitgadget@gmail.com>
Small note regarding these two - kk/merge-base-exhaustion is
(unfortunately) dependent on kk/commit-reach-find-all-fix
before a reroll.
I tried building v6 of kk/merge-base-exhaustion on top of
kk/commit-reach-find-all-fix but since that one is based
on kk/paint-down-to-common-optim it does not include
the changes from kk/commit-reach-optim which I also depend
on.
I thus think the status of kk/merge-base-exhaustion should
instead be:
"On hold, waiting for kk/commit-reach-find-all-fix to land first."
Alternatively you could rebase kk/commit-reach-find-all-fix
on master (triggers a small conflict though) and that would
also unblock a reroll (but I don't want to generate more work for you).
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH 0/2] reftable: fix quadratic behavior when re-creating deleted refs
From: brian m. carlson @ 2026-07-08 11:15 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget; +Cc: git, Kristofer Karlsson
In-Reply-To: <pull.2166.git.1783344957.gitgitgadget@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]
On 2026-07-06 at 13:35:54, Kristofer Karlsson via GitGitGadget wrote:
> This series fixes quadratic behavior in update-ref when many refs are
> deleted (tombstoned) and then new refs are created with the reftable
> backend.
[…]
> The first patch adds tests for tombstone scenarios: a perf test (p1401)
> exercising two patterns with 8000 refs, and a correctness test (t0610)
> verifying that deleted-then-recreated refs are visible.
>
> The second patch is the pure optimization. Both p1401 tests go from ~14s to
> ~0.2s with the fix.
>
> Note that auto-compaction typically merges tombstones before they accumulate
> to this degree, so the quadratic behavior may not show up in every workflow.
> But the fix ensures correct time complexity regardless of compaction state,
> and the change is fairly contained.
I had hit this before when doing some benchmarks for using reftable at
$DAYJOB. We had discussed it on the list and decided that it was
synthetic at the time, but I'm glad to see that this is being fixed now.
I don't have comments on the patches themselves because I haven't spent
enough time in the reftable code to be familiar with it, but I do
definitely appreciate the performance improvement.
--
brian m. carlson (they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]
^ permalink raw reply
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
From: Kristofer Karlsson @ 2026-07-08 10:59 UTC (permalink / raw)
To: René Scharfe
Cc: Junio C Hamano, Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <57bb0e9e-221d-4234-b5bc-a87610e8263c@web.de>
On Wed, 8 Jul 2026 at 12:44, René Scharfe <l.s.r@web.de> wrote:
>
> tl;dr: Yes, please, but I'm biased.
>
> The text size of prio-queue.o on Apple silicon increases from 1351 to
> 1563 bytes for me, 212 bytes or 16% more. OK.
>
> It makes intuitive sense to find the new position of the last item by
> searching from the bottom up instead of from the top down. Timings
> confirm it. Are there pathologic cases that perform worse, though? I
> don't see how to construct one. It would require an unbalanced heap,
> where the bottom items from one branch would rise high in other
> branches. Is this even possible?
Agreed, I also struggle to come up with such a case. Perhaps
theoretically possible to construct, but would not invalidate
the general heuristic?
> For a full drain (only _get(), no _put()) of up to 12 items the answer
> is no, at least. Cascade never needs more comparisons for any
> permutation; test code below. Here are the aggregate numbers:
>
> next cascade
> n min max mean min max mean
> 2 0 0 0.0 0 0 0.0
> 3 1 1 1.0 1 1 1.0
> 4 3 3 3.0 3 3 3.0
> 5 5 6 5.8 5 6 5.6
> 6 7 10 8.7 7 9 8.0
> 7 10 14 12.0 9 12 10.9
> 8 14 18 16.3 12 16 13.9
> 9 18 23 20.9 15 20 17.4
> 10 22 29 25.5 18 24 20.7
> 11 26 35 30.5 21 28 24.4
> 12 30 41 35.5 24 33 27.9
I am sincerely grateful that you took the time to analyze it
to this level of detail. Very nice analysis and data!
> sift_up_rebalance() is a combination of sift_down_root() with an empty
> root and the bubble-up operation from prio_queue_put(). The latter can
> easily be factored out into a sift-up function, reducing code
> duplication.
Good point! I am not sure how messy this gets in practice, but I
will see if I can implement this split for the next patch.
> Extending sift_down_root() to deal with an empty root would be easy as
> well, but also a bit tricky to avoid pointless checks for each caller.
> Not sure it's worth it. Like this perhaps?
>
> static inline size_t sift_down_root(struct prio_queue *queue, bool empty)
> {
> size_t ix, child;
>
> /* Push down the one at the root */
> for (ix = 0; ix * 2 + 1 < queue->nr_; ix = child) {
> child = ix * 2 + 1; /* left */
> if (child + 1 < queue->nr_ &&
> compare(queue, child, child + 1) >= 0)
> child++; /* use right child */
>
> if (empty)
> queue->array[ix] = queue->array[child];
> else if (compare(queue, ix, child) <= 0)
> break;
> else
> swap(queue, child, ix);
> }
> return ix;
> }
Yes, something like that would work, but I agree -- ideally we
could have something that's even nicer and avoids the boolean flag
for split behavior.
> Anyway, my point is that it's not "adding another sift function", but
> remixing existing ones, which I only count as half. :)
>
> I'd very much like to see this go in because it seems to be strictly
> faster, makes intuitive sense and adds only little code. I didn't
> find this method used anywhere else, which is a warning sign, but I
> can't find any catch.
Thanks, I think that is enough motivation for me to at least attempt
another version and then it will be easier to reason about dropping
or keeping.
I am not sure why it's a warning sign to have no other usages,
especially when it's a file local static function. I guess it could
be inlined instead (though I would not prefer that).
Thanks for the very thorough and insightful review,
Kristofer
^ permalink raw reply
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
From: René Scharfe @ 2026-07-08 10:43 UTC (permalink / raw)
To: Kristofer Karlsson, Junio C Hamano
Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <CAL71e4NZYdpw5cvi6ARn1req8xaRGGg9X4xhZKp6S9Dz4K23aQ@mail.gmail.com>
On 7/6/26 11:52 PM, Kristofer Karlsson wrote:
> On Sun, 7 Jun 2026 at 09:30, René Scharfe <l.s.r@web.de> wrote:
>>
>> So I guess we keep the full sift-down for prio_queue_replace(), knowing
>> that sometimes we have a lot of items that end up at or close to the
>> root of the heap.
>
> The lazy-fold series (kk/prio-queue-get-put-fusion) is in next now.
> I rebased this cascade patch on top of it to check if it's still
> useful.
>
> With lazy-fold in place the regression scenario you identified
> is resolved. The only remaining change is in flush_get(),
> where unfused gets now cascade instead of sifting down:
>
> - queue->array[0] = queue->array[--queue->nr_];
> - sift_down_root(queue);
> + --queue->nr_;
> + sift_up_rebalance(queue);
>
> plus the ~20-line sift_up_rebalance() implementation.
>
> I benchmarked this on the linux kernel repo and on a large
> merge-heavy repo.
>
> The results are consistent: a real but small 1-2% end-to-end
> improvement across commands. A prio-queue microbenchmark
> would likely show a larger difference, but the queue
> is only a fraction of the total work in any real git operation.
>
> The lazy-fold optimization cannibalized some of the value here,
> so cascade only helps the remaining unfused gets. As you observed,
> cascade is better there, but there are fewer of them now that there
> is more fusing happening.
>
> I am on the fence about whether 1-2% end-to-end justifies adding
> another sift function. If you (René and Junio) think the benefit
> is too small for the code cost, I am happy to drop this patch.
> Otherwise I can submit a small reroll on top of
> kk/prio-queue-get-put-fusion (or rather next, in practice).
tl;dr: Yes, please, but I'm biased.
The text size of prio-queue.o on Apple silicon increases from 1351 to
1563 bytes for me, 212 bytes or 16% more. OK.
It makes intuitive sense to find the new position of the last item by
searching from the bottom up instead of from the top down. Timings
confirm it. Are there pathologic cases that perform worse, though? I
don't see how to construct one. It would require an unbalanced heap,
where the bottom items from one branch would rise high in other
branches. Is this even possible?
For a full drain (only _get(), no _put()) of up to 12 items the answer
is no, at least. Cascade never needs more comparisons for any
permutation; test code below. Here are the aggregate numbers:
next cascade
n min max mean min max mean
2 0 0 0.0 0 0 0.0
3 1 1 1.0 1 1 1.0
4 3 3 3.0 3 3 3.0
5 5 6 5.8 5 6 5.6
6 7 10 8.7 7 9 8.0
7 10 14 12.0 9 12 10.9
8 14 18 16.3 12 16 13.9
9 18 23 20.9 15 20 17.4
10 22 29 25.5 18 24 20.7
11 26 35 30.5 21 28 24.4
12 30 41 35.5 24 33 27.9
sift_up_rebalance() is a combination of sift_down_root() with an empty
root and the bubble-up operation from prio_queue_put(). The latter can
easily be factored out into a sift-up function, reducing code
duplication.
Extending sift_down_root() to deal with an empty root would be easy as
well, but also a bit tricky to avoid pointless checks for each caller.
Not sure it's worth it. Like this perhaps?
static inline size_t sift_down_root(struct prio_queue *queue, bool empty)
{
size_t ix, child;
/* Push down the one at the root */
for (ix = 0; ix * 2 + 1 < queue->nr_; ix = child) {
child = ix * 2 + 1; /* left */
if (child + 1 < queue->nr_ &&
compare(queue, child, child + 1) >= 0)
child++; /* use right child */
if (empty)
queue->array[ix] = queue->array[child];
else if (compare(queue, ix, child) <= 0)
break;
else
swap(queue, child, ix);
}
return ix;
}
Anyway, my point is that it's not "adding another sift function", but
remixing existing ones, which I only count as half. :)
I'd very much like to see this go in because it seems to be strictly
faster, makes intuitive sense and adds only little code. I didn't
find this method used anywhere else, which is a warning sign, but I
can't find any catch.
René
$ for n in $(seq 2 2)
do
t/helper/test-tool prio-queue permute get $n |
awk -v n=$n -v max=0 '
{sum+=$2}
max < $2 {max=$2}
!min || min > $2 {min=$2}
END {printf "%2d %3d %3d %5.1f \n", n, min, max, sum/NR}
'
done
---
Makefile | 1 +
t/helper/test-prio-queue.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
4 files changed, 94 insertions(+)
diff --git a/Makefile b/Makefile
index 1f3f099f5c5..ba7d293cf5f 100644
--- a/Makefile
+++ b/Makefile
@@ -843,6 +843,7 @@ TEST_BUILTINS_OBJS += test-partial-clone.o
TEST_BUILTINS_OBJS += test-path-utils.o
TEST_BUILTINS_OBJS += test-path-walk.o
TEST_BUILTINS_OBJS += test-pcre2-config.o
+TEST_BUILTINS_OBJS += test-prio-queue.o
TEST_BUILTINS_OBJS += test-pkt-line.o
TEST_BUILTINS_OBJS += test-proc-receive.o
TEST_BUILTINS_OBJS += test-progress.o
diff --git a/t/helper/test-prio-queue.c b/t/helper/test-prio-queue.c
new file mode 100644
index 00000000000..c175021b12b
--- /dev/null
+++ b/t/helper/test-prio-queue.c
@@ -0,0 +1,91 @@
+#include "test-tool.h"
+#include "prio-queue.h"
+
+/* Generate all permutations using Heap's algorithm. */
+static int permute_ints(size_t n, void (*fn)(int *, size_t))
+{
+ int *arr;
+ size_t *c;
+
+ ALLOC_ARRAY(arr, n);
+ for (size_t i = 0; i < n; i++)
+ arr[i] = i + 1;
+ CALLOC_ARRAY(c, n);
+
+ fn(arr, n);
+ for (size_t i = 1; i < n; i++) {
+ if (c[i] < i) {
+ SWAP(arr[i & 1 ? c[i] : 0], arr[i]);
+ fn(arr, n);
+ c[i]++;
+ i = 0;
+ } else {
+ c[i] = 0;
+ }
+ }
+
+ free(arr);
+ free(c);
+
+ return 0;
+}
+
+static uintmax_t nr_of_compares;
+
+static int compare_ints(const void *a_, const void *b_, void *cb_data UNUSED)
+{
+ const int *a = a_;
+ const int *b = b_;
+ nr_of_compares++;
+ return *a - *b;
+}
+
+static void report(const char *name, const int *arr, size_t n)
+{
+ printf("%s %"PRIuMAX" for", name, nr_of_compares);
+ for (size_t i = 0; i < n; i++)
+ printf(" %d", arr[i]);
+ putchar('\n');
+}
+
+static void get_permutation(int *arr, size_t n)
+{
+ static struct prio_queue queue = { compare_ints };
+
+ for (size_t i = 0; i < n; i++)
+ prio_queue_put(&queue, &arr[i]);
+
+ nr_of_compares = 0;
+ for (size_t i = 0; i < n; i++)
+ prio_queue_get(&queue);
+
+ report("get", arr, n);
+}
+
+static void put_permutation(int *arr, size_t n)
+{
+ struct prio_queue queue = { compare_ints };
+
+ nr_of_compares = 0;
+ for (size_t i = 0; i < n; i++)
+ prio_queue_put(&queue, &arr[i]);
+
+ report("put", arr, n);
+
+ clear_prio_queue(&queue);
+}
+
+int cmd__prio_queue(int argc, const char **argv)
+{
+ if (argc == 4 && !strcmp(argv[1], "permute")) {
+ size_t n = strtoul(argv[3], NULL, 10);
+ if (!strcmp(argv[2], "get"))
+ return permute_ints(n, get_permutation);
+ if (!strcmp(argv[2], "put"))
+ return permute_ints(n, put_permutation);
+ }
+
+ fprintf(stderr, "usage: test-tool prio-queue permute get <n>\n");
+ fprintf(stderr, " or: test-tool prio-queue permute put <n>\n");
+ return 129;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index b71a22b43bb..69352f541f4 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -57,6 +57,7 @@ static struct test_cmd cmds[] = {
{ "path-walk", cmd__path_walk },
{ "pcre2-config", cmd__pcre2_config },
{ "pkt-line", cmd__pkt_line },
+ { "prio-queue", cmd__prio_queue },
{ "proc-receive", cmd__proc_receive },
{ "progress", cmd__progress },
{ "reach", cmd__reach },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index f2885b33d58..ab0d3e01d1e 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -50,6 +50,7 @@ int cmd__path_utils(int argc, const char **argv);
int cmd__path_walk(int argc, const char **argv);
int cmd__pcre2_config(int argc, const char **argv);
int cmd__pkt_line(int argc, const char **argv);
+int cmd__prio_queue(int argc, const char **argv);
int cmd__proc_receive(int argc, const char **argv);
int cmd__progress(int argc, const char **argv);
int cmd__reach(int argc, const char **argv);
^ permalink raw reply related
* Re: [PATCH v2 00/13] setup: split up repository discovery and setup
From: Toon Claes @ 2026-07-08 9:42 UTC (permalink / raw)
To: Junio C Hamano, Justin Tobler; +Cc: Patrick Steinhardt, git
In-Reply-To: <xmqqldbm4r86.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Justin Tobler <jltobler@gmail.com> writes:
>
>> The changes in this version look good to me. Thanks.
>
> Thanks, both. These indeed look good.
It's nice to see the discovery being kept in this new struct that holds
accurate information. There is quite some code shuffled around through
this series, but the changes look sensible. So as far as I am concerned,
I agree this series looks good.
Cheers,
Toon
^ 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