* Re: [PATCH 0/1] Extract only the message log body from git commit.
From: Pablo Sabater @ 2026-07-22 12:38 UTC (permalink / raw)
To: hardikxk, git
In-Reply-To: <20260722083836.744338-1-hardikxk@gmail.com>
On Wed Jul 22, 2026 at 10:38 AM CEST, hardikxk wrote:
>
> The patch fixes the `extractLogMessageFromGitCommit` function to skip all the metada of the commit object and only return back the message body.
nit: Let's wrap this at ~72 columns.
>
> Previously the function would return the entire data of the objects
> including authors tree and SHAs. This patch fixes that to skip over all
> that and just return the body of the log message.
This repeats what the commit message already says. For a single-patch
series a cover letter is usually not needed.
Documentation/MyFirstContribution [1] notes that the commit message
should already explain the change at a high level, and that any extra
context can go below the '---' line instead. I would drop this cover
letter unless there's something else to say.
>
> hardikxk (1):
> Extract only the message body from git commit.
>
> git-p4.py | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
>
> base-commit: 48bbf81c29ca9a4479ec7850fe206518682cdb2f
[1]: https://github.com/git/git/blob/master/Documentation/MyFirstContribution.adoc#bonus-chapter-one-patch-changes
Regards,
Pablo
^ permalink raw reply
* Re: [PATCH GSoC v20 00/13] cat-file: add remote-object-info to batch-command
From: Pablo Sabater @ 2026-07-22 12:59 UTC (permalink / raw)
To: Karthik Nayak, Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon, szeder.dev
In-Reply-To: <CAOLa=ZT77L39-jSZxgrvVFwdY6_rapBmKv8gCxr7QpdkCsEzXg@mail.gmail.com>
On Tue Jul 21, 2026 at 10:12 PM CEST, Karthik Nayak wrote:
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> [snip]
>
>> Changes in v20:
>> - Moved the prep patch that fixes hash_algo's type to be before
>> write_fetch_command_and_capabilities() being moved to 'connect.c'
>> - Reverted git-cat-file.adoc documentation comments about CAVEATS
>> - Fixed style for EXPAND_DATA_INIT
>> - Added more context for comman line die()
>>
>
> [snip]
>
>> Range-diff versus v19:
>>
>> 1: 6df5bcb3bc = 1: 31b02f10b5 transport-helper: fix memory leak of helper on disconnect
>> 2: 141d85a76f = 2: 7d62e0586f cat-file: declare loop counter inside for()
>> 3: 87457a1fe3 = 3: 082d593fb7 t1006: extract helper functions into new 'lib-cat-file.sh'
>> 4: c26d378931 = 4: 8646b9d7cf fetch-pack: drop the static advertise_sid variable
>> -: ---------- > 5: 5531a3c399 fetch-pack: use unsigned int for hash_algo variable
>> 5: ee16e17228 ! 6: 92b4122642 fetch-pack: move write_fetch_command_and_capabilities() to connect.c
>> @@ connect.c: int server_supports(const char *feature)
>> + }
>> +
>> + if (server_feature_v2("object-format", &hash_name)) {
>> -+ int hash_algo = hash_algo_by_name(hash_name);
>> ++ const unsigned int hash_algo = hash_algo_by_name(hash_name);
>> + if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
>> + die(_("mismatched algorithms: client %s; server %s"),
>> + the_hash_algo->name, hash_name);
>> @@ fetch-pack.c: static int add_haves(struct fetch_negotiator *negotiator,
>> - }
>> -
>> - if (server_feature_v2("object-format", &hash_name)) {
>> -- int hash_algo = hash_algo_by_name(hash_name);
>> +- const unsigned int hash_algo = hash_algo_by_name(hash_name);
>> - if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
>> - die(_("mismatched algorithms: client %s; server %s"),
>> - the_hash_algo->name, hash_name);
>> 6: bb915f57ad < -: ---------- connect: use unsigned int for hash_algo_by_name() calls
>> 7: b3b06b0cae = 7: b954a5994a connect: make write_fetch_command_and_capabilities() more generic
>> 8: 79a87c7011 = 8: 627242a7a1 fetch-pack: move fetch initialization
>> 9: 37c3f93975 = 9: 0578594733 protocol-caps: check object existence regardless of the attributes requested
>> 10: c5062ecaf2 = 10: 816bfa9162 serve: advertise object-info feature
>> 11: 22d72168bc = 11: 2323f45cb2 transport: add client support for object-info
>> 12: 2cf3b24a35 ! 12: a39975766b cat-file: add remote-object-info to batch-command
>> @@ Documentation/git-cat-file.adoc: one per line, and print information based on th
>> You can specify the information shown for each object by using a custom
>> `<format>`. The `<format>` is copied literally to stdout for each
>> @@ Documentation/git-cat-file.adoc: newline. The available atoms are:
>> - reports).
>> -
>> - `objectsize:disk`::
>> -- The size, in bytes, that the object takes up on disk. See the
>> -- note about on-disk sizes in the `CAVEATS` section below.
>> -+ The size, in bytes, that the object takes up on disk.
>> -
>> `deltabase`::
>> If the object is stored as a delta on-disk, this expands to the
>> full hex representation of the delta base object name.
>> - Otherwise, expands to the null OID (all zeroes). See `CAVEATS`
>> -- below.
>> -+ Otherwise, expands to the null OID (all zeroes).
>> ++ Otherwise, expands to the null OID (all zeroes). See `CAVEATS` section
>> + below.
>>
>
> This chagne is still unnecessary, no?
I guess it is unnecessary. I added it because on top, at objectsize:disk
it says "`CAVEATS` section". So I tried to have it consistently.
Is it worth a reroll?
>
>> `rest`::
>> - If this atom is used in the output string, input lines are split
>> @@ Documentation/git-cat-file.adoc: newline. The available atoms are:
>> after that first run of whitespace (i.e., the "rest" of the
>> line) are output in place of the `%(rest)` atom.
>> @@ builtin/cat-file.c: struct expand_data {
>> + */
>> + unsigned is_remote:1;
>> +};
>> ++
>> +#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD }
>> +
>> +static const char *remote_object_info_atoms[] = {
>> @@ builtin/cat-file.c: static void parse_cmd_mailmap(struct batch_options *opt UNUS
>> + line_to_split = xstrdup(line);
>> + count = split_cmdline(line_to_split, &argv);
>> + if (count < 0)
>> -+ die(_("remote-object-info: %s"), split_cmdline_strerror(count));
>> ++ die(_("remote-object-info: failed to parse command line: %s"),
>> ++ split_cmdline_strerror(count));
>> + if (count - 1 > MAX_ALLOWED_OBJ_LIMIT)
>> + die(_("remote-object-info supports at most %d objects"),
>> + MAX_ALLOWED_OBJ_LIMIT);
>> 13: ebdfc22fb7 ! 13: 70a11d2aea cat-file: make remote-object-info allow-list adapt to the server
>> @@ builtin/cat-file.c: struct expand_data {
>> */
>> unsigned is_remote:1;
>> -};
>> +-
>> -#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD }
>>
>> -static const char *remote_object_info_atoms[] = {
>> - "objectname",
>> - "objectsize",
>> ++ /*
>> ++ * List of atoms (i.e. "objectsize") that the server supports. Built
>> ++ * from the server's object-info advertised capabilities.
>> ++ */
>> + struct string_list remote_allowed_atoms;
>> };
>> -+#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD, \
>> -+ .remote_allowed_atoms = STRING_LIST_INIT_NODUP }
>>
>> ++#define EXPAND_DATA_INIT { .mode = S_IFINVALID, \
>> ++ .type = OBJ_BAD, \
>> ++ .remote_allowed_atoms = STRING_LIST_INIT_NODUP }
>> ++
>
> I guess this is output of running the style check script (which I did
> recommend, but I should've mentioned that you should take it with a
> pinch of salt, it doesn't always provide the best solutions.)
It comes in part from Junio's suggestion [1]. Although I don't think that
it will receive a lot of new fields, I thought that it would leave the
macro clean for that.
>
>> static int is_atom(const char *atom, const char *s, int slen)
>> {
>> + int alen = strlen(atom);
>> @@ builtin/cat-file.c: static int expand_atom(struct strbuf *sb, const char *atom, int len,
>> struct expand_data *data)
>> {
>> @@ builtin/cat-file.c: static int expand_atom(struct strbuf *sb, const char *atom,
>> - if (is_atom(remote_object_info_atoms[i], atom, len))
>> + size_t i;
>> + for (i = 0; i < data->remote_allowed_atoms.nr; i++)
>> -+ if (is_atom(data->remote_allowed_atoms.items[i].string, atom, len))
>> ++ if (is_atom(data->remote_allowed_atoms.items[i].string,
>> ++ atom, len))
>> break;
>> -
>> - /*
>>
>> ---
>> base-commit: 44de1520f08d1dfebc3ab2d9f644208eaa5ac925
>
> I reviewed v18 last, but the diff here looks good to me. Thanks!
[1]: https://lore.kernel.org/git/xmqqcxwonnkx.fsf@gitster.g/
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH 4/9] odb: lift object existence check out of the "loose" backend
From: Toon Claes @ 2026-07-22 13:25 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Justin Tobler
In-Reply-To: <20260717-pks-odb-move-loose-object-writing-v1-4-46446a3cb5b7@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> Before writing a new loose object we first check whether the object
> already exists in any of the sources attached to the object database.
> This results in a couple of issues:
>
> - We have a layering violation, where the source needs to be aware of
> objects stored in any of the other sources.
>
> - Every backend would have to reimplement this check, which feels
> somewhat pointless.
>
> - It is not possible to easily write an object into a source in case
> the same object already exists in another source.
>
> Refactor the code and lift up the object existence check from the
> "loose" backend into the generic ODB layer. No callers need adjustment
> as none of them write via a specific source, but via the ODB layer.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> odb.c | 7 +++++++
> odb/source-loose.c | 8 ++------
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/odb.c b/odb.c
> index 4adbdf8a64..bfeca76f4e 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -997,6 +997,13 @@ int odb_write_object_ext(struct object_database *odb,
>
> hash_object_file(odb->repo->hash_algo, buf, len, type, oid);
>
> + /*
> + * We can skip the write in case we already have the object available.
> + * In that case, we only freshen its mtime.
> + */
> + if (odb_freshen_object(odb, oid))
> + return 0;
> +
For the loose backend, this calls check_and_freshen_file(), which code
comment says:
/*
* All of the check_and_freshen functions return 1 if the file exists and was
* freshened (if freshening was requested), 0 otherwise. If they return
* 0, you should not assume that it is safe to skip a write of the object (it
* either does not exist on disk, or has a stale mtime and may be subject to
* pruning).
*/
And I see similar logic in the packed backend. So a non-zero value skips
the write and the file was freshened. Makes sense.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH 0/1] rebase: add --[no-]edit to --continue
From: Phillip Wood @ 2026-07-22 13:39 UTC (permalink / raw)
To: Junio C Hamano, Hugo Sales; +Cc: git
In-Reply-To: <xmqqldb4xlqa.fsf@gitster.g>
On 21/07/2026 19:04, Junio C Hamano wrote:
> Hugo Sales <hugo@hsal.es> writes:
>
>> When a rebase stops for conflicts and the user runs `git rebase --continue`, the
>> merge backend opens $EDITOR so the commit message can be revised. That is often
>> useful, but not always: sometimes the user only wants to keep the message that
>> is already there.
>>
>> This series adds:
>>
>> - `git rebase --continue --no-edit` to commit without opening an editor
>
> Meh. "GIT_SEQUENCE_EDITOR=: git rebase --continue" is your friend ;-)
Do you mean "GIT_EDITOR=:"? The sequence editor is only relevant for
editing the todo list. The last time this came up [1] I shared an alias
that only suppresses the editor if HEAD has not changed since the user
continued the rebase, but that does not stop the user suppressing the
editor when continuing a "reword" command. I share the concerns you
listed below about this patch in its current form.
Thanks
Phillip
[1]
https://lore.kernel.org/git/fbf859ca-43f4-433e-b111-377aa60f1947@gmail.com
>> - `rebase.noEdit` to make that the default on continue
>> - `git rebase --continue --edit` to override `rebase.noEdit`
>
> The new 'rebase.noEdit' configuration is especially concerning. It
> encourages users to assume by default that their rebase sessions
> will not produce notable changes worth recording in the commit logs.
>
> A few immediate edge cases come to mind:
>
> - What if 'rebase -i' stops to give control back to the user for
> reasons other than a merge conflict? If the user chose 'edit',
> their original intent was to modify both the commit message and
> the content. With 'rebase.noEdit' enabled, would they now have
> to remember to pass '--edit' when continuing? Does the answer
> depend on whether the 'edit' step resulted in a merge conflict?
>
> - What if the user chose 'reword', which is an explicit signal to
> update the commit message, but 'rebase.noEdit' is enabled? If
> the rebase does not stop with a conflict, it might open the
> editor regardless of the configuration. But if a conflict does
> occur and requires manual resolution, will the user still need to
> remember to pass '--edit' when continuing?
>
> The proposed tests only cover the code path where control returns to
> the user due to a conflict. This is understandable since that
> scenario was your primary motivation. However, they do not verify
> what happens when there are no conflicts but the user explicitly
> intended to edit the message from the start. You may want to expand
> the test coverage to address these scenarios (and potentially
> others, as this is not an exhaustive list).
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH v20 0/7] branch: delete-merged
From: Phillip Wood @ 2026-07-22 13:39 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget, git
Cc: Kristoffer Haugsbakk, Johannes Sixt, Harald Nordgren
In-Reply-To: <pull.2285.v20.git.git.1784704238.gitgitgadget@gmail.com>
Hi Harald
On 22/07/2026 08:10, Harald Nordgren via GitGitGadget wrote:
> Delete branches that have already been merged on upstream.
>
> Changes in v20:
>
> * Protect branches transitively required by a surviving local upstream
> stack. Traverse upstream chains once and defer delete-set mutation until
> traversal completes.
> * Make stacked-branch handling independent of ref iteration order and
> update the documentation accordingly.
> * Clarify variable names with regards to branch names (short) to reduce
> confusion.
I'm having a hard time reading the range diff due to the renaming of the
members of "struct stacked_branch_data". Can you explain what has
changed in the logic to protect branches that are upstreams of unmerged
branches and why please? In particular why wasn't sufficient to stop
removing the members of "spared" from "deletable" in
spare_stacked_base() and remove them after refs_for_each_branch_ref()
has returned instead.
Thanks
Phillip
> Range-diff vs v19:
>
> 1: 562648132d = 1: 51c1c9d075 branch: add --forked filter for --list mode
> 2: c7ebd9344c = 2: 711574b2e5 branch: convert delete_branches() to a flags argument
> 3: 0c4f3358e3 = 3: 47c5975dc7 branch: let delete_branches skip unmerged branches on bulk refusal
> 4: 64a202526a = 4: 46268acec5 branch: prepare delete_branches for a bulk caller
> 5: a6caa5b397 ! 5: ef9f57e735 branch: add --delete-merged <branch>
> @@ Commit message
> A branch whose work is not yet merged into its upstream is silently
> skipped, so one unmerged topic does not abort the whole sweep.
>
> - A branch that another, surviving branch tracks as its upstream is
> - also kept, so a branch is never deleted out from under one stacked
> - on top of it. Such a kept branch is itself merged, so when its own
> - upstream is being deleted, clear its now-stale upstream config.
> + A branch that a surviving branch depends on through a chain of local
> + upstreams is also kept, so no branch is deleted out from under stacked
> + work. Collect this transitive set without changing the candidate set
> + during ref iteration: walk upstream chains from surviving branches,
> + visit each branch at most once, and remove the collected bases only
> + after the iteration completes. This makes the result independent of
> + ref iteration order without repeated full scans.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>
> @@ Documentation/git-branch.adoc: This option is only applicable in non-verbose mod
> +silently skipped. Delete it with `git branch -D` if you want to
> +remove it anyway.
> ++
> -+A branch that another, surviving branch tracks as its upstream is
> -+kept, so a branch is never deleted out from under one stacked on top
> -+of it. If that kept branch in turn tracks a branch that is being
> -+deleted, its now-stale upstream configuration is cleared.
> ++A branch that a surviving branch depends on through a chain of local
> ++upstreams is kept, so a branch is never deleted out from under stacked
> ++work.
> +
> `-v`::
> `-vv`::
> @@ builtin/branch.c: static int parse_opt_forked(const struct option *opt, const ch
> return 0;
> }
>
> -+struct spare_data {
> -+ struct strset *deletable;
> -+ struct strset *spared;
> ++struct stacked_branch_data {
> ++ struct strset *deletable_branch_names;
> ++ struct strset *protected_branch_names;
> ++ struct strset *visited_branch_names;
> +};
> +
> -+/*
> -+ * A surviving branch stacked on a deletion candidate would lose its
> -+ * upstream, so drop that candidate from the delete set and remember it
> -+ * in "spared" so its own upstream can be tidied up afterwards.
> -+ */
> -+static int spare_stacked_base(const struct reference *ref, void *cb_data)
> ++static int collect_stacked_branch_bases(const struct reference *ref,
> ++ void *cb_data)
> +{
> -+ struct spare_data *data = cb_data;
> -+ struct branch *branch;
> -+ const char *upstream, *up_short;
> ++ struct stacked_branch_data *data = cb_data;
> ++ const char *branch_name;
> +
> -+ if (strset_contains(data->deletable, ref->name))
> -+ return 0;
> -+ branch = branch_get(ref->name);
> -+ upstream = branch_get_upstream(branch, NULL);
> -+ if (!upstream || !skip_prefix(upstream, "refs/heads/", &up_short) ||
> -+ !strset_contains(data->deletable, up_short))
> ++ if (!skip_prefix(ref->name, "refs/heads/", &branch_name))
> ++ BUG("expected local branch ref, got '%s'", ref->name);
> ++ if (strset_contains(data->deletable_branch_names, branch_name))
> + return 0;
> +
> -+ strset_remove(data->deletable, up_short);
> -+ strset_add(data->spared, up_short);
> ++ while (strset_add(data->visited_branch_names, branch_name)) {
> ++ struct branch *branch = branch_get(branch_name);
> ++ const char *upstream_refname = branch_get_upstream(branch, NULL);
> ++ const char *upstream_branch_name;
> ++
> ++ if (!upstream_refname ||
> ++ !skip_prefix(upstream_refname, "refs/heads/",
> ++ &upstream_branch_name) ||
> ++ !strset_contains(data->deletable_branch_names,
> ++ upstream_branch_name))
> ++ break;
> ++
> ++ strset_add(data->protected_branch_names, upstream_branch_name);
> ++ branch_name = upstream_branch_name;
> ++ }
> ++
> + return 0;
> +}
> +
> -+/*
> -+ * Keep any branch that a surviving branch tracks as its upstream, so we
> -+ * never delete a branch out from under one stacked on top of it. Such a
> -+ * base is itself merged, so when its own upstream is also going away
> -+ * (no surviving branch tracks it), clear the base's now-stale upstream.
> -+ */
> -+static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable)
> ++static void protect_stacked_branch_bases(struct ref_store *refs,
> ++ struct strset *deletable_branch_names)
> +{
> -+ struct strset spared = STRSET_INIT;
> -+ struct spare_data data = { .deletable = deletable, .spared = &spared };
> -+ struct strbuf key = STRBUF_INIT;
> ++ struct strset protected_branch_names = STRSET_INIT;
> ++ struct strset visited_branch_names = STRSET_INIT;
> ++ struct stacked_branch_data data = {
> ++ .deletable_branch_names = deletable_branch_names,
> ++ .protected_branch_names = &protected_branch_names,
> ++ .visited_branch_names = &visited_branch_names,
> ++ };
> ++ struct refs_for_each_ref_options opts = {
> ++ .prefix = "refs/heads/",
> ++ };
> + struct hashmap_iter iter;
> + struct strmap_entry *entry;
> +
> -+ refs_for_each_branch_ref(refs, spare_stacked_base, &data);
> -+
> -+ strset_for_each_entry(&spared, &iter, entry) {
> -+ struct branch *branch = branch_get(entry->key);
> -+ const char *upstream = branch_get_upstream(branch, NULL);
> -+ const char *up_short;
> -+
> -+ if (!upstream || !skip_prefix(upstream, "refs/heads/", &up_short) ||
> -+ !strset_contains(deletable, up_short))
> -+ continue;
> ++ refs_for_each_ref_ext(refs, collect_stacked_branch_bases, &data, &opts);
> +
> -+ strbuf_reset(&key);
> -+ strbuf_addf(&key, "branch.%s.merge", branch->name);
> -+ repo_config_set_gently(the_repository, key.buf, NULL);
> -+ strbuf_reset(&key);
> -+ strbuf_addf(&key, "branch.%s.remote", branch->name);
> -+ repo_config_set_gently(the_repository, key.buf, NULL);
> -+ }
> ++ strset_for_each_entry(&protected_branch_names, &iter, entry)
> ++ strset_remove(deletable_branch_names, entry->key);
> +
> -+ strbuf_release(&key);
> -+ strset_clear(&spared);
> ++ strset_clear(&visited_branch_names);
> ++ strset_clear(&protected_branch_names);
> +}
> +
> +static int branch_pushes_to_upstream(struct branch *branch,
> @@ builtin/branch.c: static int parse_opt_forked(const struct option *opt, const ch
> + struct ref_store *refs = get_main_ref_store(the_repository);
> + struct ref_filter filter = REF_FILTER_INIT;
> + struct ref_array candidates = { 0 };
> -+ struct strset deletable = STRSET_INIT;
> -+ struct strvec to_delete = STRVEC_INIT;
> ++ struct strset deletable_branch_names = STRSET_INIT;
> ++ struct strvec branches_to_delete = STRVEC_INIT;
> + struct hashmap_iter iter;
> + struct strmap_entry *entry;
> + size_t i;
> @@ builtin/branch.c: static int parse_opt_forked(const struct option *opt, const ch
> + filter_refs(&candidates, &filter, filter.kind);
> +
> + for (i = 0; i < (size_t)candidates.nr; i++) {
> -+ const char *full_name = candidates.items[i]->refname;
> -+ const char *short_name;
> ++ const char *branch_refname = candidates.items[i]->refname;
> ++ const char *branch_name;
> + struct branch *branch;
> -+ const char *upstream;
> ++ const char *upstream_refname;
> +
> -+ if (!skip_prefix(full_name, "refs/heads/", &short_name))
> -+ BUG("filter returned non-branch ref '%s'", full_name);
> -+ if (branch_checked_out(full_name))
> ++ if (!skip_prefix(branch_refname, "refs/heads/", &branch_name))
> ++ BUG("filter returned non-branch ref '%s'", branch_refname);
> ++ if (branch_checked_out(branch_refname))
> + continue;
> +
> -+ branch = branch_get(short_name);
> -+ upstream = branch_get_upstream(branch, NULL);
> -+ if (!upstream || !refs_ref_exists(refs, upstream))
> ++ branch = branch_get(branch_name);
> ++ upstream_refname = branch_get_upstream(branch, NULL);
> ++ if (!upstream_refname || !refs_ref_exists(refs, upstream_refname))
> + continue;
> -+ if (branch_pushes_to_upstream(branch, upstream))
> ++ if (branch_pushes_to_upstream(branch, upstream_refname))
> + continue;
> -+ if (check_branch_commit(short_name, short_name,
> ++ if (check_branch_commit(branch_name, branch_name,
> + &candidates.items[i]->objectname, NULL,
> + FILTER_REFS_BRANCHES, DELETE_BRANCH_SKIP_UNMERGED))
> + continue;
> +
> -+ strset_add(&deletable, short_name);
> ++ strset_add(&deletable_branch_names, branch_name);
> + }
> +
> -+ spare_stacked_bases(refs, &deletable);
> ++ protect_stacked_branch_bases(refs, &deletable_branch_names);
> +
> -+ strset_for_each_entry(&deletable, &iter, entry)
> -+ strvec_push(&to_delete, entry->key);
> ++ strset_for_each_entry(&deletable_branch_names, &iter, entry)
> ++ strvec_push(&branches_to_delete, entry->key);
> +
> -+ if (to_delete.nr)
> -+ ret = delete_branches(to_delete.nr, to_delete.v,
> ++ if (branches_to_delete.nr)
> ++ ret = delete_branches(branches_to_delete.nr, branches_to_delete.v,
> + FILTER_REFS_BRANCHES,
> + DELETE_BRANCH_SKIP_UNMERGED |
> + DELETE_BRANCH_NO_HEAD_FALLBACK |
> + flags);
> +
> -+ strvec_clear(&to_delete);
> -+ strset_clear(&deletable);
> ++ strvec_clear(&branches_to_delete);
> ++ strset_clear(&deletable_branch_names);
> + ref_array_clear(&candidates);
> + ref_filter_clear(&filter);
> + return ret;
> @@ t/t3200-branch.sh: test_expect_success '--forked requires a value' '
> + )
> +'
> +
> -+test_expect_success '--delete-merged clears the deleted upstream of a spared branch' '
> ++test_expect_success '--delete-merged keeps the upstream chain of a surviving branch' '
> + setup_repo_for_delete_merged &&
> + (
> + cd repo &&
> @@ t/t3200-branch.sh: test_expect_success '--forked requires a value' '
> + git commit --allow-empty -m "tip work" &&
> +
> + git branch --delete-merged origin/next \
> -+ --delete-merged lower &&
> ++ --delete-merged lower >actual 2>&1 &&
> ++ test_must_be_empty actual &&
> +
> + check_branches <<-\EOF &&
> ++ lower
> + main
> + mid
> + tip
> + EOF
> +
> -+ git config --local --get-regexp "branch\\.(mid|tip)\\.(merge|remote)" >actual &&
> ++ git config --local --get-regexp "branch\\.(lower|mid|tip)\\.(merge|remote)" >actual &&
> + cat >expect <<-\EOF &&
> ++ branch.lower.remote origin
> ++ branch.lower.merge refs/heads/next
> ++ branch.mid.remote .
> ++ branch.mid.merge refs/heads/lower
> + branch.tip.remote .
> + branch.tip.merge refs/heads/mid
> + EOF
> @@ t/t3200-branch.sh: test_expect_success '--forked requires a value' '
> + )
> +'
> +
> ++test_expect_success '--delete-merged result is independent of stacked branch names' '
> ++ setup_repo_for_delete_merged &&
> ++ (
> ++ cd repo &&
> ++ git branch c-lower origin/next --track &&
> ++ git branch b-mid c-lower --track &&
> ++ git checkout -b a-tip b-mid --track &&
> ++ git commit --allow-empty -m "tip work" &&
> ++
> ++ git branch --delete-merged origin/next \
> ++ --delete-merged "c-*" &&
> ++
> ++ check_branches <<-\EOF &&
> ++ a-tip
> ++ b-mid
> ++ c-lower
> ++ main
> ++ EOF
> ++
> ++ git branch --delete-merged origin/next \
> ++ --delete-merged "c-*" >actual 2>&1 &&
> ++ test_must_be_empty actual &&
> ++
> ++ check_branches <<-\EOF
> ++ a-tip
> ++ b-mid
> ++ c-lower
> ++ main
> ++ EOF
> ++ )
> ++'
> ++
> +test_expect_success '--delete-merged requires a value' '
> + test_must_fail git -C forked branch --delete-merged 2>err &&
> + test_grep "requires a value" err
> 6: 734d27c908 ! 6: fa70108611 branch: add branch.<name>.deleteMerged opt-out
> @@ Documentation/git-branch.adoc: A branch is not deleted when:
> ## builtin/branch.c ##
> @@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
> struct ref_array candidates = { 0 };
> - struct strset deletable = STRSET_INIT;
> - struct strvec to_delete = STRVEC_INIT;
> + struct strset deletable_branch_names = STRSET_INIT;
> + struct strvec branches_to_delete = STRVEC_INIT;
> + struct strbuf key = STRBUF_INIT;
> struct hashmap_iter iter;
> struct strmap_entry *entry;
> size_t i;
> @@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
> - const char *short_name;
> + const char *branch_name;
> struct branch *branch;
> - const char *upstream;
> + const char *upstream_refname;
> + int opt_out;
>
> - if (!skip_prefix(full_name, "refs/heads/", &short_name))
> - BUG("filter returned non-branch ref '%s'", full_name);
> + if (!skip_prefix(branch_refname, "refs/heads/", &branch_name))
> + BUG("filter returned non-branch ref '%s'", branch_refname);
> @@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
> FILTER_REFS_BRANCHES, DELETE_BRANCH_SKIP_UNMERGED))
> continue;
>
> + strbuf_reset(&key);
> -+ strbuf_addf(&key, "branch.%s.deletemerged", short_name);
> ++ strbuf_addf(&key, "branch.%s.deletemerged", branch_name);
> + if (!repo_config_get_bool(the_repository, key.buf, &opt_out) &&
> + !opt_out) {
> + if (!(flags & DELETE_BRANCH_QUIET))
> + fprintf(stderr,
> + _("Skipping '%s' (branch.%s.deleteMerged is false)\n"),
> -+ short_name, short_name);
> ++ branch_name, branch_name);
> + continue;
> + }
> +
> - strset_add(&deletable, short_name);
> + strset_add(&deletable_branch_names, branch_name);
> }
>
> @@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
> @@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstrea
> flags);
>
> + strbuf_release(&key);
> - strvec_clear(&to_delete);
> - strset_clear(&deletable);
> + strvec_clear(&branches_to_delete);
> + strset_clear(&deletable_branch_names);
> ref_array_clear(&candidates);
>
> ## t/t3200-branch.sh ##
> 7: 7aa9d5db14 ! 7: 13bac431a3 branch: add --dry-run for --delete-merged
> @@ Documentation/git-branch.adoc: git branch (-m|-M) [<old-branch>] <new-branch>
>
> DESCRIPTION
> -----------
> -@@ Documentation/git-branch.adoc: kept, so a branch is never deleted out from under one stacked on top
> - of it. If that kept branch in turn tracks a branch that is being
> - deleted, its now-stale upstream configuration is cleared.
> +@@ Documentation/git-branch.adoc: A branch that a surviving branch depends on through a chain of local
> + upstreams is kept, so a branch is never deleted out from under stacked
> + work.
>
> +`--dry-run`::
> + With `--delete-merged`, print which branches would be
> @@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int ki
> char *refname = name + branch_name_pos;
> if (!(flags & DELETE_BRANCH_QUIET))
> printf(remote_branch
> -@@ builtin/branch.c: static int spare_stacked_base(const struct reference *ref, void *cb_data)
> - * base is itself merged, so when its own upstream is also going away
> - * (no surviving branch tracks it), clear the base's now-stale upstream.
> - */
> --static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable)
> -+static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable,
> -+ unsigned int flags)
> - {
> - struct strset spared = STRSET_INIT;
> - struct spare_data data = { .deletable = deletable, .spared = &spared };
> -@@ builtin/branch.c: static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable
> -
> - refs_for_each_branch_ref(refs, spare_stacked_base, &data);
> -
> -- strset_for_each_entry(&spared, &iter, entry) {
> -- struct branch *branch = branch_get(entry->key);
> -- const char *upstream = branch_get_upstream(branch, NULL);
> -- const char *up_short;
> -+ if (!(flags & DELETE_BRANCH_DRY_RUN)) {
> -+ strset_for_each_entry(&spared, &iter, entry) {
> -+ struct branch *branch = branch_get(entry->key);
> -+ const char *upstream = branch_get_upstream(branch, NULL);
> -+ const char *up_short;
> -
> -- if (!upstream || !skip_prefix(upstream, "refs/heads/", &up_short) ||
> -- !strset_contains(deletable, up_short))
> -- continue;
> -+ if (!upstream || !skip_prefix(upstream, "refs/heads/", &up_short) ||
> -+ !strset_contains(deletable, up_short))
> -+ continue;
> -
> -- strbuf_reset(&key);
> -- strbuf_addf(&key, "branch.%s.merge", branch->name);
> -- repo_config_set_gently(the_repository, key.buf, NULL);
> -- strbuf_reset(&key);
> -- strbuf_addf(&key, "branch.%s.remote", branch->name);
> -- repo_config_set_gently(the_repository, key.buf, NULL);
> -+ strbuf_reset(&key);
> -+ strbuf_addf(&key, "branch.%s.merge", branch->name);
> -+ repo_config_set_gently(the_repository, key.buf, NULL);
> -+ strbuf_reset(&key);
> -+ strbuf_addf(&key, "branch.%s.remote", branch->name);
> -+ repo_config_set_gently(the_repository, key.buf, NULL);
> -+ }
> - }
> -
> - strbuf_release(&key);
> -@@ builtin/branch.c: static int delete_merged_branches(const struct strvec *upstreams,
> - strset_add(&deletable, short_name);
> - }
> -
> -- spare_stacked_bases(refs, &deletable);
> -+ spare_stacked_bases(refs, &deletable, flags);
> -
> - strset_for_each_entry(&deletable, &iter, entry)
> - strvec_push(&to_delete, entry->key);
> @@ builtin/branch.c: int cmd_branch(int argc,
> int delete = 0, rename = 0, copy = 0, list = 0,
> unset_upstream = 0, show_current = 0, edit_description = 0;
> @@ t/t3200-branch.sh: test_expect_success '--delete-merged keeps the upstream of a
> check_branches <<-\EOF &&
> feature
> main
> -@@ t/t3200-branch.sh: test_expect_success '--delete-merged clears the deleted upstream of a spared bra
> +@@ t/t3200-branch.sh: test_expect_success '--delete-merged keeps the upstream chain of a surviving bra
> git checkout -b tip mid --track &&
> git commit --allow-empty -m "tip work" &&
>
> + git branch --dry-run --delete-merged origin/next \
> -+ --delete-merged lower &&
> ++ --delete-merged lower >actual 2>&1 &&
> ++ test_must_be_empty actual &&
> +
> -+ git config --local --get-regexp "branch\\.(mid|tip)\\.(merge|remote)" >actual &&
> ++ git config --local --get-regexp "branch\\.(lower|mid|tip)\\.(merge|remote)" >actual &&
> + cat >expect <<-\EOF &&
> ++ branch.lower.remote origin
> ++ branch.lower.merge refs/heads/next
> + branch.mid.remote .
> + branch.mid.merge refs/heads/lower
> + branch.tip.remote .
> @@ t/t3200-branch.sh: test_expect_success '--delete-merged clears the deleted upstr
> + test_cmp expect actual &&
> +
> git branch --delete-merged origin/next \
> - --delete-merged lower &&
> -
> + --delete-merged lower >actual 2>&1 &&
> + test_must_be_empty actual &&
> @@ t/t3200-branch.sh: test_expect_success "branch -d still deletes a deleteMerged=false branch" '
> )
> '
>
^ permalink raw reply
* Re: [PATCH 1/1] Extract only the message body from git commit.
From: Hardik Kumar @ 2026-07-22 13:42 UTC (permalink / raw)
To: Pablo Sabater, hardikxk, git
In-Reply-To: <DK53I00U9FJS.2MUNWC5000IZ5@gmail.com>
> nit: I think this can be written more clearly. Let's use present tense
> and state things affirmatively:
Sure, I will update this in a later commit.
> Reading the code, this doesn't seem to do what the log says it does.
> Testing it against what it did before this patch:
> We can see that the previous output only shows the commit log, title
> + body. There were no SHAs, tree, etc., the opposite of what this
> patch's log claimed.
I see, I was comparing this to the how git stores your commits in
.git/objects. My assumption was the function would take an object
from there and (wanting to extract just the body) was taking the
entire object as is and not stripping out the subject.
> What this patch actually does is drop the commit subject.
Yes, since I suppose all the other places in the file where
this method is being used don't require the subject line.
> Either way, this patch does not address the '# fixme' correctly.
>
> Before continuing, I think we should try to understand what the '# fixme'
> meant in the first place.
My assumption was that the function that I made changes to was solely
responsible for sending back only the message stripping out subject and
metadata related to the commit object.
Moving forward I suppose the one suggestion would be changing the name
of the variable `foundTitle` to `foundSubject` instead since that might
result in less ambuiguity in what the function is doing (and the fixme).
Although I dont't think if that really makes any difference.
But yes the #fixme does seem very vague now. Not sure if this is worth
it.
Regards,
Hardik.
^ permalink raw reply
* Re: [PATCH 9/9] object-file: move logic to write loose objects
From: Toon Claes @ 2026-07-22 14:26 UTC (permalink / raw)
To: Patrick Steinhardt, git; +Cc: Justin Tobler
In-Reply-To: <20260717-pks-odb-move-loose-object-writing-v1-9-46446a3cb5b7@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> The logic to write loose objects is split up across "object-file.c" and
> "odb/source-loose.c". This split is somewhat weird, but it is the result
> of two things:
>
> - `force_object_loose()` used to reach into internals of how exactly
> we write objects.
>
> - The logic of writing objects is intertwined with potentially
> starting a transaction.
>
> We have refactored `force_object_loose()` over preceding commits to work
> via generic interfaces now, so this reason doesn't exist anymore. But
> the second reason still does, as our management of "files" transactions
> and their ad-hoc creation is still very messy. This area definitely
> requires further work, and that work is indeed ongoing.
>
> That being said, we can already move the writing logic into the "loose"
> backend rather easily. All we have to do is to expose two functions that
> relate to the transactions.
I'm a bit on the fence that should have gone in a separte commit, but
it's fine.
> Expose these two functions and move the writing logic into the "loose"
> backend accordingly so that it becomes more self-contained. Note that
> this requires us to drop a reference to `the_repository` in favor of
> using the source's repository in `start_loose_object_common()`.
Yay! Thanks for calling that out, it standed out in the zebra diff.
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> object-file.c | 360 +----------------------------------------------------
> object-file.h | 22 +---
> odb/source-loose.c | 354 +++++++++++++++++++++++++++++++++++++++++++++++++++-
That's a pretty large diff, but luckily the zebra diff helps a lot.
> -int write_loose_object(struct odb_source_loose *loose,
> - const struct object_id *oid, char *hdr,
> - int hdrlen, const void *buf, unsigned long len,
> - const time_t *mtime, unsigned flags)
This line is not colored being moved because it was made static, which
makes sense.
> diff --git a/object-file.h b/object-file.h
> index 31781a9c53..805f2cfa28 100644
> --- a/object-file.h
> +++ b/object-file.h
> @@ -24,20 +24,6 @@ int index_path(struct index_state *istate, struct object_id *oid, const char *pa
> struct object_info;
> struct odb_source;
>
> -/*
> - * Write the given stream into the loose object source. The only difference
> - * from the generic implementation of this function is that we don't perform an
> - * object existence check here.
> - *
> - * TODO: We should stop exposing this function altogether and move it into
> - * "odb/source-loose.c". This requires a couple of refactorings though to make
> - * `force_object_loose()` generic and is thus postponed to a later point in
> - * time.
> - */
This was added by you on 2026-06-01, so thanks for addressing this.
> @@ -611,12 +849,120 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,
> size_t len,
> struct object_id *oid)
> {
> + struct odb_source_loose *loose = odb_source_loose_downcast(source);
> + const struct git_hash_algo *compat = loose->base.odb->repo->compat_hash_algo;
> + struct object_id compat_oid;
> + int fd, ret, err = 0, flush = 0;
> + unsigned char compressed[4096];
> + git_zstream stream;
> + struct git_hash_ctx c, compat_c;
> + struct strbuf tmp_file = STRBUF_INIT;
> + struct strbuf filename = STRBUF_INIT;
> + unsigned char buf[8192];
> + int dirlen;
> + char hdr[MAX_HEADER_LEN];
> + int hdrlen;
> +
> + if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT))
> + odb_transaction_files_prepare(loose->base.odb->transaction);
> +
> + /* Since oid is not determined, save tmp file to odb path. */
> + strbuf_addf(&filename, "%s/", loose->base.path);
> + hdrlen = format_object_header(hdr, sizeof(hdr), OBJ_BLOB, len);
> +
> /*
> - * TODO: the implementation should be moved here, see the comment on
> - * the called function in "object-file.h".
So this is what you did, as suggested, by yourself.
> + * Common steps for write_loose_object and stream_loose_object to
> + * start writing loose objects:
> + *
> + * - Create tmpfile for the loose object.
> + * - Setup zlib stream for compression.
> + * - Start to feed header to zlib stream.
> */
> - struct odb_source_loose *loose = odb_source_loose_downcast(source);
> - return odb_source_loose_write_stream(loose, in_stream, len, oid);
This line is marked as removed in the zebra diff, but that's because the
code is being inlined into this odb_source_loose_write_object_stream()
function.
All good.
--
Cheers,
Toon
^ permalink raw reply
* Re: [PATCH v3 0/9] sequencer: do not record dropped commits as rewritten
From: Phillip Wood @ 2026-07-22 15:15 UTC (permalink / raw)
To: Oswald Buddenhagen, Junio C Hamano; +Cc: git
In-Reply-To: <al4RYuWKqAr-IlFC@ugly.lan>
Hi Oswald
On 20/07/2026 13:15, Oswald Buddenhagen wrote:
> On Sun, Jul 19, 2026 at 12:29:31PM -0700, Junio C Hamano wrote:
>> It looks like this is now ready to go? Any further comments?
>>
> you can add whatever footer is appropriate for "i read it, it seems to
> make sense, but i didn't double-check" for me.
>
> (same for phillip's new 2-patch series.)
Thanks for reading them through - I'm glad to hear the commit messages
make sense now.
Phillip
> (it feels silly to "spam" the list with such low-value verdicts. i
> really miss gerrit code review here, where i'd leave a +1 in passing.)
>
^ permalink raw reply
* Re: [PATCH] completion: complete paths for git send-email
From: Junio C Hamano @ 2026-07-22 15:32 UTC (permalink / raw)
To: Ben Knoble
Cc: Yury Norov, Yury Norov, git, Thiago Perrotta, Philippe Blain,
Rubén Justo, linux-kernel, Codex
In-Reply-To: <C9564DC6-6B68-46CA-A339-1A1774AFA7C0@gmail.com>
Ben Knoble <ben.knoble@gmail.com> writes:
>> As to the tests, using 40-hex is misleading, and 0-branch as you
>> said would be sufficient to reproduce and demonstrate the issue, and
>> that your code change fixes it.
>>
>> Ben, anything I missed?
>>
>> Thanks.
>
> Not from my end, though SZEDER’s review merits some thinking.
I agree that presenting both refs and paths cleanly will require a
much better structure than a flat list. I also agree that hiding
paths when we have ref matches may give us a cleaner layout than
mixing them alphabetically into a single, flat list. While I am
still not convinced it is the best way, at least that is the
principle current completion implementations use for other commands,
and it makes sense to model the updated completion for send-email
after it.
That said, since I never feed refs to send-email myself, 'if we have
matches with refs, do not show paths at all' rule makes send-email
completion completely useless, at least to me.
> Traveling the next week+; replies may be slower (than usual, hah).
Have a great trip, and have fun!
Thanks.
^ permalink raw reply
* Re: [PATCH 0/1] rebase: add --[no-]edit to --continue
From: Junio C Hamano @ 2026-07-22 15:36 UTC (permalink / raw)
To: Phillip Wood; +Cc: Hugo Sales, git
In-Reply-To: <db7edc66-9b2a-47bc-98db-87d01885cef0@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
> On 21/07/2026 19:04, Junio C Hamano wrote:
>> Hugo Sales <hugo@hsal.es> writes:
>>
>>> When a rebase stops for conflicts and the user runs `git rebase --continue`, the
>>> merge backend opens $EDITOR so the commit message can be revised. That is often
>>> useful, but not always: sometimes the user only wants to keep the message that
>>> is already there.
>>>
>>> This series adds:
>>>
>>> - `git rebase --continue --no-edit` to commit without opening an editor
>>
>> Meh. "GIT_SEQUENCE_EDITOR=: git rebase --continue" is your friend ;-)
>
> Do you mean "GIT_EDITOR=:"? The sequence editor is only relevant for
Oh, absolutely. I made a last minute change s/_EDITOR/SEQUENCE_&/
before sending it out, without realizing that I made a totally
unnecessary change X-<.
Thanks for spotting.
> editing the todo list.
^ permalink raw reply
* Re: [PATCH v20 0/7] branch: delete-merged
From: Harald Nordgren @ 2026-07-22 15:41 UTC (permalink / raw)
To: phillip.wood
Cc: Harald Nordgren via GitGitGadget, git, Kristoffer Haugsbakk,
Johannes Sixt
In-Reply-To: <20976edc-fc9d-4d4d-8919-b599d336f9cc@gmail.com>
On Wed, Jul 22, 2026 at 3:39 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> Hi Harald
>
> On 22/07/2026 08:10, Harald Nordgren via GitGitGadget wrote:
> > Delete branches that have already been merged on upstream.
> >
> > Changes in v20:
> >
> > * Protect branches transitively required by a surviving local upstream
> > stack. Traverse upstream chains once and defer delete-set mutation until
> > traversal completes.
> > * Make stacked-branch handling independent of ref iteration order and
> > update the documentation accordingly.
> > * Clarify variable names with regards to branch names (short) to reduce
> > confusion.
>
> I'm having a hard time reading the range diff due to the renaming of the
> members of "struct stacked_branch_data". Can you explain what has
> changed in the logic to protect branches that are upstreams of unmerged
> branches and why please? In particular why wasn't sufficient to stop
> removing the members of "spared" from "deletable" in
> spare_stacked_base() and remove them after refs_for_each_branch_ref()
> has returned instead.
Hmm. The main idea is to bite the bullet and actually traverse the
graph since it didn't seem possible to finish the job correctly in a
single pass anotherwise which I'm trying to demonstrate with
test_expect_success '--delete-merged keeps the upstream chain of a
surviving branch' '
It becomes a bit like whack-a-mole when Junio asks me to clarify the
code and now you cannot review it because of that. I'm still not
friends with range-diff even after using it for some time, I agree
that the diff v19...v20 doesn't look very inviting.
Harald
^ permalink raw reply
* Re: Performance regression in connectivity check during receive-pack (git 2.54)
From: Junio C Hamano @ 2026-07-22 15:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Jeff King, Wolfgang Kritzinger, git, jltobler
In-Reply-To: <amCuLpT6vYzo1GF8@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
> The alternative to this would be to instead have logic in functions like
> `odb_write()` that checks whether we have an active transaction or not.
> If so, the write would go into the transaction directly instead of going
> into the primary source, and consequently we wouldn't even have to
> modify the list of sources at all.
>
> This shouldn't create too much of a problem, as we typically don't
> intend to even read objects that we've written into the transaction
> immediately. It would avoid that we try to read objects from the
> temporary object directory. And it would also allow us to eventually
> move all the logic to write objects into the transactions exclusively.
I suspect several of those 'transactions' are actually misspelt
'temporary directories', but I catch your drift. That said, a
redesign like that feels more or less independent of the fix for our
immediate performance regression.
After all, didn't Peff show us a case where no odb sources were
being flipped in the middle? Simply setting up one object store to
borrow from another via the alternates mechanism demonstrated that
checking packs across all object stores before hunting for loose
objects in any of them makes a world of difference.
> I'm currently out of office though, and will be on vacation next week.
> I'll explore this area a bit more though once I'm back in office in two
> weeks.
Understood. Bon voyage and have fun!
^ permalink raw reply
* Re: [PATCH GSoC v20 00/13] cat-file: add remote-object-info to batch-command
From: Karthik Nayak @ 2026-07-22 16:13 UTC (permalink / raw)
To: Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon, szeder.dev
In-Reply-To: <DK549LIZS8BS.3PL9CQQIR2LAA@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 8242 bytes --]
"Pablo Sabater" <pabloosabaterr@gmail.com> writes:
> On Tue Jul 21, 2026 at 10:12 PM CEST, Karthik Nayak wrote:
>> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>>
>> [snip]
>>
>>> Changes in v20:
>>> - Moved the prep patch that fixes hash_algo's type to be before
>>> write_fetch_command_and_capabilities() being moved to 'connect.c'
>>> - Reverted git-cat-file.adoc documentation comments about CAVEATS
>>> - Fixed style for EXPAND_DATA_INIT
>>> - Added more context for comman line die()
>>>
>>
>> [snip]
>>
>>> Range-diff versus v19:
>>>
>>> 1: 6df5bcb3bc = 1: 31b02f10b5 transport-helper: fix memory leak of helper on disconnect
>>> 2: 141d85a76f = 2: 7d62e0586f cat-file: declare loop counter inside for()
>>> 3: 87457a1fe3 = 3: 082d593fb7 t1006: extract helper functions into new 'lib-cat-file.sh'
>>> 4: c26d378931 = 4: 8646b9d7cf fetch-pack: drop the static advertise_sid variable
>>> -: ---------- > 5: 5531a3c399 fetch-pack: use unsigned int for hash_algo variable
>>> 5: ee16e17228 ! 6: 92b4122642 fetch-pack: move write_fetch_command_and_capabilities() to connect.c
>>> @@ connect.c: int server_supports(const char *feature)
>>> + }
>>> +
>>> + if (server_feature_v2("object-format", &hash_name)) {
>>> -+ int hash_algo = hash_algo_by_name(hash_name);
>>> ++ const unsigned int hash_algo = hash_algo_by_name(hash_name);
>>> + if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
>>> + die(_("mismatched algorithms: client %s; server %s"),
>>> + the_hash_algo->name, hash_name);
>>> @@ fetch-pack.c: static int add_haves(struct fetch_negotiator *negotiator,
>>> - }
>>> -
>>> - if (server_feature_v2("object-format", &hash_name)) {
>>> -- int hash_algo = hash_algo_by_name(hash_name);
>>> +- const unsigned int hash_algo = hash_algo_by_name(hash_name);
>>> - if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
>>> - die(_("mismatched algorithms: client %s; server %s"),
>>> - the_hash_algo->name, hash_name);
>>> 6: bb915f57ad < -: ---------- connect: use unsigned int for hash_algo_by_name() calls
>>> 7: b3b06b0cae = 7: b954a5994a connect: make write_fetch_command_and_capabilities() more generic
>>> 8: 79a87c7011 = 8: 627242a7a1 fetch-pack: move fetch initialization
>>> 9: 37c3f93975 = 9: 0578594733 protocol-caps: check object existence regardless of the attributes requested
>>> 10: c5062ecaf2 = 10: 816bfa9162 serve: advertise object-info feature
>>> 11: 22d72168bc = 11: 2323f45cb2 transport: add client support for object-info
>>> 12: 2cf3b24a35 ! 12: a39975766b cat-file: add remote-object-info to batch-command
>>> @@ Documentation/git-cat-file.adoc: one per line, and print information based on th
>>> You can specify the information shown for each object by using a custom
>>> `<format>`. The `<format>` is copied literally to stdout for each
>>> @@ Documentation/git-cat-file.adoc: newline. The available atoms are:
>>> - reports).
>>> -
>>> - `objectsize:disk`::
>>> -- The size, in bytes, that the object takes up on disk. See the
>>> -- note about on-disk sizes in the `CAVEATS` section below.
>>> -+ The size, in bytes, that the object takes up on disk.
>>> -
>>> `deltabase`::
>>> If the object is stored as a delta on-disk, this expands to the
>>> full hex representation of the delta base object name.
>>> - Otherwise, expands to the null OID (all zeroes). See `CAVEATS`
>>> -- below.
>>> -+ Otherwise, expands to the null OID (all zeroes).
>>> ++ Otherwise, expands to the null OID (all zeroes). See `CAVEATS` section
>>> + below.
>>>
>>
>> This chagne is still unnecessary, no?
>
> I guess it is unnecessary. I added it because on top, at objectsize:disk
> it says "`CAVEATS` section". So I tried to have it consistently.
>
> Is it worth a reroll?
>
No I don't think a re-roll is necessary. But something to avoid next
time :)
>>
>>> `rest`::
>>> - If this atom is used in the output string, input lines are split
>>> @@ Documentation/git-cat-file.adoc: newline. The available atoms are:
>>> after that first run of whitespace (i.e., the "rest" of the
>>> line) are output in place of the `%(rest)` atom.
>>> @@ builtin/cat-file.c: struct expand_data {
>>> + */
>>> + unsigned is_remote:1;
>>> +};
>>> ++
>>> +#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD }
>>> +
>>> +static const char *remote_object_info_atoms[] = {
>>> @@ builtin/cat-file.c: static void parse_cmd_mailmap(struct batch_options *opt UNUS
>>> + line_to_split = xstrdup(line);
>>> + count = split_cmdline(line_to_split, &argv);
>>> + if (count < 0)
>>> -+ die(_("remote-object-info: %s"), split_cmdline_strerror(count));
>>> ++ die(_("remote-object-info: failed to parse command line: %s"),
>>> ++ split_cmdline_strerror(count));
>>> + if (count - 1 > MAX_ALLOWED_OBJ_LIMIT)
>>> + die(_("remote-object-info supports at most %d objects"),
>>> + MAX_ALLOWED_OBJ_LIMIT);
>>> 13: ebdfc22fb7 ! 13: 70a11d2aea cat-file: make remote-object-info allow-list adapt to the server
>>> @@ builtin/cat-file.c: struct expand_data {
>>> */
>>> unsigned is_remote:1;
>>> -};
>>> +-
>>> -#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD }
>>>
>>> -static const char *remote_object_info_atoms[] = {
>>> - "objectname",
>>> - "objectsize",
>>> ++ /*
>>> ++ * List of atoms (i.e. "objectsize") that the server supports. Built
>>> ++ * from the server's object-info advertised capabilities.
>>> ++ */
>>> + struct string_list remote_allowed_atoms;
>>> };
>>> -+#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD, \
>>> -+ .remote_allowed_atoms = STRING_LIST_INIT_NODUP }
>>>
>>> ++#define EXPAND_DATA_INIT { .mode = S_IFINVALID, \
>>> ++ .type = OBJ_BAD, \
>>> ++ .remote_allowed_atoms = STRING_LIST_INIT_NODUP }
>>> ++
>>
>> I guess this is output of running the style check script (which I did
>> recommend, but I should've mentioned that you should take it with a
>> pinch of salt, it doesn't always provide the best solutions.)
>
> It comes in part from Junio's suggestion [1]. Although I don't think that
> it will receive a lot of new fields, I thought that it would leave the
> macro clean for that.
>
Then wouldn't something like this be better?
#define EXPAND_DATA_INIT { \
.mode = S_IFINVALID, \
.type = OBJ_BAD, \
.remote_allowed_atoms = STRING_LIST_INIT_NODUP, \
}
Note the difference:
1. The first line no longer has the '.mode' field, this would ensure
that removal of the field is a diff which only contains the field and
not movement of other fields to the top line.
2. Similar for the last time.
3. We also add a comma for the last field, this allows addition of new
fields without modifying previous lines by adding a missing comma.
I don't think this too warrants a re-roll, but this is something to
definitely fix incase you do re-roll for other reasons :)
>>
>>> static int is_atom(const char *atom, const char *s, int slen)
>>> {
>>> + int alen = strlen(atom);
>>> @@ builtin/cat-file.c: static int expand_atom(struct strbuf *sb, const char *atom, int len,
>>> struct expand_data *data)
>>> {
>>> @@ builtin/cat-file.c: static int expand_atom(struct strbuf *sb, const char *atom,
>>> - if (is_atom(remote_object_info_atoms[i], atom, len))
>>> + size_t i;
>>> + for (i = 0; i < data->remote_allowed_atoms.nr; i++)
>>> -+ if (is_atom(data->remote_allowed_atoms.items[i].string, atom, len))
>>> ++ if (is_atom(data->remote_allowed_atoms.items[i].string,
>>> ++ atom, len))
>>> break;
>>> -
>>> - /*
>>>
>>> ---
>>> base-commit: 44de1520f08d1dfebc3ab2d9f644208eaa5ac925
>>
>> I reviewed v18 last, but the diff here looks good to me. Thanks!
>
> [1]: https://lore.kernel.org/git/xmqqcxwonnkx.fsf@gitster.g/
>
> Thanks,
> Pablo
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [PATCH] revision: honor --exclude-first-parent-only with SEEN first parent
From: Junio C Hamano @ 2026-07-22 17:03 UTC (permalink / raw)
To: git; +Cc: Jerry Zhang, Michael Hore
The '--exclude-first-parent-only' option instructs the revision
walker to follow only the first parent of a merge commit to
propagate down the UNINTERESTING bit.
However, if the first parent has already been marked SEEN (for
example, because it was explicitly specified on the command line),
process_parents() skips it with a 'continue' statement. But the
loop then continues on to process the second parent, because the
check for the '--exclude-first-parent-only' option is near the end
of the loop, which the 'continue' statement skips. Consequently, we
end up marking the second parent as UNINTERESTING.
Break out of the loop instead of continuing when the first parent is
already SEEN or fails to parse. This ensures that we do not process
subsequent parents and mark them as UNINTERESTING.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Reviewed-by: Jerry Zhang <jerry@skydio.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This time with a commit log message.
revision.c | 10 ++++++++--
t/t6012-rev-list-simplify.sh | 18 ++++++++++++++++++
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/revision.c b/revision.c
index 599b3a66c3..9b30663669 100644
--- a/revision.c
+++ b/revision.c
@@ -1152,12 +1152,18 @@ static int process_parents(struct rev_info *revs, struct commit *commit,
if (p)
p->object.flags |= UNINTERESTING |
CHILD_VISITED;
- if (repo_parse_commit_gently(revs->repo, p, 1) < 0)
+ if (repo_parse_commit_gently(revs->repo, p, 1) < 0) {
+ if (revs->exclude_first_parent_only)
+ break;
continue;
+ }
if (p->parents)
mark_parents_uninteresting(revs, p);
- if (p->object.flags & SEEN)
+ if (p->object.flags & SEEN) {
+ if (revs->exclude_first_parent_only)
+ break;
continue;
+ }
p->object.flags |= (SEEN | NOT_USER_GIVEN);
if (list)
commit_list_insert_by_date(p, list);
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index 4cecb6224c..2284bbba12 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -285,4 +285,22 @@ test_expect_success 'log --graph --simplify-merges --show-pulls' '
test_cmp expect actual
'
+test_expect_success 'exclude-first-parent-only with parent already seen' '
+ git checkout --orphan test-seen &&
+ git rm -rf . &&
+ test_commit r1 &&
+ git checkout -b branch-f &&
+ test_commit f &&
+ git checkout test-seen &&
+ git merge --no-ff --no-edit -m r2 branch-f &&
+ git tag r2 &&
+
+ git rev-list --exclude-first-parent-only f ^r2 >actual &&
+ git rev-parse f >expect &&
+ test_cmp expect actual &&
+
+ git rev-list --exclude-first-parent-only f r1 ^r2 >actual2 &&
+ test_cmp expect actual2
+'
+
test_done
--
2.55.0-496-g61638ae030
^ permalink raw reply related
* [PATCH v4 0/2] remote: url-based pushRemote with renamed remotes
From: Harald Nordgren via GitGitGadget @ 2026-07-22 18:08 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Harald Nordgren
In-Reply-To: <pull.2358.v3.git.git.1784664859.gitgitgadget@gmail.com>
Fix git status not showing the push branch after remotes are renamed, when
branch.<name>.pushRemote is a URL matching exactly one configured remote.
Changes in v4:
* Match configured remotes by effective push URL, preferring pushurl over
url.
* Update the documentation and rationale to describe where the remote would
push.
Changes in v3:
* Revamp commit messages to clarify motivation.
Changes in v2:
* Clarify that URL push destinations already work and that this change only
restores their tracking information.
* Document URL values for branch.<name>.pushRemote and their @{push}
behavior.
Harald Nordgren (2):
remote: pass repository to push tracking helper
remote: find tracking branches for URL push destinations
Documentation/config/branch.adoc | 1 +
Documentation/revisions.adoc | 3 +
remote.c | 54 ++++++++++--
remote.h | 2 +
t/t5505-remote.sh | 144 +++++++++++++++++++++++++++++++
transport.c | 5 +-
6 files changed, 203 insertions(+), 6 deletions(-)
base-commit: 5d2e7709234afea1b6ddb25cd4f60d3d5fb3c200
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2358%2FHaraldNordgren%2Fremote-resolve-url-push-tracking-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2358/HaraldNordgren/remote-resolve-url-push-tracking-v4
Pull-Request: https://github.com/git/git/pull/2358
Range-diff vs v3:
1: b1ac49de87 = 1: 494287bade remote: pass repository to push tracking helper
2: a343af9d50 ! 2: 08c432a2d4 remote: find tracking branches for URL push destinations
@@ Commit message
"git status" cannot show the push branch, and an up-to-date push can
leave its tracking information stale.
- When exactly one configured remote uses the push destination URL, use
- that remote for push tracking. Continue to push to the URL so the
- configured remote's push settings do not change existing behavior. Keep
- the current behavior when no remote matches or multiple remotes match.
+ When exactly one configured remote would push to the same URL, use that
+ remote for push tracking. Continue to push to the URL so the configured
+ remote's push settings do not change existing behavior. Keep the current
+ behavior when no remote matches or multiple remotes match.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ Documentation/revisions.adoc: some output processing may assume ref names in UTF
`git push` were run while `branchname` was checked out (or the current
`HEAD` if no branchname is specified). Like for '@\{upstream\}', we report
the remote-tracking branch that corresponds to that branch at the remote.
-+ If the push destination is a URL and exactly one configured remote has
-+ that URL among its `remote.<name>.url` values, '@\{push}' reports that
-+ remote's remote-tracking branch.
++ If the push destination is a URL and exactly one configured remote uses
++ that URL for pushing, '@\{push}' reports that remote's remote-tracking
++ branch.
+
Here's an example to make it more clear:
+
## remote.c ##
+@@ remote.c: struct strvec *push_url_of_remote(struct remote *remote)
+ return remote->pushurl.nr ? &remote->pushurl : &remote->url;
+ }
+
++static bool remote_has_push_url(struct remote *remote, const char *url)
++{
++ const struct strvec *push_urls = push_url_of_remote(remote);
++
++ for (size_t i = 0; i < push_urls->nr; i++) {
++ if (!strcmp(push_urls->v[i], url))
++ return true;
++ }
++ return false;
++}
++
+ void ref_push_report_free(struct ref_push_report *report)
+ {
+ while (report) {
@@ remote.c: const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
return branch->merge[0]->dst;
}
@@ remote.c: const char *branch_get_upstream(struct branch *branch, struct strbuf *
+
+ if (!candidate || candidate == remote ||
+ !remote_is_configured(candidate, 0) ||
-+ !remote_has_url(candidate, check_url))
++ !remote_has_push_url(candidate, check_url))
+ continue;
+ if (first_match)
+ return remote;
@@ t/t5505-remote.sh: test_expect_success 'rename a remote renames repo remote.push
+ EOF
+'
+
++test_expect_success 'configured pushurl makes URL-valued pushRemote trackable' '
++ setup_url_pushremote &&
++
++ (
++ cd client &&
++ git remote rename origin upstream &&
++ git remote add -f origin ../fork.git &&
++ git remote set-url --push origin "$fork_url"
++ ) &&
++
++ check_status <<-EOF
++ On branch topic
++ Your branch is ahead of ${SQ}upstream/main${SQ} by 1 commit.
++
++ Your branch is up to date with ${SQ}origin/topic${SQ}.
++
++ nothing to commit, working tree clean
++ EOF
++'
++
+test_expect_success 'pushInsteadOf URL pushRemote is trackable' '
+ setup_url_pushremote &&
+ (
--
gitgitgadget
^ permalink raw reply
* [PATCH v4 1/2] remote: pass repository to push tracking helper
From: Harald Nordgren via GitGitGadget @ 2026-07-22 18:08 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2358.v4.git.git.1784743738.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
The next commit needs tracking_for_push_dest() to inspect the
repository's configured remotes. Pass the repository through the
existing callers and mark the new parameter as unused.
No change in behavior.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
remote.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/remote.c b/remote.c
index b17648d6ef..0dc36956c3 100644
--- a/remote.c
+++ b/remote.c
@@ -1887,7 +1887,8 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
return branch->merge[0]->dst;
}
-static char *tracking_for_push_dest(struct remote *remote,
+static char *tracking_for_push_dest(struct repository *repo UNUSED,
+ struct remote *remote,
const char *refname,
struct strbuf *err)
{
@@ -1925,13 +1926,13 @@ static char *branch_get_push_1(struct repository *repo,
_("push refspecs for '%s' do not include '%s'"),
remote->name, branch->name);
- ret = tracking_for_push_dest(remote, dst, err);
+ ret = tracking_for_push_dest(repo, remote, dst, err);
free(dst);
return ret;
}
if (remote->mirror)
- return tracking_for_push_dest(remote, branch->refname, err);
+ return tracking_for_push_dest(repo, remote, branch->refname, err);
switch (push_default) {
case PUSH_DEFAULT_NOTHING:
@@ -1939,7 +1940,7 @@ static char *branch_get_push_1(struct repository *repo,
case PUSH_DEFAULT_MATCHING:
case PUSH_DEFAULT_CURRENT:
- return tracking_for_push_dest(remote, branch->refname, err);
+ return tracking_for_push_dest(repo, remote, branch->refname, err);
case PUSH_DEFAULT_UPSTREAM:
return xstrdup_or_null(branch_get_upstream(branch, err));
@@ -1953,7 +1954,7 @@ static char *branch_get_push_1(struct repository *repo,
up = branch_get_upstream(branch, err);
if (!up)
return NULL;
- cur = tracking_for_push_dest(remote, branch->refname, err);
+ cur = tracking_for_push_dest(repo, remote, branch->refname, err);
if (!cur)
return NULL;
if (strcmp(cur, up)) {
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 2/2] remote: find tracking branches for URL push destinations
From: Harald Nordgren via GitGitGadget @ 2026-07-22 18:08 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2358.v4.git.git.1784743738.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Git accepts a repository URL as branch.<name>.pushRemote and can push
to it. This branch setting takes precedence over remote.pushDefault.
A branch can be configured with a URL-valued pushRemote before any push
occurs. If the remotes are later rearranged with "git remote rename" and
"git remote add", the newly added remote may use that URL. The URL value
is unaffected by the rename and continues to take precedence over
remote.pushDefault. The URL and the remote then point to the same
repository, but Git does not connect them for tracking. Pushing works,
but @{push} cannot identify the remote's tracking branch. As a result,
"git status" cannot show the push branch, and an up-to-date push can
leave its tracking information stale.
When exactly one configured remote would push to the same URL, use that
remote for push tracking. Continue to push to the URL so the configured
remote's push settings do not change existing behavior. Keep the current
behavior when no remote matches or multiple remotes match.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/config/branch.adoc | 1 +
Documentation/revisions.adoc | 3 +
remote.c | 45 +++++++++-
remote.h | 2 +
t/t5505-remote.sh | 144 +++++++++++++++++++++++++++++++
transport.c | 5 +-
6 files changed, 198 insertions(+), 2 deletions(-)
diff --git a/Documentation/config/branch.adoc b/Documentation/config/branch.adoc
index a4db9fa5c8..5a85fde8de 100644
--- a/Documentation/config/branch.adoc
+++ b/Documentation/config/branch.adoc
@@ -55,6 +55,7 @@ This option defaults to `never`.
repository), you would want to set `remote.pushDefault` to
specify the remote to push to for all branches, and use this
option to override it for a specific branch.
+ The value may be the name of a configured remote or a repository URL.
`branch.<name>.merge`::
Defines, together with `branch.<name>.remote`, the upstream branch
diff --git a/Documentation/revisions.adoc b/Documentation/revisions.adoc
index 6ea6c7cead..3fbfbd3d5f 100644
--- a/Documentation/revisions.adoc
+++ b/Documentation/revisions.adoc
@@ -127,6 +127,9 @@ some output processing may assume ref names in UTF-8.
`git push` were run while `branchname` was checked out (or the current
`HEAD` if no branchname is specified). Like for '@\{upstream\}', we report
the remote-tracking branch that corresponds to that branch at the remote.
+ If the push destination is a URL and exactly one configured remote uses
+ that URL for pushing, '@\{push}' reports that remote's remote-tracking
+ branch.
+
Here's an example to make it more clear:
+
diff --git a/remote.c b/remote.c
index 0dc36956c3..3a6abf1258 100644
--- a/remote.c
+++ b/remote.c
@@ -954,6 +954,17 @@ struct strvec *push_url_of_remote(struct remote *remote)
return remote->pushurl.nr ? &remote->pushurl : &remote->url;
}
+static bool remote_has_push_url(struct remote *remote, const char *url)
+{
+ const struct strvec *push_urls = push_url_of_remote(remote);
+
+ for (size_t i = 0; i < push_urls->nr; i++) {
+ if (!strcmp(push_urls->v[i], url))
+ return true;
+ }
+ return false;
+}
+
void ref_push_report_free(struct ref_push_report *report)
{
while (report) {
@@ -1887,13 +1898,45 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
return branch->merge[0]->dst;
}
-static char *tracking_for_push_dest(struct repository *repo UNUSED,
+struct remote *repo_remote_for_push_tracking(struct repository *repo,
+ struct remote *remote)
+{
+ const struct strvec *push_urls;
+ struct remote *first_match = NULL;
+ struct remote_state *remote_state = repo->remote_state;
+ const char *check_url;
+
+ if (remote->origin != REMOTE_UNCONFIGURED)
+ return remote;
+
+ push_urls = push_url_of_remote(remote);
+ if (push_urls->nr != 1)
+ return remote;
+ check_url = push_urls->v[0];
+
+ for (int i = 0; i < remote_state->remotes_nr; i++) {
+ struct remote *candidate = remote_state->remotes[i];
+
+ if (!candidate || candidate == remote ||
+ !remote_is_configured(candidate, 0) ||
+ !remote_has_push_url(candidate, check_url))
+ continue;
+ if (first_match)
+ return remote;
+ first_match = candidate;
+ }
+
+ return first_match ? first_match : remote;
+}
+
+static char *tracking_for_push_dest(struct repository *repo,
struct remote *remote,
const char *refname,
struct strbuf *err)
{
char *ret;
+ remote = repo_remote_for_push_tracking(repo, remote);
ret = apply_refspecs(&remote->fetch, refname);
if (!ret)
return error_buf(err,
diff --git a/remote.h b/remote.h
index 72a54d84ad..cca02033b9 100644
--- a/remote.h
+++ b/remote.h
@@ -345,6 +345,8 @@ char *remote_ref_for_branch(struct branch *branch, int for_push);
const char *repo_default_remote(struct repository *repo);
const char *repo_remote_from_url(struct repository *repo, const char *url);
+struct remote *repo_remote_for_push_tracking(struct repository *repo,
+ struct remote *remote);
/* returns true if the given branch has merge configuration given. */
int branch_has_merge_config(struct branch *branch);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 6f5e86dede..9c2f140d5a 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -24,6 +24,28 @@ setup_repository () {
)
}
+setup_url_pushremote () {
+ rm -rf fork.git client &&
+ git clone --bare one fork.git &&
+ git clone one client &&
+ fork_url="file://$TRASH_DIRECTORY/fork.git" &&
+ (
+ cd client &&
+ git checkout -b topic --track origin/main &&
+ git commit --allow-empty -m topic-change &&
+ git config push.default current &&
+ git config status.compareBranches "@{upstream} @{push}" &&
+ git config branch.topic.pushRemote "$fork_url" &&
+ git push
+ )
+}
+
+check_status () {
+ git -C client status >actual &&
+ cat >expected &&
+ test_cmp expected actual
+}
+
tokens_match () {
echo "$1" | tr ' ' '\012' | sort | sed -e '/^$/d' >expect &&
echo "$2" | tr ' ' '\012' | sort | sed -e '/^$/d' >actual &&
@@ -1018,6 +1040,128 @@ test_expect_success 'rename a remote renames repo remote.pushDefault but keeps g
)
'
+test_expect_success 'URL-valued pushRemote without matching remote is not trackable' '
+ setup_url_pushremote &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}origin/main${SQ} by 1 commit.
+ (use "git push" to publish your local commits)
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'adding matching remote makes URL-valued pushRemote trackable' '
+ setup_url_pushremote &&
+
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url"
+ ) &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 1 commit.
+
+ Your branch is up to date with ${SQ}origin/topic${SQ}.
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'configured pushurl makes URL-valued pushRemote trackable' '
+ setup_url_pushremote &&
+
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin ../fork.git &&
+ git remote set-url --push origin "$fork_url"
+ ) &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 1 commit.
+
+ Your branch is up to date with ${SQ}origin/topic${SQ}.
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'pushInsteadOf URL pushRemote is trackable' '
+ setup_url_pushremote &&
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url" &&
+ git config "url.$fork_url.pushInsteadOf" fork: &&
+ git config branch.topic.pushRemote fork:
+ ) &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 1 commit.
+
+ Your branch is up to date with ${SQ}origin/topic${SQ}.
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'up-to-date URL push refreshes stale tracking branch' '
+ setup_url_pushremote &&
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url" &&
+ git commit --allow-empty -m another-topic-change &&
+ git -C ../fork.git fetch ../client topic:topic
+ ) &&
+
+ check_status <<-EOF &&
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 2 commits.
+
+ Your branch is ahead of ${SQ}origin/topic${SQ} by 1 commit.
+ (use "git push" to publish your local commits)
+
+ nothing to commit, working tree clean
+ EOF
+
+ git -C client push >actual 2>&1 &&
+ test_grep "Everything up-to-date" actual &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 2 commits.
+
+ Your branch is up to date with ${SQ}origin/topic${SQ}.
+
+ nothing to commit, working tree clean
+ EOF
+'
+
+test_expect_success 'duplicate remote URL leaves URL-valued pushRemote ambiguous' '
+ setup_url_pushremote &&
+ (
+ cd client &&
+ git remote rename origin upstream &&
+ git remote add -f origin "$fork_url" &&
+ git remote add duplicate "$fork_url"
+ ) &&
+
+ check_status <<-EOF
+ On branch topic
+ Your branch is ahead of ${SQ}upstream/main${SQ} by 1 commit.
+ (use "git push" to publish your local commits)
+
+ nothing to commit, working tree clean
+ EOF
+'
+
test_expect_success 'rename handles remote without fetch refspec' '
git clone --bare one no-refspec.git &&
# confirm assumption that bare clone does not create refspec
diff --git a/transport.c b/transport.c
index fc144f0aed..30a4ab2cd5 100644
--- a/transport.c
+++ b/transport.c
@@ -1553,8 +1553,11 @@ int transport_push(struct repository *r,
if (!(flags & (TRANSPORT_PUSH_DRY_RUN |
TRANSPORT_RECURSE_SUBMODULES_ONLY))) {
struct ref *ref;
+ struct remote *tracking_remote = repo_remote_for_push_tracking(
+ r, transport->remote);
+
for (ref = remote_refs; ref; ref = ref->next)
- transport_update_tracking_ref(transport->remote, ref, verbose);
+ transport_update_tracking_ref(tracking_remote, ref, verbose);
}
if (porcelain && !push_ret)
--
gitgitgadget
^ permalink raw reply related
* submodule path with symlinks
From: Kyle Marek @ 2026-07-22 18:20 UTC (permalink / raw)
To: git
Hello,
I've just hit this change in behavior introduced ~2 years ago, where
submodule paths are no longer allowed to contain symlinks [1].
This change has broken a practice that I have used repeatedly over the
years, where several worktrees share the same copy of their submodules.
It has been very useful for project organization, but no longer works on
updated systems for "security" reasons. Please, I understand the risks
associated with symlinks, I do not want to be "protected from myself",
and I do not want to re-architect affected projects.
I see there is even a TODO comment about exactly this issue [2]:
> TODO: allow exempting it via `safe.submodule.path` or something
I'm surprised to see that this change went through without an
accompanying commit to allowing exceptions or allowing the user accept
the risks associated with symlinks.
Is there interest in implementing the TODO? Will you accept a patch for
this?
Thank you,
Kyle Marek
[1]: https://git.kernel.org/pub/scm/git/git.git/commit/?id=e8d0608944486019ea0e1ed2ed29776811a565c2
[2]: https://git.kernel.org/pub/scm/git/git.git/tree/builtin/submodule--helper.c?id=e8d0608944486019ea0e1ed2ed29776811a565c2#n2679
^ permalink raw reply
* Re: What's cooking in git.git (Jul 2026, #09)
From: Junio C Hamano @ 2026-07-22 18:27 UTC (permalink / raw)
To: Christian Couder; +Cc: git
In-Reply-To: <CAP8UFD3qTPak-RGtJ6cG7JkvX3_NZGHUB9N+rQk8ea2S_MDssA@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
>> * cc/fast-import-usage (2026-07-16) 7 commits
>> - fast-import: use struct option for usage string
>> - fast-import: move command state globals into 'struct fast_import_state'
>> - fast-import: introduce 'struct fast_import_state'
>> - fast-import: localize 'i' into the 'for' loops using it
>> - api-parse-options.adoc: document hidden and OPT_*_F option macros
>> - api-parse-options.adoc: document per-option flags
>> - parse-options: introduce OPT_HIDDEN_GROUP
>>
>> The usage string of 'git fast-import' has been updated to use the
>> 'parse_options' API for displaying help, and its SYNOPSIS in the
>> documentation has been standardized to match.
>>
>> Waiting for response.
>> cf. <xmqq4ihyehyb.fsf@gitster.g>
>> source: <20260716165517.433849-1-christian.couder@gmail.com>
>
> I am having a vacation, so I will likely not be able to reply soon.
> Feel free to discard in the meantime.
I'll leave it as-is (unless somebody else comes in and crashes with
the changes in the topic badly, in which case I may eject one or the
other from my tree). Enjoy your vacation.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 2/2] remote: find tracking branches for URL push destinations
From: Junio C Hamano @ 2026-07-22 19:03 UTC (permalink / raw)
To: Harald Nordgren via GitGitGadget; +Cc: git, D. Ben Knoble, Harald Nordgren
In-Reply-To: <08c432a2d4f52c202a2bebaa72330a17e94aedd1.1784743738.git.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +static bool remote_has_push_url(struct remote *remote, const char *url)
> +{
> + const struct strvec *push_urls = push_url_of_remote(remote);
> +
> + for (size_t i = 0; i < push_urls->nr; i++) {
> + if (!strcmp(push_urls->v[i], url))
> + return true;
> + }
> + return false;
> +}
A new helper is very much welcome.
> void ref_push_report_free(struct ref_push_report *report)
> {
> while (report) {
> @@ -1887,13 +1898,45 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
> return branch->merge[0]->dst;
> }
>
> -static char *tracking_for_push_dest(struct repository *repo UNUSED,
> +struct remote *repo_remote_for_push_tracking(struct repository *repo,
> + struct remote *remote)
> +{
> + const struct strvec *push_urls;
> + struct remote *first_match = NULL;
> + struct remote_state *remote_state = repo->remote_state;
> + const char *check_url;
> +
> + if (remote->origin != REMOTE_UNCONFIGURED)
> + return remote;
> +
> + push_urls = push_url_of_remote(remote);
> + if (push_urls->nr != 1)
> + return remote;
> + check_url = push_urls->v[0];
> +
> + for (int i = 0; i < remote_state->remotes_nr; i++) {
> + struct remote *candidate = remote_state->remotes[i];
> +
> + if (!candidate || candidate == remote ||
> + !remote_is_configured(candidate, 0) ||
> + !remote_has_push_url(candidate, check_url))
This part used to use remote_has_url(candidate, remote->url.v[0]),
which only looked at the .url and ignored .pushurl. Now it uses
remote_has_push_url() so we grab the effective push URL for the
remote we are dealing with and match it against the effective push
URL of the candidates. Looks correct.
> diff --git a/transport.c b/transport.c
> index fc144f0aed..30a4ab2cd5 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1553,8 +1553,11 @@ int transport_push(struct repository *r,
> if (!(flags & (TRANSPORT_PUSH_DRY_RUN |
> TRANSPORT_RECURSE_SUBMODULES_ONLY))) {
> struct ref *ref;
> + struct remote *tracking_remote = repo_remote_for_push_tracking(
> + r, transport->remote);
Personally, I would have line-wrapped the above more like this:
struct remote *tracking_remote =
repo_remote_for_push_tracking(r, transport->remote);
This is just for a future reference; it is certainly not critical
enough to warrant a new iteration just for this.
Thanks.
^ permalink raw reply
* Re: [PATCH] submodule: resolve insteadof-aliases when matching remote
From: Junio C Hamano @ 2026-07-22 19:49 UTC (permalink / raw)
To: Éric NICOLAS; +Cc: git, Jacob Keller
In-Reply-To: <20260721213042.3357346-1-ccjmne@gmail.com>
Éric NICOLAS <ccjmne@gmail.com> writes:
> When ca62f524c1 introduced a mechanism to identify which remote is to be
> used by a submodule, we had it compare the URL stored in the .gitmodules
> inventory to those of each available remote.
Please refer to an existing commit using this format:
When ca62f524c1 (submodule: look up remotes by URL first,
2025-06-23) introduced ...
> However, when using URL aliasing via url.<base>.insteadOf, we store
> in .gitmodules the URL pre-resolution of the alias, whereas the
> corresponding remote set up in the submodule reports using the
> *resolved* URL. This mechanism therefore fails to find a match then,
Since anything involving the .gitmodules file is often security-
sensitive, it is always a good idea to go beyond just saying 'X fails
to do Y.' We should also explain why that failure is a bad thing (or
perhaps a good thing) and for what reason.
If this aliasing were controlled by a remote entity (for example, if
an upstream project modified the .gitmodules file to redirect us
somewhere unexpected), failing to find a match could actually be a
safety feature, shielding us from bad actors trying to hijack the
local repository. Since that is not the case here, adding 'fails to
find a match, which is unfortunate because...' would make the commit
message much stronger.
> and resorts to the fallback logic, which does use either the only
> configured remote if there is only one, or attempts using "origin"
> otherwise.
>
> Resolve the alias in the URL inventoried in .gitmodules before comparing
> it against those of the corresponding submodule's configured remotes.
>
> Signed-off-by: Éric NICOLAS <ccjmne@gmail.com>
> ---
> remote.c | 15 ++++++++++++---
> t/t7406-submodule-update.sh | 21 +++++++++++++++++++++
> 2 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index b17648d6ef..ae187fb3d6 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1821,17 +1821,26 @@ const char *repo_default_remote(struct repository *repo)
>
> const char *repo_remote_from_url(struct repository *repo, const char *url)
> {
> + char *rewritten_url;
> + const char *url_to_match;
> + const char *remote_name = NULL;
> +
> read_config(repo, 0);
> + rewritten_url = alias_url(url, &repo->remote_state->rewrites);
> + url_to_match = rewritten_url ? rewritten_url : url;
Being a bit lazy, I probably would have just reused 'url' directly:
if ((rewritten_url = alias_url(url, &repo->remote_state->rewrites)))
url = rewritten_url;
This lets us avoid introducing a brand-new 'url_to_match' variable,
whose lifetime is essentially just taking over for 'url' anyway.
> for (int i = 0; i < repo->remote_state->remotes_nr; i++) {
> struct remote *remote = repo->remote_state->remotes[i];
> if (!remote)
> continue;
>
> - if (remote_has_url(remote, url))
> - return remote->name;
> + if (remote_has_url(remote, url_to_match)) {
> + remote_name = remote->name;
> + break;
> + }
While the new code preserves the original 'first one wins' behavior,
it does make me wonder why we do not issue a warning or raise an
error when multiple URLs match. Leaving such an ambiguous
configuration unflagged feels like a silent bug waiting to happen.
But it is of course outside the scope of this topic.
> }
> - return NULL;
> + free(rewritten_url);
> + return remote_name;
> }
Thanks.
^ permalink raw reply
* Re: [PATCH GSoC v20 00/13] cat-file: add remote-object-info to batch-command
From: Pablo Sabater @ 2026-07-22 20:47 UTC (permalink / raw)
To: Karthik Nayak, Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon, szeder.dev
In-Reply-To: <CAOLa=ZS8J4t12ab1=3-LRYNuZOwqSHG861iYm97JjF3mGprvJA@mail.gmail.com>
On Wed Jul 22, 2026 at 6:13 PM CEST, Karthik Nayak wrote:
> "Pablo Sabater" <pabloosabaterr@gmail.com> writes:
>
>> On Tue Jul 21, 2026 at 10:12 PM CEST, Karthik Nayak wrote:
>>> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>>>
>>> [snip]
>>>
>>>> Changes in v20:
>>>> - Moved the prep patch that fixes hash_algo's type to be before
>>>> write_fetch_command_and_capabilities() being moved to 'connect.c'
>>>> - Reverted git-cat-file.adoc documentation comments about CAVEATS
>>>> - Fixed style for EXPAND_DATA_INIT
>>>> - Added more context for comman line die()
>>>>
>>>
>>> [snip]
>>>
>>>> Range-diff versus v19:
[snip]
>>>>
>>>> @@ Documentation/git-cat-file.adoc: one per line, and print information based on th
>>>> You can specify the information shown for each object by using a custom
>>>> `<format>`. The `<format>` is copied literally to stdout for each
>>>> @@ Documentation/git-cat-file.adoc: newline. The available atoms are:
>>>> - reports).
>>>> -
>>>> - `objectsize:disk`::
>>>> -- The size, in bytes, that the object takes up on disk. See the
>>>> -- note about on-disk sizes in the `CAVEATS` section below.
>>>> -+ The size, in bytes, that the object takes up on disk.
>>>> -
>>>> `deltabase`::
>>>> If the object is stored as a delta on-disk, this expands to the
>>>> full hex representation of the delta base object name.
>>>> - Otherwise, expands to the null OID (all zeroes). See `CAVEATS`
>>>> -- below.
>>>> -+ Otherwise, expands to the null OID (all zeroes).
>>>> ++ Otherwise, expands to the null OID (all zeroes). See `CAVEATS` section
>>>> + below.
>>>>
>>>
>>> This chagne is still unnecessary, no?
>>
>> I guess it is unnecessary. I added it because on top, at objectsize:disk
>> it says "`CAVEATS` section". So I tried to have it consistently.
>>
>> Is it worth a reroll?
>>
>
> No I don't think a re-roll is necessary. But something to avoid next
> time :)
>
[snip]
>>>
>>>> 13: ebdfc22fb7 ! 13: 70a11d2aea cat-file: make remote-object-info allow-list adapt to the server
>>>> @@ builtin/cat-file.c: struct expand_data {
>>>> */
>>>> unsigned is_remote:1;
>>>> -};
>>>> +-
>>>> -#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD }
>>>>
>>>> -static const char *remote_object_info_atoms[] = {
>>>> - "objectname",
>>>> - "objectsize",
>>>> ++ /*
>>>> ++ * List of atoms (i.e. "objectsize") that the server supports. Built
>>>> ++ * from the server's object-info advertised capabilities.
>>>> ++ */
>>>> + struct string_list remote_allowed_atoms;
>>>> };
>>>> -+#define EXPAND_DATA_INIT { .mode = S_IFINVALID, .type = OBJ_BAD, \
>>>> -+ .remote_allowed_atoms = STRING_LIST_INIT_NODUP }
>>>>
>>>> ++#define EXPAND_DATA_INIT { .mode = S_IFINVALID, \
>>>> ++ .type = OBJ_BAD, \
>>>> ++ .remote_allowed_atoms = STRING_LIST_INIT_NODUP }
>>>> ++
>>>
>>> I guess this is output of running the style check script (which I did
>>> recommend, but I should've mentioned that you should take it with a
>>> pinch of salt, it doesn't always provide the best solutions.)
>>
>> It comes in part from Junio's suggestion [1]. Although I don't think that
>> it will receive a lot of new fields, I thought that it would leave the
>> macro clean for that.
>>
>
> Then wouldn't something like this be better?
>
> #define EXPAND_DATA_INIT { \
> .mode = S_IFINVALID, \
> .type = OBJ_BAD, \
> .remote_allowed_atoms = STRING_LIST_INIT_NODUP, \
> }
>
> Note the difference:
> 1. The first line no longer has the '.mode' field, this would ensure
> that removal of the field is a diff which only contains the field and
> not movement of other fields to the top line.
> 2. Similar for the last time.
> 3. We also add a comma for the last field, this allows addition of new
> fields without modifying previous lines by adding a missing comma.
>
> I don't think this too warrants a re-roll, but this is something to
> definitely fix incase you do re-roll for other reasons :)
Yeah, this one is better formatted than what I did. Thanks for
the explanation. I'll definetely fix this two hunks if I have to reroll.
[snip]
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH 1/1] Extract only the message body from git commit.
From: Junio C Hamano @ 2026-07-22 20:53 UTC (permalink / raw)
To: hardikxk; +Cc: git
In-Reply-To: <20260722083836.744338-2-hardikxk@gmail.com>
hardikxk <hardikxk@gmail.com> writes:
> Subject: Re: [PATCH 1/1] Extract only the message body from git commit.
Please see Documentation/SubmittingPatches[[describe-changes]].
> The patch fixes the `extractLogMessageFromGitCommit` function to skip all the metada of the commit object and only return back the message body.
Line-wrap overly long lines like this one.
> Previously the function would return the entire data of the objects
> including authors tree and SHAs. This patch fixes that to skip over all
> that and just return the body of the log message.
Please see Documentation/SubmittingPatches[[meaningful-message]].
The observation on how the current code behaves should not be
described as "Previously X did Y". Just say "X does Y" instead.
There are other rules on how to write proposed commit log messages
explained there.
> Signed-off-by: hardikxk <hardikxk@gmail.com>
Please see Documentation/SubmittingPatches[[real-name]].
> ---
> git-p4.py | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
Thanks.
> diff --git a/git-p4.py b/git-p4.py
> index c0ca7be..589efcd 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1003,12 +1003,18 @@ def branchExists(ref):
> def extractLogMessageFromGitCommit(commit):
> logMessage = ""
>
> - # fixme: title is first line of commit, not 1st paragraph.
> + foundNewLine = False
> foundTitle = False
> for log in read_pipe_lines(["git", "cat-file", "commit", commit]):
> - if not foundTitle:
> + if not foundNewLine:
> + # skip anything that is not the commit message
> if len(log) == 1:
> - foundTitle = True
> + foundNewLine = True
> + continue
> +
> + # everything from here is the commit message
> + if not foundTitle:
> + foundTitle = True
> continue
>
> logMessage += log
^ permalink raw reply
* Re: [PATCH] mv: report missing destination leading directory
From: Lucas Zamboni Orioli @ 2026-07-22 21:32 UTC (permalink / raw)
To: Ben Knoble; +Cc: Lucas Zamboni Orioli via GitGitGadget, git
In-Reply-To: <C6C7AB29-7027-467B-8DCC-3443CC356628@gmail.com>
Em qua., 15 de jul. de 2026 às 13:50, Ben Knoble
<ben.knoble@gmail.com> escreveu:
> I suppose this still allows a TOCTOU issue where the check succeeds and (with lucky timing) the destination then disappears?
Thank you for the feedback, also great catch, this does end up with a
TOCTOU issue.
>
> In that case, I think a worthwhile additional change would also be for the error message to diagnose which file is missing (or at least include both source and destination).
>
> Now, without checking I somehow doubt whether rename(2) tells us which entry is missing. Worse, if we check afterwards, we could have a « TOUTOC » :p where the entry reappears to confuse the error diagnosis.
I think your suggestion of including both source and destination in the
error message is a good solution, I verified rename(2) just in case and
it does not provide the information about which file is missing.
So what I'm thinking of doing is change the error message to
fatal: renaming 'source/file' to 'destination/file' failed: No
such file or directory
'%s' to '%s' seems to be more in the pattern of other git messages
instead of the cp arrow style.
So for v2 I'll split this into two commits:
1. mv: name both source and destination when rename fails
(the die_errno change is race-free and always applicable)
2. mv: check for missing destination directory before renaming
(the checking-phase/dry-run detection)
The first stands on its own even if the second is dropped, so I'll
order it first.
Since this introduces a new message I'll leave the po/ files to the
l10n team, the new message adds one string and the early check reuses
the existing
'destination directory does not exist' one.
^ permalink raw reply
* [PATCH v2] submodule: resolve insteadOf aliases when matching remote
From: Éric NICOLAS @ 2026-07-23 0:21 UTC (permalink / raw)
To: git; +Cc: gitster, jacob.keller, Éric NICOLAS
In-Reply-To: <20260721213042.3357346-1-ccjmne@gmail.com>
When ca62f524c1 (submodule: look up remotes by URL first, 2025-06-23)
introduced a mechanism to identify which remote is to be used by a
submodule, it compared the URL stored in the .gitmodules inventory to
that of each available remote.
The URLs of remotes are rewritten according to url.<base>.insteadOf,
whereas those stored in the .gitmodules aren't. When such aliasing
applies, no match can be made between the two corresponding sides, and
the procedure degrades to its fallback logic electing either the only
configured remote if there is only one, or "origin" otherwise.
That behaviour is unfortunate when no remote is called "origin",
because its last resort will have a submodule update command look for a
non-existent remote-tracking reference and fail to proceed, instead of
using the remote whose rewritten URL matches.
Resolve the alias in the URL inventoried in .gitmodules before comparing
it against those of the corresponding submodule's configured remotes.
Signed-off-by: Éric NICOLAS <ccjmne@gmail.com>
---
Thank you for your guidance.
Changes in v2:
- Reword the commit message more purposefully
- Adjust the implementation as suggested, avoiding a superfluous
variable
- Tidy up the integration test
remote.c | 14 +++++++++++---
t/t7406-submodule-update.sh | 19 +++++++++++++++++++
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index b17648d6ef..b1fed58e79 100644
--- a/remote.c
+++ b/remote.c
@@ -1821,17 +1821,25 @@ const char *repo_default_remote(struct repository *repo)
const char *repo_remote_from_url(struct repository *repo, const char *url)
{
+ char *rewritten_url;
+ const char *remote_name = NULL;
+
read_config(repo, 0);
+ if ((rewritten_url = alias_url(url, &repo->remote_state->rewrites)))
+ url = rewritten_url;
for (int i = 0; i < repo->remote_state->remotes_nr; i++) {
struct remote *remote = repo->remote_state->remotes[i];
if (!remote)
continue;
- if (remote_has_url(remote, url))
- return remote->name;
+ if (remote_has_url(remote, url)) {
+ remote_name = remote->name;
+ break;
+ }
}
- return NULL;
+ free(rewritten_url);
+ return remote_name;
}
int branch_has_merge_config(struct branch *branch)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 9554720152..10adeabf0f 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -256,6 +256,25 @@ test_expect_success 'submodule update --remote should fetch upstream changes' '
)
'
+test_expect_success 'submodule update --remote resolves URL rewrites' '
+ test_config_global "url.$(pwd)/.insteadOf" local: &&
+ mkdir alias-super alias-submodule &&
+ (
+ cd alias-submodule &&
+ git init &&
+ git commit --allow-empty --message "Initial commit"
+ ) &&
+ (
+ cd alias-super &&
+ git init &&
+ git submodule add local:alias-submodule submodule &&
+ git submodule update --force &&
+ git -C submodule remote rename origin upstream &&
+ git -C submodule remote add fork user@host &&
+ git submodule update --remote
+ )
+'
+
test_expect_success 'submodule update --remote should fetch upstream changes with .' '
(
cd super &&
Range-diff against v1:
1: ed507998b3 ! 1: 4363eb3cb1 submodule: resolve insteadof-aliases when matching remote
@@ Metadata
Author: Éric NICOLAS <ccjmne@gmail.com>
## Commit message ##
- submodule: resolve insteadof-aliases when matching remote
+ submodule: resolve insteadOf aliases when matching remote
- When ca62f524c1 introduced a mechanism to identify which remote is to be
- used by a submodule, we had it compare the URL stored in the .gitmodules
- inventory to those of each available remote.
+ When ca62f524c1 (submodule: look up remotes by URL first, 2025-06-23)
+ introduced a mechanism to identify which remote is to be used by a
+ submodule, it compared the URL stored in the .gitmodules inventory to
+ that of each available remote.
- However, when using URL aliasing via url.<base>.insteadOf, we store
- in .gitmodules the URL pre-resolution of the alias, whereas the
- corresponding remote set up in the submodule reports using the
- *resolved* URL. This mechanism therefore fails to find a match then,
- and resorts to the fallback logic, which does use either the only
- configured remote if there is only one, or attempts using "origin"
- otherwise.
+ The URLs of remotes are rewritten according to url.<base>.insteadOf,
+ whereas those stored in the .gitmodules aren't. When such aliasing
+ applies, no match can be made between the two corresponding sides, and
+ the procedure degrades to its fallback logic electing either the only
+ configured remote if there is only one, or "origin" otherwise.
+
+ That behaviour is unfortunate when no remote is called "origin",
+ because its last resort will have a submodule update command look for a
+ non-existent remote-tracking reference and fail to proceed, instead of
+ using the remote whose rewritten URL matches.
Resolve the alias in the URL inventoried in .gitmodules before comparing
it against those of the corresponding submodule's configured remotes.
@@ remote.c: const char *repo_default_remote(struct repository *repo)
const char *repo_remote_from_url(struct repository *repo, const char *url)
{
+ char *rewritten_url;
-+ const char *url_to_match;
+ const char *remote_name = NULL;
+
read_config(repo, 0);
-+ rewritten_url = alias_url(url, &repo->remote_state->rewrites);
-+ url_to_match = rewritten_url ? rewritten_url : url;
++ if ((rewritten_url = alias_url(url, &repo->remote_state->rewrites)))
++ url = rewritten_url;
for (int i = 0; i < repo->remote_state->remotes_nr; i++) {
struct remote *remote = repo->remote_state->remotes[i];
@@ remote.c: const char *repo_default_remote(struct repository *repo)
- if (remote_has_url(remote, url))
- return remote->name;
-+ if (remote_has_url(remote, url_to_match)) {
++ if (remote_has_url(remote, url)) {
+ remote_name = remote->name;
+ break;
+ }
@@ t/t7406-submodule-update.sh: test_expect_success 'submodule update --remote shou
+test_expect_success 'submodule update --remote resolves URL rewrites' '
+ test_config_global "url.$(pwd)/.insteadOf" local: &&
-+ mkdir aliased-super aliased-submodule &&
++ mkdir alias-super alias-submodule &&
+ (
-+ cd aliased-submodule &&
++ cd alias-submodule &&
+ git init &&
-+ echo line >file &&
-+ git add file &&
-+ git commit -m "Initial commit"
++ git commit --allow-empty --message "Initial commit"
+ ) &&
+ (
-+ cd aliased-super &&
++ cd alias-super &&
+ git init &&
-+ git submodule add local:aliased-submodule submodule &&
-+ git submodule update --force submodule &&
++ git submodule add local:alias-submodule submodule &&
++ git submodule update --force &&
+ git -C submodule remote rename origin upstream &&
+ git -C submodule remote add fork user@host &&
-+ git submodule update --remote submodule
++ git submodule update --remote
+ )
+'
+
--
2.55.0
^ permalink raw reply related
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