* [PATCH v11 06/10] environment: migrate apply_default_whitespace and apply_default_ignorewhitespace
From: Tian Yuchen @ 2026-07-13 3:57 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, cirnovskyv, szeder.dev, Tian Yuchen,
Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260713035738.1606138-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 | 28 ++++++++++++++++++++--------
environment.c | 6 ++++--
environment.h | 4 ++--
3 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/apply.c b/apply.c
index 249248d4f2..f0cfd76190 100644
--- a/apply.c
+++ b/apply.c
@@ -47,11 +47,17 @@ 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);
+ struct repo_config_values *cfg = repo_config_values(repo);
+
+ FREE_AND_NULL(cfg->apply_default_whitespace);
+ repo_config_get_string(repo, "apply.whitespace",
+ &cfg->apply_default_whitespace);
+ FREE_AND_NULL(cfg->apply_default_ignorewhitespace);
+ repo_config_get_string(repo, "apply.ignorewhitespace",
+ &cfg->apply_default_ignorewhitespace);
+ repo_config(repo, git_xmerge_config, NULL);
}
static int parse_whitespace_option(struct apply_state *state, const char *option)
@@ -126,10 +132,15 @@ 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);
+
+ struct repo_config_values *cfg = repo_config_values(repo);
+
+ if (cfg->apply_default_whitespace &&
+ parse_whitespace_option(state, cfg->apply_default_whitespace))
return -1;
- if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+ if (cfg->apply_default_ignorewhitespace &&
+ parse_ignorewhitespace_option(state, cfg->apply_default_ignorewhitespace))
return -1;
return 0;
}
@@ -192,7 +203,8 @@ 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 3857818da3..20500658a2 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;
@@ -726,6 +724,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;
@@ -745,4 +745,6 @@ void repo_config_values_clear(struct repo_config_values *cfg)
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 856dc70cc4..f450242ac0 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 v11 05/10] environment: move askpass_program into repo_config_values
From: Tian Yuchen @ 2026-07-13 3:57 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, cirnovskyv, szeder.dev, Tian Yuchen,
Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260713035738.1606138-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 | 7 ++++---
environment.h | 3 +--
prompt.c | 3 ++-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/environment.c b/environment.c
index 975c9cb9eb..3857818da3 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 *askpass_program;
enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
enum eol core_eol = EOL_UNSET;
int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
@@ -464,8 +463,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")) {
@@ -726,6 +725,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;
@@ -744,4 +744,5 @@ void repo_config_values_clear(struct repo_config_values *cfg)
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 39b6691b47..856dc70cc4 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;
@@ -220,8 +221,6 @@ const char *get_commit_output_encoding(void);
extern char *git_commit_encoding;
extern char *git_log_output_encoding;
-extern char *askpass_program;
-
/*
* The character that begins a commented line in user-editable file
* that is subject to stripspace.
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 v11 04/10] environment: move pager_program into repo_config_values
From: Tian Yuchen @ 2026-07-13 3:57 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, cirnovskyv, szeder.dev, Tian Yuchen,
Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260713035738.1606138-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()'. All current callers indeed pass
'the_repository', so this new enforcement does not harm them.
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 memory leaks 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 +
| 32 +++++++++++++++++++++++---------
3 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/environment.c b/environment.c
index a65d575af4..975c9cb9eb 100644
--- a/environment.c
+++ b/environment.c
@@ -725,6 +725,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;
@@ -742,4 +743,5 @@ void repo_config_values_clear(struct repo_config_values *cfg)
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 8178ebab76..39b6691b47 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..543ef12936 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,17 @@ 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)
{
- if (!strcmp(var, "core.pager"))
- return git_config_string(&pager_program, var, value);
+ struct repository *r = data;
+
+ if (!strcmp(var, "core.pager")) {
+ struct repo_config_values *cfg = repo_config_values(r);
+
+ FREE_AND_NULL(cfg->pager_program);
+ return git_config_string(&cfg->pager_program, var, value);
+ }
+
return 0;
}
@@ -91,10 +99,12 @@ const char *git_pager(struct repository *r, int stdout_is_tty)
pager = getenv("GIT_PAGER");
if (!pager) {
- if (!pager_program)
+ struct repo_config_values *cfg = repo_config_values(r);
+
+ if (!cfg->pager_program)
read_early_config(r,
- core_pager_config, NULL);
- pager = pager_program;
+ core_pager_config, r);
+ pager = cfg->pager_program;
}
if (!pager)
pager = getenv("PAGER");
@@ -302,7 +312,11 @@ 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) {
+ struct repo_config_values *cfg = repo_config_values(r);
+
+ free(cfg->pager_program);
+ cfg->pager_program = data.value;
+ }
return data.want;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v11 03/10] environment: move editor_program into repo_config_values
From: Tian Yuchen @ 2026-07-13 3:57 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, cirnovskyv, szeder.dev, Tian Yuchen,
Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260713035738.1606138-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..0d1cb8768d 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)
+ 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 275931c213..a65d575af4 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;
@@ -437,8 +436,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") ||
@@ -725,6 +724,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;
@@ -741,4 +741,5 @@ void repo_config_values_clear(struct repo_config_values *cfg)
{
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 4776ccc657..8178ebab76 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 v11 02/10] environment: move excludes_file into repo_config_values
From: Tian Yuchen @ 2026-07-13 3:57 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, cirnovskyv, szeder.dev, Tian Yuchen,
Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260713035738.1606138-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 | 17 ++++++++++++++---
environment.h | 4 +++-
3 files changed, 19 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 ae05f16d04..275931c213 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,16 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
}
+const char *repo_excludes_file(struct repository *repo)
+{
+ struct repo_config_values *cfg = repo_config_values(repo);
+
+ if (!cfg->excludes_file)
+ cfg->excludes_file = xdg_config_home("ignore");
+
+ return cfg->excludes_file;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -461,8 +470,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 +724,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;
@@ -730,4 +740,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
void repo_config_values_clear(struct repo_config_values *cfg)
{
FREE_AND_NULL(cfg->attributes_file);
+ FREE_AND_NULL(cfg->excludes_file);
}
diff --git a/environment.h b/environment.h
index 9169d7f62d..4776ccc657 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 v11 01/10] repository: introduce repo_config_values_clear()
From: Tian Yuchen @ 2026-07-13 3:57 UTC (permalink / raw)
To: git
Cc: pabloosabaterr, cirnovskyv, szeder.dev, Tian Yuchen,
Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260713035738.1606138-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'.
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 | 5 +++++
environment.h | 9 +++++++++
repository.c | 1 +
3 files changed, 15 insertions(+)
diff --git a/environment.c b/environment.c
index ba2c60103f..ae05f16d04 100644
--- a/environment.c
+++ b/environment.c
@@ -726,3 +726,8 @@ 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 repo_config_values *cfg)
+{
+ FREE_AND_NULL(cfg->attributes_file);
+}
diff --git a/environment.h b/environment.h
index 6f18286955..9169d7f62d 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 repo_config_values *cfg);
+
/*
* 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..669e2d1200 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->config_values_private_);
if (repo->config) {
git_configset_clear(repo->config);
--
2.43.0
^ permalink raw reply related
* [PATCH v11 00/10] migrate more variables into repo_config_values
From: Tian Yuchen @ 2026-07-13 3:57 UTC (permalink / raw)
To: git; +Cc: pabloosabaterr, cirnovskyv, szeder.dev, Tian Yuchen
In-Reply-To: <20260712111734.1073514-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.
edit comment (commit 10):
Adjust the comment for config_values_private_ in repository.h.
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?
Changes since v10:
- use repo_config_values *cfg to avoid multiple calls to
repo_config_values() and avoid overly long lines.
- drop the extern declarations for askpass_program.
- in the commit message of pager_program migration, mention that the
new assertion is fine since current callers pass the_repository only.
- add FREE_AND_NULL()s before repo_config_get_strings() calls.
- create a new commit to adjust the comment for config_values_private_
since it was no longer true.
Special thanks to Pablo and Junio!
Tian Yuchen (10):
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
repository: adjust the comment of config_values_private_
apply.c | 28 ++++++++++++------
branch.c | 2 +-
builtin/push.c | 10 ++++---
dir.c | 4 +--
editor.c | 4 +--
environment.c | 76 ++++++++++++++++++++++++++++++++-----------------
environment.h | 77 ++++++++++++++++++++++++++++++--------------------
object-file.c | 3 +-
pager.c | 32 +++++++++++++++------
prompt.c | 3 +-
remote.c | 2 +-
repository.c | 1 +
repository.h | 2 +-
13 files changed, 158 insertions(+), 86 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH 0/4] send-pack: introduce a `no-ref-delta` capability
From: Taylor Blau @ 2026-07-13 1:14 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <alQ7U8TOWjhasaWk@com-79390>
On Sun, Jul 12, 2026 at 06:11:47PM -0700, Taylor Blau wrote:
> This series teaches 'send-pack' to avoid writing `REF_DELTA` entries
> when the receiving end asks it to.
Hmmph. ISTM that my scripts for sending patches to the list somehow
broke the Message-ID of the cover letter, so the patches themselves are
not properly connected to this thread.
I'll investigate why that is separately, but in the meantime, the actual
patches may be found beginning here:
https://lore.kernel.org/git/alQ7WKITYDXfiVn9@com-79390/T/#meaec3602fcf2e3c6d05f7248239c1b167a1e6ddf
Thanks,
Taylor
^ permalink raw reply
* [PATCH 4/4] send-pack: honor `no-ref-delta` capability
From: Taylor Blau @ 2026-07-13 1:12 UTC (permalink / raw)
To: git, git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1783905084.git.ttaylorr@openai.com>
Add a 'no-ref-delta' receive-pack capability and teach send-pack to pass
'--no-ref-delta' to 'pack-objects' when the server advertises it.
Keep this separate from 'ofs-delta' so that a server may request that
`send-pack` omit `REF_DELTA` without also accepting `OFS_DELTA`.
Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
Documentation/gitprotocol-capabilities.adoc | 17 ++++++++++++++---
builtin/receive-pack.c | 5 +++++
send-pack.c | 4 ++++
send-pack.h | 1 +
t/t5516-fetch-push.sh | 14 ++++++++++++++
5 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/Documentation/gitprotocol-capabilities.adoc b/Documentation/gitprotocol-capabilities.adoc
index 2cf7735be4..bbe88defdf 100644
--- a/Documentation/gitprotocol-capabilities.adoc
+++ b/Documentation/gitprotocol-capabilities.adoc
@@ -34,9 +34,9 @@ were sent. Server MUST NOT ignore capabilities that client requested
and server advertised. As a consequence of these rules, server MUST
NOT advertise capabilities it does not understand.
-The 'atomic', 'report-status', 'report-status-v2', 'delete-refs', 'quiet',
-and 'push-cert' capabilities are sent and recognized by the receive-pack
-(push to server) process.
+The 'atomic', 'report-status', 'report-status-v2', 'delete-refs',
+'no-ref-delta', 'quiet', and 'push-cert' capabilities are sent and
+recognized by the receive-pack (push to server) process.
The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized
by both upload-pack and receive-pack protocols. The 'agent' and 'session-id'
@@ -174,6 +174,17 @@ The server can send, and the client can understand, PACKv2 with delta referring
its base by position in pack rather than by an obj-id. That is, they can
send/read OBJ_OFS_DELTA (aka type 6) in a packfile.
+no-ref-delta
+------------
+
+The receive-pack server can request, and the client can send, PACKv2
+without deltas referring to their bases by an obj-id. That is, the
+client MUST NOT send OBJ_REF_DELTA (aka type 7) in a packfile when the
+server advertises this capability.
+
+This does not imply that the server understands OBJ_OFS_DELTA entries;
+that is negotiated separately with the 'ofs-delta' capability.
+
agent
-----
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 19eb6a1b61..1c516cbdc6 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -66,6 +66,7 @@ static struct strbuf fsck_msg_types = STRBUF_INIT;
static int receive_unpack_limit = -1;
static int transfer_unpack_limit = -1;
static int advertise_atomic_push = 1;
+static int advertise_no_ref_delta;
static int advertise_push_options;
static int advertise_sid;
static int unpack_limit = 100;
@@ -290,6 +291,8 @@ static void show_ref(const char *path, const struct object_id *oid)
strbuf_addstr(&cap, " atomic");
if (prefer_ofs_delta)
strbuf_addstr(&cap, " ofs-delta");
+ if (advertise_no_ref_delta)
+ strbuf_addstr(&cap, " no-ref-delta");
if (push_cert_nonce)
strbuf_addf(&cap, " push-cert=%s", push_cert_nonce);
if (advertise_push_options)
@@ -2631,6 +2634,8 @@ int cmd_receive_pack(int argc,
OPT_HIDDEN_BOOL(0, "http-backend-info-refs", &advertise_refs, NULL),
OPT_ALIAS(0, "advertise-refs", "http-backend-info-refs"),
OPT_HIDDEN_BOOL(0, "reject-thin-pack-for-testing", &reject_thin, NULL),
+ OPT_HIDDEN_BOOL(0, "advertise-no-ref-delta-for-testing",
+ &advertise_no_ref_delta, NULL),
OPT_END()
};
diff --git a/send-pack.c b/send-pack.c
index 3bb5afc687..2beb1c4be9 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -80,6 +80,8 @@ static int pack_objects(struct repository *r,
strvec_push(&po.args, "--thin");
if (args->use_ofs_delta)
strvec_push(&po.args, "--delta-base-offset");
+ if (args->no_ref_delta)
+ strvec_push(&po.args, "--no-ref-delta");
if (args->quiet || !args->progress)
strvec_push(&po.args, "-q");
if (args->progress)
@@ -570,6 +572,8 @@ int send_pack(struct repository *r,
allow_deleting_refs = 1;
if (server_supports("ofs-delta"))
args->use_ofs_delta = 1;
+ if (server_supports("no-ref-delta"))
+ args->no_ref_delta = 1;
if (server_supports("side-band-64k"))
use_sideband = 1;
if (server_supports("quiet"))
diff --git a/send-pack.h b/send-pack.h
index 13850c98bb..30be2be0f2 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -28,6 +28,7 @@ struct send_pack_args {
force_update:1,
use_thin_pack:1,
use_ofs_delta:1,
+ no_ref_delta:1,
dry_run:1,
/* One of the SEND_PACK_PUSH_CERT_* constants. */
push_cert:2,
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 1b986349a8..c00074afe8 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1548,6 +1548,20 @@ EOF
git push --no-thin --receive-pack="$rcvpck" no-thin/.git refs/heads/main:refs/heads/foo
'
+test_expect_success 'push honors no-ref-delta capability' '
+ test_commit no-ref-delta &&
+
+ rcvpck="git receive-pack --advertise-no-ref-delta-for-testing" &&
+
+ GIT_TRACE2_EVENT="$PWD/no-ref-delta" \
+ git push --receive-pack="$rcvpck" no-thin/.git \
+ refs/heads/main:refs/heads/bar &&
+
+ test_subcommand git pack-objects --all-progress-implied --revs \
+ --stdout --thin --delta-base-offset --no-ref-delta -q \
+ <no-ref-delta
+'
+
test_expect_success 'pushing a tag pushes the tagged object' '
blob=$(echo unreferenced | git hash-object -w --stdin) &&
git tag -m foo tag-of-blob $blob &&
--
2.55.0
^ permalink raw reply related
* [PATCH 3/4] pack-objects: support reuse with `--no-ref-delta`
From: Taylor Blau @ 2026-07-13 1:12 UTC (permalink / raw)
To: git, git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1783905084.git.ttaylorr@openai.com>
The previous commit disables delta- and bitmap-reuse entirely whenever
pack-objects is given '--no-ref-delta' for the sake of simplicity. This
is overly pessimistic.
When '--delta-base-offset' is also given, delta reuse can remain
enabled. A reused delta whose base is written earlier in the output can
be encoded as an `OFS_DELTA`, even when its source copy was encoded as a
`REF_DELTA`.
Preferred bases and external thin-pack bases are different: neither
appears in the output, so deltas against either still require encoding
the object as a `REF_DELTA`, and thus cannot be reused.
Without '--delta-base-offset', delta reuse remains disabled, since no
delta representation remains.
Bitmap reuse follows a different path, since selected entries may be
copied without passing through the code which chooses a delta
representation. When given '--no-ref-delta', we must inspect candidate
objects individually, and leave `REF_DELTA` entries to the normal object
path outside of pack-reuse.
We must likewise avoid the special-case for reusing either the single or
preferred pack corresponding to the bitmap by whole `eword_t`'s at a
time.
Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
builtin/pack-objects.c | 17 +++++++++++++----
pack-bitmap.c | 30 ++++++++++++++++++++----------
pack-bitmap.h | 3 ++-
t/t5300-pack-object.sh | 21 ++++++++++++++++++++-
t/t5332-multi-pack-reuse.sh | 16 ++++++++++++++++
5 files changed, 71 insertions(+), 16 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c3574fcb8a..43cd4be2e5 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2207,6 +2207,13 @@ static int can_reuse_delta(const struct object_id *base_oid,
*/
base = packlist_find(&to_pack, base_oid);
if (base) {
+ /*
+ * A preferred base is omitted from the resulting pack, so it
+ * can only be referenced by object ID.
+ */
+ if (base->preferred_base && !allow_ref_delta)
+ return 0;
+
if (!in_same_island(&delta->idx.oid, &base->idx.oid))
return 0;
*base_out = base;
@@ -2218,7 +2225,8 @@ static int can_reuse_delta(const struct object_id *base_oid,
* even if it was buried too deep in history to make it into the
* packing list.
*/
- if (thin && bitmap_has_oid_in_uninteresting(bitmap_git, base_oid)) {
+ if (allow_ref_delta && thin &&
+ bitmap_has_oid_in_uninteresting(bitmap_git, base_oid)) {
if (use_delta_islands) {
if (!in_same_island(&delta->idx.oid, base_oid))
return 0;
@@ -4668,7 +4676,7 @@ static int pack_options_allow_reuse(void)
!ignore_packed_keep_on_disk &&
!ignore_packed_keep_in_core &&
(!local || !have_non_local_packs) &&
- !incremental && allow_ref_delta;
+ !incremental && (allow_ref_delta || allow_ofs_delta);
}
static int get_object_list_from_bitmap(struct rev_info *revs)
@@ -4690,7 +4698,8 @@ static int get_object_list_from_bitmap(struct rev_info *revs)
&reuse_packfiles,
&reuse_packfiles_nr,
&reuse_packfile_bitmap,
- allow_pack_reuse == MULTI_PACK_REUSE);
+ allow_pack_reuse == MULTI_PACK_REUSE,
+ allow_ref_delta);
if (reuse_packfiles) {
reuse_packfile_objects = bitmap_popcount(reuse_packfile_bitmap);
@@ -5317,7 +5326,7 @@ int cmd_pack_objects(int argc,
if (unpack_unreachable || keep_unreachable || pack_loose_unreachable)
use_internal_rev_list = 1;
- if (!reuse_object || !allow_ref_delta)
+ if (!reuse_object || (!allow_ref_delta && !allow_ofs_delta))
reuse_delta = 0;
if (cfg->pack_compression_level == -1)
cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 83eb47a28b..36cb02e374 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2267,7 +2267,8 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
uint32_t pack_pos,
off_t offset,
struct bitmap *reuse,
- struct pack_window **w_curs)
+ struct pack_window **w_curs,
+ int allow_ref_delta)
{
off_t delta_obj_offset;
enum object_type type;
@@ -2286,6 +2287,9 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
uint32_t base_pos;
uint32_t base_bitmap_pos;
+ if (type == OBJ_REF_DELTA && !allow_ref_delta)
+ return 0;
+
/*
* Find the position of the base object so we can look it up
* in our bitmaps. If we can't come up with an offset, or if
@@ -2358,20 +2362,19 @@ static int try_partial_reuse(struct bitmap_index *bitmap_git,
static void reuse_partial_packfile_from_bitmap_1(struct bitmap_index *bitmap_git,
struct bitmapped_pack *pack,
- struct bitmap *reuse)
+ struct bitmap *reuse,
+ int allow_ref_delta)
{
struct bitmap *result = bitmap_git->result;
struct pack_window *w_curs = NULL;
size_t pos = pack->bitmap_pos / BITS_IN_EWORD;
- if (!pack->bitmap_pos) {
+ if (allow_ref_delta && !pack->bitmap_pos) {
/*
* If we're processing the first (in the case of a MIDX, the
* preferred pack) or the only (in the case of single-pack
- * bitmaps) pack, then we can reuse whole words at a time.
- *
- * This is because we know that any deltas in this range *must*
- * have their bases chosen from the same pack, since:
+ * bitmaps) pack, then any delta in this range must have its
+ * base chosen from the same pack:
*
* - In the single pack case, there is no other pack to choose
* them from.
@@ -2380,6 +2383,10 @@ static void reuse_partial_packfile_from_bitmap_1(struct bitmap_index *bitmap_git
* all ties are broken in favor of that pack (i.e. the one
* we're currently processing). So any duplicate bases will be
* resolved in favor of the pack we're processing.
+ *
+ * When REF_DELTAs are allowed, we can therefore reuse whole
+ * words at a time without inspecting object headers. Otherwise,
+ * inspect each object below to avoid reusing a REF_DELTA entry.
*/
while (pos < result->word_alloc &&
pos < pack->bitmap_nr / BITS_IN_EWORD &&
@@ -2429,7 +2436,8 @@ static void reuse_partial_packfile_from_bitmap_1(struct bitmap_index *bitmap_git
}
if (try_partial_reuse(bitmap_git, pack, bit_pos,
- pack_pos, ofs, reuse, &w_curs) < 0) {
+ pack_pos, ofs, reuse, &w_curs,
+ allow_ref_delta) < 0) {
/*
* try_partial_reuse indicated we couldn't reuse
* any bits, so there is no point in trying more
@@ -2464,7 +2472,8 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
struct bitmapped_pack **packs_out,
size_t *packs_nr_out,
struct bitmap **reuse_out,
- int multi_pack_reuse)
+ int multi_pack_reuse,
+ int allow_ref_delta)
{
struct repository *r = bitmap_repo(bitmap_git);
struct bitmapped_pack *packs = NULL;
@@ -2559,7 +2568,8 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
reuse = bitmap_word_alloc(word_alloc);
for (i = 0; i < packs_nr; i++)
- reuse_partial_packfile_from_bitmap_1(bitmap_git, &packs[i], reuse);
+ reuse_partial_packfile_from_bitmap_1(bitmap_git, &packs[i], reuse,
+ allow_ref_delta);
if (bitmap_is_empty(reuse)) {
free(packs);
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 19a8655457..39b6309736 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -116,7 +116,8 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
struct bitmapped_pack **packs_out,
size_t *packs_nr_out,
struct bitmap **reuse_out,
- int multi_pack_reuse);
+ int multi_pack_reuse,
+ int allow_ref_delta);
int rebuild_existing_bitmaps(struct bitmap_index *, struct packing_data *mapping,
kh_oid_map_t *reused_bitmaps, int show_progress);
void free_bitmap_index(struct bitmap_index *);
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index b9e36044b9..02c09e3f7d 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -229,6 +229,20 @@ test_expect_success 'pack without REF_DELTA with OFS_DELTA' '
test_grep ! " REF_DELTA " deltas
'
+test_expect_success 'pack without REF_DELTA reuses deltas as OFS_DELTA' '
+ # Install the REF_DELTA pack above and disable delta search, so any
+ # output delta must be a reused REF_DELTA rewritten as OFS_DELTA.
+ test_when_finished "rm -f .git/objects/pack/pack-$packname_2.*" &&
+ git index-pack --stdin <test-2-${packname_2}.pack >/dev/null &&
+
+ git pack-objects --window=0 --delta-base-offset \
+ --no-ref-delta --stdout <obj-list >reused.pack &&
+ git index-pack -o reused.idx reused.pack &&
+ test-tool pack-deltas --list-deltas reused.idx >deltas &&
+ test_grep " OFS_DELTA " deltas &&
+ test_grep ! " REF_DELTA " deltas
+'
+
test_expect_success 'pack without REF_DELTA skips excluded delta bases' '
test_when_finished "git read-tree $tree" &&
@@ -253,7 +267,12 @@ test_expect_success 'pack without REF_DELTA skips excluded delta bases' '
test_grep ! " OFS_DELTA " deltas &&
test_grep " REF_DELTA " deltas &&
- git pack-objects --thin --stdout --revs \
+ # Store the REF_DELTA entries above and disable delta search below,
+ # so any output delta would have to reuse an excluded-base
+ # REF_DELTA.
+ git index-pack --stdin <thin-fixed.pack >/dev/null &&
+
+ git pack-objects --thin --window=0 --stdout --revs \
--delta-base-offset --no-ref-delta \
<thin-revs >no-ref-thin.pack &&
git index-pack --fix-thin --stdin no-ref-thin-fixed.pack \
diff --git a/t/t5332-multi-pack-reuse.sh b/t/t5332-multi-pack-reuse.sh
index 881ce668e1..bc479653ec 100755
--- a/t/t5332-multi-pack-reuse.sh
+++ b/t/t5332-multi-pack-reuse.sh
@@ -111,6 +111,22 @@ test_expect_success 'reuse all objects from all packs' '
test_pack_objects_reused_all 9 3
'
+test_expect_success '--no-ref-delta reuses REF_DELTA-free bitmapped packs' '
+ # Whole-word reuse is unavailable under --no-ref-delta, so reusing
+ # every object below exercises the per-object bitmap path.
+ : >trace2.txt &&
+ GIT_TRACE2_EVENT="$PWD/trace2.txt" \
+ git pack-objects --stdout --revs --all --delta-base-offset \
+ --no-ref-delta >got.pack &&
+
+ test_pack_reused 9 <trace2.txt &&
+ test_packs_reused 3 <trace2.txt &&
+
+ git index-pack --strict -o got.idx got.pack &&
+ test-tool pack-deltas --list-deltas got.idx >deltas &&
+ test_grep ! " REF_DELTA " deltas
+'
+
test_expect_success 'reuse objects from first pack with middle gap' '
for i in D E F
do
--
2.55.0
^ permalink raw reply related
* [PATCH 2/4] pack-objects: introduce `--no-ref-delta`
From: Taylor Blau @ 2026-07-13 1:11 UTC (permalink / raw)
To: git, git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1783905084.git.ttaylorr@openai.com>
Some consumers of 'pack-objects' may wish to avoid packs which contain
`REF_DELTA` entries. For instance, a 'receive-pack' implementation which
retains the resulting pack without building an index of object IDs may
prefer every delta base to be discoverable from an earlier entry in the
same pack.
Teach 'pack-objects' a new `--no-ref-delta` option to avoid writing
`REF_DELTA` entries, without changing whether `OFS_DELTA` is allowed.
When used without `--delta-base-offset`, no delta representation
remains, so avoid delta search entirely. Otherwise, allow new deltas
whose bases appear earlier in the same pack.
For now, disable delta- and bitmap-reuse under `--no-ref-delta`, since
either may copy an existing `REF_DELTA` entry. This is overly
pessimistic, but simplifies the changes in this commit. The next commit
re-enables reuse in the cases which do not require `REF_DELTA`.
Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
Documentation/git-pack-objects.adoc | 8 ++++-
builtin/pack-objects.c | 16 ++++++---
t/t5300-pack-object.sh | 52 +++++++++++++++++++++++++++++
3 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index 65cd00c152..5e42e4429d 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -10,7 +10,8 @@ SYNOPSIS
--------
[verse]
'git pack-objects' [-q | --progress | --all-progress] [--all-progress-implied]
- [--no-reuse-delta] [--delta-base-offset] [--non-empty]
+ [--no-reuse-delta] [--delta-base-offset] [--no-ref-delta]
+ [--non-empty]
[--local] [--incremental] [--window=<n>] [--depth=<n>]
[--revs [--unpacked | --all]] [--keep-pack=<pack-name>]
[--cruft] [--cruft-expiration=<time>]
@@ -297,6 +298,11 @@ Note: Porcelain commands such as `git gc` (see linkgit:git-gc[1]),
in modern Git when they put objects in your repository into pack files.
So does `git bundle` (see linkgit:git-bundle[1]) when it creates a bundle.
+--no-ref-delta::
+ Do not emit deltas which represent their base by their literal
+ object ID. This is independent of `--delta-base-offset`;
+ without that option, no deltas are emitted.
+
--threads=<n>::
Specifies the number of threads to spawn when searching for best
delta matches. This requires that pack-objects be compiled with
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e3760b3492..c3574fcb8a 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -190,7 +190,8 @@ static inline void oe_set_delta_size(struct packing_data *pack,
static const char *const pack_usage[] = {
N_("git pack-objects [-q | --progress | --all-progress] [--all-progress-implied]\n"
- " [--no-reuse-delta] [--delta-base-offset] [--non-empty]\n"
+ " [--no-reuse-delta] [--delta-base-offset] [--no-ref-delta]\n"
+ " [--non-empty]\n"
" [--local] [--incremental] [--window=<n>] [--depth=<n>]\n"
" [--revs [--unpacked | --all]] [--keep-pack=<pack-name>]\n"
" [--cruft] [--cruft-expiration=<time>]\n"
@@ -221,6 +222,7 @@ static int ignore_packed_keep_in_core;
static int ignore_packed_keep_in_core_open;
static int ignore_packed_keep_in_core_has_cruft;
static int allow_ofs_delta;
+static int allow_ref_delta = 1;
static struct pack_idx_option pack_idx_opts;
static const char *base_name;
static int progress = 1;
@@ -3405,6 +3407,9 @@ static int should_attempt_deltas(struct object_entry *entry)
if (entry->no_try_delta)
return 0;
+ if (entry->preferred_base && !allow_ref_delta)
+ return 0;
+
if (!entry->preferred_base) {
if (oe_type(entry) < 0)
die(_("unable to get type of object %s"),
@@ -3647,7 +3652,8 @@ static void prepare_pack(int window, int depth)
if (!pack_to_stdout)
do_check_packed_object_crc = 1;
- if (!to_pack.nr_objects || !window || !depth)
+ if (!to_pack.nr_objects || !window || !depth ||
+ (!allow_ref_delta && !allow_ofs_delta))
return;
if (path_walk)
@@ -4662,7 +4668,7 @@ static int pack_options_allow_reuse(void)
!ignore_packed_keep_on_disk &&
!ignore_packed_keep_in_core &&
(!local || !have_non_local_packs) &&
- !incremental;
+ !incremental && allow_ref_delta;
}
static int get_object_list_from_bitmap(struct rev_info *revs)
@@ -5111,6 +5117,8 @@ int cmd_pack_objects(int argc,
N_("reuse existing objects")),
OPT_BOOL(0, "delta-base-offset", &allow_ofs_delta,
N_("use OFS_DELTA objects")),
+ OPT_BOOL(0, "ref-delta", &allow_ref_delta,
+ N_("use REF_DELTA objects")),
OPT_INTEGER(0, "threads", &delta_search_threads,
N_("use threads when searching for best delta matches")),
OPT_BOOL(0, "non-empty", &non_empty,
@@ -5309,7 +5317,7 @@ int cmd_pack_objects(int argc,
if (unpack_unreachable || keep_unreachable || pack_loose_unreachable)
use_internal_rev_list = 1;
- if (!reuse_object)
+ if (!reuse_object || !allow_ref_delta)
reuse_delta = 0;
if (cfg->pack_compression_level == -1)
cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 4bee490ff6..b9e36044b9 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -211,6 +211,58 @@ test_expect_success 'pack with OFS_DELTA' '
test_grep " OFS_DELTA " deltas
'
+test_expect_success 'pack without REF_DELTA' '
+ git pack-objects --no-ref-delta --stdout <obj-list >no-ref.pack &&
+ git index-pack -o no-ref.idx no-ref.pack &&
+
+ test-tool pack-deltas --list-deltas no-ref.idx >deltas &&
+ test_must_be_empty deltas
+'
+
+test_expect_success 'pack without REF_DELTA with OFS_DELTA' '
+ git pack-objects --delta-base-offset --no-ref-delta --stdout \
+ <obj-list >no-ref-ofs.pack &&
+ git index-pack -o no-ref-ofs.idx no-ref-ofs.pack &&
+
+ test-tool pack-deltas --list-deltas no-ref-ofs.idx >deltas &&
+ test_grep " OFS_DELTA " deltas &&
+ test_grep ! " REF_DELTA " deltas
+'
+
+test_expect_success 'pack without REF_DELTA skips excluded delta bases' '
+ test_when_finished "git read-tree $tree" &&
+
+ echo bar >>d &&
+ git update-index --add d &&
+ thin_tree=$(git write-tree) &&
+ thin_commit=$(git commit-tree $thin_tree -p $commit </dev/null) &&
+
+ {
+ echo $thin_commit &&
+ echo ^$commit
+ } >thin-revs &&
+
+ # Each type appears only once in the output, so any delta must
+ # use an excluded base and therefore be a REF_DELTA.
+ git pack-objects --thin --stdout --revs \
+ <thin-revs >thin.pack &&
+ git index-pack --fix-thin --stdin thin-fixed.pack \
+ <thin.pack >/dev/null &&
+
+ test-tool pack-deltas --list-deltas thin-fixed.idx >deltas &&
+ test_grep ! " OFS_DELTA " deltas &&
+ test_grep " REF_DELTA " deltas &&
+
+ git pack-objects --thin --stdout --revs \
+ --delta-base-offset --no-ref-delta \
+ <thin-revs >no-ref-thin.pack &&
+ git index-pack --fix-thin --stdin no-ref-thin-fixed.pack \
+ <no-ref-thin.pack >/dev/null &&
+
+ test-tool pack-deltas --list-deltas no-ref-thin-fixed.idx >deltas &&
+ test_must_be_empty deltas
+'
+
test_expect_success 'unpack with OFS_DELTA' '
check_unpack test-3-${packname_3} obj-list
'
--
2.55.0
^ permalink raw reply related
* [PATCH 1/4] t/helper: teach pack-deltas to list delta entries
From: Taylor Blau @ 2026-07-13 1:11 UTC (permalink / raw)
To: git, git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1783905084.git.ttaylorr@openai.com>
In the following commit(s), some tests will need to distinguish between
`REF_DELTA`s and `OFS_DELTA`s to exercise a new '--no-ref-delta' option
for 'pack-objects'.
Existing tools report delta relationships, but not how their bases are
represented in the pack.
Teach 'test-tool pack-deltas' a '--list-deltas' mode. For each delta
entry, print the object ID, its REF_DELTA or OFS_DELTA type, and the
base object ID or pack offset, respectively. This lets tests inspect
pack headers without open-coding a parser.
Signed-off-by: Taylor Blau <ttaylorr@openai.com>
---
t/helper/test-pack-deltas.c | 69 +++++++++++++++++++++++++++++++++++++
t/t5300-pack-object.sh | 8 +++--
2 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/t/helper/test-pack-deltas.c b/t/helper/test-pack-deltas.c
index 840797cf0d..4ba6fe2dd3 100644
--- a/t/helper/test-pack-deltas.c
+++ b/t/helper/test-pack-deltas.c
@@ -7,6 +7,7 @@
#include "hash.h"
#include "hex.h"
#include "pack.h"
+#include "packfile.h"
#include "pack-objects.h"
#include "parse-options.h"
#include "setup.h"
@@ -15,6 +16,7 @@
static const char *usage_str[] = {
"test-tool pack-deltas --num-objects <num-objects>",
+ "test-tool pack-deltas --list-deltas <pack>.idx",
NULL
};
@@ -80,19 +82,86 @@ static void write_ref_delta(struct hashfile *f,
free(delta_buf);
}
+static int list_delta(const struct object_id *oid,
+ struct packed_git *p,
+ uint32_t pos,
+ void *_w_curs)
+{
+ struct pack_window **w_curs = _w_curs;
+ off_t obj_offset = nth_packed_object_offset(p, pos);
+ off_t cur = obj_offset;
+ size_t size;
+ enum object_type type = unpack_object_header(p, w_curs, &cur,
+ &size);
+
+ if (type < 0)
+ die("unable to parse object at position %"PRIu32, pos);
+ if (type != OBJ_REF_DELTA && type != OBJ_OFS_DELTA)
+ return 0;
+
+ if (type == OBJ_REF_DELTA) {
+ struct object_id base_oid;
+ const unsigned char *base = use_pack(p, w_curs, cur,
+ NULL);
+
+ oidread(&base_oid, base, p->repo->hash_algo);
+ printf("%s REF_DELTA %s\n", oid_to_hex(oid),
+ oid_to_hex(&base_oid));
+ } else {
+ off_t base_offset = get_delta_base(p, w_curs, &cur,
+ type, obj_offset);
+
+ if (!base_offset)
+ die("unable to read base of object %s", oid_to_hex(oid));
+ printf("%s OFS_DELTA %"PRIuMAX"\n", oid_to_hex(oid),
+ (uintmax_t)base_offset);
+ }
+
+ return 0;
+}
+
+static void list_deltas(const char *idx_name)
+{
+ struct packed_git *p;
+ struct pack_window *w_curs = NULL;
+
+ p = add_packed_git(the_repository, idx_name, strlen(idx_name), 1);
+ if (!p || open_pack_index(p))
+ die("unable to open pack index %s", idx_name);
+
+ if (for_each_object_in_pack(p, list_delta, &w_curs,
+ ODB_FOR_EACH_OBJECT_PACK_ORDER))
+ die("unable to iterate over objects in %s", idx_name);
+
+ unuse_pack(&w_curs);
+ close_pack(p);
+ free(p);
+}
+
int cmd__pack_deltas(int argc, const char **argv)
{
int num_objects = -1;
+ int list_deltas_mode = 0;
struct hashfile *f;
struct strbuf line = STRBUF_INIT;
struct option options[] = {
OPT_INTEGER('n', "num-objects", &num_objects, N_("the number of objects to write")),
+ OPT_BOOL(0, "list-deltas", &list_deltas_mode,
+ N_("list REF_DELTA and OFS_DELTA entries")),
OPT_END()
};
argc = parse_options(argc, argv, NULL,
options, usage_str, 0);
+ if (list_deltas_mode) {
+ if (argc != 1 || num_objects >= 0)
+ usage_with_options(usage_str, options);
+ setup_git_directory(the_repository);
+ list_deltas(argv[0]);
+ return 0;
+ }
+
if (argc || num_objects < 0)
usage_with_options(usage_str, options);
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 73445782e7..4bee490ff6 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -190,7 +190,9 @@ test_expect_success 'unpack without delta (core.fsyncmethod=batch)' '
test_expect_success 'pack with REF_DELTA' '
packname_2=$(git pack-objects --progress test-2 <obj-list 2>stderr) &&
- check_deltas stderr -gt 0
+ check_deltas stderr -gt 0 &&
+ test-tool pack-deltas --list-deltas test-2-$packname_2.idx >deltas &&
+ test_grep " REF_DELTA " deltas
'
test_expect_success 'unpack with REF_DELTA' '
@@ -204,7 +206,9 @@ test_expect_success 'unpack with REF_DELTA (core.fsyncmethod=batch)' '
test_expect_success 'pack with OFS_DELTA' '
packname_3=$(git pack-objects --progress --delta-base-offset test-3 \
<obj-list 2>stderr) &&
- check_deltas stderr -gt 0
+ check_deltas stderr -gt 0 &&
+ test-tool pack-deltas --list-deltas test-3-$packname_3.idx >deltas &&
+ test_grep " OFS_DELTA " deltas
'
test_expect_success 'unpack with OFS_DELTA' '
--
2.55.0
^ permalink raw reply related
* [PATCH 0/4] send-pack: introduce a `no-ref-delta` capability
From: Taylor Blau @ 2026-07-13 1:11 UTC (permalink / raw)
To: git, git; +Cc: Jeff King, Junio C Hamano
This series teaches 'send-pack' to avoid writing `REF_DELTA` entries
when the receiving end asks it to.
Some 'receive-pack' implementations may wish to retain the incoming pack
without first building an object ID index, in which case requiring delta
bases to appear earlier in the same pack makes them easier to locate.
The new `no-ref-delta` capability is deliberately independent of
`ofs-delta`, and thus asking the sender not to write `REF_DELTA` entries
does not by itself mean that the receiver understands `OFS_DELTA`
entries. The corresponding `pack-objects` option therefore controls
`REF_DELTA` without changing whether `OFS_DELTA` is allowed.
The main complication is reuse. Ordinary delta reuse reuses the
compressed delta instructions, but rewrites the entry header and base
reference. It can therefore write an existing `REF_DELTA` as an
`OFS_DELTA` when `--delta-base-offset` is in effect and the base has
already been written in the output pack. Deltas against preferred or
external thin-pack bases cannot be reused in this way, since those bases
do not appear in the output at all.
Bitmap pack reuse is different, since it copies entries directly from
an existing pack. Under `--no-ref-delta`, it must inspect candidate
objects individually, omit `REF_DELTA` entries from direct pack reuse,
and leave them to the normal object-writing path.
The patches are organized as follows:
- The first patch teaches 'test-tool pack-deltas' to list each delta's
representation and base. I originally wrote the series without this,
but found that writing tests demonstrating which specific *kind* of
delta representation was chosen to be awkward without having a
dedicated test helper.
- The second patch introduces the `--no-ref-delta` option in
'pack-objects', though initially with delta- and bitmap-reuse
disabled for the sake of simplicity.
- The third patch re-enables ordinary delta- and bitmap-reuse where it
is safe to do so.
- The final patch advertises and consumes the new `no-ref-delta`
capability.
Thanks in advance for your review!
Taylor Blau (4):
t/helper: teach pack-deltas to list delta entries
pack-objects: introduce `--no-ref-delta`
pack-objects: support reuse with `--no-ref-delta`
send-pack: honor `no-ref-delta` capability
Documentation/git-pack-objects.adoc | 8 ++-
Documentation/gitprotocol-capabilities.adoc | 17 ++++-
builtin/pack-objects.c | 29 ++++++--
builtin/receive-pack.c | 5 ++
pack-bitmap.c | 30 +++++---
pack-bitmap.h | 3 +-
send-pack.c | 4 ++
send-pack.h | 1 +
t/helper/test-pack-deltas.c | 69 ++++++++++++++++++
t/t5300-pack-object.sh | 79 ++++++++++++++++++++-
t/t5332-multi-pack-reuse.sh | 16 +++++
t/t5516-fetch-push.sh | 14 ++++
12 files changed, 252 insertions(+), 23 deletions(-)
--
2.55.0
^ permalink raw reply
* Re: [PATCH 1/6] SubmittingPatches: clarify expected structure of commit log message
From: Junio C Hamano @ 2026-07-13 0:07 UTC (permalink / raw)
To: Michael Montalbo; +Cc: git
In-Reply-To: <CAC2QwmL05MbVS=jtk7ARj6jJUT461Ws7BcYqUAUrywvDDXjJqg@mail.gmail.com>
Michael Montalbo <mmontalbo@gmail.com> writes:
> I think collapsing the "Formatting and Style Guidelines" section with
> the above would be clearer than having a separate section.
Thanks for pointing it out; I tend to agree.
Before rerolling the series in entirety, here is what I have in my
editor buffer right now, after attempting to move the formatting and
styles into the main description.
I haven't checked if the formatting works as AsciiDoc yet, though.
--- >8 ---
[[meaningful-message]]
==== Structure of a Commit Message
1. Title:
The first line of the commit log message is the title that lets
readers of `git log --oneline` quickly understand what area the
commit touches and what problem it addresses.
- Keep it short (50 characters is the soft limit).
- Skip the full stop at the end.
- Prefix the subject with the modified area followed by a colon
and a space (e.g., "area: subject"). The area is typically a
filename or identifier (e.g., `doc:`, `transport:`, `t5601:`).
Run `git log --no-merges` on target files to see conventions.
- Do not capitalize the first word after the "area:" prefix
unless there is a specific reason (e.g., `HEAD` is always in
uppercase). For example, use "doc: clarify...", not "doc:
Clarify...".
2. Body:
A well-structured commit message body typically follows a
three-part flow: Observation, Solution Design, and
Implementation.
- Leave a blank line between the title and the body.
- Wrap lines in the body of the commit log message to around 70
columns.
- The body of the log message must be self-contained. Do not
rely on external URLs (including mailing list archives) as the
sole explanation. Summarize the relevant points of external
material so that readers can understand the change with the log
message alone.
[[present-tense]]
3. Observation (The Status Quo):
Explain the problem you are solving with your change by
describing what is wrong with the current code *without* your
change.
- As this part is always about the current state by convention,
words like "currently" are unnecessary.
- Write this problem statement in the present tense (e.g., "The
code does X when given input Y", not "The code did X").
4. Solution Design (The Approach):
Explain the approach you took, justify how it solves the problem,
and describe why you chose the particular design over other
alternatives.
- Focus on describing _why_, not _how_ (e.g., "The code does X
when given input Y, but it should do Z _because_...").
- If your change only addresses a subset of a larger problem
(e.g., it handles directories but not files because ...),
explain this limitation. This helps future developers
understand the boundaries of your work and whether it can be
safely extended.
- If your change resolves design or viability concerns raised by
the community during prior review rounds, ensure the message
records the resolution, explaining why the chosen approach was
accepted over alternatives.
[[imperative-mood]]
5. Implementation (The Execution):
Finally, describe how the changes are implemented.
- Write this in the imperative mood (e.g., "Make xyzzy do frotz",
not "This patch makes xyzzy do..." or "I changed xyzzy..."), as
if you are instructing an agent to make changes to the
codebase.
- You do not have to repeat everything readers can discern from
the patch text. Highlight the key points in your
implementation.
^ permalink raw reply
* Re: [PATCH 00/11] sequencer: do not record dropped commits as rewritten
From: Junio C Hamano @ 2026-07-13 0:06 UTC (permalink / raw)
To: Phillip Wood; +Cc: git, Uwe Kleine-König, Konstantin Ryabitsev
In-Reply-To: <dce74d17-eefd-40bb-82f3-f6b3179cc2b6@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> Hi Junio
>
> On 30/06/2026 20:57, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>
>> I have a bunch of typofixes queued on top of these 11 patches (made
>> with "git commit --fixup reword:<sha1>"); please double check when
>> you reroll after seeing more substantial reviews than mere typofixes,
>> possibly from others.
>
> Thanks, I'll squash those locally and wait before resending
>
> Phillip
Thanks.
Just responding belatedly as I was scanning topics that are marked
as "Expecting a reroll" in my draft copy of the "What's cooking"
report that I work from.
^ permalink raw reply
* Re: [PATCH 6/6] SubmittingPatches: clarify the writing style of whats-cooking
From: Michael Montalbo @ 2026-07-12 20:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20260711192650.2417665-7-gitster@pobox.com>
On Sat, Jul 11, 2026 at 12:27 PM Junio C Hamano <gitster@pobox.com> wrote:
> +TIP: When proposing a topic summary in your cover letter, write it in...
super nit: It seems like the precedent in this file is to use "NOTE" instead
of "TIP".
^ permalink raw reply
* Re: [PATCH 1/6] SubmittingPatches: clarify expected structure of commit log message
From: Michael Montalbo @ 2026-07-12 20:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20260711192650.2417665-2-gitster@pobox.com>
On Sat, Jul 11, 2026 at 12:27 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> +2. **Solution (The Approach)**:
> + Justify the way your change solves the problem. Explain why the
> + proposed approach is better and mention any alternate solutions
> + considered and discarded.
Slight reflow suggestion (answers the question "better than what?"
and is more concise):
"Explain why the proposed approach is better than any alternate
solutions that were considered and discarded."
> ++
> +If your change only addresses a subset of a larger problem (e.g.,
> +handles directories but not files because of characteristic Y),
> +explain this limitation. This helps future developers understand the
> +boundaries of your work and whether it can be safely extended.
> ++
> +If the change resolves design or viability concerns raised by the
> +community during prior review rounds, ensure the message records the
> +resolution, explaining why the chosen approach was accepted over
> +alternatives.
In the spirit of paring down text, this last section seems to overlap with
the prior "alternative solutions considered" blurb above. Maybe they can
be combined?
> +
> +3. **Command (The Instruction)**:
"Command" reads a bit awkwardly to me. I think something about
"Implementation" or another phrase that distinguishes between
the mechanics of the change and the design of the change
might be more clear.
> + [[imperative-mood]]
> + Command the codebase to change. Write this in the **imperative
> + mood** (e.g., "make xyzzy do frotz" instead of "This patch makes
> + xyzzy do..." or "I changed xyzzy..."), as if you are giving orders
> + to the codebase to change its behavior.
> +
> +#### Formatting and Style Guidelines
> +
> +* **The Subject Line (First Line)**:
> + * Keep it short (50 characters is the soft limit).
> + * Skip the full stop at the end.
> + * Prefix the subject with the modified area followed by a colon
> + and a space (e.g., "area: subject"). The area is typically a
> + filename or identifier (e.g., `doc:`, `transport:`, `t5601:`).
> + Run `git log --no-merges` on target files to see conventions.
> + * [[summary-section]]
> + Do not capitalize the first word after the "area:" prefix unless
> + there is a specific reason (e.g., `HEAD` is always in caps).
> + E.g., use "doc: clarify...", not "doc: Clarify...".
> +
> +* **The Body**:
> + * Explain the *why* rather than repeating the *what* of the diff.
I think collapsing the "Formatting and Style Guidelines" section with
the above would be clearer than having a separate section. The
content prior to this section mixes "content" and "formatting"
guidelines so maybe those concepts could be explicitly delineated
and the advice in this section could be co-located with the commit
message component it is related to above. That might also help
eliminate some redundancy (i.e., another reference to "why vs.
what").
Some more general feedback: maybe examples of well vs. poorly
formed components would help distill the advice for a reader.
Overall, I think reducing the amount of text a contributor needs to
read in order to get up to speed is a very worthwhile endeavor, so
thank you!
^ permalink raw reply
* Re: cygwin v2.55.0 test failures
From: Torsten Bögershausen @ 2026-07-12 20:04 UTC (permalink / raw)
To: Ramsay Jones
Cc: GIT Mailing-list, Johannes Schindelin, Patrick Steinhardt,
Junio C Hamano, Johannes Sixt, Adam Dinwoodie
In-Reply-To: <f65466c9-bede-472e-ad57-e72a5289be27@ramsayjones.plus.com>
On Fri, Jul 10, 2026 at 07:32:23PM +0100, Ramsay Jones wrote:
[snip]
Hej Ramsay,
Thanks for picking this up - I have some smaller comments inline,
trying to be helpful.
> As luck would have it, I left a note to myself about the remaining two
> failure cases. This leads to the remaining hunk, to connect.c, in the patch
> below; ie. the removal of a conditional (which should only fire for GfW and
> cygwin). The '#ifdef DUMMY/#endif' should probably be replaced with an
> '#ifdef GIT_WINDOWS_NATIVE/#endif' so that GfW is not affected. (Having said
> that, I suspect that even GfW should drop it ['somebody was smoking something
> exotic'], but I have no way to test it, so ...).
> Personally, I would be quite happy to rip out all win32 path handling and
> only support POSIX paths (I have been using cygwin since about 1996 and
> have only ever used win32 paths when testing git ... that is the whole
> point of cygwin! :) ), but I already know that that is a no-go. (there is
> always somebody that complains when you suggest it).
As cygwin supports/allows win32 paths: we do support them in Git as well.
(and nobody is forced to use them)
>
> So, for now anyway, it seems that I need to tidy up the patch and move in
> the opposite direction to e.g. commit 1cadad6f65 ("git clone <url>
> C:\cygwin\home\USER\repo' is working (again)", 2018-12-15).
>
> Part of the reason for vacillating on the correct way forward with this
> patch, was because I have often thought that I should use the cygwin API
> to cater to both POSIX and win32 paths. For example, we could possibly use
> the 'cygwin_conv_path()' function to do the path conversion (somewhat
> similar to the macos pre-composed-utf8 stuff, minus the directory reading).
> However, I think that would open a different can of worms, including some
> potential memory leaks. So, not exactly a slam dunk.
>
> [I also had a note-to-self about 'mixed / and \ urls' in the config file
> which is exposed by these same tests. So, another patch may be needed?]
Not sure if I follow. cygwin allows mixed / and \ . What should be patched ?
>
> Anyway, something to think about. Hmm, I suspect it would be best to just
> tidy up this patch first. ;)
>
> Just FYI. Thanks!
>
> ATB,
> Ramsay Jones
> diff --git a/connect.c b/connect.c
> index 47e39d2a73..6f5715e938 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -1088,10 +1088,12 @@ static enum url_scheme parse_connect_url(const char *url_orig, char **ret_host,
>
> if (scheme == URL_SCHEME_LOCAL)
> path = end;
> +#ifdef DUMMY
> else if (scheme == URL_SCHEME_FILE && *host != '/' &&
> !has_dos_drive_prefix(host) &&
> offset_1st_component(host - 2) > 1)
> path = host - 2; /* include the leading "//" */
> +#endif
This very lines come from
commit ebb8d2c90fb0840a0803935804e37e2205505f23
mingw: support UNC in git clone file://server/share/repo
...and I can not see a reason to remove it.
^ permalink raw reply
* Re: [PATCH v3 0/2] Silence po catalog output under "make -s"
From: Junio C Hamano @ 2026-07-12 19:41 UTC (permalink / raw)
To: Johannes Sixt, Harald Nordgren; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <CAHwyqnWsyWcggBBEZTfe5Np=xEAxe6iy+pekvUrsm4RY3VxTHw@mail.gmail.com>
Harald Nordgren <haraldnordgren@gmail.com> writes:
> Hi!
>
> What is the status here?
>
> Harald
If I understand correctly, J6t told me to expect a pull request in
https://lore.kernel.org/git/40b7eee4-6b45-449f-a3a0-0ae415097041@kdbg.org/
but it will happen after v2.55 is tagged.
And in response I said "Thanks."
I think that is where we stand right now.
Thanks.
^ permalink raw reply
* Re: [PATCH v9 0/4] graph: indent visual roots in graph
From: Mirko Faina @ 2026-07-12 19:33 UTC (permalink / raw)
To: Pablo Sabater
Cc: Chandra Pratap, git, ayu.chandekar, christian.couder, gitster,
jltobler, karthik.188, krka, peff, phillip.wood,
siddharthasthana31, Mirko Faina
In-Reply-To: <DJWR4GEV14P4.3G9N0ZL1R8VDL@gmail.com>
On Sun, Jul 12, 2026 at 06:59:58PM +0200, Pablo Sabater wrote:
> 2. Ambiguity:
>
> If it happens that the visual number on visual roots meet the condition
> (number_of_visual_roots % 3 == 0) and the next commit is NOT a visual
> root this would happen:
>
> A
> B
> C
> D
> E
> E
>
> Which would be ambiguous. The solution is to check with the lookahead
> buffer that we have since patch 3 if the next is a visual root, if it's
> not we indent D anyway:
>
> A
> B
> C
> D
> E
> E
>
> Which I find the pyramid effect uncomfortable.
> What about capping at 4 columns?
>
> 1.
>
> A
> B
> C
> D
> E
> F
> G
> H
>
> 2.
>
> A
> B
> C
> D
> E
> F
> F
>
> I prefer the 4 column wrap because it looks more abrupt and IMO shows
> better that the commits are unrelated.
>
> What do you think?
I agree, warpping beyond three levels instead of two would resolve this
ambiguity.
> Also, about the no-opt option "--no-graph-indent" is still wanted
> regardless of the final design that we choose?
Yes, I personally wouldn't want indentation on non-oneline formats.
Thank you.
^ permalink raw reply
* Re: [PATCH 4/6] MyFirstContribution: clarify that 'seen' does not mean acceptance
From: Junio C Hamano @ 2026-07-12 19:04 UTC (permalink / raw)
To: Matt Hunter; +Cc: git
In-Reply-To: <DJWSKKVJM03B.1DTV8F9FXG9IF@lfurio.us>
"Matt Hunter" <m@lfurio.us> writes:
> On Sat Jul 11, 2026 at 3:26 PM EDT, Junio C Hamano wrote:
>> +
>> +Plenty of early testers use `next` and
>> may report issues. Eventually, changes in `next` will make it to `master`,
>> which is typically considered stable. Finally, when a new release is cut,
>> `maint` is used to base bugfixes onto. As mentioned at the beginning of this
>
> It feels odd not to reflow this paragraph, where the first line now just
> stops halfway across the width of the paragraph. Though, the diff-churn
> may not be worth it in your eyes.
Yes, I did not want to force patch readers to review three extra
lines just to spot a non-existent difference caused by an
unnecessary reflow. Unlike SubmittingPatches, the target audience
for MyFirstContribution is less familiar with our source files than
experienced contributors are, so they will not be reading this in
its source form anyway. Therefore, I thought leaving an unusually
short line there was a reasonable trade-off until the entire
paragraph needs to be rewritten.
However, when the next person who wants to modify this source file
reads it, it will indeed be distracting to them. So, perhaps I
should reflow the remainder of the paragraph.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/2] Silence po catalog output under "make -s"
From: Harald Nordgren @ 2026-07-12 18:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, git, Harald Nordgren via GitGitGadget
In-Reply-To: <xmqqh5mn4eyp.fsf@gitster.g>
Hi!
What is the status here?
Harald
^ permalink raw reply
* Provide a free license for git-digestif; just add a link in the README
From: lmybizexplore @ 2026-07-12 18:07 UTC (permalink / raw)
To: git
The target organization maintains an open-source project named git-digestif on GitHub, providing digest services for Git developers. We are Weifang Leimingyun Network Technology Co., Ltd. Our team specializes in AI research and development. We have just won the second prize in the National Artificial Intelligence Competition for 2025. Our core product is an AI-based customer acquisition robot that enables automated search across the internet and human-like communication. I would like to provide you with a licensed version of this tool for free. In exchange, all you need to do is add a hyperlink in the README of the git-digestif project. I was wondering if you would be open to this collaboration model of replacing software licenses with README links?
^ permalink raw reply
* Re: [PATCH 4/6] MyFirstContribution: clarify that 'seen' does not mean acceptance
From: Matt Hunter @ 2026-07-12 18:08 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <20260711192650.2417665-5-gitster@pobox.com>
On Sat Jul 11, 2026 at 3:26 PM EDT, Junio C Hamano wrote:
> +
> +Plenty of early testers use `next` and
> may report issues. Eventually, changes in `next` will make it to `master`,
> which is typically considered stable. Finally, when a new release is cut,
> `maint` is used to base bugfixes onto. As mentioned at the beginning of this
It feels odd not to reflow this paragraph, where the first line now just
stops halfway across the width of the paragraph. Though, the diff-churn
may not be worth it in your eyes.
^ permalink raw reply
* Re: [PATCH v9 0/4] graph: indent visual roots in graph
From: Pablo Sabater @ 2026-07-12 16:59 UTC (permalink / raw)
To: Mirko Faina, Chandra Pratap
Cc: Pablo Sabater, git, ayu.chandekar, christian.couder, gitster,
jltobler, karthik.188, krka, peff, phillip.wood,
siddharthasthana31
In-Reply-To: <alOOXKGIB8BqACxR@exploit>
On Sun Jul 12, 2026 at 3:10 PM CEST, Mirko Faina wrote:
> On Sun, Jul 12, 2026 at 11:26:27AM +0530, Chandra Pratap wrote:
>> Tying graph-drawing logic to specific formatting flags could introduce
>> inconsistencies. For example, if a user relies on a custom format like
>> --format="%h %s", the output is functionally single-line and suffers
>> from the exact same ambiguity, but it would miss the fix.
>>
>> Even in multi-line formats, relying on the absence of a '|' character to spot
>> unrelated commits requires active effort. Indentation provides an immediate
>> visual cue that breaks the vertical lineage, which is helpful regardless of the
>> commit message length.
>>
>> I agree with Pablo: for users who strictly want the old behavior, an opt-out
>> flag keeps the graph logic decoupled from the formatting logic.
>
> In that case, together with --[no]-graph-indent, a configuration
> variable like "graph.indent" could be introduced to reduce the usage of
> --[no]-graph-indent for those that would like to retain the old
> behaviour for most formats.
>
>> > > Apart from having an option to disable indentation.
>> > >
>> > > We could have the cascading to have a limit or make it zig-zag:
>> > >
>> > > instead of:
>> > >
>> > > A
>> > > B
>> > > C
>> > > D
>> > >
>> > > We could do:
>> > >
>> > > A
>> > > B
>> > > C
>> > > D
>> > >
>> > > This would have its own edge cases like:
>> > >
>> > > A
>> > > B
>> > > C <- if we zig-zag here C and D become ambiguous, currently we are
>> > > D indenting only the last commits (visual roots) here we would have
>> > > D to chose between continuing cascading or indenting the first of D.
>> > >
>> > > I'm not so sure if I like the zig-zag solution because we need to think again
>> > > if it causes an ambiguity, but I wanted to mention it.
>> > >
>> > > I think we need some more opinions about the design.
>> >
>> > I don't dislike the the current solution but I can see it degenerating
>> > if someone contributes a lot of one-patch series.
>> >
>> > Maybe you could indent commits that are both head and tail up to two
>> > levels and then on the third go back to the beginning of the line. That
>> > way you kind of have a zig-zag but without ambiguity. You'd only have to
>> > add a counter to keep track of the level of indentation.
>>
>> Not sure about this. A zig-zag pattern visually mimics branching and
>> merging, which makes unrelated commits look like a complex merge topology.
>>
>> I also have a feeling that this will end up recreating the exact ambiguity this
>> patch series is trying to fix.
>
> While a zig-zag pattern might be ambiguous, what I proposed is a little
> different.
>
> What I proposed is effectively a wrapping for anything that goes beyond
> two levels of indentation. I don't think it would look anything like a
> fork/merge pattern.
>
> * A
> * B
> * C
> * D
> * E
> * F
>
> The difference between two indentation levels and no indentation is very
> noticeble, I don't think anyone confused this. This would fix the
> staircase pattern on adjacent one-patch series.
I agree that having an infinite stair is not a good solution. the 3
column wrap looks reasonable.
I see two cases with this wrap:
1. No conflict case:
A
B
C
D
E
F
No ambiguity, this would be the ideal case.
2. Ambiguity:
If it happens that the visual number on visual roots meet the condition
(number_of_visual_roots % 3 == 0) and the next commit is NOT a visual
root this would happen:
A
B
C
D
E
E
Which would be ambiguous. The solution is to check with the lookahead
buffer that we have since patch 3 if the next is a visual root, if it's
not we indent D anyway:
A
B
C
D
E
E
Which I find the pyramid effect uncomfortable.
What about capping at 4 columns?
1.
A
B
C
D
E
F
G
H
2.
A
B
C
D
E
F
F
I prefer the 4 column wrap because it looks more abrupt and IMO shows
better that the commits are unrelated.
What do you think?
Also, about the no-opt option "--no-graph-indent" is still wanted
regardless of the final design that we choose?
Thanks,
Pablo
^ 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