Git development
 help / color / mirror / Atom feed
* Re: [PATCH v3] config.mak.uname: avoid macOS dup-library warning
From: D. Ben Knoble @ 2026-06-20 20:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git, Harald Nordgren
In-Reply-To: <xmqqv7bei2tf.fsf@gitster.g>

On Fri, Jun 19, 2026 at 6:27 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Harald Nordgren <haraldnordgren@gmail.com>
> >
> > Building on macOS with Xcode 15 or newer emits:
> >
> >     ld: warning: ignoring duplicate libraries: 'libgit.a',
> >     'target/release/libgitcore.a'
> >
> > Some link recipes list the same archive twice, which is harmless.
> > Quiet the warning instead.
> >
> > Pass -Wl,-no_warn_duplicate_libraries on Xcode 15 and newer, whose
> > linkers added both the warning and the suppression flag (ld64-907
> > and dyld-1009). Earlier linkers reject the flag, so gate on the
> > linker version. Broaden the existing -fno-common version probe to
> > also match the "ld64-NNN" and "dyld-NNN" forms Xcode 15 reports.
> >
> > Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> > ---
>
> Yeah, this looks like what I expected.
>
> A few things to note.
>
>  * Can folks with different versions of Xcode (or is 15 sufficiently
>    old that practically nobody is expected to have anything older?)
>    test this patch?
>
>  * We only patch Makefile here; can folks who use meson report how
>    well your build goes?
>
> Thanks.

On one (old) machine I have available:

    $ pkgutil --pkg-info=com.apple.pkg.CLTools_Executables
    [trimmed]
    version: 14.2.0.0.1.1668646533

On said machine, I don't get the duplicate warnings on a Meson build.
No issues with the patch when running make.

I think I have seen this on my other machine, which is much newer.
When I get around to trying it there, I'll report back as well.

^ permalink raw reply

* [PATCH v2] gitk, git-gui: drop msgfmt --statistics output
From: Harald Nordgren via GitGitGadget @ 2026-06-20 22:46 UTC (permalink / raw)
  To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2339.git.git.1781459539.gitgitgadget@gmail.com>

From: Harald Nordgren <haraldnordgren@gmail.com>

