* [PATCH v7 5/9] environment: move askpass_program into repo_config_values
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-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 v7 4/9] environment: move pager_program into repo_config_values
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-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()'.
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 +
| 17 ++++++++++-------
3 files changed, 13 insertions(+), 7 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..450ad053b6 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");
@@ -303,6 +306,6 @@ 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;
+ repo_config_values(r)->pager_program = data.value;
return data.want;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v7 3/9] environment: move editor_program into repo_config_values
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-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 v7 2/9] environment: move excludes_file into repo_config_values
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-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 v7 1/9] repository: introduce repo_config_values_clear()
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260706142530.3681520-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 v7 0/9] migrate more variables into repo_config_values
From: Tian Yuchen @ 2026-07-06 14:25 UTC (permalink / raw)
To: git; +Cc: cirnovskyv, szeder.dev, Tian Yuchen
In-Reply-To: <20260701180813.776173-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?
Changes since v6:
Only the first two commits in this patch overlap with v6. The reason the
subsequent commits were not released separately is that Christian suggested
placing the introduction of 'repo_config_values_clear()' as a standalone
commit at the very beginning.
In other words, the v6 structure has been discarded, and this series is
being released as almost a new patch.
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 | 17 ++++++----
prompt.c | 3 +-
remote.c | 2 +-
repository.c | 1 +
12 files changed, 145 insertions(+), 80 deletions(-)
--
2.43.0
^ permalink raw reply
* Re* CARGO trouble appeared from 2.54.0 to 2.55.0
From: Junio C Hamano @ 2026-07-06 14:13 UTC (permalink / raw)
To: Ben Knoble; +Cc: Kurt Mielke, git
In-Reply-To: <81CE676D-CBBD-44E9-8DD6-B34068E35769@gmail.com>
Ben Knoble <ben.knoble@gmail.com> writes:
> Git’s source code has included some optional Rust components for
> a few releases now. Rust is planned to be a requirement for 3.0
> (IIUC), but I don’t have any recollection of the proposed
> timeline. [There was also some discussion of delaying the Rust
> mandate timeline for platforms which currently lack support?]
> ...
> 2.55 is the first version to flip the default to « build with Rust
> », but it remains optional as you’ve discovered.
> ...
> Searching the internet shows Alma has packages for Rust, which you
> should be able to install if you want to compile with the Rust
> components.
As you said above, what was reported is totally expected (I
understand that 2.55 built properly for the OP with "make
NO_RUST=NoThanks" set).
I do not know why nobody complained, but we have been carrying this
incomplete sentence in the release notes forever X-<.
Documentation/RelNotes/2.55.0.adoc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git c/Documentation/RelNotes/2.55.0.adoc w/Documentation/RelNotes/2.55.0.adoc
index f5643534dc..696ad15c1e 100644
--- c/Documentation/RelNotes/2.55.0.adoc
+++ w/Documentation/RelNotes/2.55.0.adoc
@@ -85,8 +85,8 @@ Performance, Internal Implementation, Development Support etc.
* Promisor remote handling has been refactored and fixed in
preparation for auto-configuration of advertised remotes.
- * Rust support is enabled by default (but still allows opting out) in
- some future version of Git.
+ * Rust support is enabled by default (but still allows opting out); in
+ some future version of Git, this will become mandatory.
* Preparation of the xdiff/ codebase to work with Rust.
^ permalink raw reply related
* Re: [PATCH v7 0/5] history: add squash subcommand to fold a range
From: Phillip Wood @ 2026-07-06 14:06 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Harald Nordgren, Patrick Steinhardt, Junio C Hamano, Matt Hunter
In-Reply-To: <pull.2337.v7.git.git.1783327849.gitgitgadget@gmail.com>
Hi Harald
On 06/07/2026 09:50, Harald Nordgren via GitGitGadget wrote:
> Adds git history squash <revision-range> to fold a range of commits.
>
> Changes in v7:
>
> * --reedit-message
There was some discussion [1] about making that the default and renaming
it - was that overlooked? If not it would be helpful to comment on those
discussions to explain why you don't think it is a good idea.
> now builds the same editor template git rebase -i shows
> for a squash (a combination of N commits banner with each folded message
> under its own header) and follows autosquash for markers: a fixup!
> message falls out (commented under a will be skipped header), while a
> squash! or amend! keeps its body with only the marker subject commented
> so its remark can be reworded in. Only the message text is affected,
> every commit's changes are always folded in.
Rebase re-orders commits so that fixups immediately follow their target
- do you do that here? I think that is very relevant because here we may
be dealing with several different commits each being targeted by a set
of fixups and presenting them mixed together will be confusing. When
rebase sees an "amend!" commit it comments out the message that is being
replaced - it is not clear from this description whether that happens here.
As I've said before I think we would be better off with a summary of the
commits that are being squashed and a more compact template message that
only contains the messages we want to keep [2]. What are the advantages
of having lots of commented lines (or redundant messages if you don't
comment out the original when there is an amend! commit) in the middle
of the template message?
> * Reuse git rebase -i's squash-message code: a preparatory sequencer:
> commit extracts the banner, header and marker-comment helpers so both
> rebase and git history squash build the identical template from one
> source.
> * Refuse a range whose oldest commit is a fixup!, squash! or amend!, since
> the marker's target cannot be inside the range.
I think it should allow squashing a bunch of fixups together though. I
thought there was a plan [3] to refuse to squash a fixup unless the
range included its target.
The range-diff does not show any input sanitization - what happens when
the user passes "--reverse" for example? As I said in [4] we should copy
what "git replay" does to sanity check the rev-list options, otherwise
we've got no idea whether the parent of the first commit returned by
get_revision() is the commit we want to use as the parent of the
squashed commit.
Thanks
Phillip
[1]
https://lore.kernel.org/git/3c35bd17-e884-432d-a400-36a89964ed89@gmail.com/
[2]
https://lore.kernel.org/git/4b505228-4846-4a48-9255-e249f4e70a1f@gmail.com
[3]
https://lore.kernel.org/git/CAHwyqnWQmObWr3N81_EU6F13iyKp3FfY8KSNFfoAjS4r_0qJrQ@mail.gmail.com/
[4]
https://lore.kernel.org/git/f3fe7ff2-3ce9-4e90-95e7-8c620de5628a@gmail.com/
> * Reorder the squash usage so dashed options come before <revision-range>,
> and spell out HEAD instead of @ in the documentation and examples.
> * Expand the squash commit message and documentation with this overview,
> and scope the merge limitation so it no longer contradicts squash folding
> a single-base interior merge.
>
> Changes in v6:
>
> * git history squash now accepts multiple revision arguments, read like the
> arguments to git-rev-list, so a compound range such as @~3.. ^topic
> works.
> * The base to reparent onto is now the oldest in-range commit's parent; a
> boundary other than that base means the range has more than one base and
> is rejected. This also fixes the earlier overly-restrictive handling of
> merges and side branches.
> * A single-commit range (e.g. @^!) is rejected with "nothing to squash"
> (this also covers the @^!-style example that previously succeeded
> silently).
> * Commit messages reworded: the squash commit now gives an overview of
> fixup!/squash!/amend! handling, rewording, merge-parent and ref behavior.
>
> Changes in v5:
>
> * The range walk now uses --ancestry-path, so only commits descended from
> the base are folded; a single revision such as HEAD or HEAD~1 is now
> rejected as "not a <base>..<tip> range" rather than treated as a squash
> down to the root.
> * This adopts the --ancestry-path suggestion; the multi-base rejection is
> unchanged, so a side branch that forked before the base and merged in is
> still refused.
> * Added tests covering more merge topologies: two interior merges, a nested
> merge, an octopus merge, an octopus arm forked before the base, a merge
> among the descendants replayed above the range, and a ref pointing at an
> interior merge commit.
>
> Changes in v4:
>
> * git history squash now detects when another ref points at a commit inside
> the range being folded and refuses, with an advice.historyUpdateRefs hint
> to use --update-refs=head.
> * A merge inside the range is folded fine as long as the range has a single
> base; a range with merge commit at the tip or base also folds correctly.
> Only a range with more than one base is rejected.
>
> Changes in v3:
>
> * Moved the feature out of git rebase and into a new git history squash
> <revision-range> subcommand, per the list discussion. git rebase --squash
> is dropped.
> * Takes an arbitrary range (git history squash @~3.., git history squash
> @~5..@~2), folding it into the oldest commit and replaying any
> descendants on top.
> * Implemented as a single tree operation rather than picking each commit,
> so there are no repeated conflict stops (addresses Phillip's efficiency
> point).
> * A merge inside the range is folded fine, only a range with more than one
> base is rejected.
> * --reedit-message seeds the editor with every folded-in message, not just
> the oldest.
>
> Harald Nordgren (5):
> history: extract helper for a commit's parent tree
> history: give commit_tree_ext a message template
> history: add squash subcommand to fold a range
> sequencer: extract helpers for the squash message markers
> history: re-edit a squash with every message
>
> Documentation/config/advice.adoc | 4 +
> Documentation/git-history.adoc | 49 ++-
> advice.c | 1 +
> advice.h | 1 +
> builtin/history.c | 390 +++++++++++++++++--
> sequencer.c | 64 ++--
> sequencer.h | 23 ++
> t/meson.build | 1 +
> t/t3455-history-squash.sh | 632 +++++++++++++++++++++++++++++++
> 9 files changed, 1099 insertions(+), 66 deletions(-)
> create mode 100755 t/t3455-history-squash.sh
>
>
> base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v7
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v7
> Pull-Request: https://github.com/git/git/pull/2337
>
> Range-diff vs v6:
>
> 1: fea6b79e60 = 1: 56ed8fadbb history: extract helper for a commit's parent tree
> 2: e2674e0bc4 = 2: 212e9c228f history: give commit_tree_ext a message template
> 3: 811e393ab4 ! 3: cf3346a1cd history: add squash subcommand to fold a range
> @@ Commit message
> Add "git history squash <revision-range>" to do this directly. It folds
> every commit in the range into the oldest one, keeping that commit's
> message and authorship and taking the tree of the newest commit, then
> - replays the commits above the range on top. fixup!, squash! and amend!
> - commits are folded like any other and are not interpreted, so the
> - squashed message comes from the oldest commit, or from an editor with
> - --reedit-message.
> + replays the commits above the range on top. The squashed message comes
> + from the oldest commit, or from an editor with --reedit-message. As that
> + message is reused, a range whose oldest commit is a fixup!, squash! or
> + amend! is refused, since the marker's target cannot be in the range.
>
> The range is read like the arguments to "git rev-list", so several
> - arguments such as "@~3.. ^topic" are allowed. A merge inside the range
> - is folded when its other parent is reachable from the base, otherwise
> - the range has more than one base and is rejected. By default the command
> - also refuses when a ref points at a commit that the fold would discard.
> - Use --update-refs=head to rewrite only the current branch instead.
> + arguments such as "HEAD~3..HEAD ^topic" are allowed. A merge inside the
> + range is folded when its other parent is reachable from the base,
> + otherwise the range has more than one base and is rejected. By default
> + the command also refuses when a ref points at a commit that the fold
> + would discard. Use --update-refs=head to rewrite only the current branch
> + instead.
>
> Inspired-by: Sergey Chernov <serega.morph@gmail.com>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> @@ Documentation/git-history.adoc: SYNOPSIS
> git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
> git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
> git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
> -+git history squash <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
> ++git history squash [--dry-run] [--update-refs=(branches|head)] [--reedit-message] <revision-range>
>
> DESCRIPTION
> -----------
> +@@ Documentation/git-history.adoc: at once.
> + LIMITATIONS
> + -----------
> +
> +-This command does not (yet) work with histories that contain merges. You
> +-should use linkgit:git-rebase[1] with the `--rebase-merges` flag instead.
> ++This command does not (yet) replay merge commits onto the rewritten
> ++history: if a commit that would be replayed is a merge, the operation is
> ++rejected, and you should use linkgit:git-rebase[1] with the
> ++`--rebase-merges` flag instead. The `squash` subcommand can still fold a
> ++merge that lies inside the range, as long as the range has a single base.
> +
> + Furthermore, the command does not support operations that can result in merge
> + conflicts. This limitation is by design as history rewrites are not intended to
> @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
> It is invalid to select either all or no hunks, as that would lead to
> one of the commits becoming empty.
> @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
> ++
> +The range is given in the usual `<base>..<tip>` form, where _<base>_ is
> +the commit just below the oldest commit to squash. For example, `git
> -+history squash @~3..` folds the three most recent commits into one, and
> -+`git history squash @~5..@~2` squashes an interior range while leaving
> -+the two newest commits in place. _<revision-range>_ is read like the
> -+arguments to linkgit:git-rev-list[1], so several arguments may be given,
> -+for example `@~3.. ^topic` to additionally exclude what is already on
> -+`topic`.
> ++history squash HEAD~3..HEAD` folds the three most recent commits into
> ++one, and `git history squash HEAD~5..HEAD~2` squashes an interior range
> ++while leaving the two newest commits in place. _<revision-range>_ is read
> ++like the arguments to linkgit:git-rev-list[1], so several arguments may be
> ++given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is
> ++already on `topic`.
> ++
> +The oldest commit's message and authorship are preserved by default,
> +unless you specify `--reedit-message`. A merge commit inside the range is
> @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
> +that reaches more than one entry point (for example a side branch that
> +forked before the range and was later merged into it) is rejected.
> ++
> -+The folded commits disappear from the history, so with the default
> -+`--update-refs=branches` the command refuses when another ref points at
> -+one of them. Rerun with `--update-refs=head` to rewrite only the current
> -+branch and leave those refs pointing at the old commits.
> ++Because the oldest commit's message is reused, the range may not begin
> ++with a `fixup!`, `squash!`, or `amend!` commit, whose target is
> ++necessarily outside the range.
> +++
> ++A branch or tag that points at a commit inside the range would be left
> ++dangling once those commits are folded away, so with the default
> ++`--update-refs=branches` the command refuses. Rerun with
> ++`--update-refs=head` to rewrite only the current branch and leave such
> ++refs pointing at the old commits.
> +
> OPTIONS
> -------
>
> +@@ Documentation/git-history.adoc: OPTIONS
> + ref updates is generally safe.
> +
> + `--reedit-message`::
> +- Open an editor to modify the target commit's message.
> ++ Open an editor to modify the rewritten commit's message. For `squash`
> ++ the editor is pre-filled with the messages of all the folded commits.
> +
> + `--empty=(drop|keep|abort)`::
> + Control what happens when a commit becomes empty as a result of the
>
> ## advice.c ##
> @@ advice.c: static struct {
> @@ builtin/history.c
> #define GIT_HISTORY_SPLIT_USAGE \
> N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]")
> +#define GIT_HISTORY_SQUASH_USAGE \
> -+ N_("git history squash <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]")
> ++ N_("git history squash [--dry-run] [--update-refs=(branches|head)] [--reedit-message] <revision-range>")
>
> static void change_data_free(void *util, const char *str UNUSED)
> {
> @@ builtin/history.c: out:
> + return ret;
> +}
> +
> ++static int reject_fixupish_oldest(struct repository *repo,
> ++ struct commit *oldest)
> ++{
> ++ const char *message, *subject;
> ++ int ret = 0;
> ++
> ++ message = repo_logmsg_reencode(repo, oldest, NULL, NULL);
> ++ find_commit_subject(message, &subject);
> ++ if (starts_with(subject, "fixup! ") ||
> ++ starts_with(subject, "squash! ") ||
> ++ starts_with(subject, "amend! "))
> ++ ret = error(_("the range begins with a fixup!, squash! or amend! "
> ++ "commit whose target is not in the range"));
> ++ repo_unuse_commit_buffer(repo, oldest, message);
> ++ return ret;
> ++}
> ++
> +struct interior_ref_cb {
> + const struct oidset *interior;
> + const char *name;
> @@ builtin/history.c: out:
> + if (ret < 0)
> + goto out;
> +
> ++ ret = reject_fixupish_oldest(repo, oldest);
> ++ if (ret < 0)
> ++ goto out;
> ++
> + if (action == REF_ACTION_BRANCHES) {
> + struct interior_ref_cb cb = { .interior = &interior };
> +
> @@ t/t3455-history-squash.sh (new)
> +
> +test_expect_success 'squashes a range into a single commit without changing the tree' '
> + git reset --hard three &&
> ++ head_before=$(git rev-parse HEAD) &&
> + tip_tree=$(git rev-parse HEAD^{tree}) &&
> +
> ++ git history squash --dry-run start.. >out &&
> ++ predicted=$(awk "/^update refs\/heads\// {print \$3}" out) &&
> ++ test_cmp_rev "$head_before" HEAD &&
> ++
> + git history squash start.. &&
> +
> ++ test "$predicted" = "$(git rev-parse HEAD)" &&
> + git rev-list --count start..HEAD >count &&
> + echo 1 >expect &&
> + test_cmp expect count &&
> @@ t/t3455-history-squash.sh (new)
> + test_cmp expect actual
> +'
> +
> -+test_expect_success 'keeps the oldest message even if it is a fixup!' '
> ++test_expect_success 'refuses a range whose oldest commit is a fixup!' '
> + git reset --hard start &&
> + test_commit --no-tag "fixup! something" file b &&
> -+ test_commit tail file c &&
> ++ test_commit --no-tag tail file c &&
> ++ head_before=$(git rev-parse HEAD) &&
> ++
> ++ test_must_fail git history squash start.. 2>err &&
> ++ test_grep "target is not in the range" err &&
> ++ test_cmp_rev "$head_before" HEAD
> ++'
> ++
> ++test_expect_success 'does not interpret squash! or amend! markers' '
> ++ git reset --hard start &&
> ++ test_commit --no-tag marker-oldest file b &&
> ++ git commit --allow-empty -m "squash! marker-oldest" &&
> ++ git commit --allow-empty -m "amend! marker-oldest" &&
> ++ test_commit --no-tag marker-newest file c &&
> +
> + git history squash start.. &&
> +
> ++ git rev-list --count start..HEAD >count &&
> ++ echo 1 >expect &&
> ++ test_cmp expect count &&
> + git log --format="%s" -1 >actual &&
> -+ echo "fixup! something" >expect &&
> ++ echo marker-oldest >expect &&
> + test_cmp expect actual
> +'
> +
> @@ t/t3455-history-squash.sh (new)
> + test_cmp expect actual
> +'
> +
> -+test_expect_success '--dry-run predicts the rewrite without performing it' '
> -+ git reset --hard three &&
> -+ head_before=$(git rev-parse HEAD) &&
> -+ tip_tree=$(git rev-parse HEAD^{tree}) &&
> -+
> -+ git history squash --dry-run start.. >out &&
> -+ predicted=$(awk "/^update refs\/heads\// {print \$3}" out) &&
> -+ test_cmp_rev "$head_before" HEAD &&
> -+
> -+ git history squash start.. &&
> -+ test "$predicted" = "$(git rev-parse HEAD)" &&
> -+ git rev-list --count start..HEAD >count &&
> -+ echo 1 >expect &&
> -+ test_cmp expect count &&
> -+ test_cmp_rev start HEAD^ &&
> -+ test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
> -+'
> -+
> +test_expect_success '--update-refs=head only moves HEAD' '
> + git reset --hard three &&
> + git branch -f other HEAD &&
> -: ---------- > 4: 001356db93 sequencer: extract helpers for the squash message markers
> 4: 4edf012b77 ! 5: 615fe4dd3f history: re-edit a squash with every message
> @@ Commit message
> When --reedit-message is given it only reopened that one message, so the
> messages of the folded-in commits were lost.
>
> - Gather the messages of every commit in the range, oldest first, and use
> - them as the editor template when re-editing, mirroring how "git rebase
> - -i" presents a squash.
> + Gather the messages of every commit in the range, oldest first, and build
> + the same editor template that "git rebase -i" shows for a squash, using
> + add_squash_combination_header(), add_squash_message_header() and
> + squash_subject_comment_len(). Only the message text differs, the changes
> + are always folded in. Following autosquash, a fixup!'s message is
> + commented out in full under a "will be skipped" header, while a squash! or
> + amend! keeps its body with only the marker subject commented.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>
> ## Documentation/git-history.adoc ##
> -@@ Documentation/git-history.adoc: arguments to linkgit:git-rev-list[1], so several arguments may be given,
> - for example `@~3.. ^topic` to additionally exclude what is already on
> - `topic`.
> +@@ Documentation/git-history.adoc: like the arguments to linkgit:git-rev-list[1], so several arguments may be
> + given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is
> + already on `topic`.
> +
> -The oldest commit's message and authorship are preserved by default,
> -unless you specify `--reedit-message`. A merge commit inside the range is
> @@ Documentation/git-history.adoc: arguments to linkgit:git-rev-list[1], so several
> folded like any other, but the range must have a single base, so a range
> that reaches more than one entry point (for example a side branch that
> forked before the range and was later merged into it) is rejected.
> + +
> + Because the oldest commit's message is reused, the range may not begin
> + with a `fixup!`, `squash!`, or `amend!` commit, whose target is
> +-necessarily outside the range.
> ++necessarily outside the range. The changes from every commit in the range
> ++are always folded in. Only the message text differs. With
> ++`--reedit-message` the template mirrors `git rebase -i`: the message of a
> ++`fixup!` elsewhere in the range is commented out in full, while a
> ++`squash!` or `amend!` keeps its message body with only the marker subject
> ++commented, so you can fold the remark into the result.
> + +
> + A branch or tag that points at a commit inside the range would be left
> + dangling once those commits are folded away, so with the default
>
> ## builtin/history.c ##
> @@ builtin/history.c: static int find_interior_ref(const struct reference *ref, void *cb_data)
> @@ builtin/history.c: static int find_interior_ref(const struct reference *ref, voi
> + struct commit *tip,
> + struct strbuf *out)
> +{
> ++ struct commit_list *commits = NULL, **tail = &commits, *c;
> + struct rev_info revs;
> + struct commit *commit;
> + struct strvec args = STRVEC_INIT;
> -+ int n = 0, ret;
> ++ int n = 0, total, ret;
> +
> + repo_init_revisions(repo, &revs, NULL);
> + strvec_push(&args, "ignored");
> @@ builtin/history.c: static int find_interior_ref(const struct reference *ref, voi
> + goto out;
> + }
> +
> -+ while ((commit = get_revision(&revs))) {
> ++ while ((commit = get_revision(&revs)))
> ++ tail = &commit_list_insert(commit, tail)->next;
> ++ total = commit_list_count(commits);
> ++
> ++ for (c = commits; c; c = c->next) {
> + const char *message, *body;
> -+ struct strbuf one = STRBUF_INIT;
> ++ size_t commented_len;
> ++ int skip;
> +
> -+ message = repo_logmsg_reencode(repo, commit, NULL, NULL);
> ++ message = repo_logmsg_reencode(repo, c->item, NULL, NULL);
> + find_commit_subject(message, &body);
> -+ strbuf_addstr(&one, body);
> -+ strbuf_trim_trailing_newline(&one);
> +
> -+ if (n++)
> -+ strbuf_addch(out, '\n');
> -+ strbuf_addbuf(out, &one);
> ++ skip = starts_with(body, "fixup! ");
> ++ commented_len = skip ? strlen(body) :
> ++ squash_subject_comment_len(body, 1);
> ++
> ++ if (!n)
> ++ add_squash_combination_header(out, total);
> + strbuf_addch(out, '\n');
> ++ add_squash_message_header(out, ++n, skip);
> ++ strbuf_addstr(out, "\n\n");
> ++ strbuf_add_commented_lines(out, body, commented_len, comment_line_str);
> ++ strbuf_addstr(out, body + commented_len);
> ++ strbuf_complete_line(out);
> +
> -+ strbuf_release(&one);
> -+ repo_unuse_commit_buffer(repo, commit, message);
> ++ repo_unuse_commit_buffer(repo, c->item, message);
> + }
> +
> + ret = 0;
> +
> +out:
> ++ commit_list_free(commits);
> + reset_revision_walk();
> + release_revisions(&revs);
> + strvec_clear(&args);
> @@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
> + test_commit re-three file d &&
> +
> + write_script editor <<-\EOF &&
> -+ cp "$1" buffer &&
> ++ cat "$1" >edited &&
> + echo combined >"$1"
> + EOF
> + test_set_editor "$(pwd)/editor" &&
> + git history squash --reedit-message start.. &&
> +
> -+ test_grep "re-one subject" buffer &&
> -+ test_grep "re-one body line" buffer &&
> -+ test_grep re-two buffer &&
> -+ test_grep re-three buffer &&
> -+ git log --format="%s" -1 >actual &&
> ++ cat >expect <<-EOF &&
> ++ # This is a combination of 3 commits.
> ++ # This is the 1st commit message:
> ++
> ++ re-one subject
> ++
> ++ re-one body line
> ++
> ++ # This is the commit message #2:
> ++
> ++ re-two
> ++
> ++ # This is the commit message #3:
> ++
> ++ re-three
> ++
> ++ # Please enter the commit message for the squash changes. Lines starting
> ++ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
> ++ # Changes to be committed:
> ++ # modified: file
> ++ #
> ++ EOF
> ++ test_cmp expect edited &&
> + echo combined >expect &&
> ++ git log --format="%s" -1 >actual &&
> + test_cmp expect actual
> +'
> +
> ++test_expect_success '--reedit-message handles fixup!, squash! and amend! like rebase' '
> ++ git reset --hard start &&
> ++ test_commit --no-tag mark-base file b &&
> ++ printf "fixup! mark-base\n\nfixup body\n" >msg &&
> ++ echo c >file &&
> ++ git add file &&
> ++ git commit -qF msg &&
> ++ printf "squash! mark-base\n\nsquash remark\n" >msg &&
> ++ echo d >file &&
> ++ git add file &&
> ++ git commit -qF msg &&
> ++ printf "amend! mark-base\n\namended message\n" >msg &&
> ++ echo e >file &&
> ++ git add file &&
> ++ git commit -qF msg &&
> ++
> ++ write_script editor <<-\EOF &&
> ++ cat "$1" >edited
> ++ EOF
> ++ test_set_editor "$(pwd)/editor" &&
> ++ git history squash --reedit-message start.. &&
> ++
> ++ cat >expect <<-EOF &&
> ++ # This is a combination of 4 commits.
> ++ # This is the 1st commit message:
> ++
> ++ mark-base
> ++
> ++ # The commit message #2 will be skipped:
> ++
> ++ # fixup! mark-base
> ++ #
> ++ # fixup body
> ++
> ++ # This is the commit message #3:
> ++
> ++ # squash! mark-base
> ++
> ++ squash remark
> ++
> ++ # This is the commit message #4:
> ++
> ++ # amend! mark-base
> ++
> ++ amended message
> ++
> ++ # Please enter the commit message for the squash changes. Lines starting
> ++ # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit.
> ++ # Changes to be committed:
> ++ # modified: file
> ++ #
> ++ EOF
> ++ test_cmp expect edited &&
> ++ git log -1 --format="%B" >final &&
> ++ test_grep ! "fixup body" final &&
> ++ test_grep "squash remark" final &&
> ++ test_grep "amended message" final
> ++'
> ++
> +test_expect_success '--reedit-message aborts on an empty message' '
> + git reset --hard three &&
> + head_before=$(git rev-parse HEAD) &&
> @@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the olde
> + test_cmp_rev "$head_before" HEAD
> +'
> +
> - test_expect_success '--dry-run predicts the rewrite without performing it' '
> + test_expect_success '--update-refs=head only moves HEAD' '
> git reset --hard three &&
> - head_before=$(git rev-parse HEAD) &&
> + git branch -f other HEAD &&
>
^ permalink raw reply
* [PATCH] sparse-index: avoid crash on intent-to-add entry outside the cone
From: Derrick Stolee via GitGitGadget @ 2026-07-06 13:50 UTC (permalink / raw)
To: git; +Cc: gitster, Derrick Stolee, Derrick Stolee
From: Derrick Stolee <stolee@gmail.com>
When collapsing a full index to a sparse index, the recursive
convert_to_sparse_rec() walks the cache tree to determine if any
of the cache tree entries can be used to represent a sparse directory.
As it goes, the method tracks how many cache entries are being represented
by the cache tree entry. The cache tree node's 'entry_count' represents how
many cache entries are covered by the node.
However, this value can be negative, representing that a node is invalid,
and is no longer reflecting the number of cache entries fit within. This can
happen when the user uses 'git add --intent-to-add' to mark an untracked
file with the intent-to-add bit to avoid committing without finishing the
add.
When such an intent-to-add file exists and the sparse-checkout changes to no
longer contain its parent directory, this leads to a segfault. Two tests are
added to demonstrate this fault:
* One test is added to t3705-add-sparse-checkout.sh to demonstrate
how 'git add' behaves with sparse-checkout.
* One test is added to t1092-sparse-checkout-compatibility.sh to demonstrate
the interaction with the sparse index and to compare it directly to how
the commands behave with a full index or no sparse-checkout.
The fix involves engaging with the loop that iterates over all cache entries
within the parent cache tree node (from 'start' to 'end') and to set the
'span' variable slightly earlier. At this point, the cache entry is for a
file that is at least one directory deeper than the current cache tree node.
The path is also not in the sparse-checkout because of an earlier
path_in_sparse_checkout() check above the loop. So we are trying to collapse
this directory by recursively calling convert_to_sparse_rec() over that span
of entries, but the negative value prevents us from predicting that number
without scanning.
Theoretically, we could scan to find the range of entries that match this
directory and determine if they truly do have an intent-to-add bit and then
collapse as many child trees as possible (the ones with valid cache tree
nodes). That would be a non-trivial change for performance-only benefit.
Since this combination of the intent-to-add and sparse index features has so
far gone undetected by real users, this scenario is unlikely to be worth
such a change.
We settle for the simplest change that prevents a bug: don't try to collapse
a node that is invalid for this reason. The tests that would demonstrate a
segfault now pass. Further, they demonstrate that the intent-to-add bit
persists in the index file after changing the sparse-checkout scope. The
test in t1092 demonstrates how some sparse directories could be collapsed
further with a more involved fix, if so desired in the future.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
sparse-index: avoid crash on intent-to-add entry outside the cone
I discovered this while taking inventory of the un-audited
ensure_full_index() calls, finding this block:
/* TODO: audit for interaction with sparse-index. */
ensure_full_index(the_repository->index);
for (i = 0; i < the_repository->index->cache_nr; i++)
if (ce_intent_to_add(the_repository->index->cache[i]))
ita_nr++;
committable = the_repository->index->cache_nr > ita_nr;
This led me to realize that the sparse-index collapse algorithm didn't
take intent-to-add into account for avoiding a collapse. We already
avoid collapse to a sparse directory if there exists a submodule
somewhere, but we don't do the same for intent-to-add.
I thought I'd just find a normal bug, not a segfault, but that made the
fix somewhat simpler though less efficient in the final result.
Thanks, -Stolee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2167%2Fderrickstolee%2Fita-segfault-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2167/derrickstolee/ita-segfault-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2167
sparse-index.c | 9 ++++-
t/t1092-sparse-checkout-compatibility.sh | 48 ++++++++++++++++++++++++
t/t3705-add-sparse-checkout.sh | 26 +++++++++++++
3 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/sparse-index.c b/sparse-index.c
index 1ed769b78d..c1fa231a89 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -113,10 +113,17 @@ static int convert_to_sparse_rec(struct index_state *istate,
continue;
}
+ span = ct->down[pos]->cache_tree->entry_count;
+ if (span < 0) {
+ /* cache-tree entry is invalidated, cannot collapse. */
+ istate->cache[num_converted++] = ce;
+ i++;
+ continue;
+ }
+
strbuf_setlen(&child_path, 0);
strbuf_add(&child_path, ce->name, slash - ce->name + 1);
- span = ct->down[pos]->cache_tree->entry_count;
count = convert_to_sparse_rec(istate,
num_converted, i, i + span,
child_path.buf, child_path.len,
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 8186da5c88..c433de2c1e 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -384,6 +384,54 @@ test_expect_success 'add, commit, checkout' '
test_all_match git checkout -
'
+test_expect_success 'intent-to-add entries outside sparse-checkout' '
+ init_repos &&
+
+ write_script edit-contents <<-\EOF &&
+ echo text >>$1
+ EOF
+
+ test_sparse_match git sparse-checkout set deep folder1 &&
+ run_on_sparse mkdir -p folder1 &&
+ run_on_all ../edit-contents folder1/newita &&
+ test_sparse_match git add -N folder1/newita &&
+
+ test_sparse_match git sparse-checkout set deep &&
+ test_sparse_match git status --porcelain=v2 &&
+ test_sparse_match git ls-files --stage
+'
+
+test_expect_success 'intent-to-add with --sparse outside sparse-checkout' '
+ init_repos &&
+
+ write_script edit-contents <<-\EOF &&
+ echo text >>$1
+ EOF
+
+ run_on_all mkdir -p folder1 &&
+ run_on_all ../edit-contents folder1/newita &&
+ test_all_match git add --sparse --intent-to-add folder1/newita &&
+
+ test_all_match git status --porcelain=v2 &&
+ test_all_match git ls-files --stage &&
+ test_all_match git diff --cached --stat &&
+
+ # Ensure sparse index stores correct sparse directories and
+ # intent-to-add path.
+ git -C sparse-index ls-files --format="%(path)" --sparse >out &&
+
+ # These paths should be present in index as-is.
+ test_grep "^before/\$" out &&
+ test_grep "^folder1/newita\$" out &&
+ test_grep "^folder2/\$" out &&
+ test_grep "^x/\$" out &&
+
+ # folder/0/ could theoretically be collapsed to a sparse
+ # directory entry, but the current implementation avoids the
+ # reduction because of folder1/newita
+ test_grep "^folder1/0/0/0\$" out
+'
+
test_expect_success 'git add, checkout, and reset with -p' '
init_repos &&
diff --git a/t/t3705-add-sparse-checkout.sh b/t/t3705-add-sparse-checkout.sh
index 53a4782267..cf3f42a353 100755
--- a/t/t3705-add-sparse-checkout.sh
+++ b/t/t3705-add-sparse-checkout.sh
@@ -233,4 +233,30 @@ test_expect_success 'refuse to add non-skip-worktree file from sparse dir' '
test_cmp expect stderr
'
+test_expect_success 'intent-to-add entry and sparse index' '
+ test_when_finished "git sparse-checkout disable" &&
+ test_when_finished "git reset --hard" &&
+
+ git sparse-checkout disable &&
+ mkdir -p in out &&
+ echo base >in/file &&
+ echo base >out/file &&
+ git add in/file out/file &&
+ git commit -m "in and out directories" &&
+
+ # enable sparse-checkout, but with all child directories.
+ git config index.sparse true &&
+ git sparse-checkout set in out &&
+
+ # create a new path and set intent-to-add bit
+ echo new >out/newita &&
+ git add -N out/newita &&
+
+ # collapse sparse-checkout, and make sure that the sparse index
+ # maintains the intent-to-add bit.
+ git sparse-checkout set in &&
+ git ls-files --error-unmatch out/newita &&
+ git status --porcelain
+'
+
test_done
base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v7 2/3] graph: add a 2 commit buffer for lookahead
From: Kristofer Karlsson @ 2026-07-06 13:44 UTC (permalink / raw)
To: Chandra Pratap
Cc: Pablo Sabater, git, ayu.chandekar, christian.couder, gitster,
jltobler, karthik.188, peff, phillip.wood, siddharthasthana31
In-Reply-To: <CA+J6zkQFsTA3QfU5VVjQ=KhJCg_pCrTgW9zinAUC4D9YwsyOkQ@mail.gmail.com>
The hardcoded size-2 lookahead buffer was my suggestion,
so I am responding inline with my thoughts although Pablo is
the right person for making further changes (if any).
On Mon, 6 Jul 2026, Chandra Pratap <chandrapratap3519@gmail.com> wrote:
> Do we need to NULL out the retrieved buffer entries? If so, it is
> worthwhile asserting that the entire buffer is NULLed out in the
> !graph->lookahead_nr check above.
You're right, it's not technically needed, and there are many places
in the repo where stale data remains in buffers, and it would be possible
to do that here too. I don't think it matters much in practice though,
and NULLing them out would perhaps prevent some accidental reuse on bugs
(NULL would crash instead).
As for asserting: rather than checking that empty slots are NULL
(which just verifies our own cleanup), it might be more useful to
assert that a slot is non-NULL when lookahead_nr says it should be
populated, i.e. assert on read rather than on empty. But even that
may be overkill for a 2-element internal buffer.
> Not the best engineering practice, but I guess it is fine to constrain
> the logic to _only_ a 2-entry buffer since that's what we'll always
> deal with anyway.
I did consider making it a proper ring buffer, but it felt like
overkill (and I could not find any other existing ring buffer to
piggy-back on in the repo), and the lookahead depth is
structurally tied to the algorithm - we only ever need two more
elements.
It also helps that this is entirely internal to graph.c. If the
buffer were part of a broader API, a less hardcoded approach
would be more appropriate indeed.
> We should use ARRAY_SIZE(graph->lookahead) instead of hardcoding
> the value 2.
Agreed, that is a nice improvement. What do you think Pablo?
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH 08/11] sequencer: simplify pick_one_commit()
From: Phillip Wood @ 2026-07-06 13:40 UTC (permalink / raw)
To: Oswald Buddenhagen, Phillip Wood
Cc: git, Uwe Kleine-König, Junio C Hamano
In-Reply-To: <akuMQ45aQejRcQ_Y@ugly.lan>
On 06/07/2026 12:06, Oswald Buddenhagen wrote:
> On Tue, Jun 30, 2026 at 04:28:58PM +0100, Phillip Wood wrote:
>> +++ b/sequencer.c
>> @@ -4981,14 +4983,13 @@ static int pick_one_commit(struct repository *r,
>> }
>> return error_with_patch(r, commit,
>> arg, item->arg_len, opts, res, !res);
>> - }
>> - if (is_rebase_i(opts) && !res)
>> + } else if (!res) {
>>
> because of this ...
>
>> record_in_rewritten(&item->commit->object.oid,
>> peek_command(todo_list, 1));
>> - if (res && is_fixup(item->command)) {
>> + } else if (res && is_fixup(item->command)) {
>>
> .. the res conditional is pointless here.
>
>> return error_failed_squash(r, item->commit, opts,
>> item->arg_len, arg);
>> - } else if (res && is_rebase_i(opts)) {
>> + } else if (res) {
>>
> and here as well.
I meant to add a comment about that to the commit message. I
deliberately left them alone so that when we convert them to use the
enum it is clear that these arms are handling cases with conflicts.
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH 10/11] sequencer: use an enum to represent result of picking a commit
From: Phillip Wood @ 2026-07-06 13:39 UTC (permalink / raw)
To: Oswald Buddenhagen, Phillip Wood
Cc: git, Uwe Kleine-König, Junio C Hamano
In-Reply-To: <akuNmMFST8W2H2Ru@ugly.lan>
On 06/07/2026 12:12, Oswald Buddenhagen wrote:
> On Tue, Jun 30, 2026 at 04:29:00PM +0100, Phillip Wood wrote:
>> Rather than using an integer where -1 is an error, 0 is success and
>> 1 means there were conflicts use an enum. This is clearer and lets
>> us add a separate return value for commits that are dropped because
>> they become empty in the next commit.
>>
> have you attempted widening the scope of the enum? the three conversions
> between the new enum and existing int return values irk me.
I know what you mean, but how wide should be go? Using the enum just one
level up the call chain means converting a whole load of functions which
creates a lot of churn that someone needs to review. I decided to keep
the enum limited to this scope for now to avoid that.
Thanks
Phillip
^ permalink raw reply
* [PATCH 2/2] reftable: fix quadratic behavior when re-creating deleted refs
From: Kristofer Karlsson via GitGitGadget @ 2026-07-06 13:35 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2166.git.1783344957.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
When many refs are deleted and then re-created, update-ref exhibits
quadratic behavior. With 8000 refs deleted and re-created, the
runtime is ~15s, quadrupling for each doubling of input size.
The root cause is the merged iterator's suppress_deletions flag.
When set, merged_iter_next_void() silently consumes tombstone records
in a tight internal loop before returning to the caller. This
prevents higher-level code from checking iteration bounds (such as
prefix or refname comparisons) until after all tombstones have been
scanned.
This affects two code paths during ref creation:
- refs_verify_refnames_available() seeks to "refs/tags/foo-1/" to
check for D/F conflicts and must scan through all subsequent
tombstones before the caller can see that they are past the prefix
of interest.
- reftable_backend_read_ref() seeks to a specific refname and must
scan through all subsequent tombstones before returning "not
found", because the merged iterator skips the matching tombstone
and searches for the next live record.
Fix this by removing suppress_deletions from the merged iterator and
instead handling deletion records at each call site in the reftable
backend, where prefix and refname bounds are available. Tombstones
are now returned to callers, which skip them after their existing
bounds checks. This allows iteration to terminate as soon as a
tombstone past the relevant bound is encountered.
This also requires adding deletion checks to the log iteration paths,
since suppress_deletions applied to both ref and log iterators.
Both tests in p1401 go from ~14s to ~0.2s with this change.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
refs/reftable-backend.c | 54 ++++++++++++++++++++++++++++++++---------
reftable/merged.c | 12 +--------
reftable/merged.h | 4 ---
reftable/stack.c | 1 -
4 files changed, 44 insertions(+), 27 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 4ae22922de..8c4f119ff1 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -86,7 +86,8 @@ static int reftable_backend_read_ref(struct reftable_backend *be,
if (ret)
goto done;
- if (strcmp(ref.refname, refname)) {
+ if (strcmp(ref.refname, refname) ||
+ reftable_ref_record_is_deletion(&ref)) {
ret = 1;
goto done;
}
@@ -112,7 +113,6 @@ static int reftable_backend_read_ref(struct reftable_backend *be,
oidread(oid, reftable_ref_record_val1(&ref),
&hash_algos[hash_id]);
} else {
- /* We got a tombstone, which should not happen. */
BUG("unhandled reference value type %d", ref.value_type);
}
@@ -633,6 +633,9 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
break;
}
+ if (iter->ref.value_type == REFTABLE_REF_DELETION)
+ continue;
+
if (iter->exclude_patterns && should_exclude_current_ref(iter))
continue;
@@ -1492,6 +1495,8 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&log))
+ continue;
ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
tombstone = &logs[logs_nr++];
@@ -1889,6 +1894,8 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&old_log))
+ continue;
free(old_log.refname);
@@ -2019,6 +2026,9 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
if (iter->err)
break;
+ if (reftable_log_record_is_deletion(&iter->log))
+ continue;
+
/*
* We want the refnames that we have reflogs for, so we skip if
* we've already produced this name. This could be faster by
@@ -2178,6 +2188,8 @@ static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&log))
+ continue;
ret = yield_log_record(refs, &log, fn, cb_data);
if (ret)
@@ -2230,6 +2242,10 @@ static int reftable_be_for_each_reflog_ent(struct ref_store *ref_store,
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&log)) {
+ reftable_log_record_release(&log);
+ continue;
+ }
ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
logs[logs_nr++] = log;
@@ -2276,18 +2292,26 @@ static int reftable_be_reflog_exists(struct ref_store *ref_store,
goto done;
/*
- * Check whether we get at least one log record for the given ref name.
- * If so, the reflog exists, otherwise it doesn't.
+ * Check whether we get at least one non-deleted log record for the
+ * given ref name. If so, the reflog exists, otherwise it doesn't.
*/
- ret = reftable_iterator_next_log(&it, &log);
- if (ret < 0)
- goto done;
- if (ret > 0) {
- ret = 0;
- goto done;
+ while (1) {
+ ret = reftable_iterator_next_log(&it, &log);
+ if (ret < 0)
+ goto done;
+ if (ret > 0) {
+ ret = 0;
+ goto done;
+ }
+ if (strcmp(log.refname, refname)) {
+ ret = 0;
+ goto done;
+ }
+ if (!reftable_log_record_is_deletion(&log))
+ break;
}
- ret = strcmp(log.refname, refname) == 0;
+ ret = 1;
done:
reftable_iterator_destroy(&it);
@@ -2399,6 +2423,8 @@ static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_da
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&log))
+ continue;
tombstone.refname = (char *)arg->refname;
tombstone.value_type = REFTABLE_LOG_DELETION;
@@ -2580,6 +2606,10 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
reftable_log_record_release(&log);
break;
}
+ if (reftable_log_record_is_deletion(&log)) {
+ reftable_log_record_release(&log);
+ continue;
+ }
oidread(&old_oid, log.value.update.old_hash,
ref_store->repo->hash_algo);
@@ -2746,6 +2776,8 @@ static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,
report.path = refname.buf;
switch (ref.value_type) {
+ case REFTABLE_REF_DELETION:
+ continue;
case REFTABLE_REF_VAL1:
case REFTABLE_REF_VAL2: {
struct object_id oid;
diff --git a/reftable/merged.c b/reftable/merged.c
index 733de07454..2f9a361234 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -26,7 +26,6 @@ struct merged_iter {
struct merged_subiter *subiters;
struct merged_iter_pqueue pq;
size_t subiters_len;
- int suppress_deletions;
ssize_t advance_index;
};
@@ -166,15 +165,7 @@ static int merged_iter_seek_void(void *it, struct reftable_record *want)
static int merged_iter_next_void(void *p, struct reftable_record *rec)
{
- struct merged_iter *mi = p;
- while (1) {
- int err = merged_iter_next_entry(mi, rec);
- if (err)
- return err;
- if (mi->suppress_deletions && reftable_record_is_deletion(rec))
- continue;
- return 0;
- }
+ return merged_iter_next_entry(p, rec);
}
static struct reftable_iterator_vtable merged_iter_vtable = {
@@ -278,7 +269,6 @@ int merged_table_init_iter(struct reftable_merged_table *mt,
goto out;
}
mi->advance_index = -1;
- mi->suppress_deletions = mt->suppress_deletions;
mi->subiters = subiters;
mi->subiters_len = mt->tables_len;
diff --git a/reftable/merged.h b/reftable/merged.h
index 4317e5f5f6..6fafd1d080 100644
--- a/reftable/merged.h
+++ b/reftable/merged.h
@@ -17,10 +17,6 @@ struct reftable_merged_table {
size_t tables_len;
enum reftable_hash hash_id;
- /* If unset, produce deletions. This is useful for compaction. For the
- * full stack, deletions should be produced. */
- int suppress_deletions;
-
uint64_t min;
uint64_t max;
};
diff --git a/reftable/stack.c b/reftable/stack.c
index 1fba96ddb3..77aeac4715 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -337,7 +337,6 @@ static int reftable_stack_reload_once(struct reftable_stack *st,
/* Update the stack to point to the new tables. */
if (st->merged)
reftable_merged_table_free(st->merged);
- new_merged->suppress_deletions = 1;
st->merged = new_merged;
if (st->tables)
--
gitgitgadget
^ permalink raw reply related
* [PATCH 1/2] t: add tests for ref tombstone scenarios
From: Kristofer Karlsson via GitGitGadget @ 2026-07-06 13:35 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2166.git.1783344957.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Add a performance test and a correctness test for update-ref when
many tombstones are present in a reftable.
The performance test (p1401) exercises two scenarios:
- All refs are deleted (creating tombstones) and then re-created
with the same names, which currently exhibits quadratic behavior.
- An asymmetric variant where refs are deleted and then new,
differently-named refs are created. When the tombstones sort
after the new refs, every create scans all tombstones, making
this case even worse than re-creating the same refs.
The correctness test (t0610) verifies that refs deleted and then
re-created with the same names are visible afterwards.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
t/perf/p1401-ref-store-tombstones.sh | 44 ++++++++++++++++++++++++++++
t/t0610-reftable-basics.sh | 22 ++++++++++++++
2 files changed, 66 insertions(+)
create mode 100755 t/perf/p1401-ref-store-tombstones.sh
diff --git a/t/perf/p1401-ref-store-tombstones.sh b/t/perf/p1401-ref-store-tombstones.sh
new file mode 100755
index 0000000000..e40a6dcbf4
--- /dev/null
+++ b/t/perf/p1401-ref-store-tombstones.sh
@@ -0,0 +1,44 @@
+#!/bin/sh
+
+test_description="Tests performance of ref operations with many tombstones"
+
+. ./perf-lib.sh
+
+test_expect_success "setup" '
+ git init --ref-format=reftable repo &&
+ blob=$(echo foo | git -C repo hash-object -w --stdin) &&
+ for i in $(test_seq 8000)
+ do
+ printf "create refs/tags/tag-%d %s\n" "$i" "$blob" ||
+ return 1
+ done >repo/input &&
+ git -C repo update-ref --stdin <repo/input &&
+ git -C repo for-each-ref --format="delete %(refname)" |
+ git -C repo update-ref --stdin
+'
+
+test_perf "recreate refs after mass delete" '
+ git -C repo update-ref --stdin <repo/input &&
+ git -C repo for-each-ref --format="delete %(refname)" |
+ git -C repo update-ref --stdin
+'
+
+test_expect_success "setup asymmetric" '
+ for i in $(test_seq 8000)
+ do
+ printf "create refs/tags/old-%d %s\n" "$i" "$blob" ||
+ return 1
+ done >repo/input-old &&
+ sed "s/old-/new-/" <repo/input-old >repo/input-new &&
+ git -C repo update-ref --stdin <repo/input-old &&
+ git -C repo for-each-ref --format="delete %(refname)" |
+ git -C repo update-ref --stdin
+'
+
+test_perf "create new refs after deleting differently-named refs" '
+ git -C repo update-ref --stdin <repo/input-new &&
+ git -C repo for-each-ref --format="delete %(refname)" |
+ git -C repo update-ref --stdin
+'
+
+test_done
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index e19e036898..4b7cfe38e4 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -1163,4 +1163,26 @@ test_expect_success 'writes do not persist peeled value for invalid tags' '
)
'
+test_expect_success 'delete and re-create refs with tombstones' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ test_commit -C repo A &&
+ A=$(git -C repo rev-parse HEAD) &&
+ cat >input <<-EOF &&
+ create refs/tags/a $A
+ create refs/tags/b $A
+ create refs/tags/c $A
+ EOF
+ git -C repo update-ref --stdin <input &&
+
+ # delete all tags, leaving tombstones
+ git -C repo for-each-ref --format="delete %(refname)" refs/tags/ |
+ git -C repo update-ref --stdin &&
+
+ # re-create the same refs and verify they are visible
+ git -C repo update-ref --stdin <input &&
+ git -C repo tag -l >actual &&
+ test_line_count = 3 actual
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH 0/2] reftable: fix quadratic behavior when re-creating deleted refs
From: Kristofer Karlsson via GitGitGadget @ 2026-07-06 13:35 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson
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 root cause is the merged iterator's suppress_deletions flag, which
silently consumes tombstone records in a tight internal loop. This prevents
higher-level code from checking iteration bounds until after all tombstones
have been scanned, making both refs_verify_refnames_available() and
reftable_backend_read_ref() O(n) per call in the presence of tombstones.
The fix removes suppress_deletions from the merged iterator and instead
handles deletion records at each call site in the reftable backend, where
prefix and refname bounds are available. This lets existing bounds checks
terminate iteration early when encountering tombstones past the relevant
bound.
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.
Previous discussion:
https://lore.kernel.org/git/20260701080014.GA3748390@coredump.intra.peff.net/
Kristofer Karlsson (2):
t: add tests for ref tombstone scenarios
reftable: fix quadratic behavior when re-creating deleted refs
refs/reftable-backend.c | 54 ++++++++++++++++++++++------
reftable/merged.c | 12 +------
reftable/merged.h | 4 ---
reftable/stack.c | 1 -
t/perf/p1401-ref-store-tombstones.sh | 44 +++++++++++++++++++++++
t/t0610-reftable-basics.sh | 22 ++++++++++++
6 files changed, 110 insertions(+), 27 deletions(-)
create mode 100755 t/perf/p1401-ref-store-tombstones.sh
base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2166%2Fspkrka%2Freftable-tombstone-perf-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2166/spkrka/reftable-tombstone-perf-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2166
--
gitgitgadget
^ permalink raw reply
* [PATCH v4 5/5] builtin/refs: add "rename" subcommand
From: Patrick Steinhardt @ 2026-07-06 13:27 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Toon Claes
In-Reply-To: <20260706-pks-refs-writing-subcommands-v4-0-d51f6ce7f830@pks.im>
Add a "rename" subcommand to git-refs(1) with the syntax:
$ git refs rename <oldref> <newref>
It renames <oldref> together with its reflog to <newref>; even when used
on a local branch ref, the current value and the reflog of the ref are
the only things that are renamed. Document it and redirect casual users
to "git branch -m" if that is what they wanted to do.
Co-authored-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-refs.adoc | 6 ++
builtin/refs.c | 49 +++++++++++++++
t/meson.build | 1 +
t/t1467-refs-rename.sh | 144 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 200 insertions(+)
diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index e6a3528349..ce278c59bf 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -23,6 +23,7 @@ git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude
git refs create [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value>
git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
+git refs rename [--message=<reason>] <old-ref> <new-ref>
DESCRIPTION
-----------
@@ -71,6 +72,11 @@ update::
`<new-value>` deletes the branch, whereas an all-zeroes `<old-value>`
ensures that the branch does not yet exist.
+rename::
+ Rename the reference `<oldref>` to `<newref>`. The old reference must
+ exist and the new reference must not yet exist, and both must have a
+ well-formed name (see linkgit:git-check-ref-format[1]).
+
OPTIONS
-------
diff --git a/builtin/refs.c b/builtin/refs.c
index 1ebaf30149..a9ca2058ee 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -30,6 +30,9 @@
#define REFS_UPDATE_USAGE \
N_("git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]")
+#define REFS_RENAME_USAGE \
+ N_("git refs rename [--message=<reason>] <old-ref> <new-ref>")
+
static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
struct repository *repo)
{
@@ -327,6 +330,50 @@ static int cmd_refs_update(int argc, const char **argv, const char *prefix,
return ret;
}
+static int cmd_refs_rename(int argc, const char **argv, const char *prefix,
+ struct repository *repo)
+{
+ static char const * const refs_rename_usage[] = {
+ REFS_RENAME_USAGE,
+ NULL
+ };
+ const char *message = NULL;
+ struct option opts[] = {
+ OPT_STRING(0, "message", &message, N_("reason"),
+ N_("reason of the update")),
+ OPT_END(),
+ };
+ const char *oldref, *newref;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, opts, refs_rename_usage, 0);
+ if (argc != 2)
+ usage(_("rename requires old and new reference name"));
+ if (message && !*message)
+ die(_("refusing to perform update with empty message"));
+
+ repo_config(repo, git_default_config, NULL);
+
+ oldref = argv[0];
+ newref = argv[1];
+
+ if (check_refname_format(oldref, 0))
+ die(_("invalid ref format: '%s'"), oldref);
+ if (check_refname_format(newref, 0))
+ die(_("invalid ref format: '%s'"), newref);
+
+ if (!refs_ref_exists(get_main_ref_store(repo), oldref))
+ die(_("reference does not exist: '%s'"), oldref);
+ if (refs_ref_exists(get_main_ref_store(repo), newref))
+ die(_("reference already exists: '%s'"), newref);
+
+ ret = refs_rename_ref(get_main_ref_store(repo), oldref, newref, message);
+
+ if (ret < 0)
+ ret = 1;
+ return ret;
+}
+
int cmd_refs(int argc,
const char **argv,
const char *prefix,
@@ -341,6 +388,7 @@ int cmd_refs(int argc,
REFS_CREATE_USAGE,
REFS_DELETE_USAGE,
REFS_UPDATE_USAGE,
+ REFS_RENAME_USAGE,
NULL,
};
parse_opt_subcommand_fn *fn = NULL;
@@ -353,6 +401,7 @@ int cmd_refs(int argc,
OPT_SUBCOMMAND("create", &fn, cmd_refs_create),
OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
+ OPT_SUBCOMMAND("rename", &fn, cmd_refs_rename),
OPT_END(),
};
diff --git a/t/meson.build b/t/meson.build
index 541e6f919c..a39fd8c4c4 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -226,6 +226,7 @@ integration_tests = [
't1464-refs-delete.sh',
't1465-refs-update.sh',
't1466-refs-create.sh',
+ 't1467-refs-rename.sh',
't1500-rev-parse.sh',
't1501-work-tree.sh',
't1502-rev-parse-parseopt.sh',
diff --git a/t/t1467-refs-rename.sh b/t/t1467-refs-rename.sh
new file mode 100755
index 0000000000..2b28be75c8
--- /dev/null
+++ b/t/t1467-refs-rename.sh
@@ -0,0 +1,144 @@
+#!/bin/sh
+
+test_description='git refs rename'
+
+. ./test-lib.sh
+
+setup_repo () {
+ git init "$1" &&
+ test_commit -C "$1" A &&
+ test_commit -C "$1" B
+}
+
+test_ref_matches () {
+ git rev-parse "$1" >expect &&
+ echo "$2" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'rename an existing reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A &&
+ git refs rename refs/heads/foo refs/heads/bar &&
+ test_must_fail git refs exists refs/heads/foo &&
+ test_ref_matches refs/heads/bar $A
+ )
+'
+
+test_expect_success 'rename moves the reflog along with the reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update --message="rename me" refs/heads/foo $A &&
+ git refs rename refs/heads/foo refs/heads/bar &&
+ git reflog show refs/heads/bar >reflog &&
+ test_grep "rename me" reflog &&
+ test_must_fail git reflog exists refs/heads/foo
+ )
+'
+
+test_expect_success 'rename with message records reason in reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A &&
+ git refs rename --message="rename reason" refs/heads/foo refs/heads/bar &&
+ git reflog show refs/heads/bar >actual &&
+ test_grep "rename reason" actual
+ )
+'
+
+test_expect_success 'rename a nonexistent reference fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ test_must_fail git refs rename refs/heads/foo refs/heads/bar 2>err &&
+ test_grep "reference does not exist" err
+ )
+'
+
+test_expect_success 'rename to an existing reference fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ git refs update refs/heads/bar $B &&
+ test_must_fail git refs rename refs/heads/foo refs/heads/bar 2>err &&
+ test_grep "reference already exists" err
+ )
+'
+
+test_expect_success 'rename with symbolic ref fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs create refs/heads/target $A &&
+ git symbolic-ref refs/heads/symref refs/heads/target &&
+ ! git refs rename refs/heads/symref refs/heads/renamed 2>err &&
+ test_grep "is a symbolic ref, .* not supported" err
+ )
+'
+
+test_expect_success 'rename with empty message fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs rename --message= refs/heads/foo refs/heads/bar 2>err &&
+ test_grep "empty message" err
+ )
+'
+
+test_expect_success 'rename with invalid old reference name fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ test_must_fail git refs rename "refs/heads/foo..bar" refs/heads/bar 2>err &&
+ test_grep "invalid ref format" err
+ )
+'
+
+test_expect_success 'rename with invalid new reference name fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs rename refs/heads/foo "refs/heads/bar..baz" 2>err &&
+ test_grep "invalid ref format" err
+ )
+'
+
+test_expect_success 'rename with too few arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs rename refs/heads/foo 2>err &&
+ test_grep "requires old and new reference name" err
+'
+
+test_expect_success 'rename with too many arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs rename refs/heads/foo refs/heads/bar refs/heads/baz 2>err &&
+ test_grep "requires old and new reference name" err
+'
+
+test_done
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH v4 4/5] builtin/refs: add "create" subcommand
From: Patrick Steinhardt @ 2026-07-06 13:27 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Toon Claes
In-Reply-To: <20260706-pks-refs-writing-subcommands-v4-0-d51f6ce7f830@pks.im>
The "update" subcommand cannot only update an existing reference, but it
can also create new branches and delete existing branches by specifying
the all-zeroes object ID as either old or new value. Despite that, we
already have the "delete" subcommand as a handy shortcut so that a user
can easily delete a branch. This relieves them of needing to understand
the more arcane uses of the "update" command, and of counting the number
of zeroes they need to pass.
But while we have a "delete" subcommand, we don't have an equivalent
that would allow the user to create a new branch, which creates a
certain asymmetry.
Add a new "create" subcommand to plug this gap.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-refs.adoc | 5 ++
builtin/refs.c | 52 +++++++++++++++
t/meson.build | 1 +
t/t1466-refs-create.sh | 151 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 209 insertions(+)
diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index 6475bdcc62..e6a3528349 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -20,6 +20,7 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
[ --stdin | (<pattern>...)]
git refs exists <ref>
git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
+git refs create [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value>
git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
@@ -53,6 +54,10 @@ optimize::
usage. This subcommand is an alias for linkgit:git-pack-refs[1] and
offers identical functionality.
+create::
+ Create the given reference, which must not already exist, pointing at
+ `<new-value>`.
+
delete::
Delete the given reference. This subcommand mirrors `git update-ref -d`
(see linkgit:git-update-ref[1]). When `<old-value>` is given, the
diff --git a/builtin/refs.c b/builtin/refs.c
index 08453ae1c8..1ebaf30149 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -21,6 +21,9 @@
#define REFS_OPTIMIZE_USAGE \
N_("git refs optimize " PACK_REFS_OPTS)
+#define REFS_CREATE_USAGE \
+ N_("git refs create [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value>")
+
#define REFS_DELETE_USAGE \
N_("git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]")
@@ -181,6 +184,53 @@ static int cmd_refs_optimize(int argc, const char **argv, const char *prefix,
return pack_refs_core(argc, argv, prefix, repo, refs_optimize_usage);
}
+static int cmd_refs_create(int argc, const char **argv, const char *prefix,
+ struct repository *repo)
+{
+ static char const * const refs_create_usage[] = {
+ REFS_CREATE_USAGE,
+ NULL
+ };
+ const char *message = NULL;
+ unsigned flags = 0;
+ struct option opts[] = {
+ OPT_STRING(0, "message", &message, N_("reason"),
+ N_("reason of the update")),
+ OPT_BIT(0 ,"no-deref", &flags,
+ N_("update <refname> not the one it points to"),
+ REF_NO_DEREF),
+ OPT_BIT(0, "create-reflog", &flags, N_("create a reflog"),
+ REF_FORCE_CREATE_REFLOG),
+ OPT_END(),
+ };
+ struct object_id newoid;
+ const char *refname;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, opts, refs_create_usage, 0);
+ if (argc != 2)
+ usage(_("create requires reference name and an object ID"));
+
+ if (message && !*message)
+ die(_("refusing to perform update with empty message"));
+
+ repo_config(repo, git_default_config, NULL);
+
+ refname = argv[0];
+ if (repo_get_oid_with_flags(repo, argv[1], &newoid, GET_OID_SKIP_AMBIGUITY_CHECK))
+ die(_("invalid object ID: '%s'"), argv[1]);
+ if (is_null_oid(&newoid))
+ die(_("cannot create reference with null new object ID"));
+
+ ret = refs_update_ref(get_main_ref_store(repo), message, refname,
+ &newoid, null_oid(repo->hash_algo), flags,
+ UPDATE_REFS_MSG_ON_ERR);
+
+ if (ret < 0)
+ ret = 1;
+ return ret;
+}
+
static int cmd_refs_delete(int argc, const char **argv, const char *prefix,
struct repository *repo)
{
@@ -288,6 +338,7 @@ int cmd_refs(int argc,
"git refs list " COMMON_USAGE_FOR_EACH_REF,
REFS_EXISTS_USAGE,
REFS_OPTIMIZE_USAGE,
+ REFS_CREATE_USAGE,
REFS_DELETE_USAGE,
REFS_UPDATE_USAGE,
NULL,
@@ -299,6 +350,7 @@ int cmd_refs(int argc,
OPT_SUBCOMMAND("list", &fn, cmd_refs_list),
OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
+ OPT_SUBCOMMAND("create", &fn, cmd_refs_create),
OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
OPT_END(),
diff --git a/t/meson.build b/t/meson.build
index 2063962dab..541e6f919c 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -225,6 +225,7 @@ integration_tests = [
't1463-refs-optimize.sh',
't1464-refs-delete.sh',
't1465-refs-update.sh',
+ 't1466-refs-create.sh',
't1500-rev-parse.sh',
't1501-work-tree.sh',
't1502-rev-parse-parseopt.sh',
diff --git a/t/t1466-refs-create.sh b/t/t1466-refs-create.sh
new file mode 100755
index 0000000000..cfb21bf863
--- /dev/null
+++ b/t/t1466-refs-create.sh
@@ -0,0 +1,151 @@
+#!/bin/sh
+
+test_description='git refs create'
+
+. ./test-lib.sh
+
+setup_repo () {
+ git init "$1" &&
+ test_commit -C "$1" A &&
+ test_commit -C "$1" B
+}
+
+test_ref_matches () {
+ git rev-parse "$1" >expect &&
+ echo "$2" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'create a new reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs create refs/heads/foo $A &&
+ test_ref_matches refs/heads/foo "$A"
+ )
+'
+
+test_expect_success 'create fails when the reference already exists' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs create refs/heads/foo $A &&
+ test_must_fail git refs create refs/heads/foo $B 2>err &&
+ test_grep "reference already exists" err &&
+ test_ref_matches refs/heads/foo "$A"
+ )
+'
+
+test_expect_success 'create with null new value fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ test_must_fail git refs create refs/heads/foo $ZERO_OID 2>err &&
+ test_grep "null new object ID" err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'create with invalid new value fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ test_must_fail git refs create refs/heads/foo invalid-oid 2>err &&
+ test_grep "invalid object ID" err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'create does not create a reflog by default' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs create refs/foo $A &&
+ test_must_fail git reflog exists refs/foo
+ )
+'
+
+test_expect_success 'create creates a reflog with --create-reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs create --create-reflog refs/foo $A &&
+ git reflog exists refs/foo
+ )
+'
+
+test_expect_success 'create with message records reason in reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs create --message="create reason" refs/heads/foo $A &&
+ git reflog show refs/heads/foo >actual &&
+ test_grep "create reason$" actual
+ )
+'
+
+test_expect_success 'create with symref target creates target reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git symbolic-ref refs/heads/symref refs/heads/target &&
+ git refs create refs/heads/symref $A &&
+ git reflog exists refs/heads/target
+ )
+'
+
+test_expect_success 'create with symref target and --no-deref refuses to create reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git symbolic-ref refs/heads/symref refs/heads/target &&
+ test_must_fail git refs create --no-deref refs/heads/symref $A 2>err &&
+ test_grep "dangling symref already exists" err &&
+ test_must_fail git reflog exists refs/heads/target
+ )
+'
+
+test_expect_success 'create with empty message fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ test_must_fail git refs create --message= refs/heads/foo $A 2>err &&
+ test_grep "empty message" err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'create without arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs create 2>err &&
+ test_grep "requires reference name" err
+'
+
+test_expect_success 'create with too many arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs create refs/heads/foo a b 2>err &&
+ test_grep "requires reference name" err
+'
+
+test_done
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH v4 3/5] builtin/refs: add "update" subcommand
From: Patrick Steinhardt @ 2026-07-06 13:27 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Toon Claes
In-Reply-To: <20260706-pks-refs-writing-subcommands-v4-0-d51f6ce7f830@pks.im>
Add a new "update" subcommand which mirrors `git update-ref <refname>
<oldoid> <newoid>`. This follows the same reasoning as the preceding
commit.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-refs.adoc | 12 ++
builtin/refs.c | 55 +++++++++
t/meson.build | 1 +
t/t1465-refs-update.sh | 268 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 336 insertions(+)
diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index 2633934463..6475bdcc62 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -21,6 +21,7 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
git refs exists <ref>
git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
+git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
DESCRIPTION
-----------
@@ -58,6 +59,13 @@ delete::
reference is only deleted after verifying that it currently contains
`<old-value>`.
+update::
+ Update the given reference to point at `<new-value>`. If `<old-value>`
+ is given, the reference is only updated after verifying that it
+ currently contains `<old-value>`. As a special case, an all-zeroes
+ `<new-value>` deletes the branch, whereas an all-zeroes `<old-value>`
+ ensures that the branch does not yet exist.
+
OPTIONS
-------
@@ -99,6 +107,10 @@ include::pack-refs-options.adoc[]
The following options are specific to commands which write references:
+`--create-reflog`::
+ Create a reflog for the reference even if one would not ordinarily be
+ created.
+
`--message=<reason>`::
Use the given <reason> string for the reflog entry associated with the
update. An empty message is rejected.
diff --git a/builtin/refs.c b/builtin/refs.c
index edb7d61663..08453ae1c8 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -24,6 +24,9 @@
#define REFS_DELETE_USAGE \
N_("git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]")
+#define REFS_UPDATE_USAGE \
+ N_("git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]")
+
static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
struct repository *repo)
{
@@ -224,6 +227,56 @@ static int cmd_refs_delete(int argc, const char **argv, const char *prefix,
return ret;
}
+static int cmd_refs_update(int argc, const char **argv, const char *prefix,
+ struct repository *repo)
+{
+ static char const * const refs_update_usage[] = {
+ REFS_UPDATE_USAGE,
+ NULL
+ };
+ const char *message = NULL;
+ unsigned flags = 0;
+ struct option opts[] = {
+ OPT_STRING(0, "message", &message, N_("reason"),
+ N_("reason of the update")),
+ OPT_BIT(0 ,"no-deref", &flags,
+ N_("update <refname> not the one it points to"),
+ REF_NO_DEREF),
+ OPT_BIT(0, "create-reflog", &flags, N_("create a reflog"),
+ REF_FORCE_CREATE_REFLOG),
+ OPT_END(),
+ };
+ struct object_id newoid, oldoid;
+ const char *refname;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, opts, refs_update_usage, 0);
+ if (argc < 2 || argc > 3)
+ usage(_("update requires reference name, new value and an optional old value"));
+
+ if (message && !*message)
+ die(_("refusing to perform update with empty message"));
+
+ repo_config(repo, git_default_config, NULL);
+
+ refname = argv[0];
+ if (repo_get_oid_with_flags(repo, argv[1], &newoid,
+ GET_OID_SKIP_AMBIGUITY_CHECK))
+ die(_("invalid new object ID: '%s'"), argv[1]);
+ if (argc == 3 &&
+ repo_get_oid_with_flags(repo, argv[2], &oldoid,
+ GET_OID_SKIP_AMBIGUITY_CHECK))
+ die(_("invalid old object ID: '%s'"), argv[2]);
+
+ ret = refs_update_ref(get_main_ref_store(repo), message, refname,
+ &newoid, argc == 3 ? &oldoid : NULL, flags,
+ UPDATE_REFS_MSG_ON_ERR);
+
+ if (ret < 0)
+ ret = 1;
+ return ret;
+}
+
int cmd_refs(int argc,
const char **argv,
const char *prefix,
@@ -236,6 +289,7 @@ int cmd_refs(int argc,
REFS_EXISTS_USAGE,
REFS_OPTIMIZE_USAGE,
REFS_DELETE_USAGE,
+ REFS_UPDATE_USAGE,
NULL,
};
parse_opt_subcommand_fn *fn = NULL;
@@ -246,6 +300,7 @@ int cmd_refs(int argc,
OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
+ OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
OPT_END(),
};
diff --git a/t/meson.build b/t/meson.build
index 1ccf08a3b5..2063962dab 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -224,6 +224,7 @@ integration_tests = [
't1462-refs-exists.sh',
't1463-refs-optimize.sh',
't1464-refs-delete.sh',
+ 't1465-refs-update.sh',
't1500-rev-parse.sh',
't1501-work-tree.sh',
't1502-rev-parse-parseopt.sh',
diff --git a/t/t1465-refs-update.sh b/t/t1465-refs-update.sh
new file mode 100755
index 0000000000..a9becdda99
--- /dev/null
+++ b/t/t1465-refs-update.sh
@@ -0,0 +1,268 @@
+#!/bin/sh
+
+test_description='git refs update'
+
+. ./test-lib.sh
+
+setup_repo () {
+ git init "$1" &&
+ test_commit -C "$1" A &&
+ test_commit -C "$1" B
+}
+
+test_ref_matches () {
+ git rev-parse "$1" >expect &&
+ echo "$2" >actual &&
+ test_cmp expect actual
+}
+
+test_expect_success 'update creates a new reference' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A &&
+ test_ref_matches refs/heads/foo "$A"
+ )
+'
+
+test_expect_success 'update an existing reference without oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ git refs update refs/heads/foo $B &&
+ test_ref_matches refs/heads/foo $B
+ )
+'
+
+test_expect_success 'update with matching oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ git refs update refs/heads/foo $B $A &&
+ test_ref_matches refs/heads/foo $B
+ )
+'
+
+test_expect_success 'update with stale oldvalue fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update refs/heads/foo $B $B 2>err &&
+ test_grep " but expected " err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update can create a new branch with oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A $ZERO_OID 2>err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update can create a new branch without oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A 2>err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update refuses to create preexisting branch' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update refs/heads/foo $B $ZERO_OID 2>err &&
+ test_grep "reference already exists" err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update can delete a branch with oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A 2>err &&
+ git refs update refs/heads/foo $ZERO_OID $A 2>err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'update can delete a branch without oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/heads/foo $A 2>err &&
+ git refs update refs/heads/foo $ZERO_OID 2>err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'update refuses to delete a branch with mismatching value' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A 2>err &&
+ test_must_fail git refs update refs/heads/foo $ZERO_OID $B 2>err &&
+ test_grep " but expected " err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'update refuses to create preexisting branch' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update refs/heads/foo $B $ZERO_OID 2>err &&
+ test_grep "reference already exists" err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+
+test_expect_success 'update with invalid new value fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ test_must_fail git refs update refs/heads/foo invalid-oid 2>err &&
+ test_grep "invalid new object ID" err &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'update with invalid old value fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update refs/heads/foo $B invalid-oid 2>err &&
+ test_grep "invalid old object ID" err &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update --no-deref rewrites the symref itself' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ git symbolic-ref refs/heads/symref refs/heads/foo &&
+ git refs update --no-deref refs/heads/symref $B &&
+ test_must_fail git symbolic-ref refs/heads/symref &&
+ test_ref_matches refs/heads/symref $B &&
+ test_ref_matches refs/heads/foo $A
+ )
+'
+
+test_expect_success 'update does not create a reflog by default' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update refs/foo $A &&
+ test_must_fail git reflog exists refs/foo
+ )
+'
+
+test_expect_success 'update creates a reflog with --create-reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git refs update --create-reflog refs/foo $A &&
+ git reflog exists refs/foo
+ )
+'
+
+test_expect_success 'update with message records reason in reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ git refs update --message=update-reason refs/heads/foo $B &&
+ git reflog show refs/heads/foo >actual &&
+ test_grep "update-reason$" actual
+ )
+'
+
+test_expect_success 'update with empty message fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git refs update refs/heads/foo $A &&
+ test_must_fail git refs update --message= refs/heads/foo $B 2>err &&
+ test_grep "empty message" err
+ )
+'
+
+test_expect_success 'update with too few arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs update refs/heads/foo 2>err &&
+ test_grep "requires reference name, new value" err
+'
+
+test_expect_success 'update with too many arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ test_must_fail git refs update refs/heads/foo $A $B extra 2>err &&
+ test_grep "requires reference name, new value" err
+ )
+'
+
+test_done
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH v4 2/5] builtin/refs: add "delete" subcommand
From: Patrick Steinhardt @ 2026-07-06 13:27 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Toon Claes
In-Reply-To: <20260706-pks-refs-writing-subcommands-v4-0-d51f6ce7f830@pks.im>
Reference-related functionality in Git is currently spread across many
different commands: git-update-ref(1), git-for-each-ref(1),
git-show-ref(1), git-pack-refs(1) and git-symbolic-ref(1). This makes it
hard for users to discover what functionality we have available to work
with references.
We have thus started to consolidate this functionality into git-refs(1),
which is a toolbox of everything related to references. Until now, the
command doesn't handle functionality of git-update-ref(1).
Fix this gap by introducing a new "delete" subcommand, which is the
equivalent of `git update-ref -d`.
Note that we're intentionally not using a generic "write" subcommand
with a "-d" flag. This is rather harder to discover, and subcommands
that are implmented as flags tend to be hard to reason about in the code
as we'd have to handle mutually-exclusive flags that stem from the other
subcommand-like modes.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Documentation/git-refs.adoc | 17 +++++
builtin/refs.c | 51 +++++++++++++++
t/meson.build | 1 +
t/t1464-refs-delete.sh | 152 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 221 insertions(+)
diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index fa33680cc7..2633934463 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -20,6 +20,7 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
[ --stdin | (<pattern>...)]
git refs exists <ref>
git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
+git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
DESCRIPTION
-----------
@@ -51,6 +52,12 @@ optimize::
usage. This subcommand is an alias for linkgit:git-pack-refs[1] and
offers identical functionality.
+delete::
+ Delete the given reference. This subcommand mirrors `git update-ref -d`
+ (see linkgit:git-update-ref[1]). When `<old-value>` is given, the
+ reference is only deleted after verifying that it currently contains
+ `<old-value>`.
+
OPTIONS
-------
@@ -90,6 +97,16 @@ The following options are specific to 'git refs optimize':
include::pack-refs-options.adoc[]
+The following options are specific to commands which write references:
+
+`--message=<reason>`::
+ Use the given <reason> string for the reflog entry associated with the
+ update. An empty message is rejected.
+
+`--no-deref`::
+ Operate on <ref> itself rather than the reference it points to via a
+ symbolic ref.
+
KNOWN LIMITATIONS
-----------------
diff --git a/builtin/refs.c b/builtin/refs.c
index f0faabf45a..edb7d61663 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -21,6 +21,9 @@
#define REFS_OPTIMIZE_USAGE \
N_("git refs optimize " PACK_REFS_OPTS)
+#define REFS_DELETE_USAGE \
+ N_("git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]")
+
static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
struct repository *repo)
{
@@ -175,6 +178,52 @@ static int cmd_refs_optimize(int argc, const char **argv, const char *prefix,
return pack_refs_core(argc, argv, prefix, repo, refs_optimize_usage);
}
+static int cmd_refs_delete(int argc, const char **argv, const char *prefix,
+ struct repository *repo)
+{
+ static char const * const refs_delete_usage[] = {
+ REFS_DELETE_USAGE,
+ NULL
+ };
+ const char *message = NULL;
+ unsigned flags = 0;
+ struct option opts[] = {
+ OPT_STRING(0, "message", &message, N_("reason"),
+ N_("reason of the update")),
+ OPT_BIT(0 ,"no-deref", &flags,
+ N_("update <refname> not the one it points to"),
+ REF_NO_DEREF),
+ OPT_END(),
+ };
+ struct object_id oldoid;
+ const char *refname;
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, opts, refs_delete_usage, 0);
+ if (argc < 1 || argc > 2)
+ usage(_("delete requires reference name and an optional old object ID"));
+
+ if (message && !*message)
+ die(_("refusing to perform update with empty message"));
+
+ repo_config(repo, git_default_config, NULL);
+
+ refname = argv[0];
+ if (argc == 2) {
+ if (repo_get_oid_with_flags(repo, argv[1], &oldoid, GET_OID_SKIP_AMBIGUITY_CHECK))
+ die(_("invalid old object ID: '%s'"), argv[1]);
+ if (is_null_oid(&oldoid))
+ die(_("cannot delete reference with null old object ID"));
+ }
+
+ ret = refs_delete_ref(get_main_ref_store(repo), message, refname,
+ argc == 2 ? &oldoid : NULL, flags);
+
+ if (ret < 0)
+ ret = 1;
+ return ret;
+}
+
int cmd_refs(int argc,
const char **argv,
const char *prefix,
@@ -186,6 +235,7 @@ int cmd_refs(int argc,
"git refs list " COMMON_USAGE_FOR_EACH_REF,
REFS_EXISTS_USAGE,
REFS_OPTIMIZE_USAGE,
+ REFS_DELETE_USAGE,
NULL,
};
parse_opt_subcommand_fn *fn = NULL;
@@ -195,6 +245,7 @@ int cmd_refs(int argc,
OPT_SUBCOMMAND("list", &fn, cmd_refs_list),
OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
+ OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
OPT_END(),
};
diff --git a/t/meson.build b/t/meson.build
index c5832fee05..1ccf08a3b5 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -223,6 +223,7 @@ integration_tests = [
't1461-refs-list.sh',
't1462-refs-exists.sh',
't1463-refs-optimize.sh',
+ 't1464-refs-delete.sh',
't1500-rev-parse.sh',
't1501-work-tree.sh',
't1502-rev-parse-parseopt.sh',
diff --git a/t/t1464-refs-delete.sh b/t/t1464-refs-delete.sh
new file mode 100755
index 0000000000..c88063e494
--- /dev/null
+++ b/t/t1464-refs-delete.sh
@@ -0,0 +1,152 @@
+#!/bin/sh
+
+test_description='git refs delete'
+
+. ./test-lib.sh
+
+setup_repo () {
+ git init "$1" &&
+ test_commit -C "$1" A &&
+ test_commit -C "$1" B
+}
+
+test_expect_success 'delete without oldvalue verification' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ git refs delete refs/heads/foo &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete with matching oldvalue' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ git refs delete refs/heads/foo $A &&
+ test_must_fail git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete with stale oldvalue fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git update-ref refs/heads/foo $A &&
+ test_must_fail git refs delete refs/heads/foo $B 2>err &&
+ test_grep " but expected " err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete with null oldvalue fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ test_must_fail git refs delete refs/heads/foo $ZERO_OID 2>err &&
+ test_grep "null old object ID" err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete with invalid oldvalue fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ test_must_fail git refs delete refs/heads/foo invalid-oid 2>err &&
+ test_grep "invalid old object ID" err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete symref with --no-deref leaves target intact' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ git symbolic-ref refs/heads/symref refs/heads/foo &&
+ git refs delete --no-deref refs/heads/symref &&
+ test_must_fail git refs exists refs/heads/symref &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete symref with --no-deref verifies target OID' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ B=$(git rev-parse B) &&
+ git update-ref refs/heads/foo $A &&
+ git symbolic-ref refs/heads/symref refs/heads/foo &&
+
+ test_must_fail git refs delete --no-deref refs/heads/symref $B &&
+ git refs exists refs/heads/symref &&
+
+ git refs delete --no-deref refs/heads/symref $A &&
+ test_must_fail git refs exists refs/heads/symref &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete with message records reason in reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ git symbolic-ref HEAD refs/heads/foo &&
+ git refs delete --message=delete-reason refs/heads/foo &&
+ test_must_fail git refs exists refs/heads/foo &&
+ test-tool ref-store main for-each-reflog-ent HEAD >actual &&
+ test_grep "delete-reason$" actual
+ )
+'
+
+test_expect_success 'delete with empty message fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ (
+ cd repo &&
+ A=$(git rev-parse A) &&
+ git update-ref refs/heads/foo $A &&
+ test_must_fail git refs delete --message= refs/heads/foo 2>err &&
+ test_grep "empty message" err &&
+ git refs exists refs/heads/foo
+ )
+'
+
+test_expect_success 'delete without arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git -C repo refs delete 2>err &&
+ test_grep "requires reference name" err
+'
+
+test_expect_success 'delete with too many arguments fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
+ test_must_fail git refs delete one two three 2>err &&
+ test_grep "requires reference name" err
+'
+
+test_done
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH v4 1/5] builtin/refs: drop `the_repository`
From: Patrick Steinhardt @ 2026-07-06 13:27 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Toon Claes
In-Reply-To: <20260706-pks-refs-writing-subcommands-v4-0-d51f6ce7f830@pks.im>
We still have a couple of uses of `the_repository` in "builtin/refs.c".
All of those are trivial to convert though as the command always
requires a repository to exist.
Convert them to use the passed-in repository and drop
`USE_THE_REPOSITORY_VARIABLE`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/refs.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/builtin/refs.c b/builtin/refs.c
index e3125bc61b..f0faabf45a 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "config.h"
#include "fsck.h"
@@ -23,7 +22,7 @@
N_("git refs optimize " PACK_REFS_OPTS)
static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
- struct repository *repo UNUSED)
+ struct repository *repo)
{
const char * const migrate_usage[] = {
REFS_MIGRATE_USAGE,
@@ -59,13 +58,13 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
goto out;
}
- if (the_repository->ref_storage_format == format) {
+ if (repo->ref_storage_format == format) {
err = error(_("repository already uses '%s' format"),
ref_storage_format_to_name(format));
goto out;
}
- if (repo_migrate_ref_storage_format(the_repository, format, flags, &errbuf) < 0) {
+ if (repo_migrate_ref_storage_format(repo, format, flags, &errbuf) < 0) {
err = error("%s", errbuf.buf);
goto out;
}
@@ -99,8 +98,8 @@ static int cmd_refs_verify(int argc, const char **argv, const char *prefix,
if (argc)
usage(_("'git refs verify' takes no arguments"));
- repo_config(the_repository, git_fsck_config, &fsck_refs_options);
- prepare_repo_settings(the_repository);
+ repo_config(repo, git_fsck_config, &fsck_refs_options);
+ prepare_repo_settings(repo);
worktrees = get_worktrees_without_reading_head();
for (size_t i = 0; worktrees[i]; i++)
@@ -124,7 +123,7 @@ static int cmd_refs_list(int argc, const char **argv, const char *prefix,
}
static int cmd_refs_exists(int argc, const char **argv, const char *prefix,
- struct repository *repo UNUSED)
+ struct repository *repo)
{
struct strbuf unused_referent = STRBUF_INIT;
struct object_id unused_oid;
@@ -145,7 +144,7 @@ static int cmd_refs_exists(int argc, const char **argv, const char *prefix,
die(_("'git refs exists' requires a reference"));
ref = *argv++;
- if (refs_read_raw_ref(get_main_ref_store(the_repository), ref,
+ if (refs_read_raw_ref(get_main_ref_store(repo), ref,
&unused_oid, &unused_referent, &unused_type,
&failure_errno)) {
if (failure_errno == ENOENT || failure_errno == EISDIR) {
--
2.55.0.795.g602f6c329a.dirty
^ permalink raw reply related
* [PATCH v4 0/5] builtin/refs: add ability to write references
From: Patrick Steinhardt @ 2026-07-06 13:27 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Toon Claes
In-Reply-To: <20260616-pks-refs-writing-subcommands-v1-0-9f5219b6109d@pks.im>
Hi,
Reference-related functionality in Git is currently spread across many
different commands: git-update-ref(1), git-for-each-ref(1),
git-show-ref(1), git-pack-refs(1) and git-symbolic-ref(1). This makes it
hard for users to discover what functionality we have available to work
with references.
We have thus started to consolidate this functionality into git-refs(1),
which is a toolbox of everything related to references. Until now, the
command doesn't handle functionality of git-update-ref(1).
This patch series backfills most of the functionality by introducing
three new commands:
- `git refs delete` to delete references. This is the equivalent of
`git update-ref -d`.
- `git refs update` to update references. This is the equivalent of
`git update-ref <refname> <oldvalue> <newvalue>`.
- `git refs rename` to rename a reference, including its reflog. This
does not have an equivalent in git-update-ref(1), but is inspired by
and supersedes [1].
Changes in v4:
- Add a couple more tests around symrefs.
- Use a subshell in one of the tests for consistency.
- Link to v3: https://patch.msgid.link/20260630-pks-refs-writing-subcommands-v3-0-deb04de1ecef@pks.im
Changes in v3:
- Fix confused error message.
- Link to v2: https://patch.msgid.link/20260617-pks-refs-writing-subcommands-v2-0-07f3d18336f9@pks.im
Changes in v2:
- Add a new "create" subcommand.
- Consistently quote in error messages.
- Consistently use `<old-value>` in the synopsis.
- Don't return negative exit codes.
- Improve documentation of "update" subcommand to mention that you can
create and delete branches.
- Add tests to verify that we can use "update" to do this, both in
racy and raceless ways.
- Add missing calls to `repo_config()`.
- Drop useless `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME` variable.
- Link to v1: https://patch.msgid.link/20260616-pks-refs-writing-subcommands-v1-0-9f5219b6109d@pks.im
Thanks!
Patrick
[1]: <xmqqv7brz9ba.fsf@gitster.g>
---
Patrick Steinhardt (5):
builtin/refs: drop `the_repository`
builtin/refs: add "delete" subcommand
builtin/refs: add "update" subcommand
builtin/refs: add "create" subcommand
builtin/refs: add "rename" subcommand
Documentation/git-refs.adoc | 40 +++++++
builtin/refs.c | 222 ++++++++++++++++++++++++++++++++++--
t/meson.build | 4 +
t/t1464-refs-delete.sh | 152 +++++++++++++++++++++++++
t/t1465-refs-update.sh | 268 ++++++++++++++++++++++++++++++++++++++++++++
t/t1466-refs-create.sh | 151 +++++++++++++++++++++++++
t/t1467-refs-rename.sh | 144 ++++++++++++++++++++++++
7 files changed, 973 insertions(+), 8 deletions(-)
Range-diff versus v3:
1: dc87ed0ebc = 1: 3b40441317 builtin/refs: drop `the_repository`
2: cec7d978f1 ! 2: 8089847912 builtin/refs: add "delete" subcommand
@@ t/t1464-refs-delete.sh (new)
+test_expect_success 'delete without oldvalue verification' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
-+ A=$(git -C repo rev-parse A) &&
-+ git -C repo update-ref refs/heads/foo $A &&
-+ git -C repo refs delete refs/heads/foo &&
-+ test_must_fail git -C repo show-ref --verify -q refs/heads/foo
++ (
++ cd repo &&
++ A=$(git rev-parse A) &&
++ git update-ref refs/heads/foo $A &&
++ git refs delete refs/heads/foo &&
++ test_must_fail git refs exists refs/heads/foo
++ )
+'
+
+test_expect_success 'delete with matching oldvalue' '
@@ t/t1464-refs-delete.sh (new)
+ )
+'
+
++test_expect_success 'delete symref with --no-deref verifies target OID' '
++ test_when_finished "rm -rf repo" &&
++ setup_repo repo &&
++ (
++ cd repo &&
++ A=$(git rev-parse A) &&
++ B=$(git rev-parse B) &&
++ git update-ref refs/heads/foo $A &&
++ git symbolic-ref refs/heads/symref refs/heads/foo &&
++
++ test_must_fail git refs delete --no-deref refs/heads/symref $B &&
++ git refs exists refs/heads/symref &&
++
++ git refs delete --no-deref refs/heads/symref $A &&
++ test_must_fail git refs exists refs/heads/symref &&
++ git refs exists refs/heads/foo
++ )
++'
++
+test_expect_success 'delete with message records reason in reflog' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
3: 8e73b0f711 = 3: fb830f8f9e builtin/refs: add "update" subcommand
4: f5ad0c9b18 = 4: f3c17471c1 builtin/refs: add "create" subcommand
5: 5c01f2e828 ! 5: 82e4efb2a9 builtin/refs: add "rename" subcommand
@@ t/t1467-refs-rename.sh (new)
+ )
+'
+
++test_expect_success 'rename with symbolic ref fails' '
++ test_when_finished "rm -rf repo" &&
++ setup_repo repo &&
++ (
++ cd repo &&
++ A=$(git rev-parse A) &&
++ git refs create refs/heads/target $A &&
++ git symbolic-ref refs/heads/symref refs/heads/target &&
++ ! git refs rename refs/heads/symref refs/heads/renamed 2>err &&
++ test_grep "is a symbolic ref, .* not supported" err
++ )
++'
++
+test_expect_success 'rename with empty message fails' '
+ test_when_finished "rm -rf repo" &&
+ setup_repo repo &&
---
base-commit: 700432b2ba22603a0bcb71475c9c333d17c9b0d1
change-id: 20260616-pks-refs-writing-subcommands-7a77be5bda9b
^ permalink raw reply
* [PATCH 0/2] git-subtree: Bail out if we find output from Rust rewrite
From: Ian Jackson @ 2026-07-06 11:58 UTC (permalink / raw)
To: git; +Cc: Ian Jackson, Colin Stagner, Johannes Schindelin
My rewrite of git-subtree is coming along fairly nicely. I have done
a bunch of data model design work, as well as successfully implemented
the "split" operation. (It's several orders of magnitude faster than
the shell script implementation, and produces much better output.)
I have concluded that it is going to be too difficult to have
bidirectional interoperability, for a number of reasons. One reason
is that I want to change the way split commits are constructed and
they should be as stable as we can make them.
Another, bigger, reason is that current git-subtree generates unmarked
subtree merges (ie, without any git-subtree trailers); reliably
figuring out whether something is such a merge requires a complete
history walk, which is very unfortunate. To avoid having to do this
in perpetuity, I plan to have new git-subtree assume that commits
which show signs of use of new git-subtree are not unmarked subtree
merges generated by old git-subtree.
So mycompatibility plan is:
- New git-subtree can read old git-subtree data
- Old git-subtree (this one here) will *not* handle new data
New git-subtree needs an in-downstream-tree config file with at least
an indication of the downstream project name, because otherwise split
commits can be quite inscrutable. So the presence of that file is a
convenient way to signal the change.
The purpose of the present patch is to help prevent messes, where old
git-subtree is used *afer* new git-subtree, generating output that no
tooling can handle correctly. We change old git-subtree to spot the
new git-subtree's config file, and bail out.
Right now there is no new data in existence, becauwe new git-subtree
is not useable. So right now this change has no effect.
But if we ship this change now, users with this change will be
defended from this lossage, as their collaborators start to use new
git-subtree. I'm hoping to possibly persuade distros etc. to take
this patch as a backport.
I wasn't sure how precisely to word the error message. I chose to
refer to the new tool, as if it really exists. By the time anyone
actually seex this message, it will do.
Ian Jackson (2):
git-subtree: Bail out if we find output from Rust rewrite
git-subtree: Bail out if we find output from Rust rewrite (test)
contrib/subtree/git-subtree.sh | 18 ++++++++++++++++++
contrib/subtree/t/t7900-subtree.sh | 18 ++++++++++++++++++
2 files changed, 36 insertions(+)
--
2.47.3
^ permalink raw reply
* [PATCH 2/2] git-subtree: Bail out if we find output from Rust rewrite (test)
From: Ian Jackson @ 2026-07-06 11:58 UTC (permalink / raw)
To: git; +Cc: Ian Jackson
In-Reply-To: <20260706115816.20267-1-ijackson@chiark.greenend.org.uk>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
contrib/subtree/t/t7900-subtree.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 4194687cfb..e8fa640166 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -439,6 +439,24 @@ test_expect_success 'split sub dir/ with --rejoin' '
)
'
+test_expect_success 'split fail on RIIR git subtree data' '
+ subtree_test_create_repo "$test_count" &&
+ subtree_test_create_repo "$test_count/sub proj" &&
+ test_create_commit "$test_count" main1 &&
+ test_create_commit "$test_count/sub proj" sub1 &&
+ (
+ cd "$test_count" &&
+ git fetch ./"sub proj" HEAD &&
+ git subtree add --prefix="sub dir" FETCH_HEAD &&
+ # simulate RIIR git-subtree generated data
+ mkdir .git-subtree &&
+ echo "# sabotage" >.git-subtree/config &&
+ git add .git-subtree/config &&
+ git commit -m sabotage &&
+ test_must_fail git subtree split -P "sub dir" HEAD
+ )
+'
+
# Tests that commits from other subtrees are not processed as
# part of a split.
#
--
2.47.3
^ permalink raw reply related
* [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite
From: Ian Jackson @ 2026-07-06 11:58 UTC (permalink / raw)
To: git; +Cc: Ian Jackson, Colin Stagner, Johannes Schindelin
In-Reply-To: <20260706115816.20267-1-ijackson@chiark.greenend.org.uk>
This is going to be forward compatible, but not backward compatible:
projects are expected to adopt the new tool, but not go back to this
old one.
CC: Colin Stagner <ask+git@howdoi.land>
CC: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
contrib/subtree/git-subtree.sh | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 791fd8260c..e9c7ca7cf5 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -278,6 +278,20 @@ main () {
"cmd_$arg_command" "$@"
}
+# Usage: reject_if_v2_config REV
+#
+# Bails if we find .git-subtree/config. This file is used by the RIIR
+# git-subtree, which can read data from this script, but which generates
+# data that this script cannot cope with. So if we find that the user's
+# project has already been processed with the new tool, we stop, to
+# avoid generating broken output.
+reject_if_v2_config () {
+ local config=.git-subtree/config
+ if git rev-parse --verify -q "$rev:$config"; then
+ die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one. See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
+ fi
+}
+
# Usage: cache_setup
cache_setup () {
assert test $# = 0
@@ -846,6 +860,7 @@ process_split_commit () {
# Or: cmd_add REPOSITORY REF
cmd_add () {
+ reject_if_v2_config HEAD
ensure_clean
if test $# -eq 1
@@ -934,6 +949,8 @@ cmd_split () {
die "fatal: you must provide exactly one revision, and optionally a repository. Got: '$*'"
fi
+ reject_if_v2_config "$rev"
+
# Now validate prefix against the commit, not the working tree
if ! git cat-file -e "$rev:$dir" 2>/dev/null
then
@@ -1034,6 +1051,7 @@ cmd_merge () {
then
repository="$2"
fi
+ reject_if_v2_config HEAD
ensure_clean
if test -n "$arg_addmerge_squash"
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v6 2/2] config: add "worktree" and "worktree/i" includeIf conditions
From: Chen Linxuan @ 2026-07-06 12:18 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: me, git, Kristoffer Haugsbakk, Junio C Hamano, Phillip Wood
In-Reply-To: <akeW4yFC8uuu2o8a@pks.im>
On Fri, Jul 3, 2026 at 7:03 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Fri, Jul 03, 2026 at 11:13:18AM +0800, Chen Linxuan via B4 Relay wrote:
> > diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
> > index f3892578e4ff..4e840dfdb35b 100755
> > --- a/t/t1305-config-include.sh
> > +++ b/t/t1305-config-include.sh
> > @@ -396,4 +396,132 @@ test_expect_success 'onbranch without repository but explicit nonexistent Git di
> [snip]
> > +test_expect_success SYMLINKS 'conditional include, worktree resolves symlinks' '
> > + mkdir real-wt &&
> > + ln -s real-wt link-wt &&
> > + git init link-wt/repo &&
> > + (
> > + cd link-wt/repo &&
> > + # repo->worktree resolves symlinks, so use real path in pattern
> > + echo "[includeIf \"worktree:**/real-wt/repo\"]path=bar-link" >>.git/config &&
> > + echo "[test]wtlink=2" >.git/bar-link &&
> > + echo 2 >expect &&
> > + git config test.wtlink >actual &&
> > + test_cmp expect actual
> > + )
> > +'
>
> Okay, this covers one scenario. But with "gitdir:" we're actually able
> to use both the symlinked and the real location:
>
> test_expect_success SYMLINKS 'conditional include, worktree matching symlink' '
> mkdir sym-real &&
> ln -s sym-real sym-link &&
> git init sym-link/repo &&
> (
> cd sym-link/repo &&
> link_path="$(pwd)" &&
> real_path="$(test-tool path-utils real_path "$link_path")" &&
> cat >>.git/config <<-EOF &&
> [includeIf "gitdir:$link_path/.git"]
> path = gitdir-link
> [includeIf "gitdir:$real_path/.git"]
> path = gitdir-real
> [includeIf "worktree:$link_path"]
> path = worktree-link
> [includeIf "worktree:$real_path"]
> path = worktree-real
> EOF
> echo "[test]gitdirlink=1" >.git/gitdir-link &&
> echo "[test]gitdirreal=1" >.git/gitdir-real &&
> echo "[test]worktreelink=1" >.git/worktree-link &&
> echo "[test]worktreereal=1" >.git/worktree-real &&
>
> git config get test.gitdirlink &&
> git config get test.gitdirreal &&
> git config get test.worktreereal &&
> test_must_fail git config test.worktreelink
> )
> '
>
> The last call to git-config(1) fails, which is inconsistent with how
> resolve the path for "gitdir".
>
I investigated the symlink mismatch.
`gitdir:` works because `opts->git_dir` still preserves the discovered or
user-provided spelling, and `include_by_path()` matches both its realpath
and its absolute non-realpath form.
`worktree:` is different: `repo_get_work_tree()` returns
`repo->worktree`, which is stored by `repo_set_worktree()` via
`real_pathdup(path, 1)`. So the symlink spelling is already lost before
we evaluate includeIf conditions.
Changing `repo->worktree` itself to preserve the original spelling looks
risky, because several users access `repo->worktree` directly, and setup
code appears to rely on it being canonical.
My current possible v7 approach is to keep `repo->worktree` canonical,
but store an additional absolute, normalized, non-realpath worktree path
for `includeIf.worktree`. For the ordinary discovered-repository case,
this has to be derived in `setup_discovered_git_dir()` from physical
`cwd`, the worktree-root offset, and a validated `$PWD`, because
`set_git_work_tree()` is otherwise only called with `"."`.
This makes your suggested test pass, but the plumbing is less trivial
than the original patch. Does this approach sound reasonable, or would
you prefer different semantics for symlinked worktree paths?
Chen Linxuan
> Other than that I didn't have anything to add, thanks!
>
> Patrick
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox