* [PATCH v3 4/5] builtin/refs: add "create" subcommand
From: Patrick Steinhardt @ 2026-06-30 11:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260630-pks-refs-writing-subcommands-v3-0-deb04de1ecef@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 v3 5/5] builtin/refs: add "rename" subcommand
From: Patrick Steinhardt @ 2026-06-30 11:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <20260630-pks-refs-writing-subcommands-v3-0-deb04de1ecef@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 | 131 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 187 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..f80d58e0f4
--- /dev/null
+++ b/t/t1467-refs-rename.sh
@@ -0,0 +1,131 @@
+#!/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 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
* Re: [PATCH v2] history: streamline message preparation and plug file stream leak
From: Patrick Steinhardt @ 2026-06-30 12:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <xmqqmrwdxrat.fsf@gitster.g>
On Mon, Jun 29, 2026 at 09:08:42AM -0700, Junio C Hamano wrote:
[snip]
> * Changes from v1 are two additional error checks to notice failure
> from fwrite() and fclose() to die. Interdiff appears at the end.
Technically speaking the first error check for fwrite() should be
unnecessary as the errors accumulate. But it doesn't hurt, either.
> Interdiff against v1:
> diff --git a/builtin/history.c b/builtin/history.c
> index f17ec049c0..365e81379b 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -59,13 +59,15 @@ static int fill_commit_message(struct repository *repo,
> strbuf_addstr(out, default_message);
> strbuf_addch(out, '\n');
> strbuf_commented_addf(out, comment_line_str, hint, action, comment_line_str);
> - fwrite(out->buf, 1, out->len, s.fp);
> + if (fwrite(out->buf, 1, out->len, s.fp) != out->len)
> + die_errno(_("could not write to '%s'"), path);
>
> wt_status_collect_changes_trees(&s, old_tree, new_tree);
> wt_status_print(&s);
> wt_status_collect_free_buffers(&s);
> string_list_clear_func(&s.change, change_data_free);
> - fclose(s.fp);
> + if (fclose(s.fp))
> + die_errno(_("could not write to '%s'"), path);
>
> strbuf_reset(out);
> if (launch_editor(path, out, NULL)) {
Yup, this looks good to me. Thanks!
Patrick
^ permalink raw reply
* [PATCH v1] config: fix case-insensitive match for old-style [section.subsection]
From: RISHAV DEWAN @ 2026-06-30 12:48 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, RISHAV DEWAN
When writing to an old-style "[section.subsection]" header, the
config file parser always folds the subsection to lower case
(get_base_var() lowercases unconditionally), while the key given on
the command line keeps whatever case the caller typed
(do_parse_config_key() deliberately leaves the subsection segment
untouched). matches() compared these two forms with a plain strcmp(),
so a caller passing an upper-cased subsection (e.g. 'git config
section.Subsection.key value2' against a file containing
'[section.subsection]') never matched the existing key and a
duplicate line was appended instead of replacing the value. This was
a known, documented limitation (see the now-removed BUGS section of
git-config.adoc).
Track whether the currently active section was parsed case-
insensitively (old-style) or case-sensitively (new-style, quoted) in
struct config_store_data, and have matches() use that information to
compare the subsection segment of the key accordingly, instead of an
unconditional case-sensitive strcmp().
Signed-off-by: RISHAV DEWAN <rishavdewan10@gmail.com>
---
Documentation/git-config.adoc | 21 ---------------------
config.c | 18 +++++++++++++++++-
t/t1300-config.sh | 2 --
3 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/Documentation/git-config.adoc b/Documentation/git-config.adoc
index 57af010ade..11dfd802b4 100644
--- a/Documentation/git-config.adoc
+++ b/Documentation/git-config.adoc
@@ -634,27 +634,6 @@ http.sslverify false
include::config.adoc[]
-BUGS
-----
-When using the deprecated `[section.subsection]` syntax, changing a value
-will result in adding a multi-line key instead of a change, if the subsection
-is given with at least one uppercase character. For example when the config
-looks like
-
---------
- [section.subsection]
- key = value1
---------
-
-and running `git config section.Subsection.key value2` will result in
-
---------
- [section.subsection]
- key = value1
- key = value2
---------
-
-
GIT
---
Part of the linkgit:git[1] suite
diff --git a/config.c b/config.c
index 6a0de86e3a..7f086fcd75 100644
--- a/config.c
+++ b/config.c
@@ -2594,6 +2594,7 @@ struct config_store_data {
} *parsed;
unsigned int parsed_nr, parsed_alloc, *seen, seen_nr, seen_alloc;
unsigned int key_seen:1, section_seen:1, is_keys_section:1;
+ unsigned int subsection_case_sensitive:1;
};
#define CONFIG_STORE_INIT { 0 }
@@ -2613,7 +2614,21 @@ static void config_store_data_clear(struct config_store_data *store)
static int matches(const char *key, const char *value,
const struct config_store_data *store)
{
- if (strcmp(key, store->key))
+ /*
+ * The subsection part of "key" (key[0..store->baselen)) was parsed
+ * out of the config file using the case sensitivity of whichever
+ * section header it came from (see store_aux_event()): old-style
+ * "[section.subsection]" headers are folded to lower case while
+ * parsing, so they must be compared case-insensitively against
+ * store->key, which preserves whatever case the caller passed on
+ * the command line. New-style "[section "Subsection"]" headers keep
+ * their case, so they need an exact, case-sensitive comparison.
+ */
+ int (*cmpfn)(const char *, const char *, size_t) =
+ store->subsection_case_sensitive ? strncasecmp : strncmp;
+
+ if (cmpfn(key, store->key, store->baselen) ||
+ strcmp(key + store->baselen, store->key + store->baselen))
return 0; /* not ours */
if (store->fixed_value && value)
return !strcmp(store->fixed_value, value);
@@ -2654,6 +2669,7 @@ static int store_aux_event(enum config_event_t type, size_t begin, size_t end,
!cmpfn(cs->var.buf, store->key, store->baselen);
if (store->is_keys_section) {
store->section_seen = 1;
+ store->subsection_case_sensitive = cs->subsection_case_sensitive;
ALLOC_GROW(store->seen, store->seen_nr + 1,
store->seen_alloc);
store->seen[store->seen_nr] = store->parsed_nr;
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 87ca11a127..eaa3b83990 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1499,7 +1499,6 @@ test_expect_success 'old-fashioned settings are case insensitive' '
EOF
q_to_tab >testConfig_expect <<-EOF &&
[V.A]
- r = value1
Qr = value2
EOF
git config -f testConfig_actual "V.A.r" value2 &&
@@ -1511,7 +1510,6 @@ test_expect_success 'old-fashioned settings are case insensitive' '
EOF
q_to_tab >testConfig_expect <<-EOF &&
[V.A]
- r = value1
Qr = value2
EOF
git config -f testConfig_actual "v.A.r" value2 &&
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* RUST_LIB dependency on LIB_FILE in Makefile
From: Jan Palus @ 2026-06-30 13:07 UTC (permalink / raw)
To: git
According to Makefile $(RUST_LIB) target depends on $(LIB_FILE):
$(RUST_LIB): Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
but is that really the case? As far as I can tell $(RUST_LIB) does not
use $(LIB_FILE) in any way and there's no such dependency in
meson.build.
^ permalink raw reply
* Re: [PATCH v5 0/3] Teach git-replay(1) to linearize merge commits
From: Toon Claes @ 2026-06-30 13:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Elijah Newren, Patrick Steinhardt
In-Reply-To: <f8b520d1-edeb-9e45-c503-025c8b5833c3@gmx.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> So I fear that the `replayed_base` design _is_ needed, and the only way
> `git replay --linearize` can work with multiple branches is by linearizing
> all of the replayed commits into one single, linear commit topology.
Well, I'm getting the feeling you're right here. But then this change is
no longer related to merge commits only. Replaying multiple branches
with --onto and --linearize would always replay them into a single line
hiearchy?
Personally, I would be totally fine with that. We need to lay that out
very clearly in the docs, but that is in my humble opinion also a strong
argument to name it `--linearize` and not `--replay-merge=linearize` or
whatever we've been discussing.
> Obviously, there are ways one could _try_ to rescue the previous idea, so
> that at least replaying just branches A and B would keep the replayed
> commits non-reachable from each other, but I strongly suspect that any
> such design will invariably surprise users in nasty ways when the logic
> has to fall back to the simple idea I outlined anyway.
I don't like the "try" in there. I think it's better to have predictable
behavior. Users always have the choice to replay branches in separate
git-replay(1) calls, although that comes with a downside that commits
shared by those branches will be replayed separately and will get
duplicated, unless they fiddle with the COMMITTER_DATE.
--
Cheers,
Toon
^ permalink raw reply
* Draft of Git Rev News edition 136
From: Christian Couder @ 2026-06-30 13:44 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jakub Narebski, Markus Jansen, Kaartic Sivaraam,
Štěpán Němec, Taylor Blau,
Johannes Schindelin, Derrick Stolee, Elijah Newren, Toon Claes,
Paulo Gomes, Phillip Wood
Hi everyone,
A draft of a new Git Rev News edition is available here:
https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-136.md
Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:
https://github.com/git/git.github.io/issues/846
You can also reply to this email.
In general all kinds of contributions, for example proofreading,
suggestions for articles or links, help on the issues in GitHub,
volunteering for being interviewed and so on, are very much
appreciated.
I tried to Cc everyone who appears in this edition, but maybe I missed
some people, sorry about that.
Jakub, Markus, Kaartic and I plan to publish this edition on Thursday
July 2nd, 2026.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH v5 0/4] history: add squash subcommand to fold a range
From: Phillip Wood @ 2026-06-30 13:48 UTC (permalink / raw)
To: Harald Nordgren, phillip.wood
Cc: Patrick Steinhardt, Harald Nordgren via GitGitGadget, git
In-Reply-To: <CAHwyqnVN=McZjtQGcPnoVOHAd0+VDNPXy_N949VMsqZty3RDjQ@mail.gmail.com>
On 29/06/2026 22:13, Harald Nordgren wrote:
>> You've trimmed the line where I said
>>
>> >> Possibly with a comment before each message saying where it came
>> >> from.
>
> Sorry about that! Can you show me the example of what it would look
> like? It's much easier for me to reason about it then.
For the commits
123 Commit one
456 Commit two
789 fixup! Commit one
abc squash! Commit two
def amend! Commit one
It would look something like
# This is the combination of the following commits:
# 123 Commit one
# 789 fixup! Commit one
# def amend! Commit one
# 456 Commit two
# abc squash! Commit two
A better commit one subject
A better commit one body
# ------------------------------------
Commit two
Commit two body
# squash! Commit two
More text for commit two
I've rearranged the commits in the summary to keep fixup commits with
their targets instead of listing them in the same order of "git log
--reverse". As we're dropping uninteresting messages I not sure there is
much point in numbering them[1] so I've just used a plain separator to
show where a message ends. For squash! commits commenting out the
subject shows where the original message ends and the message from the
squash! commit begins. I'm on the fence as to whether it is worth having
# amend! Commit one
above the first message
Thanks
Phillip
[1] If we really want to number things we could number each commit in
the summary and then use that to label it in the message template
with separators like
# This is the message from commit <n>
instead of separating them with
# ----------------------------
^ permalink raw reply
* Re: [PATCH v5 0/4] history: add squash subcommand to fold a range
From: Phillip Wood @ 2026-06-30 14:01 UTC (permalink / raw)
To: Matt Hunter, phillip.wood, Harald Nordgren
Cc: Patrick Steinhardt, Harald Nordgren via GitGitGadget, git
In-Reply-To: <DJM1N17VMUM5.3V5Y6YMFLIFQJ@lfurio.us>
Hi Matt
On 30/06/2026 03:55, Matt Hunter wrote:
> I haven't experimented much with the new 'git history' commands, but
> this discussion caught my eye and wanted to chime in. (aka: please
> forgive my stupid questions)
>
> Note: I have built and am testing with v6 of this topic.
Thanks for testing it, and also for your questions - it is always
helpful to get the perspective of someone using the command, especially
while we're still designing it.
> [...]
> I agree with this idea as well. And letting fixup! messages completely
> fall out makes more sense here than I want to say in git rebase. The
> commented list of commits at the top is a nice compromise for the todo
> list you would have seen had you run 'git rebase -i' instead, and a
> glance at the list would confirm that the fixup!s you thought you
> included _actually are_ in the squash range.
>
> I assume the same treatment would _not_ be completely given to squash!
> messages, since there is at least some expectation that they carry an
> additional remark that the author might want to reword into the base
> commit?
Yes for "squash! Some commit" I think we'd want to have
Some commit
Some commit body
# squash! Some commit
The body of the squash! commit
> Given the above, and how 'git rebase' usually works, I'm suprised that
> --reedit-message isn't the default behavior. This may be my bias
> towards rebase showing... Perhaps the typical use of this command is to
> just work fast, and expect a follow up 'git history reword' if needed -
> when the original just needs a little extra context?
That's a good point - if we want to encourage good commit histories then
opening the editor by default gives the user an opportunity to double
check which commits are being squashed and edit the message as appropriate.
"--reedit-message" is a bit of a mouthful - maybe we should call it
"--edit" (or "--no-edit" if we want to open the editor by default) like
"git commit"
> This is probably a larger question, since (according to the man page) it
> affects the other 'git history' commands as well. When I run
> 'git history ...' and discover that I made a mistake after inspecting
> the results, is there a fool-proof way to undo the change and return to
> the previous state? My first thought was to run 'git reset --hard ...',
> but the default behavior of --update-refs (moving other branches) can
> make this more complicated.
Yes this is a problem to which we don't have a good solution at the
moment. I believe Jujitsu and git-branchless both have some kind of
operations log that lets you revert a whole operation rather than just a
single ref-update. We'd need some way to tie all the ref updates from a
single ref transaction together either by logging the separately or
adding some form of transaction id to the reflog. That would be a big
change.
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH 2/6] object-file: propagate files transaction errors
From: Justin Tobler @ 2026-06-30 14:14 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <akOCPk55yi3lerL-@pks.im>
On 26/06/30 10:45AM, Patrick Steinhardt wrote:
> On Mon, Jun 29, 2026 at 02:04:08PM -0500, Justin Tobler wrote:
> > On 26/06/29 01:58PM, Justin Tobler wrote:
> > > On 26/06/24 01:26PM, Patrick Steinhardt wrote:
> > > > On Tue, Jun 23, 2026 at 11:19:16PM -0500, Justin Tobler wrote:
> > > > > @@ -511,11 +511,15 @@ static void odb_transaction_files_prepare(struct odb_transaction *base)
> > > > > * added at the time they call odb_transaction_files_begin.
> > > > > */
> > > > > if (!transaction || transaction->objdir)
> > > > > - return;
> > > > > + return 0;
> > > > >
> > > > > transaction->objdir = tmp_objdir_create(base->source->odb->repo, "bulk-fsync");
> > > > > - if (transaction->objdir)
> > > > > - tmp_objdir_replace_primary_odb(transaction->objdir, 0);
> > > > > + if (!transaction->objdir)
> > > > > + return -1;
> > > >
> > > > Huh. So previously we just didn't handle this error at all and just
> > > > continued to tag along? Did that result in anything sensible or was this
> > > > just YOLOing it?
> > >
> > > Good question. Previously if there was an error, we wouldn't end up
> > > creating any tmpdir and would instead continue to use the primary ODB to
> > > write objects in. This change would make it a hard error if we fail to
> > > create the temp dir. This matches the behavior that git-receive-pack(1)
> > > expects, but I didn't consider that the existing callers could
> > > transparently handle there being no temp dir.
> > >
> > > I suspect we may want existing ODB transaction users to continue being
> > > resilient in the same manner. In the next version, I'll maintain the
> > > same behavior.
> >
> > I think I got a bit ahead of myself. The existing callers of
> > odb_transaction_files_prepare() still continue to ignore this error. So
> > the behavior already does remain the same here.
>
> Oh, well, okay. I think this behaviour is plain bad -- if the caller
> wants to have a transaction, then we should bail in case we cannot
> create one. But this doesn't need to be fixed in this patch series.
Ya, I tend to agree. The problem here is that
odb_transaction_files_prepare() is being invoked lazily during the
object write. This would be another argument against lazily creating the
temporary directory though. In a followup series I'll explore removing
this and investigate if it has any meaninful performance implications.
-Justin
^ permalink raw reply
* [PATCH 00/11] sequencer: do not record dropped commits as rewritten
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <67dbfb5c-5f07-49b8-aa32-a4635c585028@gmail.com>
On 19/06/2026 11:13, Phillip Wood wrote:
> I'm happy to take this forward and try and fix at least some of the
> other bugs I've listed above. Uwe - if I don't cc you on some patches
> within the next couple of weeks please feel free to send a reminder.
Here is the first batch that fixes the same problem as Uwe's patch. I've
taken a slightly different approach that uses the return value from
do_pick_commit() to signal that a commit was dropped rather than
adding another function argument. That involves a number of preparatory
patches, but they are hopefully reasonably small and easy to follow.
If a commit gets dropped because its changes are already upstream
then we should not record it as rewritten. As well as confusing any
post-rewrite hooks this means we end up copying the notes from the
dropped commit to the commit that was picked immediately before the
one that was dropped.
This series is structured as follows:
Patch 1 restores some test coverage that was lost when the default
rebase backend was changed.
Patch 2 moves a function so it can be called without a forward
declaration in Patch 11.
Patches 3 & 4 fix the return value of do_pick_commit() when an external
command fails (this is in preparation for patch 10).
Patches 5-9 try and simplify the control flow in pick_one_commit()
in preparation for patch 10.
Patch 10 changes the return type of do_pick_commit() to an enum.
Patch 11 adds a new member to the enum from patch 10 for commits that
are dropped when they become empty and uses that to stop them from
being recorded as rewritten.
Base-Commit: 6c3d7b73556db708feb3b16232fab1efc4353428
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Frebase-drop-notes-with-commit%2Fv1
View-Changes-At: https://github.com/phillipwood/git/compare/6c3d7b735...26551f268
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/rebase-drop-notes-with-commit/v1
Phillip Wood (11):
t3400: restore coverage for note copying with apply backend
sequencer: move definition of is_final_fixup()
sequencer: be more careful with external merge
sequencer: never reschedule on failed commit
sequencer: remove unnecessary "or" in pick_one_commit()
sequencer: simplify handing of fixup with conflicts
sequencer: remove unnecessary condition in pick_one_commit()
sequencer: simplify pick_one_commit()
sequencer: return early from pick_one_commit() on success
sequencer: use an enum to represent result of picking a commit
sequencer: do not record dropped commits as rewritten
sequencer.c | 154 +++++++++++++++++++++++-----------
t/t3400-rebase.sh | 16 +++-
t/t3404-rebase-interactive.sh | 11 +++
t/t5407-post-rewrite-hook.sh | 23 +++++
4 files changed, 155 insertions(+), 49 deletions(-)
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply
* [PATCH 01/11] t3400: restore coverage for note copying with apply backend
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Now that the merge backend is the default we have lost coverage for
"git rebase --apply" copying notes. Fix this by replacing "-m" with
"--apply" as the previous test which uses the default backend now
checks the merge backend.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
t/t3400-rebase.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index c0c00fbb7b1..f0e7fcf649a 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -270,9 +270,9 @@ test_expect_success 'rebase can copy notes' '
test "a note" = "$(git notes show HEAD)"
'
-test_expect_success 'rebase -m can copy notes' '
+test_expect_success 'rebase --apply can copy notes' '
git reset --hard n3 &&
- git rebase -m --onto n1 n2 &&
+ git rebase --apply --onto n1 n2 &&
test "a note" = "$(git notes show HEAD)"
'
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 02/11] sequencer: move definition of is_final_fixup()
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Move this function earlier in the file in preparation for adding a
new caller in a later commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 57855b0066a..32a09b6e87d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4627,21 +4627,6 @@ static int do_update_refs(struct repository *r, int quiet)
strbuf_release(&update_msg);
strbuf_release(&error_msg);
return res;
-}
-
-static int is_final_fixup(struct todo_list *todo_list)
-{
- int i = todo_list->current;
-
- if (!is_fixup(todo_list->items[i].command))
- return 0;
-
- while (++i < todo_list->nr)
- if (is_fixup(todo_list->items[i].command))
- return 0;
- else if (!is_noop(todo_list->items[i].command))
- break;
- return 1;
}
static enum todo_command peek_command(struct todo_list *todo_list, int offset)
@@ -4925,6 +4910,21 @@ static int reread_todo_if_changed(struct repository *r,
strbuf_release(&buf);
return 0;
+}
+
+static int is_final_fixup(struct todo_list *todo_list)
+{
+ int i = todo_list->current;
+
+ if (!is_fixup(todo_list->items[i].command))
+ return 0;
+
+ while (++i < todo_list->nr)
+ if (is_fixup(todo_list->items[i].command))
+ return 0;
+ else if (!is_noop(todo_list->items[i].command))
+ break;
+ return 1;
}
static const char rescheduled_advice[] =
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 03/11] sequencer: be more careful with external merge
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If an external merge strategy cannot merge (for example because it
would overwrite an untracked file) it exits with a non-zero exit
code other than 1. This should be treated differently to a merge
with conflicts which is signalled by an exit code of 1 because as
the merge failed we need to reschedule the last pick. The caller
expects us to return -1 in this case. Also reschedule without trying
to merge if the commit message cannot be written as that prevents us
from successfully picking the commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 19 +++++++++++++++----
t/t3404-rebase-interactive.sh | 11 +++++++++++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 32a09b6e87d..e6626c4db4e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2453,14 +2453,25 @@ static int do_pick_commit(struct repository *r,
struct commit_list *common = NULL;
struct commit_list *remotes = NULL;
- res = write_message(ctx->message.buf, ctx->message.len,
- git_path_merge_msg(r), 0);
+ if (write_message(ctx->message.buf, ctx->message.len,
+ git_path_merge_msg(r), 0)) {
+ res = -1;
+ goto leave;
+ }
commit_list_insert(base, &common);
commit_list_insert(next, &remotes);
- res |= try_merge_command(r, opts->strategy,
- opts->xopts.nr, opts->xopts.v,
+ res = try_merge_command(r, opts->strategy,
+ opts->xopts.nr, opts->xopts.v,
common, oid_to_hex(&head), remotes);
+ /*
+ * If the there were conflicts, try_merge_command() returns 1,
+ * any other no-zero return code means that either the merge
+ * command could not be run, or it failed to merge.
+ */
+ if (res && res != 1)
+ res = -1;
+
commit_list_free(common);
commit_list_free(remotes);
}
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 58b3bb0c271..297b84e60d5 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1249,6 +1249,17 @@ test_expect_success 'interrupted rebase -i with --strategy and -X' '
git rebase --continue &&
test $(git show conflict-branch:conflict) = $(cat conflict) &&
test $(cat file1) = Z
+'
+
+test_expect_success 'failing pick with --strategy is rescheduled' '
+ test_when_finished "rm -rf bin; test_might_fail git rebase --abort" &&
+ mkdir bin &&
+ echo exit 2 | write_script bin/git-merge-fail &&
+ git log -1 --format="pick %H # %s" HEAD >expect &&
+ test_must_fail env PATH="$PWD/bin:$PATH" \
+ git rebase --no-ff --strategy fail HEAD^ &&
+ test_cmp expect .git/rebase-merge/git-rebase-todo &&
+ test_cmp expect .git/rebase-merge/done
'
test_expect_success 'rebase -i error on commits with \ in message' '
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 04/11] sequencer: never reschedule on failed commit
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If "git commit" fails to run then run_git_commit() returns -1 which
causes the current command to be rescheduled. This is incorrect as
we have successfully picked the commit and have written all the state
files we need to successfully commit when the user continues. Fix this
by converting -1 to 1 which matches what do_merge() does.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index e6626c4db4e..d7e439b1feb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2542,6 +2542,12 @@ static int do_pick_commit(struct repository *r,
res = run_git_commit(NULL, reflog_action, opts, flags);
*check_todo = 1;
}
+ /*
+ * If "git commit" failed to run than res == -1 but we dont
+ * want reschedule the last command because the picking the
+ * commit was successful.
+ */
+ res = !!res;
}
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 05/11] sequencer: remove unnecessary "or" in pick_one_commit()
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If error_with_patch(..., res, ...) succeeds then it returns "res", if
it fails then it returns -1. This means that or-ing the return value
with "res" is pointless the result is the same as the return value.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index d7e439b1feb..39cbb7b6e3e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5007,9 +5007,8 @@ static int pick_one_commit(struct repository *r,
oideq(&opts->squash_onto, &oid))))
to_amend = 1;
- return res | error_with_patch(r, item->commit,
- arg, item->arg_len, opts,
- res, to_amend);
+ return error_with_patch(r, item->commit, arg, item->arg_len,
+ opts, res, to_amend);
}
return res;
}
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 06/11] sequencer: simplify handing of fixup with conflicts
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Commit e032abd5a0 (rebase: fix rewritten list for failed pick,
2023-09-06) introduced an early return when res == -1, so if we enter
this conditional block then res is positive. After the last couple
of commits the only possible positive value is 1 so we can simplify
the code by removing the conditional call to intend_to_amend() and
call it error_with_patch() instead.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 39cbb7b6e3e..bcfbda018a7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3874,7 +3874,7 @@ static int error_failed_squash(struct repository *r,
return error(_("could not copy '%s' to '%s'"),
rebase_path_message(),
git_path_merge_msg(r));
- return error_with_patch(r, commit, subject, subject_len, opts, 1, 0);
+ return error_with_patch(r, commit, subject, subject_len, opts, 1, 1);
}
static int do_exec(struct repository *r, const char *command_line, int quiet)
@@ -4986,8 +4986,6 @@ static int pick_one_commit(struct repository *r,
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
if (res && is_fixup(item->command)) {
- if (res == 1)
- intend_to_amend();
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
} else if (res && is_rebase_i(opts) && item->commit) {
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 07/11] sequencer: remove unnecessary condition in pick_one_commit()
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
item->commit holds the commit to be picked and so it must be non-NULL
otherwise pick_one_commit() would not know which commit to pick.
It is also unconditionally dereferenced in do_pick_commit() which is
called at the top of this function. Therefore the check to see if it
is non-NULL is superfluous.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index bcfbda018a7..ff28873d21c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4988,7 +4988,7 @@ static int pick_one_commit(struct repository *r,
if (res && is_fixup(item->command)) {
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
- } else if (res && is_rebase_i(opts) && item->commit) {
+ } else if (res && is_rebase_i(opts)) {
int to_amend = 0;
struct object_id oid;
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 08/11] sequencer: simplify pick_one_commit()
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Unless we're rebasing all we do in pick_one_commit() is call
do_pick_commit() and return its result. Simplify the code by returing
early if we're not rebasing so that we don't have to continually call
is_rebase_i() in the rest of the function. Note that there are a couple
of conditions that do not call is_rebase_i() but they check for either
an "edit" or a "fixup" command, both of which imply we're rebasing.
As the conditional blocks are all mutually exclusive (either the
conditions are mutually exclusive, or an earlier conditional block
that would match a later one contains a "return" statement) chain
them together with "else if" to make that clear.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index ff28873d21c..416729f30a7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4966,12 +4966,14 @@ static int pick_one_commit(struct repository *r,
res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),
check_todo);
- if (is_rebase_i(opts) && res < 0) {
+ if (!is_rebase_i(opts))
+ return res;
+
+ if (res < 0) {
/* Reschedule */
*reschedule = 1;
return -1;
- }
- if (item->command == TODO_EDIT) {
+ } else if (item->command == TODO_EDIT) {
struct commit *commit = item->commit;
if (!res) {
if (!opts->verbose)
@@ -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) {
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)) {
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
- } else if (res && is_rebase_i(opts)) {
+ } else if (res) {
int to_amend = 0;
struct object_id oid;
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 09/11] sequencer: return early from pick_one_commit() on success
From: Phillip Wood @ 2026-06-30 15:28 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
The only block that does not return early is the one guarded by
"!res". Move the return into that block to make it clear that after
recording the commit as rewritten all we do is return from the function.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 416729f30a7..655a2e84bef 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4986,6 +4986,7 @@ static int pick_one_commit(struct repository *r,
} else if (!res) {
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
+ return 0;
} else if (res && is_fixup(item->command)) {
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
@@ -5009,7 +5010,8 @@ static int pick_one_commit(struct repository *r,
return error_with_patch(r, item->commit, arg, item->arg_len,
opts, res, to_amend);
}
- return res;
+
+ BUG("Unhandled return value from do_pick_commit()");
}
static int pick_commits(struct repository *r,
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 11/11] sequencer: do not record dropped commits as rewritten
From: Phillip Wood @ 2026-06-30 15:29 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If a commit gets dropped because its changes are already upstream
then we should not record it as rewritten. As well as confusing any
post-rewrite hooks this means we end up copying the notes from the
dropped commit to the commit that was picked immediately before the
one that was dropped.
While we do not want to record the dropped commit is rewritten, if
it is the final commit in a chain of fixups then we need to flush
the list of rewritten commits. The behavior of an "edit" command
where the commit is dropped is changed so that "rebase --continue"
will not amend the previous pick. However, as the code comment notes
it will still be erroneously recorded as rewritten when the rebase
continues. That will need to be addressed separately along with not
recording skipped commits as rewritten.
The initialization of "drop_commit" is moved to ensure it is initialized
when rewording a fast-forwarded commit.
Reported-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 24 +++++++++++++++++++-----
t/t3400-rebase.sh | 12 ++++++++++++
t/t5407-post-rewrite-hook.sh | 23 +++++++++++++++++++++++
3 files changed, 54 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index ca005b969c4..a85f9e8b77d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2264,6 +2264,7 @@ enum pick_result {
PICK_RESULT_ERROR = -1,
PICK_RESULT_OK,
PICK_RESULT_CONFLICTS,
+ PICK_RESULT_DROPPED,
};
static enum pick_result do_pick_commit(struct repository *r,
@@ -2279,7 +2280,7 @@ static enum pick_result do_pick_commit(struct repository *r,
const char *base_label, *next_label, *reflog_action;
char *author = NULL;
struct commit_message msg = { NULL, NULL, NULL, NULL };
- int res, unborn = 0, reword = 0, allow, drop_commit;
+ int res, unborn = 0, reword = 0, allow, drop_commit = 0;
enum todo_command command = item->command;
struct commit *commit = item->commit;
@@ -2509,7 +2510,6 @@ static enum pick_result do_pick_commit(struct repository *r,
goto leave;
}
- drop_commit = 0;
allow = allow_empty(r, opts, commit);
if (allow < 0) {
res = allow;
@@ -2574,6 +2574,8 @@ static enum pick_result do_pick_commit(struct repository *r,
return PICK_RESULT_ERROR;
else if (res > 0)
return PICK_RESULT_CONFLICTS;
+ else if (drop_commit)
+ return PICK_RESULT_DROPPED;
else
return PICK_RESULT_OK;
}
@@ -4994,18 +4996,30 @@ static int pick_one_commit(struct repository *r,
} else if (item->command == TODO_EDIT) {
struct commit *commit = item->commit;
int res = pick_res == PICK_RESULT_CONFLICTS;
+ int to_amend = pick_res != PICK_RESULT_CONFLICTS &&
+ pick_res != PICK_RESULT_DROPPED;
- if (pick_res == PICK_RESULT_OK) {
+ /*
+ * NEEDSWORK: Do not record the commit as rewritten when
+ * continuing if it was dropped. Does it even make sense
+ * to stop if the commit was dropped?
+ */
+ if (pick_res == PICK_RESULT_OK ||
+ pick_res == PICK_RESULT_DROPPED) {
if (!opts->verbose)
term_clear_line();
fprintf(stderr, _("Stopped at %s... %.*s\n"),
short_commit_name(r, commit), item->arg_len, arg);
}
- return error_with_patch(r, commit,
- arg, item->arg_len, opts, res, !res);
+ return error_with_patch(r, commit, arg, item->arg_len, opts,
+ res, to_amend);
} else if (pick_res == PICK_RESULT_OK) {
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
+ return 0;
+ } else if (pick_res == PICK_RESULT_DROPPED) {
+ if (is_final_fixup(todo_list))
+ flush_rewritten_pending();
return 0;
} else if (pick_res == PICK_RESULT_CONFLICTS &&
is_fixup(item->command)) {
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index f0e7fcf649a..1d09886ea35 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -274,6 +274,18 @@ test_expect_success 'rebase --apply can copy notes' '
git reset --hard n3 &&
git rebase --apply --onto n1 n2 &&
test "a note" = "$(git notes show HEAD)"
+'
+
+test_expect_success 'rebase drops notes of dropped commits' '
+ git checkout n1 &&
+ echo n3 >n3.t &&
+ echo n4 >n4.t &&
+ git add n3.t n4.t &&
+ git commit -m n34 &&
+ git rebase HEAD n3 &&
+ test_commit_message HEAD -m n2 &&
+ test_must_fail git notes list HEAD >actual &&
+ test_must_be_empty actual
'
test_expect_success 'rebase commit with an ancient timestamp' '
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index ad7f8c6f002..51991956d1d 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -306,6 +306,29 @@ test_expect_success 'git rebase -i (exec)' '
cat >expected.data <<-EOF &&
$(git rev-parse C) $(git rev-parse HEAD^)
$(git rev-parse D) $(git rev-parse HEAD)
+ EOF
+ verify_hook_input
+'
+
+test_expect_success 'rebase with commits that become empty' '
+ cat >todo <<-\EOF &&
+ pick H
+ pick E
+ fixup I
+ fixup H
+ pick G
+ pick I
+ EOF
+ (
+ set_replace_editor todo &&
+ git rebase -i --empty=drop A A
+ ) &&
+ echo rebase >expected.args &&
+ cat >expected.data <<-EOF &&
+ $(git rev-parse H) $(git rev-parse HEAD~2)
+ $(git rev-parse E) $(git rev-parse HEAD~1)
+ $(git rev-parse I) $(git rev-parse HEAD~1)
+ $(git rev-parse G) $(git rev-parse HEAD)
EOF
verify_hook_input
'
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
* [PATCH 10/11] sequencer: use an enum to represent result of picking a commit
From: Phillip Wood @ 2026-06-30 15:29 UTC (permalink / raw)
To: git; +Cc: Uwe Kleine-König, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1782833268.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
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.
Note we continue to use "return error(...)" to return errors and
take advantage of C's lax typing of enums
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
sequencer.c | 61 +++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 16 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 655a2e84bef..ca005b969c4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2260,10 +2260,16 @@ static const char *reflog_message(struct replay_opts *opts,
return buf.buf;
}
-static int do_pick_commit(struct repository *r,
- struct todo_item *item,
- struct replay_opts *opts,
- int final_fixup, int *check_todo)
+enum pick_result {
+ PICK_RESULT_ERROR = -1,
+ PICK_RESULT_OK,
+ PICK_RESULT_CONFLICTS,
+};
+
+static enum pick_result do_pick_commit(struct repository *r,
+ struct todo_item *item,
+ struct replay_opts *opts,
+ int final_fixup, int *check_todo)
{
struct replay_ctx *ctx = opts->ctx;
unsigned int flags = should_edit(opts) ? EDIT_MSG : 0;
@@ -2564,7 +2570,12 @@ static int do_pick_commit(struct repository *r,
free(author);
update_abort_safety_file();
- return res;
+ if (res < 0)
+ return PICK_RESULT_ERROR;
+ else if (res > 0)
+ return PICK_RESULT_CONFLICTS;
+ else
+ return PICK_RESULT_OK;
}
static int prepare_revs(struct replay_opts *opts)
@@ -4960,37 +4971,47 @@ static int pick_one_commit(struct repository *r,
struct replay_opts *opts,
int *check_todo, int* reschedule)
{
- int res;
+ enum pick_result pick_res;
struct todo_item *item = todo_list->items + todo_list->current;
const char *arg = todo_item_get_arg(todo_list, item);
- res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),
- check_todo);
+ pick_res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),
+ check_todo);
if (!is_rebase_i(opts))
- return res;
+ switch (pick_res) {
+ case PICK_RESULT_ERROR:
+ return -1;
+ case PICK_RESULT_CONFLICTS:
+ return 1;
+ default:
+ return 0;
+ }
- if (res < 0) {
+ if (pick_res == PICK_RESULT_ERROR) {
/* Reschedule */
*reschedule = 1;
return -1;
} else if (item->command == TODO_EDIT) {
struct commit *commit = item->commit;
- if (!res) {
+ int res = pick_res == PICK_RESULT_CONFLICTS;
+
+ if (pick_res == PICK_RESULT_OK) {
if (!opts->verbose)
term_clear_line();
fprintf(stderr, _("Stopped at %s... %.*s\n"),
short_commit_name(r, commit), item->arg_len, arg);
}
return error_with_patch(r, commit,
arg, item->arg_len, opts, res, !res);
- } else if (!res) {
+ } else if (pick_res == PICK_RESULT_OK) {
record_in_rewritten(&item->commit->object.oid,
peek_command(todo_list, 1));
return 0;
- } else if (res && is_fixup(item->command)) {
+ } else if (pick_res == PICK_RESULT_CONFLICTS &&
+ is_fixup(item->command)) {
return error_failed_squash(r, item->commit, opts,
item->arg_len, arg);
- } else if (res) {
+ } else if (pick_res == PICK_RESULT_CONFLICTS) {
int to_amend = 0;
struct object_id oid;
@@ -5008,7 +5029,7 @@ static int pick_one_commit(struct repository *r,
to_amend = 1;
return error_with_patch(r, item->commit, arg, item->arg_len,
- opts, res, to_amend);
+ opts, 1, to_amend);
}
BUG("Unhandled return value from do_pick_commit()");
@@ -5547,7 +5568,15 @@ static int single_pick(struct repository *r,
TODO_PICK : TODO_REVERT;
item.commit = cmit;
- return do_pick_commit(r, &item, opts, 0, &check_todo);
+ switch (do_pick_commit(r, &item, opts, 0, &check_todo)) {
+ case PICK_RESULT_ERROR:
+ return -1;
+ case PICK_RESULT_CONFLICTS:
+ return 1;
+ default:
+ return 0;
+ }
+
}
int sequencer_pick_revisions(struct repository *r,
--
2.54.0.200.gfd8d68259e3
^ permalink raw reply related
page: | 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