The catalog rules ran msgfmt with --statistics, whose output went to
stderr and so survived "make -s" (gitk also echoed "Generating
catalog").

The statistics are not needed, as in 2f12b31b746c (Makefile: don't
invoke msgfmt with --statistics, 2021-12-17), and the "Generating
catalog" line is not needed either. Remove them so a quiet build stays
quiet.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    Silence po catalog output under "make -s"
    
    The gitk and git-gui catalog rules sent msgfmt --statistics output (and
    a "Generating catalog" line) to stderr, so it survived "make -s". Emit
    it only when "-s" is absent, keeping a quiet build silent and a verbose
    build unchanged.
    
    Changes in v2:
    
     * Reworked from conditionally silencing msgfmt output under make -s to
       just removing --statistics outright, following 2f12b31b74 (Makefile:
       don't invoke msgfmt with --statistics, 2021-12-17)
     * Also drop gitk's Generating catalog echo, which is not needed either

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2339%2FHaraldNordgren%2Fsilence-catalog-output-under-make-s-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2339/HaraldNordgren/silence-catalog-output-under-make-s-v2
Pull-Request: https://github.com/git/git/pull/2339

Range-diff vs v1:

 1:  18451d46e4 < -:  ---------- gitk: silence catalog output under "make -s"
 2:  5071c5106a ! 1:  ee57c25009 git-gui: silence statistics under "make -s"
     @@ Metadata
      Author: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## Commit message ##
     -    git-gui: silence statistics under "make -s"
     +    gitk, git-gui: drop msgfmt --statistics output
      
     -    The catalog rule passed --statistics to msgfmt unconditionally, and its
     -    output went to stderr, so it survived "make -s".
     +    The catalog rules ran msgfmt with --statistics, whose output went to
     +    stderr and so survived "make -s" (gitk also echoed "Generating
     +    catalog").
      
     -    Pass --statistics only when "-s" is absent, leaving a quiet build silent
     -    while default and V=1 builds are unchanged.
     +    The statistics are not needed, as in 2f12b31b746c (Makefile: don't
     +    invoke msgfmt with --statistics, 2021-12-17), and the "Generating
     +    catalog" line is not needed either. Remove them so a quiet build stays
     +    quiet.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## git-gui/Makefile ##
     -@@ git-gui/Makefile: ifndef V
     - 	REMOVE_F0 = dst=
     - 	REMOVE_F1 = && echo '   ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
     - endif
     -+	MSGFMT_STATS = --statistics
     - endif
     - 
     - TCLTK_PATH ?= wish
      @@ git-gui/Makefile: $(PO_TEMPLATE): $(SCRIPT_SH) $(ALL_LIBFILES)
       update-po:: $(PO_TEMPLATE)
       	$(foreach p, $(ALL_POFILES), echo Updating $p ; msgmerge -U $p $(PO_TEMPLATE) ; )
       $(ALL_MSGFILES): %.msg : %.po
      -	$(QUIET_MSGFMT0)$(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
     -+	$(QUIET_MSGFMT0)$(MSGFMT) $(MSGFMT_STATS) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
     ++	$(QUIET_MSGFMT0)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
       
       lib/tclIndex: $(ALL_LIBFILES) generate-tclindex.sh GIT-GUI-BUILD-OPTIONS
       	$(QUIET_INDEX)$(SHELL_PATH) generate-tclindex.sh . ./GIT-GUI-BUILD-OPTIONS $(ALL_LIBFILES)
     +
     + ## gitk-git/Makefile ##
     +@@ gitk-git/Makefile: update-po:: $(PO_TEMPLATE)
     + 	echo; \
     + 	echo "	git config filter.gettext-no-location.clean \"msgcat --no-location -\""
     + $(ALL_MSGFILES): %.msg : %.po
     +-	@echo Generating catalog $@
     +-	$(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
     ++	$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
     + 
     + .PHONY: all install uninstall clean update-po
     + .PHONY: FORCE


 git-gui/Makefile  | 2 +-
 gitk-git/Makefile | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-gui/Makefile b/git-gui/Makefile
index d33204e875..48d848a59d 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -155,7 +155,7 @@ $(PO_TEMPLATE): $(SCRIPT_SH) $(ALL_LIBFILES)
 update-po:: $(PO_TEMPLATE)
 	$(foreach p, $(ALL_POFILES), echo Updating $p ; msgmerge -U $p $(PO_TEMPLATE) ; )
 $(ALL_MSGFILES): %.msg : %.po
-	$(QUIET_MSGFMT0)$(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
+	$(QUIET_MSGFMT0)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
 
 lib/tclIndex: $(ALL_LIBFILES) generate-tclindex.sh GIT-GUI-BUILD-OPTIONS
 	$(QUIET_INDEX)$(SHELL_PATH) generate-tclindex.sh . ./GIT-GUI-BUILD-OPTIONS $(ALL_LIBFILES)
diff --git a/gitk-git/Makefile b/gitk-git/Makefile
index 41116d8a14..0ae083c1ca 100644
--- a/gitk-git/Makefile
+++ b/gitk-git/Makefile
@@ -75,8 +75,7 @@ update-po:: $(PO_TEMPLATE)
 	echo; \
 	echo "	git config filter.gettext-no-location.clean \"msgcat --no-location -\""
 $(ALL_MSGFILES): %.msg : %.po
-	@echo Generating catalog $@
-	$(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
+	$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
 
 .PHONY: all install uninstall clean update-po
 .PHONY: FORCE

base-commit: 8d96f09e9245ddf80c1981476fcbac8c4bb4125f
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH v4] SubmittingPatches: address design critiques
From: Junio C Hamano @ 2026-06-20 23:43 UTC (permalink / raw)
  To: git; +Cc: Michael Montalbo, Kristoffer Haugsbakk
In-Reply-To: <xmqqik7eld2g.fsf_-_@gitster.g>

Contributors sometimes fail to answer fundamental design or
viability comments from reviewers and submit subsequent rounds
without addressing them.  When design decisions are resolved on the
mailing list, the final justification should be recorded in the
commit messages.

Instruct authors to be particularly mindful of critiques regarding
high-level design or viability, to defend their choices on the list,
and to accompany new iterations with clearer explanations in the cover
letter, responses, and revised commit messages. Also instruct them to
explicitly document the resolution of these concerns in the commit
message body to keep the historical record complete.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Hopefully this will be the last iteration.

 Documentation/SubmittingPatches | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index f042bb5aaf..28b4f2f795 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -51,6 +51,22 @@ area.
   respond to them with "Reply-All" on the mailing list, while taking
   them into account while preparing an updated set of patches.
 +
+Be particularly mindful of critiques regarding the high-level design
+or viability of your proposal (e.g., questioning if the feature is
+worth implementing, or if the chosen approach is appropriate).  Defend
+your design decisions on the list first and work with reviewers and
+other members to improve the design before revising the implementation.
+This will avoid wasting effort on an implementation before its design is
+solid.
++
+Make sure that any new version explains and justifies those design
+decisions more clearly, in the cover letter and in the revised commit
+messages.  Aim to make the reviewers say "it is now clear why we may
+want to do this with the updated version".
++
+Topics with unresolved fundamental design critiques will not be
+considered ready for merging.
++
 It is often beneficial to allow some time for reviewers to provide
 feedback before sending a new version, rather than sending an updated
 series immediately after receiving a review. This helps collect broader
@@ -323,6 +339,10 @@ The body should provide a meaningful commit message, which:
 
 . alternate solutions considered but discarded, if any.
 
+. records the resolution of design or viability concerns raised by the
+  community during the review, if any, ensuring the historical record
+  explains why the chosen approach was accepted over alternatives.
+
 [[present-tense]]
 The problem statement that describes the status quo is written in the
 present tense.  Write "The code does X when it is given input Y",

Interdiff against v2:
  diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
  index a9789e5303..28b4f2f795 100644
  --- a/Documentation/SubmittingPatches
  +++ b/Documentation/SubmittingPatches
  @@ -54,9 +54,10 @@ area.
   Be particularly mindful of critiques regarding the high-level design
   or viability of your proposal (e.g., questioning if the feature is
   worth implementing, or if the chosen approach is appropriate).  Defend
  -your design decisions on the list first, work with reviewers and other
  -members to improve the design before revising the implementation, to
  -avoid wasting effort on an implementation before its design is solid.
  +your design decisions on the list first and work with reviewers and
  +other members to improve the design before revising the implementation.
  +This will avoid wasting effort on an implementation before its design is
  +solid.
   +
   Make sure that any new version explains and justifies those design
   decisions more clearly, in the cover letter and in the revised commit
-- 
2.55.0-rc1-134-gb7e3543e07


^ permalink raw reply related

* Re: git-diff in a worktree is an order of magnitude slower?
From: Junio C Hamano @ 2026-06-21  0:53 UTC (permalink / raw)
  To: D. Ben Knoble; +Cc: Jeff King, Git
In-Reply-To: <CALnO6CAx91kbJ84d6Ef655UNG0y0rhyknBRh6Y+0o7Xn-uVytQ@mail.gmail.com>

"D. Ben Knoble" <ben.knoble@gmail.com> writes:

> But the refresh_index_quietly call is guarded by (effectively; the
> actual code uses rev.diffopt.skip_stat_unmatch)
>
>     1 < !!diff_auto_refresh_index

It is not quite that, is it?  In aecbf914 (git-diff: resurrect the
traditional empty "diff --git" behaviour, 2007-08-31), it read more
like

	if (1 < rev.diffopt.skip_stat_unmatch)
		refresh_index_quietly();

where rev.diffopt.skip_stat_unmatch was initialized to 1 if
diff_auto_refresh_index (boolean) is set to true.

Now, cmd_diff() dispatches to various diff backends to compare two
sets (like "a tree object vs the index", "the index vs the working
tree files"), each of which ends with a call to diffcore_std() and
diffcore_flush() to conclude.   In diffcore_std() there is a call
to diffcore_skip_stat_unmatch() ONLY when skip_stat_unmatch member
is set (we initialize it to 1 when auto-refrresh-index is enabled,
as you saw above).  The function is used to squelch the paths that
remain in diff_queued_diff only because they were stat-dirty without
having an actual content change, and _counts_ how many such ghost
changes existed by incrementing the .skip_stat_unmatch counter.

> which dates to aecbf914c4 (git-diff: resurrect the traditional empty
> "diff --git" behaviour, 2007-08-31). On my system that comparison is
> false because the double-negation produces 1
> (diff_auto_refresh_index=1 or the result of git_config_bool). 

Not quite.  It was false because double-negation initializes the
member to 1, which causes a call to diffcore_skip_stat_unmatch()
be made, *and* the diffcore_skip_stat_unmatch() function did not
find any ghost changes, i.e., paths that were only stat-dirty hence
needed a call to refresh_index_quietly().

> So… has that conditional been quietly dead all this time? I can't
> imagine that's right, but…

I initially thought it was an embarrassing thinko, but after seeing
how .skip_stat_unmatch is used as a 1-based counter (i.e., if the
member says 42, it means it saw 41 paths that were stat-dirty but
without actual content change), I do not think so.

Now, it is a different matter if such a "dual" purpose "more than a
simple boolean" counter is a good idea.  Apparently it confused both
of us in this case ;-).

Thanks.

^ permalink raw reply

* Re: [PATCH] meson: wire up USE_NSEC build knob
From: Junio C Hamano @ 2026-06-21  1:01 UTC (permalink / raw)
  To: D. Ben Knoble
  Cc: git, brian m . carlson, Jeff King, Patrick Steinhardt,
	Ramsay Jones
In-Reply-To: <c4c5ade901ff95b0f95939ea818870e4f3d59da1.1781971201.git.ben.knoble+github@gmail.com>

"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:

> Autotools-style builds permit enabling USE_NSEC for cases where that's
> desired; the equivalent knob is missing from meson-based builds.

With or without autoconf, Makefile based build can use USE_NSEC.  It
is a welcome addition to the other side of thw world.  I do not know
if 'meson setup -Dnanosec=true' is a name that is easy to discover,
though.

Will queue.  Thanks.

^ permalink raw reply

* Re: [PATCH v3 0/2] completion: hide dotfiles for selected path completion
From: Junio C Hamano @ 2026-06-21  1:17 UTC (permalink / raw)
  To: Zakariyah Ali via GitGitGadget; +Cc: git, Zakariyah Ali
In-Reply-To: <pull.2311.v3.git.git.1781978156.gitgitgadget@gmail.com>

"Zakariyah Ali via GitGitGadget" <gitgitgadget@gmail.com> writes:

> The completion helper for index paths uses git ls-files rather than shell
> filename completion. As a result, leading-dot paths such as a tracked
> .gitignore were offered even when the user had not started the path with ..
>
> Hide leading-dot path components for git rm, git mv, and git ls-files when
> completing an empty path component. Explicit dot completion is still
> preserved, so git rm . can still complete .gitignore.
>
> This removes the existing TODO expectations in t/t9902-completion.sh and
> adds coverage for explicit dot completion.

OK.

> Validation:
>
>  * git diff --check -- contrib/completion/git-completion.bash
>    t/t9902-completion.sh
>  * bash -n contrib/completion/git-completion.bash
>  * ./t9902-completion.sh

I am not sure what you wanted to say with these lines.  If you did
the above to build confidence that your patch works, that would be
great.  Or are you telling readers to do these things and when they
do not see any issues consider your patch perfect?

What is missing around here in this cover letter is a description of
how this iteration is different from the previous one.  And ...

> Zakariyah Ali (2):
>   completion: hide dotfiles for selected path completion
>   completion: hide dotfiles by default for path completion
>
>  contrib/completion/git-completion.bash | 53 +++++++++++++++-----------
>  t/t9902-completion.sh                  | 19 ++++-----
>  2 files changed, 40 insertions(+), 32 deletions(-)
>
>
> base-commit: 9b7fa37559a1b95ee32e32858b0d038b4cf583e5
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2311%2Falibaba0010%2Fcompletion-hide-dotfiles-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2311/alibaba0010/completion-hide-dotfiles-v3
> Pull-Request: https://github.com/git/git/pull/2311
>
> Range-diff vs v2:
>
>  1:  056e239e06 = 1:  056e239e06 completion: hide dotfiles for selected path completion
>  -:  ---------- > 2:  7482ee4645 completion: hide dotfiles by default for path completion

... I find this range diff very troubling.  If we look at patch 2,
it seems that it redoes some part of what is done in patch 1 saying
"oops that was wrong, so let's do it better this time".  Such a
drunken-mans' walk that goes in one direction in an earlier step,
only to be corrected to move to a different course, is now how we
want a new topic to be presented.

The end result may be much easier to read, mostly thanks to updated
loop in the awk script, so if we really want to pretend this as two
patches for "small pieces are easier to digest" value, perhaps have
[PATCH 1/2] that updates the awk script (without doing anything
related to hide-dotfiles theme) to make it easier to read by not
having multiple "print pfx p" in it, and then build on top of that
improved base, have [PATCH 2/2] that adds the support to hide
dotfiles, perhaps?

Since the initial iteration was quite a while ago, I no longer
remember the details of the review I gave, but I recall having hard
time telling which callers of the complete-index-file helper hide
dotfiles from their output and which callers do not hide them, and
how the patch decided to choose which ones should and should not
hide.  Has it been improved and if so how?  That is something we
expect the cover letter to tell, too.

Thanks.

^ permalink raw reply

* Re: [PATCH v6 2/3] revision: add peek functions for lookahead
From: Junio C Hamano @ 2026-06-21  1:48 UTC (permalink / raw)
  To: Pablo Sabater
  Cc: git, krka, ayu.chandekar, chandrapratap3519, christian.couder,
	jltobler, karthik.188, peff, phillip.wood, siddharthasthana31,
	Kristofer Karlsson
In-Reply-To: <20260620-ps-pre-commit-indent-v6-2-cdc6d8fd5fbc@gmail.com>

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> +int revision_has_commits_after (struct rev_info *revs, int n)
> +{
> +	struct topo_walk_info *info = revs->topo_walk_info;
> +
> +	if (info) {
> +		int visible = 0;
> +		for (size_t i = 0; i < info->topo_queue.nr && visible < n; i++) {
> +			struct commit *c = info->topo_queue.array[i].data;
> +			if (get_commit_action(revs, c) == commit_show)
> +				visible++;
> +		}
> +		return visible > n-1;

The loop needs to be rethought, perhaps with a better abstraction
than ".nr is the number of elements in the queue and we can walk
them over as a dense array", using prio_queue_for_each(), once this
topic meets the kk/prio-queue-get-put-fusion topic.

I see Kristofer is already on the CC: line.  Depending on the
done-ness of the topic, we may want to include the topic in the
updated base for this topic to resolve semantic conflicts early, or
the other way around (i.e., let this topic graduate first and then
rebuild the prio-queue topic on top of the updated 'master').

Thanks.


^ permalink raw reply

* Re: git-diff in a worktree is an order of magnitude slower?
From: Junio C Hamano @ 2026-06-21  3:58 UTC (permalink / raw)
  To: D. Ben Knoble; +Cc: Jeff King, Git
In-Reply-To: <xmqqa4sog1e9.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> I initially thought it was an embarrassing thinko, but after seeing
> how .skip_stat_unmatch is used as a 1-based counter (i.e., if the
> member says 42, it means it saw 41 paths that were stat-dirty but
> without actual content change), I do not think so.
>
> Now, it is a different matter if such a "dual" purpose "more than a
> simple boolean" counter is a good idea.  Apparently it confused both
> of us in this case ;-).

FWIW the patch was done as part of this discussion thread:

  https://lore.kernel.org/git/20070830063810.GD16312@mellanox.co.il/


^ permalink raw reply

* Re: [PATCH GSoC RFC v13 00/12] cat-file: add remote-object-info to batch-command
From: Chandra Pratap @ 2026-06-21  5:25 UTC (permalink / raw)
  To: Pablo Sabater
  Cc: gitster, peff, eric.peijian, chriscool, git, jltobler,
	karthik.188, toon
In-Reply-To: <20260619-ps-eric-work-rebase-v13-0-3d4c7315d2f8@gmail.com>

On Fri, 19 Jun 2026 at 20:26, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
>
> This path series is a continuation of Eric Ju's (eric.peijian@gmail.com) and

s/path/patch

> Calvin Wan's (calvinwan@google.com) patch series [1] and [2] respectively.
>
> Sometimes it is beneficial to retrieve information about an object without
> having to download it completely. The server logic for retrieving size has
> already been implemented and merged in "a2ba162cda (object-info: support for
> retrieving object info, 2021-04-20)"[3]. This patch series implement the client
> option for it.
>
> Eric's series adds the `remote-object-info` command to
> `cat-file --batch-command`. This command allows the client to make an
> object-info command request to a server that supports protocol v2.
>
> If the server uses protocol v2 but does not support the object-info capability,
> `cat-file --batch-command` will die.
>
> If a user attempts to use `remote-object-info` with protocol v1,
> `cat-file --batch-command` will die.
>
> Currently, only the size (%(objectsize)) is supported end to end in this
> implementation. The type (%(objecttype)) is known by the client's allow-list
> and request path but is not supported on the server side nor the response
> parsing. A follow up series will add full end-to-end support for %(objecttype).
>
> The default format for remote-object-info is set to %(objectname) %(objectsize).
> Once %(objecttype) is supported, the default format will be unified accordingly.
>
> If the batch command format includes unsupported fields such as %(objecttype),
> %(objectsize:disk), or %(deltabase), the command will return empty strings for
> each unsupported field.
>
> This series completes Eric's work mainly with the refactor of the validation
> of the placeholder with an allow-list that filters what the client asks with
> what the server is capable of provide following Jeff King's idea [4].
>
> I have a question for the design:
>
> 1. If the format includes unsupported fields such as %(objecttype) or
>    %(deltabase) it currently returns an empty string for each unsupported
>    field, this follows what for-each-ref does with known but inapplicable
>    atoms. However future placeholders that will be implemented: %(rest),
>    %(objectmode) can return empty strings. How should we differentiate
>    "unsupported" vs "no data".
>    Eric proposed to use a placeholder like "???" [5].
>    Should a placeholder be used?

Maybe it's best to fail cleanly if the user requests an unsupported atom?
I don't really like the placeholder idea though. If a placeholder like "???"
is introduced, any script/test parsing the output must add explicit logic
to check for literal question marks, and that sounds flaky. Not to
mention some atom's response may legitimately contain "???".

> 2. _tangent/not related with this series_
>    'a2ba162cda' is designed to only work with full OIDs, which is
>    inconsistent with local `info` that does support short OIDs and in
>    case of being ambiguous returns a list of what possibly the user meant.
>
>    Because V2 protocol is thought to be stateless supporting short OIDs
>    could become more inconsistent with other remote commands that do not
>    support short OIDs. Maybe a --pick-first option? That does accept
>    short oids and picks the first match.

We might return the wrong object's info if we do this. With the server
giving us no information to verify whether the returned value _really_
corresponds to our intended object, I'd say this isn't the right choice.

>    Alternatively, would sending a list of possible OIDs to the client so
>    it can re-request with the correct one be ok?

As far as I know, disambiguation like this is treated purely as a local
UI convenience in Git, never a network-level operation.

`fetch` already requires users to input exact, full OIDs for their `want`
lines (obtained via a prior `ls-remote` or ref advertisement), and dies
if one isn't provided. Thus, I think erroring out is fine here.

^ permalink raw reply

* Re: [PATCH GSoC RFC v13 05/12] fetch-pack: move function to connect.c
From: Chandra Pratap @ 2026-06-21  5:37 UTC (permalink / raw)
  To: Pablo Sabater
  Cc: gitster, peff, eric.peijian, chriscool, git, jltobler,
	karthik.188, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <20260619-ps-eric-work-rebase-v13-5-3d4c7315d2f8@gmail.com>

On Fri, 19 Jun 2026 at 20:26, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
>
> write_fetch_command_and_capabilities will be refactored in a subsequent

Nit: the rest of this patch's body referes to this function as:
`write_fetch_command_and_capabilities()`

Let's use that here as well.

> commit where it will become a more general-purpose function, making it
> more accessible to additional commands in the future.
>
> To move `write_fetch_command_and_capabilities()` to `connect.c`, we need
> to adjust how `advertise_sid` is managed. Previously in `fetch_pack.c`,
> `advertise_sid` was a static variable, modified using
> `repo_config_get_bool()`.
>
> In `connect.c`, we now initialize `advertise_sid` at the begining by
> directly using `repo_config_get_bool()`. This change is safe because:
>
> In the original `fetch-pack.c` code, there are only two places that write
> `advertise_sid`:
>
> 1. In function `do_fetch_pack()`:
>         if (!sever_supports("session_id"))

s/sever/server

>                advertise_sid = 0;
> 2. In function `fetch_pack_config()`:
>         repo_config_get_bool("transfer.advertisesid", &advertise_sid);
>
> About 1, since `do_fetch_pack()` is only relevant for protocol v1, this
> assignment can be ignored, as `write_fetch_command_and_capabilities()`
> is only used in v2.
>
> About 2, `repo_config_get_bool()` is from `config.h` and it's an out-of-box
> dependency of `connect.c`, so we can reuse it directly.
>
> Move `write_fetch_command_and_capabilities()` to `connect.c`

Nit: this is a better patch header than "move function to connect.c",
since it better describes the exact change we intend to make.

Let's use it instead.

^ permalink raw reply

* [PATCH v4 0/4] history: add squash subcommand to fold a range
From: Harald Nordgren via GitGitGadget @ 2026-06-21  5:53 UTC (permalink / raw)
  To: git; +Cc: Harald Nordgren
In-Reply-To: <pull.2337.v3.git.git.1781810226.gitgitgadget@gmail.com>

Adds git history squash <revision-range> to fold a range of commits.

Changes in v4:

 * git history squash now detects when another ref points at a commit inside
   the range being folded and refuses, with an advice.historyUpdateRefs hint
   to use --update-refs=head.
 * A merge inside the range is folded fine as long as the range has a single
   base; a range with merge commit at the tip or base also folds correctly.
   Only a range with more than one base is rejected.

Changes in v3:

 * Moved the feature out of git rebase and into a new git history squash
   <revision-range> subcommand, per the list discussion. git rebase --squash
   is dropped.
 * Takes an arbitrary range (git history squash @~3.., git history squash
   @~5..@~2), folding it into the oldest commit and replaying any
   descendants on top.
 * Implemented as a single tree operation rather than picking each commit,
   so there are no repeated conflict stops (addresses Phillip's efficiency
   point).
 * A merge inside the range is folded fine, only a range with more than one
   base is rejected.
 * --reedit-message seeds the editor with every folded-in message, not just
   the oldest.

Harald Nordgren (4):
  history: extract helper for a commit's parent tree
  history: give commit_tree_ext a message template
  history: add squash subcommand to fold a range
  history: re-edit a squash with every message

 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  26 +++
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 328 +++++++++++++++++++++++++----
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 340 +++++++++++++++++++++++++++++++
 7 files changed, 663 insertions(+), 38 deletions(-)
 create mode 100755 t/t3455-history-squash.sh


base-commit: 8d96f09e9245ddf80c1981476fcbac8c4bb4125f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2337%2FHaraldNordgren%2Frebase-fixup-fold-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2337/HaraldNordgren/rebase-fixup-fold-v4
Pull-Request: https://github.com/git/git/pull/2337

Range-diff vs v3:

 1:  1e31474ef6 = 1:  fc2801c0b1 history: extract helper for a commit's parent tree
 2:  498da64046 = 2:  ee591e83b4 history: give commit_tree_ext a message template
 3:  66b2f49fb4 ! 3:  80bfea642e history: add squash subcommand to fold a range
     @@ Commit message
          other commit, but the range must have a single base, so a range with
          more than one entry point is rejected.
      
     +    The folded commits leave the history, so by default the command refuses
     +    when another ref points at one of them. Use "--update-refs=head" to
     +    rewrite only the current branch and leave those refs untouched.
     +
          Inspired-by: Sergey Chernov <serega.morph@gmail.com>
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     + ## Documentation/config/advice.adoc ##
     +@@ Documentation/config/advice.adoc: all advice messages.
     + 	forceDeleteBranch::
     + 		Shown when the user tries to delete a not fully merged
     + 		branch without the force option set.
     ++	historyUpdateRefs::
     ++		Shown when `git history squash` refuses because a ref points
     ++		into the range being folded, to tell the user about
     ++		`--update-refs=head`.
     + 	ignoredHook::
     + 		Shown when a hook is ignored because the hook is not
     + 		set as executable.
     +
       ## Documentation/git-history.adoc ##
      @@ Documentation/git-history.adoc: SYNOPSIS
       git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
     @@ Documentation/git-history.adoc: linkgit:gitglossary[7].
      +folded like any other, but the range must have a single base, so a range
      +that reaches more than one entry point (for example a side branch that
      +forked before the range and was later merged into it) is rejected.
     +++
     ++The folded commits disappear from the history, so with the default
     ++`--update-refs=branches` the command refuses when another ref points at
     ++one of them. Rerun with `--update-refs=head` to rewrite only the current
     ++branch and leave those refs pointing at the old commits.
      +
       OPTIONS
       -------
       
      
     + ## advice.c ##
     +@@ advice.c: static struct {
     + 	[ADVICE_FETCH_SHOW_FORCED_UPDATES]		= { "fetchShowForcedUpdates" },
     + 	[ADVICE_FORCE_DELETE_BRANCH]			= { "forceDeleteBranch" },
     + 	[ADVICE_GRAFT_FILE_DEPRECATED]			= { "graftFileDeprecated" },
     ++	[ADVICE_HISTORY_UPDATE_REFS]			= { "historyUpdateRefs" },
     + 	[ADVICE_IGNORED_HOOK]				= { "ignoredHook" },
     + 	[ADVICE_IMPLICIT_IDENTITY]			= { "implicitIdentity" },
     + 	[ADVICE_MERGE_CONFLICT]				= { "mergeConflict" },
     +
     + ## advice.h ##
     +@@ advice.h: enum advice_type {
     + 	ADVICE_FETCH_SHOW_FORCED_UPDATES,
     + 	ADVICE_FORCE_DELETE_BRANCH,
     + 	ADVICE_GRAFT_FILE_DEPRECATED,
     ++	ADVICE_HISTORY_UPDATE_REFS,
     + 	ADVICE_IGNORED_HOOK,
     + 	ADVICE_IMPLICIT_IDENTITY,
     + 	ADVICE_MERGE_CONFLICT,
     +
       ## builtin/history.c ##
     +@@
     + #define USE_THE_REPOSITORY_VARIABLE
     + 
     + #include "builtin.h"
     ++#include "advice.h"
     + #include "cache-tree.h"
     + #include "commit.h"
     + #include "commit-reach.h"
      @@
       	N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
       #define GIT_HISTORY_SPLIT_USAGE \
     @@ builtin/history.c: out:
      +				const char *range,
      +				struct commit **base_out,
      +				struct commit **oldest_out,
     -+				struct commit **tip_out)
     ++				struct commit **tip_out,
     ++				struct oidset *interior_out)
      +{
      +	struct rev_info revs;
      +	struct commit *commit, *base = NULL, *oldest = NULL, *tip = NULL;
     @@ builtin/history.c: out:
      +		}
      +		if (!oldest)
      +			oldest = commit;
     ++		if (tip)
     ++			oidset_insert(interior_out, &tip->object.oid);
      +		tip = commit;
      +	}
      +
     @@ builtin/history.c: out:
      +	return ret;
      +}
      +
     ++struct interior_ref_cb {
     ++	const struct oidset *interior;
     ++	const char *name;
     ++};
     ++
     ++static int find_interior_ref(const struct reference *ref, void *cb_data)
     ++{
     ++	struct interior_ref_cb *data = cb_data;
     ++
     ++	if (oidset_contains(data->interior, ref->oid)) {
     ++		data->name = xstrdup(ref->name);
     ++		return 1;
     ++	}
     ++
     ++	return 0;
     ++}
     ++
      +static int cmd_history_squash(int argc,
      +			      const char **argv,
      +			      const char *prefix,
     @@ builtin/history.c: out:
      +		OPT_END(),
      +	};
      +	struct strbuf reflog_msg = STRBUF_INIT;
     ++	struct oidset interior = OIDSET_INIT;
      +	struct commit *base, *oldest, *tip, *rewritten;
      +	const struct object_id *base_tree_oid, *tip_tree_oid;
      +	struct commit_list *parents = NULL;
     @@ builtin/history.c: out:
      +	if (action == REF_ACTION_DEFAULT)
      +		action = REF_ACTION_BRANCHES;
      +
     -+	ret = resolve_squash_range(repo, argv[0], &base, &oldest, &tip);
     ++	ret = resolve_squash_range(repo, argv[0], &base, &oldest, &tip,
     ++				   &interior);
      +	if (ret < 0)
      +		goto out;
      +
     ++	if (action == REF_ACTION_BRANCHES) {
     ++		struct interior_ref_cb cb = { .interior = &interior };
     ++
     ++		refs_for_each_ref(get_main_ref_store(repo),
     ++				  find_interior_ref, &cb);
     ++		if (cb.name) {
     ++			ret = error(_("'%s' points into the squashed range"),
     ++				    cb.name);
     ++			advise_if_enabled(ADVICE_HISTORY_UPDATE_REFS,
     ++					  _("Use --update-refs=head to rewrite only "
     ++					    "the current branch and leave such refs "
     ++					    "untouched."));
     ++			free((char *)cb.name);
     ++			goto out;
     ++		}
     ++	}
     ++
      +	ret = setup_revwalk(repo, action, tip, &revs);
      +	if (ret < 0)
      +		goto out;
     @@ builtin/history.c: out:
      +
      +out:
      +	strbuf_release(&reflog_msg);
     ++	oidset_clear(&interior);
      +	commit_list_free(parents);
      +	release_revisions(&revs);
      +	return ret;
     @@ t/meson.build: integration_tests = [
         't3451-history-reword.sh',
         't3452-history-split.sh',
         't3453-history-fixup.sh',
     -+  't3454-history-squash.sh',
     ++  't3455-history-squash.sh',
         't3500-cherry.sh',
         't3501-revert-cherry-pick.sh',
         't3502-cherry-pick-merge.sh',
      
     - ## t/t3454-history-squash.sh (new) ##
     + ## t/t3455-history-squash.sh (new) ##
      @@
      +#!/bin/sh
      +
     @@ t/t3454-history-squash.sh (new)
      +test_expect_success 'setup linear history touching two files' '
      +	test_commit base file a &&
      +	git tag start &&
     -+	test_commit one other x &&
     -+	test_commit two file c &&
     ++	test_commit --no-tag one other x &&
     ++	test_commit --no-tag two file c &&
      +	test_commit three file d
      +'
      +
     @@ t/t3454-history-squash.sh (new)
      +
      +test_expect_success 'reuses the message of a fixup! commit in the range' '
      +	git reset --hard start &&
     -+	test_commit reg1 file b &&
     ++	test_commit --no-tag reg1 file b &&
      +	git commit --allow-empty -m "fixup! reg1" &&
      +	test_commit reg2 file c &&
      +
     @@ t/t3454-history-squash.sh (new)
      +test_expect_success 'preserves authorship of the oldest commit' '
      +	git reset --hard start &&
      +	GIT_AUTHOR_NAME=Squasher GIT_AUTHOR_EMAIL=squash@example.com \
     -+		test_commit oldest file b &&
     ++		test_commit --no-tag oldest file b &&
      +	test_commit newest file c &&
      +
      +	git history squash start.. &&
     @@ t/t3454-history-squash.sh (new)
      +test_expect_success '--dry-run predicts the rewrite without performing it' '
      +	git reset --hard three &&
      +	head_before=$(git rev-parse HEAD) &&
     ++	tip_tree=$(git rev-parse HEAD^{tree}) &&
      +
      +	git history squash --dry-run start.. >out &&
     -+	grep "^update refs/heads/" out >update &&
     -+	predicted=$(awk "{print \$3}" update) &&
     ++	predicted=$(awk "/^update refs\/heads\// {print \$3}" out) &&
      +	test_cmp_rev "$head_before" HEAD &&
      +
      +	git history squash start.. &&
     -+	test "$predicted" = "$(git rev-parse HEAD)"
     ++	test "$predicted" = "$(git rev-parse HEAD)" &&
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	test_cmp_rev start HEAD^ &&
     ++	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
      +'
      +
      +test_expect_success '--update-refs=head only moves HEAD' '
     @@ t/t3454-history-squash.sh (new)
      +	test_cmp_rev "$other_before" other
      +'
      +
     -+test_expect_success '--update-refs=branches moves a branch pointing into the range' '
     ++test_expect_success 'refuses to fold a range a ref points into' '
     ++	git reset --hard three &&
     ++	git branch -f mid HEAD~1 &&
     ++	head_before=$(git rev-parse HEAD) &&
     ++
     ++	test_must_fail git history squash start.. 2>err &&
     ++	test_grep "error: .* points into the squashed range" err &&
     ++	test_grep "hint: .*--update-refs=head" err &&
     ++	test_cmp_rev "$head_before" HEAD &&
     ++
     ++	git branch -D mid
     ++'
     ++
     ++test_expect_success 'advice.historyUpdateRefs silences the hint' '
     ++	git reset --hard three &&
     ++	git branch -f mid HEAD~1 &&
     ++
     ++	test_must_fail git -c advice.historyUpdateRefs=false \
     ++		history squash start.. 2>err &&
     ++	test_grep "points into the squashed range" err &&
     ++	test_grep ! "hint:" err &&
     ++
     ++	git branch -D mid
     ++'
     ++
     ++test_expect_success '--update-refs=head folds past a ref pointing into the range' '
      +	git reset --hard three &&
     -+	git branch -f mid HEAD~2 &&
     ++	git branch -f mid HEAD~1 &&
      +	mid_before=$(git rev-parse mid) &&
      +
     -+	git history squash start..@~1 &&
     ++	git history squash --update-refs=head start.. &&
      +
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
      +	test_cmp_rev "$mid_before" mid &&
     -+	test_commit_message mid -m one
     ++
     ++	git branch -D mid
     ++'
     ++
     ++test_expect_success 'refuses to fold a range a tag points into' '
     ++	git reset --hard three &&
     ++	git tag -f mark HEAD~1 &&
     ++	head_before=$(git rev-parse HEAD) &&
     ++
     ++	test_must_fail git history squash start.. 2>err &&
     ++	test_grep "refs/tags/mark" err &&
     ++	test_grep "points into the squashed range" err &&
     ++	test_cmp_rev "$head_before" HEAD &&
     ++
     ++	git tag -d mark
      +'
      +
      +test_expect_success 'squashes a range whose internal merge has a single base' '
      +	git reset --hard start &&
     -+	test_commit before-side file b &&
     ++	test_commit --no-tag before-side file b &&
      +	git checkout -b inner-side &&
     -+	test_commit on-inner-side inner x &&
     ++	test_commit --no-tag on-inner-side inner x &&
      +	git checkout - &&
     -+	test_commit after-side file c &&
     ++	test_commit --no-tag after-side file c &&
      +	git merge --no-ff -m merge inner-side &&
     -+	test_commit after-merge file d &&
     ++	git branch -D inner-side &&
     ++	test_commit --no-tag after-merge file d &&
      +	tip_tree=$(git rev-parse HEAD^{tree}) &&
      +
      +	git history squash start.. &&
     @@ t/t3454-history-squash.sh (new)
      +	test_path_is_file inner
      +'
      +
     ++test_expect_success 'folds a range whose tip is a merge commit' '
     ++	git reset --hard start &&
     ++	test_commit --no-tag tipmerge-base file b &&
     ++	git checkout -b tipmerge-side &&
     ++	test_commit --no-tag tipmerge-side side x &&
     ++	git checkout - &&
     ++	test_commit --no-tag tipmerge-main file c &&
     ++	git merge --no-ff -m "merge tipmerge-side" tipmerge-side &&
     ++	git branch -D tipmerge-side &&
     ++	tip_tree=$(git rev-parse HEAD^{tree}) &&
     ++
     ++	git history squash start.. &&
     ++
     ++	git rev-list --count start..HEAD >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
     ++	test_path_is_file side
     ++'
     ++
     ++test_expect_success 'folds a range whose base is a merge commit' '
     ++	git reset --hard start &&
     ++	git checkout -b basemerge-side &&
     ++	test_commit --no-tag basemerge-side side x &&
     ++	git checkout - &&
     ++	test_commit --no-tag basemerge-main file b &&
     ++	git merge --no-ff -m "merge basemerge-side" basemerge-side &&
     ++	git branch -D basemerge-side &&
     ++	base=$(git rev-parse HEAD) &&
     ++	test_commit --no-tag basemerge-one file c &&
     ++	test_commit --no-tag basemerge-two file d &&
     ++	tip_tree=$(git rev-parse HEAD^{tree}) &&
     ++
     ++	git history squash "$base.." &&
     ++
     ++	git rev-list --count "$base..HEAD" >count &&
     ++	echo 1 >expect &&
     ++	test_cmp expect count &&
     ++	test_cmp_rev "$base" HEAD^ &&
     ++	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
     ++'
     ++
      +test_expect_success 'refuses to squash a range with more than one base' '
      +	git reset --hard start &&
      +	head_before=$(git rev-parse HEAD) &&
 4:  43e4270614 ! 4:  85c7817d7e history: re-edit a squash with every message
     @@ Documentation/git-history.adoc: history squash @~3..` folds the three most recen
       forked before the range and was later merged into it) is rejected.
      
       ## builtin/history.c ##
     -@@ builtin/history.c: out:
     - 	return ret;
     +@@ builtin/history.c: static int find_interior_ref(const struct reference *ref, void *cb_data)
     + 	return 0;
       }
       
      +static int build_squash_message(struct repository *repo,
     @@ builtin/history.c: static int cmd_history_squash(int argc,
       	};
       	struct strbuf reflog_msg = STRBUF_INIT;
      +	struct strbuf message = STRBUF_INIT;
     + 	struct oidset interior = OIDSET_INIT;
       	struct commit *base, *oldest, *tip, *rewritten;
       	const struct object_id *base_tree_oid, *tip_tree_oid;
     - 	struct commit_list *parents = NULL;
      @@ builtin/history.c: static int cmd_history_squash(int argc,
     - 	if (ret < 0)
     - 		goto out;
     + 		}
     + 	}
       
      +	if (flags & COMMIT_TREE_EDIT_MESSAGE) {
      +		ret = build_squash_message(repo, base, tip, &message);
     @@ builtin/history.c: static int cmd_history_squash(int argc,
       out:
       	strbuf_release(&reflog_msg);
      +	strbuf_release(&message);
     + 	oidset_clear(&interior);
       	commit_list_free(parents);
       	release_revisions(&revs);
     - 	return ret;
      
     - ## t/t3454-history-squash.sh ##
     -@@ t/t3454-history-squash.sh: test_expect_success 'preserves authorship of the oldest commit' '
     + ## t/t3455-history-squash.sh ##
     +@@ t/t3455-history-squash.sh: test_expect_success 'preserves authorship of the oldest commit' '
       	test_cmp expect actual
       '
       
     @@ t/t3454-history-squash.sh: test_expect_success 'preserves authorship of the olde
      +	echo b >file &&
      +	git add file &&
      +	git commit -m "re-one subject" -m "re-one body line" &&
     -+	test_commit re-two file c &&
     ++	test_commit --no-tag re-two file c &&
      +	test_commit re-three file d &&
      +
      +	write_script editor <<-\EOF &&
     @@ t/t3454-history-squash.sh: test_expect_success 'preserves authorship of the olde
      +	test_set_editor "$(pwd)/editor" &&
      +	git history squash --reedit-message start.. &&
      +
     -+	grep "re-one subject" buffer &&
     -+	grep "re-one body line" buffer &&
     -+	grep re-two buffer &&
     -+	grep re-three buffer &&
     ++	test_grep "re-one subject" buffer &&
     ++	test_grep "re-one body line" buffer &&
     ++	test_grep re-two buffer &&
     ++	test_grep re-three buffer &&
      +	git log --format="%s" -1 >actual &&
      +	echo combined >expect &&
      +	test_cmp expect actual

-- 
gitgitgadget

^ permalink raw reply

* [PATCH v4 1/4] history: extract helper for a commit's parent tree
From: Harald Nordgren via GitGitGadget @ 2026-06-21  5:53 UTC (permalink / raw)
  To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v4.git.git.1782021195.gitgitgadget@gmail.com>

From: Harald Nordgren <haraldnordgren@gmail.com>

Three places resolve the tree of a commit's first parent, falling back
to the empty tree for a root commit, each repeating the same parse and
oidcpy dance. Extract a first_parent_tree_oid() helper and route the
existing callers through it.

No change in behavior.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/history.c | 58 +++++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/builtin/history.c b/builtin/history.c
index 091465a59e..f95f26e684 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -157,6 +157,25 @@ out:
 	return ret;
 }
 
+static int first_parent_tree_oid(struct repository *repo,
+				 struct commit *commit,
+				 struct object_id *out)
+{
+	struct commit *parent = commit->parents ? commit->parents->item : NULL;
+
+	if (!parent) {
+		oidcpy(out, repo->hash_algo->empty_tree);
+		return 0;
+	}
+
+	if (repo_parse_commit(repo, parent))
+		return error(_("unable to parse parent commit %s"),
+			     oid_to_hex(&parent->object.oid));
+
+	oidcpy(out, &repo_get_commit_tree(repo, parent)->object.oid);
+	return 0;
+}
+
 static int commit_tree_with_edited_message(struct repository *repo,
 					   const char *action,
 					   struct commit *original,
@@ -164,21 +183,11 @@ static int commit_tree_with_edited_message(struct repository *repo,
 {
 	struct object_id parent_tree_oid;
 	const struct object_id *tree_oid;
-	struct commit *parent;
 
 	tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
 
-	parent = original->parents ? original->parents->item : NULL;
-	if (parent) {
-		if (repo_parse_commit(repo, parent)) {
-			return error(_("unable to parse parent commit %s"),
-				     oid_to_hex(&parent->object.oid));
-		}
-
-		parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid;
-	} else {
-		oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
-	}
+	if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
+		return -1;
 
 	return commit_tree_ext(repo, action, original, original->parents,
 			       &parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
@@ -444,18 +453,10 @@ static int commit_became_empty(struct repository *repo,
 			       struct commit *original,
 			       struct tree *result)
 {
-	struct commit *parent = original->parents ? original->parents->item : NULL;
 	struct object_id parent_tree_oid;
 
-	if (parent) {
-		if (repo_parse_commit(repo, parent))
-			return error(_("unable to parse parent of %s"),
-				     oid_to_hex(&original->object.oid));
-
-		parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid;
-	} else {
-		oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
-	}
+	if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
+		return -1;
 
 	return oideq(&result->object.oid, &parent_tree_oid);
 }
@@ -799,16 +800,9 @@ static int split_commit(struct repository *repo,
 	struct tree *split_tree;
 	int ret;
 
-	if (original->parents) {
-		if (repo_parse_commit(repo, original->parents->item)) {
-			ret = error(_("unable to parse parent commit %s"),
-				    oid_to_hex(&original->parents->item->object.oid));
-			goto out;
-		}
-
-		parent_tree_oid = *get_commit_tree_oid(original->parents->item);
-	} else {
-		oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree);
+	if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0) {
+		ret = -1;
+		goto out;
 	}
 	original_commit_tree_oid = get_commit_tree_oid(original);
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v4 2/4] history: give commit_tree_ext a message template
From: Harald Nordgren via GitGitGadget @ 2026-06-21  5:53 UTC (permalink / raw)
  To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v4.git.git.1782021195.gitgitgadget@gmail.com>

From: Harald Nordgren <haraldnordgren@gmail.com>

commit_tree_ext() reuses the message of the commit it is handed. A
caller that folds several commits together wants to seed the message
from more than that single commit, so add an optional message_template
parameter. When NULL, the behavior is unchanged.

Pass NULL from the existing fixup and split callers.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/history.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/builtin/history.c b/builtin/history.c
index f95f26e684..305bde3102 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -101,6 +101,7 @@ enum commit_tree_flags {
 static int commit_tree_ext(struct repository *repo,
 			   const char *action,
 			   struct commit *commit_with_message,
+			   const char *message_template,
 			   const struct commit_list *parents,
 			   const struct object_id *old_tree,
 			   const struct object_id *new_tree,
@@ -130,13 +131,16 @@ static int commit_tree_ext(struct repository *repo,
 		original_author = xmemdupz(ptr, len);
 	find_commit_subject(original_message, &original_body);
 
+	if (!message_template)
+		message_template = original_body;
+
 	if (flags & COMMIT_TREE_EDIT_MESSAGE) {
 		ret = fill_commit_message(repo, old_tree, new_tree,
-					  original_body, action, &commit_message);
+					  message_template, action, &commit_message);
 		if (ret < 0)
 			goto out;
 	} else {
-		strbuf_addstr(&commit_message, original_body);
+		strbuf_addstr(&commit_message, message_template);
 	}
 
 	original_extra_headers = read_commit_extra_headers(commit_with_message,
@@ -189,7 +193,7 @@ static int commit_tree_with_edited_message(struct repository *repo,
 	if (first_parent_tree_oid(repo, original, &parent_tree_oid) < 0)
 		return -1;
 
-	return commit_tree_ext(repo, action, original, original->parents,
+	return commit_tree_ext(repo, action, original, NULL, original->parents,
 			       &parent_tree_oid, tree_oid, out, COMMIT_TREE_EDIT_MESSAGE);
 }
 
@@ -644,7 +648,7 @@ static int cmd_history_fixup(int argc,
 		goto out;
 
 	if (!skip_commit) {
-		ret = commit_tree_ext(repo, "fixup", original, original->parents,
+		ret = commit_tree_ext(repo, "fixup", original, NULL, original->parents,
 				      &original_tree->object.oid, &merge_result.tree->object.oid,
 				      &rewritten, flags);
 		if (ret < 0) {
@@ -855,7 +859,7 @@ static int split_commit(struct repository *repo,
 	 * The first commit is constructed from the split-out tree. The base
 	 * that shall be diffed against is the parent of the original commit.
 	 */
-	ret = commit_tree_ext(repo, "split-out", original, original->parents, &parent_tree_oid,
+	ret = commit_tree_ext(repo, "split-out", original, NULL, original->parents, &parent_tree_oid,
 			      &split_tree->object.oid, &first_commit, COMMIT_TREE_EDIT_MESSAGE);
 	if (ret < 0) {
 		ret = error(_("failed writing first commit"));
@@ -872,7 +876,7 @@ static int split_commit(struct repository *repo,
 	old_tree_oid = &repo_get_commit_tree(repo, first_commit)->object.oid;
 	new_tree_oid = &repo_get_commit_tree(repo, original)->object.oid;
 
-	ret = commit_tree_ext(repo, "split-out", original, parents, old_tree_oid,
+	ret = commit_tree_ext(repo, "split-out", original, NULL, parents, old_tree_oid,
 			      new_tree_oid, &second_commit, COMMIT_TREE_EDIT_MESSAGE);
 	if (ret < 0) {
 		ret = error(_("failed writing second commit"));
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v4 3/4] history: add squash subcommand to fold a range
From: Harald Nordgren via GitGitGadget @ 2026-06-21  5:53 UTC (permalink / raw)
  To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v4.git.git.1782021195.gitgitgadget@gmail.com>

From: Harald Nordgren <haraldnordgren@gmail.com>

Folding a series of commits into one required either an interactive
rebase where each commit after the first was hand-edited to "fixup", or
a "git reset --soft" to the merge base followed by "git commit --amend".

Add "git history squash <revision-range>" to do this directly. It folds
every commit in the range into the oldest one, keeping that commit's
message and authorship and taking the tree of the newest commit, so the
range collapses into a single commit. Commits above the range are
replayed on top of the result.

The range is given as <base>..<tip>, so "git history squash @~3.."
folds the three most recent commits and "git history squash @~5..@~2"
squashes an interior range. A merge inside the range is folded like any
other commit, but the range must have a single base, so a range with
more than one entry point is rejected.

The folded commits leave the history, so by default the command refuses
when another ref points at one of them. Use "--update-refs=head" to
rewrite only the current branch and leave those refs untouched.

Inspired-by: Sergey Chernov <serega.morph@gmail.com>
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/config/advice.adoc |   4 +
 Documentation/git-history.adoc   |  25 +++
 advice.c                         |   1 +
 advice.h                         |   1 +
 builtin/history.c                | 195 ++++++++++++++++++++
 t/meson.build                    |   1 +
 t/t3455-history-squash.sh        | 303 +++++++++++++++++++++++++++++++
 7 files changed, 530 insertions(+)
 create mode 100755 t/t3455-history-squash.sh

diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db58918..f4d692d136 100644
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -55,6 +55,10 @@ all advice messages.
 	forceDeleteBranch::
 		Shown when the user tries to delete a not fully merged
 		branch without the force option set.
+	historyUpdateRefs::
+		Shown when `git history squash` refuses because a ref points
+		into the range being folded, to tell the user about
+		`--update-refs=head`.
 	ignoredHook::
 		Shown when a hook is ignored because the hook is not
 		set as executable.
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 2ba8121795..6716749cde 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -11,6 +11,7 @@ SYNOPSIS
 git history fixup <commit> [--dry-run] [--update-refs=(branches|head)] [--reedit-message] [--empty=(drop|keep|abort)]
 git history reword <commit> [--dry-run] [--update-refs=(branches|head)]
 git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]
+git history squash <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]
 
 DESCRIPTION
 -----------
@@ -97,6 +98,30 @@ linkgit:gitglossary[7].
 It is invalid to select either all or no hunks, as that would lead to
 one of the commits becoming empty.
 
+`squash <revision-range>`::
+	Fold all commits in _<revision-range>_ into the oldest commit of that
+	range. The resulting commit keeps the oldest commit's message and
+	authorship and takes the tree of the range's newest commit, so the
+	whole range collapses into a single commit. Commits above the range
+	are replayed on top of the result.
++
+The range is given in the usual `<base>..<tip>` form, where _<base>_ is
+the commit just below the oldest commit to squash. For example, `git
+history squash @~3..` folds the three most recent commits into one, and
+`git history squash @~5..@~2` squashes an interior range while leaving
+the two newest commits in place.
++
+The oldest commit's message and authorship are preserved by default,
+unless you specify `--reedit-message`. A merge commit inside the range is
+folded like any other, but the range must have a single base, so a range
+that reaches more than one entry point (for example a side branch that
+forked before the range and was later merged into it) is rejected.
++
+The folded commits disappear from the history, so with the default
+`--update-refs=branches` the command refuses when another ref points at
+one of them. Rerun with `--update-refs=head` to rewrite only the current
+branch and leave those refs pointing at the old commits.
+
 OPTIONS
 -------
 
diff --git a/advice.c b/advice.c
index 0018501b7b..5c6ff95e31 100644
--- a/advice.c
+++ b/advice.c
@@ -58,6 +58,7 @@ static struct {
 	[ADVICE_FETCH_SHOW_FORCED_UPDATES]		= { "fetchShowForcedUpdates" },
 	[ADVICE_FORCE_DELETE_BRANCH]			= { "forceDeleteBranch" },
 	[ADVICE_GRAFT_FILE_DEPRECATED]			= { "graftFileDeprecated" },
+	[ADVICE_HISTORY_UPDATE_REFS]			= { "historyUpdateRefs" },
 	[ADVICE_IGNORED_HOOK]				= { "ignoredHook" },
 	[ADVICE_IMPLICIT_IDENTITY]			= { "implicitIdentity" },
 	[ADVICE_MERGE_CONFLICT]				= { "mergeConflict" },
diff --git a/advice.h b/advice.h
index 8def280688..911b4e4643 100644
--- a/advice.h
+++ b/advice.h
@@ -25,6 +25,7 @@ enum advice_type {
 	ADVICE_FETCH_SHOW_FORCED_UPDATES,
 	ADVICE_FORCE_DELETE_BRANCH,
 	ADVICE_GRAFT_FILE_DEPRECATED,
+	ADVICE_HISTORY_UPDATE_REFS,
 	ADVICE_IGNORED_HOOK,
 	ADVICE_IMPLICIT_IDENTITY,
 	ADVICE_MERGE_CONFLICT,
diff --git a/builtin/history.c b/builtin/history.c
index 305bde3102..4f1baea56c 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1,6 +1,7 @@
 #define USE_THE_REPOSITORY_VARIABLE
 
 #include "builtin.h"
+#include "advice.h"
 #include "cache-tree.h"
 #include "commit.h"
 #include "commit-reach.h"
@@ -30,6 +31,8 @@
 	N_("git history reword <commit> [--dry-run] [--update-refs=(branches|head)]")
 #define GIT_HISTORY_SPLIT_USAGE \
 	N_("git history split <commit> [--dry-run] [--update-refs=(branches|head)] [--] [<pathspec>...]")
+#define GIT_HISTORY_SQUASH_USAGE \
+	N_("git history squash <revision-range> [--dry-run] [--update-refs=(branches|head)] [--reedit-message]")
 
 static void change_data_free(void *util, const char *str UNUSED)
 {
@@ -973,6 +976,196 @@ out:
 	return ret;
 }
 
+/*
+ * Resolve a "<base>..<tip>" revision range into the base commit just outside
+ * the range (which becomes the parent of the squashed commit), the oldest
+ * commit contained in the range (whose message the squash reuses), and the
+ * range tip (whose tree becomes the result). A merge inside the range is fine,
+ * but the range must have a single base and must not reach a root commit.
+ */
+static int resolve_squash_range(struct repository *repo,
+				const char *range,
+				struct commit **base_out,
+				struct commit **oldest_out,
+				struct commit **tip_out,
+				struct oidset *interior_out)
+{
+	struct rev_info revs;
+	struct commit *commit, *base = NULL, *oldest = NULL, *tip = NULL;
+	struct strvec args = STRVEC_INIT;
+	int ret;
+
+	repo_init_revisions(repo, &revs, NULL);
+	strvec_push(&args, "ignored");
+	strvec_push(&args, "--reverse");
+	strvec_push(&args, "--topo-order");
+	strvec_push(&args, "--boundary");
+	strvec_push(&args, range);
+	setup_revisions_from_strvec(&args, &revs, NULL);
+	if (args.nr != 1) {
+		ret = error(_("'%s' does not name a revision range"), range);
+		goto out;
+	}
+
+	if (prepare_revision_walk(&revs) < 0) {
+		ret = error(_("error preparing revisions"));
+		goto out;
+	}
+
+	while ((commit = get_revision(&revs))) {
+		if (commit->object.flags & BOUNDARY) {
+			if (base) {
+				ret = error(_("range '%s' has more than one base; "
+					      "cannot squash"), range);
+				goto out;
+			}
+			base = commit;
+			continue;
+		}
+		if (!oldest)
+			oldest = commit;
+		if (tip)
+			oidset_insert(interior_out, &tip->object.oid);
+		tip = commit;
+	}
+
+	if (!oldest) {
+		ret = error(_("the range '%s' is empty"), range);
+		goto out;
+	}
+
+	if (!base) {
+		ret = error(_("cannot squash the root commit"));
+		goto out;
+	}
+
+	*base_out = base;
+	*oldest_out = oldest;
+	*tip_out = tip;
+	ret = 0;
+
+out:
+	reset_revision_walk();
+	release_revisions(&revs);
+	strvec_clear(&args);
+	return ret;
+}
+
+struct interior_ref_cb {
+	const struct oidset *interior;
+	const char *name;
+};
+
+static int find_interior_ref(const struct reference *ref, void *cb_data)
+{
+	struct interior_ref_cb *data = cb_data;
+
+	if (oidset_contains(data->interior, ref->oid)) {
+		data->name = xstrdup(ref->name);
+		return 1;
+	}
+
+	return 0;
+}
+
+static int cmd_history_squash(int argc,
+			      const char **argv,
+			      const char *prefix,
+			      struct repository *repo)
+{
+	const char * const usage[] = {
+		GIT_HISTORY_SQUASH_USAGE,
+		NULL,
+	};
+	enum ref_action action = REF_ACTION_DEFAULT;
+	enum commit_tree_flags flags = 0;
+	int dry_run = 0;
+	struct option options[] = {
+		OPT_CALLBACK_F(0, "update-refs", &action, "(branches|head)",
+			       N_("control which refs should be updated"),
+			       PARSE_OPT_NONEG, parse_ref_action),
+		OPT_BOOL('n', "dry-run", &dry_run,
+			 N_("perform a dry-run without updating any refs")),
+		OPT_BIT(0, "reedit-message", &flags,
+			N_("open an editor to modify the commit message"),
+			COMMIT_TREE_EDIT_MESSAGE),
+		OPT_END(),
+	};
+	struct strbuf reflog_msg = STRBUF_INIT;
+	struct oidset interior = OIDSET_INIT;
+	struct commit *base, *oldest, *tip, *rewritten;
+	const struct object_id *base_tree_oid, *tip_tree_oid;
+	struct commit_list *parents = NULL;
+	struct rev_info revs = { 0 };
+	int ret;
+
+	argc = parse_options(argc, argv, prefix, options, usage, 0);
+	if (argc != 1) {
+		ret = error(_("command expects a single revision range"));
+		goto out;
+	}
+	repo_config(repo, git_default_config, NULL);
+
+	if (action == REF_ACTION_DEFAULT)
+		action = REF_ACTION_BRANCHES;
+
+	ret = resolve_squash_range(repo, argv[0], &base, &oldest, &tip,
+				   &interior);
+	if (ret < 0)
+		goto out;
+
+	if (action == REF_ACTION_BRANCHES) {
+		struct interior_ref_cb cb = { .interior = &interior };
+
+		refs_for_each_ref(get_main_ref_store(repo),
+				  find_interior_ref, &cb);
+		if (cb.name) {
+			ret = error(_("'%s' points into the squashed range"),
+				    cb.name);
+			advise_if_enabled(ADVICE_HISTORY_UPDATE_REFS,
+					  _("Use --update-refs=head to rewrite only "
+					    "the current branch and leave such refs "
+					    "untouched."));
+			free((char *)cb.name);
+			goto out;
+		}
+	}
+
+	ret = setup_revwalk(repo, action, tip, &revs);
+	if (ret < 0)
+		goto out;
+
+	base_tree_oid = &repo_get_commit_tree(repo, base)->object.oid;
+	tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
+	commit_list_append(base, &parents);
+
+	ret = commit_tree_ext(repo, "squash", oldest, NULL, parents,
+			      base_tree_oid, tip_tree_oid, &rewritten, flags);
+	if (ret < 0) {
+		ret = error(_("failed writing squashed commit"));
+		goto out;
+	}
+
+	strbuf_addf(&reflog_msg, "squash: updating %s", argv[0]);
+
+	ret = handle_reference_updates(&revs, action, tip, rewritten,
+				       reflog_msg.buf, dry_run,
+				       REPLAY_EMPTY_COMMIT_ABORT);
+	if (ret < 0) {
+		ret = error(_("failed replaying descendants"));
+		goto out;
+	}
+
+	ret = 0;
+
+out:
+	strbuf_release(&reflog_msg);
+	oidset_clear(&interior);
+	commit_list_free(parents);
+	release_revisions(&revs);
+	return ret;
+}
+
 int cmd_history(int argc,
 		const char **argv,
 		const char *prefix,
@@ -982,6 +1175,7 @@ int cmd_history(int argc,
 		GIT_HISTORY_FIXUP_USAGE,
 		GIT_HISTORY_REWORD_USAGE,
 		GIT_HISTORY_SPLIT_USAGE,
+		GIT_HISTORY_SQUASH_USAGE,
 		NULL,
 	};
 	parse_opt_subcommand_fn *fn = NULL;
@@ -989,6 +1183,7 @@ int cmd_history(int argc,
 		OPT_SUBCOMMAND("fixup", &fn, cmd_history_fixup),
 		OPT_SUBCOMMAND("reword", &fn, cmd_history_reword),
 		OPT_SUBCOMMAND("split", &fn, cmd_history_split),
+		OPT_SUBCOMMAND("squash", &fn, cmd_history_squash),
 		OPT_END(),
 	};
 
diff --git a/t/meson.build b/t/meson.build
index 3219264fe7..63ea26b8ed 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -399,6 +399,7 @@ integration_tests = [
   't3451-history-reword.sh',
   't3452-history-split.sh',
   't3453-history-fixup.sh',
+  't3455-history-squash.sh',
   't3500-cherry.sh',
   't3501-revert-cherry-pick.sh',
   't3502-cherry-pick-merge.sh',
diff --git a/t/t3455-history-squash.sh b/t/t3455-history-squash.sh
new file mode 100755
index 0000000000..821c801153
--- /dev/null
+++ b/t/t3455-history-squash.sh
@@ -0,0 +1,303 @@
+#!/bin/sh
+
+test_description='tests for git-history squash subcommand'
+
+. ./test-lib.sh
+
+test_expect_success 'setup linear history touching two files' '
+	test_commit base file a &&
+	git tag start &&
+	test_commit --no-tag one other x &&
+	test_commit --no-tag two file c &&
+	test_commit three file d
+'
+
+test_expect_success 'errors on missing range argument' '
+	test_must_fail git history squash 2>err &&
+	test_grep "command expects a single revision range" err
+'
+
+test_expect_success 'errors on too many arguments' '
+	test_must_fail git history squash start.. HEAD 2>err &&
+	test_grep "command expects a single revision range" err
+'
+
+test_expect_success 'errors on an empty range' '
+	test_must_fail git history squash HEAD..HEAD 2>err &&
+	test_grep "the range .* is empty" err
+'
+
+test_expect_success 'errors when the range includes the root commit' '
+	test_must_fail git history squash HEAD 2>err &&
+	test_grep "cannot squash the root commit" err
+'
+
+test_expect_success 'squashes a range into a single commit without changing the tree' '
+	git reset --hard three &&
+	tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+	git history squash start.. &&
+
+	git rev-list --count start..HEAD >count &&
+	echo 1 >expect &&
+	test_cmp expect count &&
+	test_cmp_rev start HEAD^ &&
+	test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+	git log --format="%s" -1 >subject &&
+	echo one >expect &&
+	test_cmp expect subject &&
+	git reflog >reflog &&
+	test_grep "squash: updating" reflog
+'
+
+test_expect_success 'squashes an interior range and replays descendants verbatim' '
+	git reset --hard three &&
+	final_tree=$(git rev-parse HEAD^{tree}) &&
+
+	git history squash start..@~1 &&
+
+	git log --format="%s" start..HEAD >actual &&
+	cat >expect <<-\EOF &&
+	three
+	one
+	EOF
+	test_cmp expect actual &&
+
+	test_cmp_rev start HEAD~2 &&
+	test "$final_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'squashes when the base is the root commit' '
+	git reset --hard three &&
+	root=$(git rev-list --max-parents=0 HEAD) &&
+	tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+	git history squash "$root.." &&
+
+	git rev-list --count "$root..HEAD" >count &&
+	echo 1 >expect &&
+	test_cmp expect count &&
+	test_cmp_rev "$root" HEAD^ &&
+	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'squashing a single-commit range replays the rest' '
+	git reset --hard three &&
+	tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+	git history squash start..@~2 &&
+
+	git log --format="%s" start..HEAD >actual &&
+	cat >expect <<-\EOF &&
+	three
+	two
+	one
+	EOF
+	test_cmp expect actual &&
+	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'reuses the message of a fixup! commit in the range' '
+	git reset --hard start &&
+	test_commit --no-tag reg1 file b &&
+	git commit --allow-empty -m "fixup! reg1" &&
+	test_commit reg2 file c &&
+
+	git history squash start.. &&
+
+	git log --format="%s" -1 >actual &&
+	echo reg1 >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'keeps the oldest message even if it is a fixup!' '
+	git reset --hard start &&
+	test_commit --no-tag "fixup! something" file b &&
+	test_commit tail file c &&
+
+	git history squash start.. &&
+
+	git log --format="%s" -1 >actual &&
+	echo "fixup! something" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'preserves authorship of the oldest commit' '
+	git reset --hard start &&
+	GIT_AUTHOR_NAME=Squasher GIT_AUTHOR_EMAIL=squash@example.com \
+		test_commit --no-tag oldest file b &&
+	test_commit newest file c &&
+
+	git history squash start.. &&
+
+	git log -1 --format="%an <%ae>" >actual &&
+	echo "Squasher <squash@example.com>" >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success '--dry-run predicts the rewrite without performing it' '
+	git reset --hard three &&
+	head_before=$(git rev-parse HEAD) &&
+	tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+	git history squash --dry-run start.. >out &&
+	predicted=$(awk "/^update refs\/heads\// {print \$3}" out) &&
+	test_cmp_rev "$head_before" HEAD &&
+
+	git history squash start.. &&
+	test "$predicted" = "$(git rev-parse HEAD)" &&
+	git rev-list --count start..HEAD >count &&
+	echo 1 >expect &&
+	test_cmp expect count &&
+	test_cmp_rev start HEAD^ &&
+	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success '--update-refs=head only moves HEAD' '
+	git reset --hard three &&
+	git branch -f other HEAD &&
+	other_before=$(git rev-parse other) &&
+
+	git history squash --update-refs=head start.. &&
+
+	git rev-list --count start..HEAD >count &&
+	echo 1 >expect &&
+	test_cmp expect count &&
+	test_cmp_rev "$other_before" other
+'
+
+test_expect_success 'refuses to fold a range a ref points into' '
+	git reset --hard three &&
+	git branch -f mid HEAD~1 &&
+	head_before=$(git rev-parse HEAD) &&
+
+	test_must_fail git history squash start.. 2>err &&
+	test_grep "error: .* points into the squashed range" err &&
+	test_grep "hint: .*--update-refs=head" err &&
+	test_cmp_rev "$head_before" HEAD &&
+
+	git branch -D mid
+'
+
+test_expect_success 'advice.historyUpdateRefs silences the hint' '
+	git reset --hard three &&
+	git branch -f mid HEAD~1 &&
+
+	test_must_fail git -c advice.historyUpdateRefs=false \
+		history squash start.. 2>err &&
+	test_grep "points into the squashed range" err &&
+	test_grep ! "hint:" err &&
+
+	git branch -D mid
+'
+
+test_expect_success '--update-refs=head folds past a ref pointing into the range' '
+	git reset --hard three &&
+	git branch -f mid HEAD~1 &&
+	mid_before=$(git rev-parse mid) &&
+
+	git history squash --update-refs=head start.. &&
+
+	git rev-list --count start..HEAD >count &&
+	echo 1 >expect &&
+	test_cmp expect count &&
+	test_cmp_rev "$mid_before" mid &&
+
+	git branch -D mid
+'
+
+test_expect_success 'refuses to fold a range a tag points into' '
+	git reset --hard three &&
+	git tag -f mark HEAD~1 &&
+	head_before=$(git rev-parse HEAD) &&
+
+	test_must_fail git history squash start.. 2>err &&
+	test_grep "refs/tags/mark" err &&
+	test_grep "points into the squashed range" err &&
+	test_cmp_rev "$head_before" HEAD &&
+
+	git tag -d mark
+'
+
+test_expect_success 'squashes a range whose internal merge has a single base' '
+	git reset --hard start &&
+	test_commit --no-tag before-side file b &&
+	git checkout -b inner-side &&
+	test_commit --no-tag on-inner-side inner x &&
+	git checkout - &&
+	test_commit --no-tag after-side file c &&
+	git merge --no-ff -m merge inner-side &&
+	git branch -D inner-side &&
+	test_commit --no-tag after-merge file d &&
+	tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+	git history squash start.. &&
+
+	git rev-list --count start..HEAD >count &&
+	echo 1 >expect &&
+	test_cmp expect count &&
+	git log --format="%s" -1 >subject &&
+	echo before-side >expect &&
+	test_cmp expect subject &&
+	test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+	test_path_is_file inner
+'
+
+test_expect_success 'folds a range whose tip is a merge commit' '
+	git reset --hard start &&
+	test_commit --no-tag tipmerge-base file b &&
+	git checkout -b tipmerge-side &&
+	test_commit --no-tag tipmerge-side side x &&
+	git checkout - &&
+	test_commit --no-tag tipmerge-main file c &&
+	git merge --no-ff -m "merge tipmerge-side" tipmerge-side &&
+	git branch -D tipmerge-side &&
+	tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+	git history squash start.. &&
+
+	git rev-list --count start..HEAD >count &&
+	echo 1 >expect &&
+	test_cmp expect count &&
+	test "$tip_tree" = "$(git rev-parse HEAD^{tree})" &&
+	test_path_is_file side
+'
+
+test_expect_success 'folds a range whose base is a merge commit' '
+	git reset --hard start &&
+	git checkout -b basemerge-side &&
+	test_commit --no-tag basemerge-side side x &&
+	git checkout - &&
+	test_commit --no-tag basemerge-main file b &&
+	git merge --no-ff -m "merge basemerge-side" basemerge-side &&
+	git branch -D basemerge-side &&
+	base=$(git rev-parse HEAD) &&
+	test_commit --no-tag basemerge-one file c &&
+	test_commit --no-tag basemerge-two file d &&
+	tip_tree=$(git rev-parse HEAD^{tree}) &&
+
+	git history squash "$base.." &&
+
+	git rev-list --count "$base..HEAD" >count &&
+	echo 1 >expect &&
+	test_cmp expect count &&
+	test_cmp_rev "$base" HEAD^ &&
+	test "$tip_tree" = "$(git rev-parse HEAD^{tree})"
+'
+
+test_expect_success 'refuses to squash a range with more than one base' '
+	git reset --hard start &&
+	head_before=$(git rev-parse HEAD) &&
+	git checkout -b forked-before &&
+	test_commit forked-side fside x &&
+	git checkout - &&
+	test_commit forked-main file b &&
+	git merge --no-ff -m merge forked-before &&
+	merged=$(git rev-parse HEAD) &&
+
+	test_must_fail git history squash forked-main.. 2>err &&
+	test_grep "more than one base" err &&
+	test_cmp_rev "$merged" HEAD
+'
+
+test_done
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v4 4/4] history: re-edit a squash with every message
From: Harald Nordgren via GitGitGadget @ 2026-06-21  5:53 UTC (permalink / raw)
  To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v4.git.git.1782021195.gitgitgadget@gmail.com>

From: Harald Nordgren <haraldnordgren@gmail.com>

By default "git history squash" reuses the oldest commit's message.
When --reedit-message is given it only reopened that one message, so the
messages of the folded-in commits were lost.

Gather the messages of every commit in the range, oldest first, and use
them as the editor template when re-editing, mirroring how "git rebase
-i" presents a squash. The combined message is built before the
descendant walk so it is not disturbed by the flags that walk leaves on
the commits.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-history.adoc |  5 +--
 builtin/history.c              | 61 +++++++++++++++++++++++++++++++++-
 t/t3455-history-squash.sh      | 37 +++++++++++++++++++++
 3 files changed, 100 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 6716749cde..df389015aa 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -111,8 +111,9 @@ history squash @~3..` folds the three most recent commits into one, and
 `git history squash @~5..@~2` squashes an interior range while leaving
 the two newest commits in place.
 +
-The oldest commit's message and authorship are preserved by default,
-unless you specify `--reedit-message`. A merge commit inside the range is
+The oldest commit's message and authorship are preserved by default. With
+`--reedit-message`, an editor opens pre-filled with the messages of all the
+folded commits so you can combine them. A merge commit inside the range is
 folded like any other, but the range must have a single base, so a range
 that reaches more than one entry point (for example a side branch that
 forked before the range and was later merged into it) is rejected.
diff --git a/builtin/history.c b/builtin/history.c
index 4f1baea56c..ff3bc9f945 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1068,6 +1068,56 @@ static int find_interior_ref(const struct reference *ref, void *cb_data)
 	return 0;
 }
 
+static int build_squash_message(struct repository *repo,
+				struct commit *base,
+				struct commit *tip,
+				struct strbuf *out)
+{
+	struct rev_info revs;
+	struct commit *commit;
+	struct strvec args = STRVEC_INIT;
+	int n = 0, ret;
+
+	repo_init_revisions(repo, &revs, NULL);
+	strvec_push(&args, "ignored");
+	strvec_push(&args, "--reverse");
+	strvec_push(&args, "--topo-order");
+	strvec_pushf(&args, "%s..%s", oid_to_hex(&base->object.oid),
+		     oid_to_hex(&tip->object.oid));
+	setup_revisions_from_strvec(&args, &revs, NULL);
+
+	if (prepare_revision_walk(&revs) < 0) {
+		ret = error(_("error preparing revisions"));
+		goto out;
+	}
+
+	while ((commit = get_revision(&revs))) {
+		const char *message, *body;
+		struct strbuf one = STRBUF_INIT;
+
+		message = repo_logmsg_reencode(repo, commit, NULL, NULL);
+		find_commit_subject(message, &body);
+		strbuf_addstr(&one, body);
+		strbuf_trim_trailing_newline(&one);
+
+		if (n++)
+			strbuf_addch(out, '\n');
+		strbuf_addbuf(out, &one);
+		strbuf_addch(out, '\n');
+
+		strbuf_release(&one);
+		repo_unuse_commit_buffer(repo, commit, message);
+	}
+
+	ret = 0;
+
+out:
+	reset_revision_walk();
+	release_revisions(&revs);
+	strvec_clear(&args);
+	return ret;
+}
+
 static int cmd_history_squash(int argc,
 			      const char **argv,
 			      const char *prefix,
@@ -1092,6 +1142,7 @@ static int cmd_history_squash(int argc,
 		OPT_END(),
 	};
 	struct strbuf reflog_msg = STRBUF_INIT;
+	struct strbuf message = STRBUF_INIT;
 	struct oidset interior = OIDSET_INIT;
 	struct commit *base, *oldest, *tip, *rewritten;
 	const struct object_id *base_tree_oid, *tip_tree_oid;
@@ -1131,6 +1182,12 @@ static int cmd_history_squash(int argc,
 		}
 	}
 
+	if (flags & COMMIT_TREE_EDIT_MESSAGE) {
+		ret = build_squash_message(repo, base, tip, &message);
+		if (ret < 0)
+			goto out;
+	}
+
 	ret = setup_revwalk(repo, action, tip, &revs);
 	if (ret < 0)
 		goto out;
@@ -1139,7 +1196,8 @@ static int cmd_history_squash(int argc,
 	tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
 	commit_list_append(base, &parents);
 
-	ret = commit_tree_ext(repo, "squash", oldest, NULL, parents,
+	ret = commit_tree_ext(repo, "squash", oldest,
+			      message.len ? message.buf : NULL, parents,
 			      base_tree_oid, tip_tree_oid, &rewritten, flags);
 	if (ret < 0) {
 		ret = error(_("failed writing squashed commit"));
@@ -1160,6 +1218,7 @@ static int cmd_history_squash(int argc,
 
 out:
 	strbuf_release(&reflog_msg);
+	strbuf_release(&message);
 	oidset_clear(&interior);
 	commit_list_free(parents);
 	release_revisions(&revs);
diff --git a/t/t3455-history-squash.sh b/t/t3455-history-squash.sh
index 821c801153..1fb3b9b63e 100755
--- a/t/t3455-history-squash.sh
+++ b/t/t3455-history-squash.sh
@@ -135,6 +135,43 @@ test_expect_success 'preserves authorship of the oldest commit' '
 	test_cmp expect actual
 '
 
+test_expect_success '--reedit-message offers every folded-in message' '
+	git reset --hard start &&
+	echo b >file &&
+	git add file &&
+	git commit -m "re-one subject" -m "re-one body line" &&
+	test_commit --no-tag re-two file c &&
+	test_commit re-three file d &&
+
+	write_script editor <<-\EOF &&
+	cp "$1" buffer &&
+	echo combined >"$1"
+	EOF
+	test_set_editor "$(pwd)/editor" &&
+	git history squash --reedit-message start.. &&
+
+	test_grep "re-one subject" buffer &&
+	test_grep "re-one body line" buffer &&
+	test_grep re-two buffer &&
+	test_grep re-three buffer &&
+	git log --format="%s" -1 >actual &&
+	echo combined >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success '--reedit-message aborts on an empty message' '
+	git reset --hard three &&
+	head_before=$(git rev-parse HEAD) &&
+
+	write_script editor <<-\EOF &&
+	>"$1"
+	EOF
+	test_set_editor "$(pwd)/editor" &&
+	test_must_fail git history squash --reedit-message start.. &&
+
+	test_cmp_rev "$head_before" HEAD
+'
+
 test_expect_success '--dry-run predicts the rewrite without performing it' '
 	git reset --hard three &&
 	head_before=$(git rev-parse HEAD) &&
-- 
gitgitgadget

^ permalink raw reply related

* [GSoC Patch v7 0/3] teach git repo info to handle path keys
From: K Jayatheerth @ 2026-06-21  5:55 UTC (permalink / raw)
  To: jayatheerthkulkarni2005
  Cc: a3205153416, git, gitster, jltobler, kumarayushjha123,
	lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260601151950.30686-1-jayatheerthkulkarni2005@gmail.com>

Hi!

This series teaches `git repo info` to handle `path.*`
keys, allowing scripts to reliably discover core
repository paths without resorting to `git rev-parse`.

The patches are structured as follows:

1. path: Extract the localized path-formatting logic
   out of `rev-parse` and expose it globally via
   `path.h` using clear append semantics.

2. repo: Introduce `path.commondir.absolute` and
   `path.commondir.relative` alongside a robust,
   isolated test helper.

3. repo: Introduce `path.gitdir.absolute` and
   `path.gitdir.relative` using the same standardized
   formatting rules.

Changes since v6:

Squashed patches 1 and 2 to avoid dead code in the tree.

Tagging Justin Tobler, Lucas Seiki Oshiro, Junio,
Phillip Wood, brian m. carlson, and Ayush Jha.

Thanks for helping improve this series!

K Jayatheerth (3):
  path: extract append_formatted_path() and use in rev-parse
  repo: add path.commondir with absolute and relative suffix formatting
  repo: add path.gitdir with absolute and relative suffix formatting

 Documentation/git-repo.adoc | 15 ++++++++
 builtin/repo.c              | 50 +++++++++++++++++++++++++
 builtin/rev-parse.c         | 73 +++++++++++++++----------------------
 path.c                      | 69 +++++++++++++++++++++++++++++++++++
 path.h                      | 30 +++++++++++++++
 t/t1900-repo-info.sh        | 58 +++++++++++++++++++++++++++++
 6 files changed, 251 insertions(+), 44 deletions(-)

-- 
2.55.0-rc1

^ permalink raw reply

* [GSoC Patch v7 1/3] path: extract append_formatted_path() and use in rev-parse
From: K Jayatheerth @ 2026-06-21  5:55 UTC (permalink / raw)
  To: jayatheerthkulkarni2005
  Cc: a3205153416, git, gitster, jltobler, kumarayushjha123,
	lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260621055534.46798-1-jayatheerthkulkarni2005@gmail.com>

Path formatting logic in builtin/rev-parse.c writes directly to
stdout. Other builtins cannot reuse it.

Extract this logic into append_formatted_path() in path.c and expose
a path_format enum in path.h.

Convert rev-parse to use the new helper in the same step to validate
the API against existing tests and avoid introducing dead code.

Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 builtin/rev-parse.c | 73 ++++++++++++++++++---------------------------
 path.c              | 69 ++++++++++++++++++++++++++++++++++++++++++
 path.h              | 30 +++++++++++++++++++
 3 files changed, 128 insertions(+), 44 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index bb882678fe..6de01466db 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -653,53 +653,38 @@ enum default_type {
 	DEFAULT_UNMODIFIED,
 };
 
-static void print_path(const char *path, const char *prefix, enum format_type format, enum default_type def)
+static void print_path(const char *path, const char *prefix,
+		       enum format_type format, enum default_type def)
 {
-	char *cwd = NULL;
-	/*
-	 * We don't ever produce a relative path if prefix is NULL, so set the
-	 * prefix to the current directory so that we can produce a relative
-	 * path whenever possible.  If we're using RELATIVE_IF_SHARED mode, then
-	 * we want an absolute path unless the two share a common prefix, so don't
-	 * set it in that case, since doing so causes a relative path to always
-	 * be produced if possible.
-	 */
-	if (!prefix && (format != FORMAT_DEFAULT || def != DEFAULT_RELATIVE_IF_SHARED))
-		prefix = cwd = xgetcwd();
-	if (format == FORMAT_DEFAULT && def == DEFAULT_UNMODIFIED) {
-		puts(path);
-	} else if (format == FORMAT_RELATIVE ||
-		  (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE)) {
-		/*
-		 * In order for relative_path to work as expected, we need to
-		 * make sure that both paths are absolute paths.  If we don't,
-		 * we can end up with an unexpected absolute path that the user
-		 * didn't want.
-		 */
-		struct strbuf buf = STRBUF_INIT, realbuf = STRBUF_INIT, prefixbuf = STRBUF_INIT;
-		if (!is_absolute_path(path)) {
-			strbuf_realpath_forgiving(&realbuf, path,  1);
-			path = realbuf.buf;
-		}
-		if (!is_absolute_path(prefix)) {
-			strbuf_realpath_forgiving(&prefixbuf, prefix, 1);
-			prefix = prefixbuf.buf;
+	struct strbuf sb = STRBUF_INIT;
+	enum path_format fmt;
+
+	if (format == FORMAT_RELATIVE) {
+		fmt = PATH_FORMAT_RELATIVE;
+	} else if (format == FORMAT_CANONICAL) {
+		fmt = PATH_FORMAT_CANONICAL;
+	} else /* FORMAT_DEFAULT */ {
+		switch (def) {
+		case DEFAULT_RELATIVE:
+			fmt = PATH_FORMAT_RELATIVE;
+			break;
+		case DEFAULT_RELATIVE_IF_SHARED:
+			fmt = PATH_FORMAT_RELATIVE_IF_SHARED;
+			break;
+		case DEFAULT_CANONICAL:
+			fmt = PATH_FORMAT_CANONICAL;
+			break;
+		case DEFAULT_UNMODIFIED:
+		default:
+			fmt = PATH_FORMAT_UNMODIFIED;
+			break;
 		}
-		puts(relative_path(path, prefix, &buf));
-		strbuf_release(&buf);
-		strbuf_release(&realbuf);
-		strbuf_release(&prefixbuf);
-	} else if (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE_IF_SHARED) {
-		struct strbuf buf = STRBUF_INIT;
-		puts(relative_path(path, prefix, &buf));
-		strbuf_release(&buf);
-	} else {
-		struct strbuf buf = STRBUF_INIT;
-		strbuf_realpath_forgiving(&buf, path, 1);
-		puts(buf.buf);
-		strbuf_release(&buf);
 	}
-	free(cwd);
+
+	append_formatted_path(&sb, path, prefix, fmt);
+	puts(sb.buf);
+
+	strbuf_release(&sb);
 }
 
 int cmd_rev_parse(int argc,
diff --git a/path.c b/path.c
index d7e17bf174..6d8e892ada 100644
--- a/path.c
+++ b/path.c
@@ -1579,6 +1579,75 @@ char *xdg_cache_home(const char *filename)
 	return NULL;
 }
 
+void append_formatted_path(struct strbuf *dest, const char *path,
+			   const char *prefix, enum path_format format)
+{
+	switch (format) {
+	case PATH_FORMAT_UNMODIFIED:
+		strbuf_addstr(dest, path);
+		break;
+
+	case PATH_FORMAT_RELATIVE: {
+		struct strbuf relative_buf = STRBUF_INIT;
+		struct strbuf real_path = STRBUF_INIT;
+		struct strbuf real_prefix = STRBUF_INIT;
+		char *cwd = NULL;
+
+		/*
+		 * We don't ever produce a relative path if prefix is NULL,
+		 * so set the prefix to the current directory so that we can
+		 * produce a relative path whenever possible.
+		 */
+		if (!prefix)
+			prefix = cwd = xgetcwd();
+
+		if (!is_absolute_path(path)) {
+			strbuf_realpath_forgiving(&real_path, path, 1);
+			path = real_path.buf;
+		}
+		if (!is_absolute_path(prefix)) {
+			strbuf_realpath_forgiving(&real_prefix, prefix, 1);
+			prefix = real_prefix.buf;
+		}
+
+		strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
+
+		strbuf_release(&relative_buf);
+		strbuf_release(&real_path);
+		strbuf_release(&real_prefix);
+		free(cwd);
+		break;
+	}
+
+	case PATH_FORMAT_RELATIVE_IF_SHARED: {
+		struct strbuf relative_buf = STRBUF_INIT;
+
+		/*
+		 * If we're using RELATIVE_IF_SHARED mode, then we want an
+		 * absolute path unless the two share a common prefix, so don't
+		 * default the prefix to the current working directory. Doing so
+		 * would cause a relative path to always be produced if possible.
+		 */
+		strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
+		strbuf_release(&relative_buf);
+		break;
+	}
+
+	case PATH_FORMAT_CANONICAL: {
+		struct strbuf canonical_buf = STRBUF_INIT;
+
+		strbuf_realpath_forgiving(&canonical_buf, path, 1);
+		strbuf_addbuf(dest, &canonical_buf);
+
+		strbuf_release(&canonical_buf);
+		break;
+	}
+
+	default:
+		BUG("unknown path_format value %d", format);
+	}
+}
+
 REPO_GIT_PATH_FUNC(squash_msg, "SQUASH_MSG")
 REPO_GIT_PATH_FUNC(merge_msg, "MERGE_MSG")
 REPO_GIT_PATH_FUNC(merge_rr, "MERGE_RR")
diff --git a/path.h b/path.h
index 4c2958a903..4d982a2c8e 100644
--- a/path.h
+++ b/path.h
@@ -262,6 +262,36 @@ enum scld_error safe_create_leading_directories_no_share(char *path);
 int safe_create_file_with_leading_directories(struct repository *repo,
 					      const char *path);
 
+/**
+ * The formatting strategy to apply when writing a path into a buffer.
+ */
+enum path_format {
+	/* Output the path exactly as-is without any modifications. */
+	PATH_FORMAT_UNMODIFIED,
+
+	/* Output a path relative to the provided directory prefix. */
+	PATH_FORMAT_RELATIVE,
+
+	/* Output a relative path only if the path shares a root with the prefix. */
+	PATH_FORMAT_RELATIVE_IF_SHARED,
+
+	/* Output a fully resolved, absolute canonical path. */
+	PATH_FORMAT_CANONICAL
+};
+
+/**
+ * Format a path according to the specified formatting strategy and append
+ * the result to the given strbuf.
+ *
+ * `dest`   : The string buffer to append the formatted path to.
+ * `path`   : The path string that needs to be formatted.
+ * `prefix` : The directory prefix to calculate relative offsets against.
+ * Pass NULL to default to the current working directory where applicable.
+ * `format` : The formatting behavior rule to execute.
+ */
+void append_formatted_path(struct strbuf *dest, const char *path,
+			   const char *prefix, enum path_format format);
+
 # ifdef USE_THE_REPOSITORY_VARIABLE
 #  include "strbuf.h"
 #  include "repository.h"
-- 
2.55.0-rc1


^ permalink raw reply related

* [GSoC Patch v7 2/3] repo: add path.commondir with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-06-21  5:55 UTC (permalink / raw)
  To: jayatheerthkulkarni2005
  Cc: a3205153416, git, gitster, jltobler, kumarayushjha123,
	lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260621055534.46798-1-jayatheerthkulkarni2005@gmail.com>

Scripts working with worktree setups need a reliable way to discover
the common directory, which diverges from the git directory when
multiple worktrees are in use. There is no way to retrieve this path
from git repo info today.

Introduce path.commondir.absolute and path.commondir.relative keys.
Exposing explicit format variants rather than a single key with a
default avoids ambiguity for scripts that require predictable output.

Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/git-repo.adoc |  9 +++++++
 builtin/repo.c              | 26 +++++++++++++++++++
 t/t1900-repo-info.sh        | 52 +++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+)

diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 42262c1983..890c34051d 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -104,6 +104,15 @@ values that they return:
 `object.format`::
 	The object format (hash algorithm) used in the repository.
 
+`path.commondir.absolute`::
+	The canonical absolute path to the Git repository's common
+	directory (the shared `.git` directory containing objects,
+	refs, and global configuration).
+
+`path.commondir.relative`::
+	The path to the Git repository's common directory relative to
+	the current working directory.
+
 `references.format`::
 	The reference storage format. The valid values are:
 +
diff --git a/builtin/repo.c b/builtin/repo.c
index 71a5c1c29c..c4cc3bf3fc 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -7,12 +7,14 @@
 #include "hex.h"
 #include "odb.h"
 #include "parse-options.h"
+#include "path.h"
 #include "path-walk.h"
 #include "progress.h"
 #include "quote.h"
 #include "ref-filter.h"
 #include "refs.h"
 #include "revision.h"
+#include "setup.h"
 #include "strbuf.h"
 #include "string-list.h"
 #include "shallow.h"
@@ -75,6 +77,28 @@ static int get_object_format(struct repository *repo, struct strbuf *buf)
 	return 0;
 }
 
+static int get_path_commondir_absolute(struct repository *repo, struct strbuf *buf)
+{
+	const char *common_dir = repo_get_common_dir(repo);
+
+	if (!common_dir)
+		return error(_("unable to get common directory"));
+
+	append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+	return 0;
+}
+
+static int get_path_commondir_relative(struct repository *repo, struct strbuf *buf)
+{
+	const char *common_dir = repo_get_common_dir(repo);
+
+	if (!common_dir)
+		return error(_("unable to get common directory"));
+
+	append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+	return 0;
+}
+
 static int get_references_format(struct repository *repo, struct strbuf *buf)
 {
 	strbuf_addstr(buf,
@@ -87,6 +111,8 @@ static const struct repo_info_field repo_info_field[] = {
 	{ "layout.bare", get_layout_bare },
 	{ "layout.shallow", get_layout_shallow },
 	{ "object.format", get_object_format },
+	{ "path.commondir.absolute", get_path_commondir_absolute },
+	{ "path.commondir.relative", get_path_commondir_relative },
 	{ "references.format", get_references_format },
 };
 
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 39bb77dda0..09158d29f9 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -155,4 +155,56 @@ test_expect_success 'git repo info -h shows only repo info usage' '
 	test_grep ! "git repo structure" actual
 '
 
+# Helper function to test path keys in both absolute and relative formats.
+# $1: label for the test
+# $2: field_name (e.g., commondir)
+# $3: expected_dir (the directory name, e.g., .git or custom-common)
+# $4: init_command (extra setup like exporting env vars)
+test_repo_info_path () {
+	label=$1
+	field_name=$2
+	expected_dir=$3
+	init_command=$4
+
+	test_expect_success "absolute: $label" '
+		test_when_finished "rm -rf repo" &&
+		git init repo &&
+		(
+			mkdir -p repo/sub &&
+			cd repo/sub &&
+			ROOT="$(test-tool path-utils real_path ..)" && export ROOT &&
+			eval "$init_command" &&
+			echo "path.$field_name.absolute=$ROOT/$expected_dir" >expect &&
+			git repo info "path.$field_name.absolute" >actual &&
+			test_cmp expect actual
+		)
+	'
+
+	test_expect_success "relative: $label" '
+		test_when_finished "rm -rf repo" &&
+		git init repo &&
+		(
+			mkdir -p repo/sub &&
+			cd repo/sub &&
+			ROOT="$(test-tool path-utils real_path ..)" && export ROOT &&
+			eval "$init_command" &&
+			echo "path.$field_name.relative=../$expected_dir" >expect &&
+			git repo info "path.$field_name.relative" >actual &&
+			test_cmp expect actual
+		)
+	'
+}
+
+test_repo_info_path 'commondir standard' 'commondir' '.git'
+
+test_repo_info_path 'commondir with GIT_COMMON_DIR and GIT_DIR' 'commondir' \
+	'custom-common' \
+	'GIT_COMMON_DIR="$ROOT/custom-common" && export GIT_COMMON_DIR &&
+	 GIT_DIR="../.git" && export GIT_DIR &&
+	 git init --bare "$ROOT/custom-common"'
+
+test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
+	'.git' \
+	'GIT_DIR="../.git" && export GIT_DIR'
+
 test_done
-- 
2.55.0-rc1


^ permalink raw reply related

* [GSoC Patch v7 3/3] repo: add path.gitdir with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-06-21  5:55 UTC (permalink / raw)
  To: jayatheerthkulkarni2005
  Cc: a3205153416, git, gitster, jltobler, kumarayushjha123,
	lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260621055534.46798-1-jayatheerthkulkarni2005@gmail.com>

Scripts need a stable way to locate the git directory without
parsing rev-parse output or relying on its flag-driven path format
selection. There is no way to retrieve this path from git repo info
today.

Introduce path.gitdir.absolute and path.gitdir.relative keys,
consistent with the path.commondir keys added in the previous patch.
Reuse the test_repo_info_path helper introduced there to validate
both variants.

Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 Documentation/git-repo.adoc |  6 ++++++
 builtin/repo.c              | 24 ++++++++++++++++++++++++
 t/t1900-repo-info.sh        |  6 ++++++
 3 files changed, 36 insertions(+)

diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 890c34051d..ed7d80c690 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -113,6 +113,12 @@ values that they return:
 	The path to the Git repository's common directory relative to
 	the current working directory.
 
+`path.gitdir.absolute`::
+	The canonical absolute path to the Git repository directory (the `.git` directory).
+
+`path.gitdir.relative`::
+	The path to the Git repository directory relative to the current working directory.
+
 `references.format`::
 	The reference storage format. The valid values are:
 +
diff --git a/builtin/repo.c b/builtin/repo.c
index c4cc3bf3fc..9a312d127a 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -99,6 +99,28 @@ static int get_path_commondir_relative(struct repository *repo, struct strbuf *b
 	return 0;
 }
 
+static int get_path_gitdir_absolute(struct repository *repo, struct strbuf *buf)
+{
+	const char *git_dir = repo_get_git_dir(repo);
+
+	if (!git_dir)
+		return error(_("unable to get git directory"));
+
+	append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+	return 0;
+}
+
+static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
+{
+	const char *git_dir = repo_get_git_dir(repo);
+
+	if (!git_dir)
+		return error(_("unable to get git directory"));
+
+	append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+	return 0;
+}
+
 static int get_references_format(struct repository *repo, struct strbuf *buf)
 {
 	strbuf_addstr(buf,
@@ -113,6 +135,8 @@ static const struct repo_info_field repo_info_field[] = {
 	{ "object.format", get_object_format },
 	{ "path.commondir.absolute", get_path_commondir_absolute },
 	{ "path.commondir.relative", get_path_commondir_relative },
+	{ "path.gitdir.absolute", get_path_gitdir_absolute },
+	{ "path.gitdir.relative", get_path_gitdir_relative },
 	{ "references.format", get_references_format },
 };
 
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 09158d29f9..ae8c22c817 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -207,4 +207,10 @@ test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
 	'.git' \
 	'GIT_DIR="../.git" && export GIT_DIR'
 
+test_repo_info_path 'gitdir standard' 'gitdir' '.git'
+
+test_repo_info_path 'gitdir with explicit GIT_DIR' 'gitdir' \
+	'.git' \
+	'GIT_DIR="../.git" && export GIT_DIR'
+
 test_done
-- 
2.55.0-rc1


^ permalink raw reply related

* Re: [PATCH GSoC RFC v13 10/12] cat-file: add remote-object-info to batch-command
From: Chandra Pratap @ 2026-06-21  6:01 UTC (permalink / raw)
  To: Pablo Sabater
  Cc: gitster, peff, eric.peijian, chriscool, git, jltobler,
	karthik.188, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <20260619-ps-eric-work-rebase-v13-10-3d4c7315d2f8@gmail.com>

[snip]
> +static void parse_cmd_remote_object_info(struct batch_options *opt,
> +                                        const char *line, struct strbuf *output,
> +                                        struct expand_data *data)
> +{
> +       int count;
> +       const char **argv;
> +       char *line_to_split;
> +       static struct object_info *remote_object_info;
> +       static struct oid_array object_info_oids = OID_ARRAY_INIT;

I don't get the point of remote_object_info and object_info_oids
being static here? These variables are allocated, utilized, and
completely freed/disconnected within a single command cycle.

Making them static gives me the false impression that state
needs to persist between calls.

> +       if (strlen(line) >= MAX_REMOTE_OBJ_INFO_LINE)
> +               die(_("remote-object-info command too long"));
> +
> +       line_to_split = xstrdup(line);
> +       count = split_cmdline(line_to_split, &argv);
> +       if (count < 0)
> +               die(_("split remote-object-info command"));
> +       if (count - 1 > MAX_ALLOWED_OBJ_LIMIT)
> +               die(_("remote-object-info supports at most %d objects"),
> +                   MAX_ALLOWED_OBJ_LIMIT);
> +
> +       if (get_remote_info(opt, count, argv, &remote_object_info,
> +                           &object_info_oids))
> +               goto cleanup;
> +
> +       data->skip_object_info = 1;
> +       for (size_t i = 0; i < object_info_oids.nr; i++) {
> +               data->oid = object_info_oids.oid[i];
> +               if (remote_object_info[i].sizep) {
> +                       /*
> +                        * When reaching here, it means remote-object-info can retrieve
> +                        * information from server without downloading them.
> +                        */
> +                       data->size = *remote_object_info[i].sizep;
> +                       opt->batch_mode = BATCH_MODE_INFO;
> +                       batch_object_write(argv[i + 1], output, opt, data, NULL, 0);
> +               } else {
> +                       report_object_status(opt, oid_to_hex(&data->oid), &data->oid, "missing");
> +               }
> +       }
> +       data->skip_object_info = 0;
> +
> +cleanup:
> +       for (size_t i = 0; i < object_info_oids.nr; i++)
> +               free_object_info_contents(&remote_object_info[i]);
> +       free(line_to_split);
> +       free(argv);
> +       free(remote_object_info);
> +}
> +
[snip]

^ permalink raw reply

* Re: [PATCH GSoC RFC v13 12/12] cat-file: make remote-object-info allow-list dynamic
From: Chandra Pratap @ 2026-06-21  6:19 UTC (permalink / raw)
  To: Pablo Sabater
  Cc: gitster, peff, eric.peijian, chriscool, git, jltobler,
	karthik.188, toon
In-Reply-To: <20260619-ps-eric-work-rebase-v13-12-3d4c7315d2f8@gmail.com>

On Fri, 19 Jun 2026 at 20:27, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
>
> The static allow-list in expand_atom() is hardcoded to only allow
> "objectname" and "objectsize" for remote queries. This works because
> up to this point all servers will either support object-info with name
> and size or they do not support them at all, but we cannot expect that
> in a future different servers with different git versions to have the
> same object-info capabilities. Therefore, the allow_list needs to be
> dynamic depending on what does the server advertise.

Nit: "...depending on what does the server advertise." ->
"...depending on what the server advertises."

^ permalink raw reply

* Re: [PATCH GSoC RFC v13 12/12] cat-file: make remote-object-info allow-list dynamic
From: Chandra Pratap @ 2026-06-21  6:27 UTC (permalink / raw)
  To: Pablo Sabater
  Cc: gitster, peff, eric.peijian, chriscool, git, jltobler,
	karthik.188, toon
In-Reply-To: <CA+J6zkRoS5uZFkW1jJv1JO7jPMPO-ZANOYerbUxn4WPaApPV6g@mail.gmail.com>

Forgot to mention this in the earlier email, but the comment explaining
why the backward iteration of the list is needed should also explain why
it is fine to cast `args->object_info_options->nr` to `int` from `size_t`
(it will always be a small number, so no risk of overflow).

^ permalink raw reply

* Re: [PATCH v6 2/3] revision: add peek functions for lookahead
From: Chandra Pratap @ 2026-06-21  6:42 UTC (permalink / raw)
  To: Pablo Sabater
  Cc: git, krka, ayu.chandekar, christian.couder, gitster, jltobler,
	karthik.188, peff, phillip.wood, siddharthasthana31,
	Kristofer Karlsson
In-Reply-To: <20260620-ps-pre-commit-indent-v6-2-cdc6d8fd5fbc@gmail.com>

> +int revision_has_commits_after (struct rev_info *revs, int n)
> +{
> +       struct topo_walk_info *info = revs->topo_walk_info;
> +
> +       if (info) {
> +               int visible = 0;
> +               for (size_t i = 0; i < info->topo_queue.nr && visible < n; i++) {
> +                       struct commit *c = info->topo_queue.array[i].data;
> +                       if (get_commit_action(revs, c) == commit_show)
> +                               visible++;
> +               }
> +               return visible > n-1;

Nit: I think 'return visible >= n' will be more readable here. As in,
more in-line with this function's description (below).

> +       if (revs->commits) {
> +               struct commit_list *cl;
> +               int visible = 0;
> +               for (cl = revs->commits; cl && visible < n; cl = cl->next) {
> +                       if (get_commit_action(revs, cl->item) == commit_show)
> +                               visible++;
> +               }
> +               return visible > n-1;

Same here.

> +       }
> +
> +       return 0;
> +}
> +
>  static void trace2_topo_walk_statistics_atexit(void)
>  {
>         struct json_writer jw = JSON_WRITER_INIT;
> diff --git a/revision.h b/revision.h
> index 00c392be37..a10c6b0940 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -572,4 +572,14 @@ int rewrite_parents(struct rev_info *revs,
>   */
>  struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);
>
> +/*
> + * Peek into revision's next commit without consuming it.
> + */
> +struct commit *revision_peek_next_commit(struct rev_info *revs);
> +
> +/*
> + * Check if there are n more commits to be shown yet.

Shouldn't this be "n or more"?


> +int revision_has_commits_after(struct rev_info *revs, int n);
> +
>  #endif
>
> --
> 2.54.0

^ permalink raw reply

* [PATCH v3 0/2] doc: clarify review replies and reroll timing
From: Weijie Yuan @ 2026-06-21  8:04 UTC (permalink / raw)
  To: git; +Cc: gitster, ps
In-Reply-To: <cover.1781714757.git.wy@wyuan.org>

This small series updates the 2 documentations: MyFirstContribution and
SubmittingPatches.

The first patch clarifies that review feedback should not be answered
only by sending a new version of the patches, which is talked in [1].
Contributors are encouraged to and should discuss their planned response in
the existing review thread, so that the next version does not become the
only place where reviewers can infer the author's reasoning.

The second patch is originally from an email from Patrick [2], which
documents a rough expectation around reroll frequency.

Patrick suggests: There is no hard rule for when to send a new version,
but batching feedback and avoiding multiple rerolls of the same series
in a single day is a useful default. The text also mentions factors that
may affect this, such as the size of the series, the depth of review,
and whether the topic is close to being picked up.
(Edit: the last point is discussed by Junio[3], which is improved in v3)

Since I am the newbie here, please tell me how to attribute the credit
to Patrick. Thank you Patrick!
(Edit: finished with the Helped-by trailer in v2)

[1]: <xmqq7bo5nf31.fsf@gitster.g>
[2]: <aietF4BX1Ewt3cpG@pks.im>
[3]: <xmqq4ij1vywy.fsf@gitster.g>

---
Changes in v3:

  - Reworked the substantial-rework case.  Instead of suggesting that
    authors send a new version sooner, the text now advises authors not
    to rush out an updated version before reviewing the larger changes
    carefully.  It recommends replying to the review that prompted the
    rewrite, saying that a substantial rework is planned, and pointing
    out which parts of the current series will become obsolete.

  - Dropped the advice that a topic close to being accepted may justify
    a quicker reroll.

  - Removed "how close the topic is to being accepted" from the short
    reroll-timing guidance in Documentation/SubmittingPatches.

  - Updated the commit message of patch 2 accordingly.


Changes in v2:

For [PATCH 1/2] doc: encourage review replies before rerolling:

  - Add "social interactions" in commit message.

  I didn't do any changes for Patrick's comments in [a]:

  > I feel like the new version doesn't really add anything significant
  > to this paragraph that it didn't already say before your patch, but
  > it does so with more words.
  > I'm of course biased though, so maybe more words help newcomers?

  Thinking about whether to delete/revert or not. Comments welcome.

For [PATCH 2/2] doc: advise batching patch rerolls:

  - Add a trailer to thank Patrick.

  Suggestions from Junio:

  - Mention that waiting between rerolls gives reviewers across time
    zones a fair chance to participate.
  - Mention that waiting also encourages authors to polish patches
    before sending them.

[a] <ai_7Wh7hrD8PZozg@pks.im>
---
base commit: 700432b2ba (topic flush before -rc1 (batch 1), 2026-06-15)

Weijie Yuan (2):
  doc: encourage review replies before rerolling
  doc: advise batching patch rerolls

 Documentation/MyFirstContribution.adoc | 34 ++++++++++++++++++++++----
 Documentation/SubmittingPatches        | 23 ++++++++++++++---
 2 files changed, 48 insertions(+), 9 deletions(-)

Range-diff against v2:
1:  4bb1efe71d = 1:  4bb1efe71d doc: encourage review replies before rerolling
2:  496a08c74d ! 2:  e1050a6ef5 doc: advise batching patch rerolls
    @@ Commit message
         time to comment regardless of their time zones.
     
         Mention factors that can affect the timing, such as series size, review
    -    depth, substantial rework, and how close the topic is to being accepted.
    -    Also point out that avoiding rapid rerolls encourages authors to polish
    -    each version before sending it, so reviewers can focus on substantial
    -    issues.
    +    depth, and substantial rework. Also point out that avoiding rapid
    +    rerolls encourages authors to polish each version before sending it, so
    +    reviewers can focus on substantial issues.
     
         Helped-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Weijie Yuan <wy@wyuan.org>
    @@ Documentation/MyFirstContribution.adoc: previous one" patches over 2 days), revi
     +The right timing depends on the topic and the feedback. Larger series usually
     +need more review time. If the only comments so far are minor, such as typo
     +fixes, it often makes sense to wait a little longer in case deeper reviews are
    -+still coming. If the comments require substantial rework, sending a new version
    -+sooner may save reviewers from spending time on a version you already know will
    -+change significantly. If the topic is close to being accepted and the remaining
    -+comments are small, a quicker new version may also be fine.
    ++still coming. If the comments call for substantial rework, do not rush out an
    ++updated version before you have reviewed the larger changes carefully. Instead,
    ++reply to the review that prompted the rewrite, say that you are preparing a
    ++substantial rework, and mention which parts of the current series will become
    ++obsolete so reviewers can avoid spending time on them until the updated series
    ++is ready.
     +
      
      [[reviewing]]
    @@ Documentation/SubmittingPatches: area.
     -input and avoids unnecessary churn from many rapid iterations.
     +input, gives reviewers in different time zones a fair chance to comment,
     +and avoids unnecessary churn from many rapid iterations.  Waiting also
    -+encourages you to polish each version before sending it, so reviewers can
    -+focus on substantial issues rather than typos or other small mistakes.
    ++encourages you to polish each version before sending it, so reviewers
    ++can focus on substantial issues rather than typos or other small
    ++mistakes. (only changes textwidth here)
     ++
     +As a rough default, avoid sending more than one new version of the same
    -+series per day, while considering the size of the series, the depth of
    -+review, and how close the topic is to being accepted.
    ++series per day, while considering the size of the series and the depth
    ++of review.
      
      . These early update iterations are expected to be full replacements,
        not incremental updates on top of what you posted already.  If you
-- 
2.54.0


^ permalink raw reply

* [PATCH v3 1/2] doc: encourage review replies before rerolling
From: Weijie Yuan @ 2026-06-21  8:05 UTC (permalink / raw)
  To: git; +Cc: gitster, ps
In-Reply-To: <cover.1782028813.git.wy@wyuan.org>

Review feedback should not be answered only by sending a new patch
version. Encourage contributors to discuss their planned response in the
mailing-list thread before rerolling.

This makes the author's reasoning explicit before the next version is
prepared, instead of forcing reviewers to infer it from the rerolled
patches. It also encourages more direct social interaction between
contributors and helps foster a more collaborative review process.

Signed-off-by: Weijie Yuan <wy@wyuan.org>
---
 Documentation/MyFirstContribution.adoc | 12 +++++++-----
 Documentation/SubmittingPatches        | 12 +++++++++---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/Documentation/MyFirstContribution.adoc b/Documentation/MyFirstContribution.adoc
index b9fdefce02..00704ab91e 100644
--- a/Documentation/MyFirstContribution.adoc
+++ b/Documentation/MyFirstContribution.adoc
@@ -1337,11 +1337,13 @@ fewer mistakes were the only one they would need to review.
 After a few days, you will hopefully receive a reply to your patchset with some
 comments. Woohoo! Now you can get back to work.
 
-It's good manners to reply to each comment, notifying the reviewer that you have
-made the change suggested, feel the original is better, or that the comment
-inspired you to do something a new way which is superior to both the original
-and the suggested change. This way reviewers don't need to inspect your v2 to
-figure out whether you implemented their comment or not.
+It's good manners to reply to each comment in the mailing list discussion
+instead of letting the next version of your patch be your only response. Tell
+the reviewer whether you plan to make the suggested change, keep the original,
+or pursue a different approach. This way reviewers can respond to your reasoning
+before you spend time preparing a version they may not agree with, and later do
+not need to inspect your v2 to figure out whether you implemented their comment
+or not.
 
 Reviewers may ask you about what you wrote in the patchset, either in
 the proposed commit log message or in the changes themselves.  You
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index f042bb5aaf..6c1e1f6423 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -48,8 +48,12 @@ area.
 
 . You get comments and suggestions for improvements.  You may even get
   them in an "on top of your change" patch form.  You are expected to
-  respond to them with "Reply-All" on the mailing list, while taking
-  them into account while preparing an updated set of patches.
+  respond to them with "Reply-All" on the mailing list, instead of
+  letting an updated patch series be your only response.  Tell
+  reviewers which suggestions you plan to use, which ones you disagree
+  with, and when a comment leads you to consider a different approach.
+  Use these replies and any follow-up discussion as input when
+  preparing an updated set of patches.
 +
 It is often beneficial to allow some time for reviewers to provide
 feedback before sending a new version, rather than sending an updated
@@ -613,7 +617,9 @@ grouped into their own e-mail thread to help readers find all parts of the
 series.  To that end, send them as replies to either an additional "cover
 letter" message (see below), the first patch, or the respective preceding patch.
 Here is a link:MyFirstContribution.html#v2-git-send-email[step-by-step guide] on
-how to submit updated versions of a patch series.
+how to submit updated versions of a patch series.  Before sending another
+version, make sure you have answered meaningful review comments in the existing
+discussion.
 
 If your log message (including your name on the
 `Signed-off-by` trailer) is not writable in ASCII, make sure that
-- 
2.54.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox