Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2] update-ref: add --rename option
From: Junio C Hamano @ 2026-06-12 16:31 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <xmqqwlw4nccr.fsf@gitster.g>

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

> [Appendix]
> ...  Ideally, these cherry-pickable commits
> that are stored under refs/merge-fix hierarchies SHOULD be indexable
> by a pair of topic (i.e. "when topic A and topic B first meets, apply
> this evil merge"), but this computation is cumbersome to write.

One scheme might be to use "refs/merge-fix/$A--$B" to store the
interaction between topic $A and topic $B, with the convention that
no topic is named with double-dash in its name.

We sequencially merge these in-flight topic into the integration
branch (e.g., 'seen').  When merging topic X, we roughly would need
to do the following.

 (0) Skip if X is already in the integration branch.

 (1) See what topic Y that would also be merged for the first time
     to the integration branch.  This is because a complex topic X
     often is done by merging in-flight Y into then-current master
     and applyng patches on top, and depending on the state of the
     integration branch, such topic Y may or may not have already
     been mergeed there.  Enumerate all these topic Ys that would
     be pulled into the integration branch as a side effect of
     merging X.

 (2) Enumerate all merge-fix refs that has any of the topic Ys or X.
     For each such refs/merge-fix/$A--$B (where either $A or $B is X
     or one of Ys), call the other side of "--" Z.  If Z is already
     in the integration branch, then we found the merge-fix we need
     to apply.

The "topic X might pull other topics that haven't been merged
together with it when it gets merged" is what makes it cumbersome to
write.  If we do not have to worry about it, it would be fairly
straight forward, but then the bulk-merge driver probably needs to
learn a safety check to make sure at the point of merging each topic
that the merge is not pulling another topic (base) into as a side
effect.  It would mean that you prepare a new topic X on top of a
merge of Y into 'master', and X can never be merged into 'seen'
before Y is.  Which may or may not be what we really want, and I
need to think about it a bit.


^ permalink raw reply

* Re: [PATCH v2] commit-reach: remove get_reachable_subset()
From: Weijie Yuan @ 2026-06-12 16:21 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Derrick Stolee, Kristofer Karlsson via GitGitGadget, git,
	Kristofer Karlsson, Patrick Steinhardt
In-Reply-To: <aiww2oXXDQXk0dgu@wyuan.org>

On Sat, Jun 13, 2026 at 12:16:34AM +0800, Weijie Yuan wrote:
> On Fri, Jun 12, 2026 at 07:41:48AM -0700, Junio C Hamano wrote:
> > Weijie Yuan <wy@wyuan.org> writes:
> > 
> > > On Thu, Jun 11, 2026 at 10:48:18AM -0700, Junio C Hamano wrote:
> > >> I wonder if we should talk about it in the SubmittingPatches and/or
> > >> MyFirstContribution document?
> > >
> > > Hi, I think it might be a good idea to cover these details in
> > > MyFirstContribution, then cross-reference them from the part of
> > > SubmittingPatches that discusses sending a new version.
> > 
> > Sorry to be nitpicky, but the above is omitting too much from your
> > quote.  "it" in "talk about it" is totally unclear to a reader who
> > haven't seen the message you are replying to.
> 
> Oops, so sorry! You are not nitpicky at all, this is totally my
> carelessness and fault. Sorry readers!
> 
> Thank you for catching this! It shows that I still need to really
> understand the previous patch I wrote, and put it into real practice:
> 
> > It is usually helpful to trim away unrelated context, such as large
> > portions of the patch that are not being discussed, while _keeping
> > enough quoted text_ for readers to understand *what* you are
> > responding to.
> 
> Thank you! I'll immediately set a solid "pre-reply" hook in my .git
> folder ;-)

Sorry, here I mean setting a pre-reply hook to remind me how to do a
good quote when writting a reply mail :-)

Sorry for the unnecessary noise, and thank you.

^ permalink raw reply

* Re: [PATCH v2] commit-reach: remove get_reachable_subset()
From: Weijie Yuan @ 2026-06-12 16:16 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Derrick Stolee, Kristofer Karlsson via GitGitGadget, git,
	Kristofer Karlsson, Patrick Steinhardt
In-Reply-To: <xmqqecibizwz.fsf@gitster.g>

On Fri, Jun 12, 2026 at 07:41:48AM -0700, Junio C Hamano wrote:
> Weijie Yuan <wy@wyuan.org> writes:
> 
> > On Thu, Jun 11, 2026 at 10:48:18AM -0700, Junio C Hamano wrote:
> >> I wonder if we should talk about it in the SubmittingPatches and/or
> >> MyFirstContribution document?
> >
> > Hi, I think it might be a good idea to cover these details in
> > MyFirstContribution, then cross-reference them from the part of
> > SubmittingPatches that discusses sending a new version.
> 
> Sorry to be nitpicky, but the above is omitting too much from your
> quote.  "it" in "talk about it" is totally unclear to a reader who
> haven't seen the message you are replying to.

Oops, so sorry! You are not nitpicky at all, this is totally my
carelessness and fault. Sorry readers!

Thank you for catching this! It shows that I still need to really
understand the previous patch I wrote, and put it into real practice:

> It is usually helpful to trim away unrelated context, such as large
> portions of the patch that are not being discussed, while _keeping
> enough quoted text_ for readers to understand *what* you are
> responding to.

Thank you! I'll immediately set a solid "pre-reply" hook in my .git
folder ;-)

^ permalink raw reply

* [PATCH v3 3/3] environment: move trust_executable_bit into repo_config_values
From: Tian Yuchen @ 2026-06-12 16:05 UTC (permalink / raw)
  To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello
In-Reply-To: <20260612160527.167203-1-cat@malon.dev>

Move the global 'trust_executable_bit' configuration
into the repository-specific 'repo_config_values'
struct. To ensure code readability, the getter function
'repo_trust_executable_bit()' has been introduced.

For now, associated functions access this configuration by
explicitly falling back to 'the_repository'.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 apply.c       |  2 +-
 environment.c | 11 +++++++++--
 environment.h |  9 ++++++++-
 read-cache.c  |  8 ++++----
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/apply.c b/apply.c
index 249248d4f2..fbb907d3c0 100644
--- a/apply.c
+++ b/apply.c
@@ -3893,7 +3893,7 @@ static int check_preimage(struct apply_state *state,
 		if (*ce && !(*ce)->ce_mode)
 			BUG("ce_mode == 0 for path '%s'", old_name);
 
-		if (trust_executable_bit || !S_ISREG(st->st_mode))
+		if (repo_trust_executable_bit(the_repository) || !S_ISREG(st->st_mode))
 			st_mode = ce_mode_from_stat(*ce, st->st_mode);
 		else if (*ce)
 			st_mode = (*ce)->ce_mode;
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..75069a884d 100644
--- a/environment.c
+++ b/environment.c
@@ -41,7 +41,6 @@
 static int pack_compression_seen;
 static int zlib_compression_seen;
 
-int trust_executable_bit = 1;
 int trust_ctime = 1;
 int check_stat = 1;
 int has_symlinks = 1;
@@ -142,6 +141,13 @@ int is_bare_repository(void)
 	return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
 }
 
+int repo_trust_executable_bit(struct repository *repo)
+{
+	return repo->gitdir?
+		repo_config_values(repo)->trust_executable_bit :
+		1;
+}
+
 int have_git_dir(void)
 {
 	return startup_info->have_repository
@@ -305,7 +311,7 @@ int git_default_core_config(const char *var, const char *value,
 
 	/* This needs a better name */
 	if (!strcmp(var, "core.filemode")) {
-		trust_executable_bit = git_config_bool(var, value);
+		cfg->trust_executable_bit = git_config_bool(var, value);
 		return 0;
 	}
 	if (!strcmp(var, "core.trustctime")) {
@@ -720,5 +726,6 @@ void repo_config_values_init(struct repo_config_values *cfg)
 {
 	cfg->attributes_file = NULL;
 	cfg->apply_sparse_checkout = 0;
+	cfg->trust_executable_bit = 1;
 	cfg->branch_track = BRANCH_TRACK_REMOTE;
 }
diff --git a/environment.h b/environment.h
index 123a71cdc8..d602d2cd95 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,7 @@ struct repo_config_values {
 	/* section "core" config values */
 	char *attributes_file;
 	int apply_sparse_checkout;
+	int trust_executable_bit;
 
 	/* section "branch" config values */
 	enum branch_track branch_track;
@@ -123,6 +124,13 @@ int git_default_config(const char *, const char *,
 int git_default_core_config(const char *var, const char *value,
 			    const struct config_context *ctx, void *cb);
 
+/*
+ * Getter for the `trust_executable_bit` field of `struct repo_config_values`.
+ * It checks `repo->gitdir` to prevent calling repo_config_values()
+ * before the configuration is loaded or in bare environments.
+ */
+int repo_trust_executable_bit(struct repository *repo);
+
 void repo_config_values_init(struct repo_config_values *cfg);
 
 /*
@@ -160,7 +168,6 @@ int is_bare_repository(void);
 extern char *git_work_tree_cfg;
 
 /* Environment bits from configuration mechanism */
-extern int trust_executable_bit;
 extern int trust_ctime;
 extern int check_stat;
 extern int has_symlinks;
diff --git a/read-cache.c b/read-cache.c
index 54150fe756..757249a449 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -207,7 +207,7 @@ unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
 	if (!has_symlinks && S_ISREG(mode) &&
 	    ce && S_ISLNK(ce->ce_mode))
 		return ce->ce_mode;
-	if (!trust_executable_bit && S_ISREG(mode)) {
+	if (!repo_trust_executable_bit(the_repository) && S_ISREG(mode)) {
 		if (ce && S_ISREG(ce->ce_mode))
 			return ce->ce_mode;
 		return create_ce_mode(0666);
@@ -221,7 +221,7 @@ static unsigned int st_mode_from_ce(const struct cache_entry *ce)
 	case S_IFLNK:
 		return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
 	case S_IFREG:
-		return (ce->ce_mode & (trust_executable_bit ? 0755 : 0644)) | S_IFREG;
+		return (ce->ce_mode & (repo_trust_executable_bit(the_repository) ? 0755 : 0644)) | S_IFREG;
 	case S_IFGITLINK:
 		return S_IFDIR | 0755;
 	case S_IFDIR:
@@ -331,7 +331,7 @@ static int ce_match_stat_basic(const struct cache_entry *ce, struct stat *st)
 		/* We consider only the owner x bit to be relevant for
 		 * "mode changes"
 		 */
-		if (trust_executable_bit &&
+		if (repo_trust_executable_bit(the_repository) &&
 		    (0100 & (ce->ce_mode ^ st->st_mode)))
 			changed |= MODE_CHANGED;
 		break;
@@ -752,7 +752,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 		ce->ce_flags |= CE_INTENT_TO_ADD;
 
 
-	if (trust_executable_bit && has_symlinks) {
+	if (repo_trust_executable_bit(the_repository) && has_symlinks) {
 		ce->ce_mode = create_ce_mode(st_mode);
 	} else {
 		/* If there is an existing entry, pick the mode bits and type
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 1/3] read-cache: remove redundant extern declarations
From: Tian Yuchen @ 2026-06-12 16:05 UTC (permalink / raw)
  To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello
In-Reply-To: <20260612160527.167203-1-cat@malon.dev>

The 'read-cache.c' file already includes 'environment.h', which provides
the extern declarations for variables like 'trust_executable_bit' and
'has_symlinks'.

Remove the redundant extern declarations inside 'st_mode_from_ce()' to
clean up the code.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 read-cache.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 38a04b8de3..c44e4d128f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -204,8 +204,6 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
 
 static unsigned int st_mode_from_ce(const struct cache_entry *ce)
 {
-	extern int trust_executable_bit, has_symlinks;
-
 	switch (ce->ce_mode & S_IFMT) {
 	case S_IFLNK:
 		return has_symlinks ? S_IFLNK : (S_IFREG | 0644);
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 2/3] read-cache: move 'ce_mode_from_stat()' to 'read-cache.c'
From: Tian Yuchen @ 2026-06-12 16:05 UTC (permalink / raw)
  To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello
In-Reply-To: <20260612160527.167203-1-cat@malon.dev>

The ce_mode_from_stat() function is declared as a static inline function
in 'read-cache.h'. As we want to migrate configuration variables, this
helper function will need access to corresponding repository-specific
configuration logic. Move the implementation to 'read-cache.c' to
cleanly encapsulate its dependencies.

Note that the 'extern int trust_executable_bit, has_symlinks;' line is
discarded because it's not necessary when the function lives in
"read-cache.c".

At present, this change has no visible impact, but it is crucial
for our future plans to pass in the repo context. Comment
has been added whilst we are at it.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
 read-cache.c | 13 +++++++++++++
 read-cache.h | 23 +++++++++--------------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index c44e4d128f..54150fe756 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -202,6 +202,19 @@ void fill_stat_cache_info(struct index_state *istate, struct cache_entry *ce, st
 	}
 }
 
+unsigned int ce_mode_from_stat(const struct cache_entry *ce, unsigned int mode)
+{
+	if (!has_symlinks && S_ISREG(mode) &&
+	    ce && S_ISLNK(ce->ce_mode))
+		return ce->ce_mode;
+	if (!trust_executable_bit && S_ISREG(mode)) {
+		if (ce && S_ISREG(ce->ce_mode))
+			return ce->ce_mode;
+		return create_ce_mode(0666);
+	}
+	return create_ce_mode(mode);
+}
+
 static unsigned int st_mode_from_ce(const struct cache_entry *ce)
 {
 	switch (ce->ce_mode & S_IFMT) {
diff --git a/read-cache.h b/read-cache.h
index 043da1f1aa..9088a0724a 100644
--- a/read-cache.h
+++ b/read-cache.h
@@ -5,20 +5,15 @@
 #include "object.h"
 #include "pathspec.h"
 
-static inline unsigned int ce_mode_from_stat(const struct cache_entry *ce,
-					     unsigned int mode)
-{
-	extern int trust_executable_bit, has_symlinks;
-	if (!has_symlinks && S_ISREG(mode) &&
-	    ce && S_ISLNK(ce->ce_mode))
-		return ce->ce_mode;
-	if (!trust_executable_bit && S_ISREG(mode)) {
-		if (ce && S_ISREG(ce->ce_mode))
-			return ce->ce_mode;
-		return create_ce_mode(0666);
-	}
-	return create_ce_mode(mode);
-}
+/*
+ * Determine the appropriate index mode for a file based on its stat()
+ * information and the existing cache entry (if any).
+ *
+ * This function handles degradation for filesystems that lack
+ * symlink support or reliable executable bits.
+ */
+unsigned int ce_mode_from_stat(const struct cache_entry *ce,
+				unsigned int mode);
 
 static inline int ce_to_dtype(const struct cache_entry *ce)
 {
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 0/3] environment: migrate 'trust_executable_bit' into 'repo_config_values'
From: Tian Yuchen @ 2026-06-12 16:05 UTC (permalink / raw)
  To: git; +Cc: ps, Tian Yuchen, Christian Couder, Ayush Chandekar,
	Olamide Caleb Bello
In-Reply-To: <20260610093635.139719-1-cat@malon.dev>

The 'core.filemode' (stored as 'trust_executable_bit') configuration
act as a core filesystem capability flag.

This series moves it into 'struct repo_config_values' to tie it to
the specific repository instance it was read from. Eager parsing
is maintained because this flag is heavily consulted in hot paths.

Note: 'repo_config_values()' still does not support any struct
repository other than the_repository due to how deeply these flags
are accessed. In other words, this series of patches is laying
the groundwork for the eventual elimination of the_repository.

Previous related work:
[PATCH 2/6] config: add trust_executable_bit to global config [1]
[PATCH] Refactor 'trust_executable_bit' to repository-scoped setting [2]
(This previous attempt was unsuccessful because the target location
selected was 'struct repo_settings', which our analysis indicated
was not the optimal choice. For further details, please see: [3])

Changes since V2:

Fixed a whole bunch of typos;

Moved the comment for 'ce_mode_from_stat()' from 'read-cache.c' to
'read-cache.h'.

Thanks!

[1] https://lore.kernel.org/git/837b5360b40f992351f489a0ae05fedf49884c6e.1685716420.git.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/20260301190017.53539-1-dronarajgyawali@gmail.com/
[3] https://lore.kernel.org/git/xmqq1pht6nyx.fsf@gitster.g/

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>

Tian Yuchen (3):
  read-cache: remove redundant extern declarations
  read-cache: move 'ce_mode_from_stat()' to 'read-cache.c'
  environment: move trust_executable_bit into repo_config_values

 apply.c       |  2 +-
 environment.c | 11 +++++++++--
 environment.h |  9 ++++++++-
 read-cache.c  | 21 ++++++++++++++++-----
 read-cache.h  | 23 +++++++++--------------
 5 files changed, 43 insertions(+), 23 deletions(-)

-- 
2.43.0


^ permalink raw reply

* Re: [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Derrick Stolee @ 2026-06-12 15:48 UTC (permalink / raw)
  To: Kristofer Karlsson; +Cc: git
In-Reply-To: <CAL71e4MFb3UUKBr1P4ZwtK3o1gvUHMs+siCpLTXKkW6Vx=BxRg@mail.gmail.com>

On 6/12/2026 11:21 AM, Kristofer Karlsson wrote:
> On Fri, 12 Jun 2026 at 17:04, Derrick Stolee <stolee@gmail.com> wrote:
>>> So the actual halt condition would be:
>>>
>>>     no non-stale P1|P2 candidates in the queue
>>>     AND (no pure-P1 OR no pure-P2)
>>
>> And since STALE is added only after both P1 and P2 bits, the two
>> conditions are identical to how queue_has_nonstale() terminates the
>> loop.
> 
> No, I think this part is different. I can demonstrate with an example queue
> state: [P1, stale, P1, stale, stale]
> With the old code, the non-stale tracker would consider this to be non-stale
> since it still has two P1 commits to process.
> My new approach would instead consider that a valid halt state - we
> can't find any new merge-bases at that point.

Ah. this is indeed the detail I missed. For any i in {1, 2}, if Pi
only appears alongside the STALE bit, then we can stop the walk. This
tracks because the other bit can't contribute any new information.

A data shape that makes this particularly helpful is the "release
branch" data shape that I used to justify the --negotiation-include
option [1].

[1] https://lore.kernel.org/git/62e5ef1a4b800cb18b2e934f45303095d545613b.1779207896.git.gitgitgadget@gmail.com/

Suppose developers are merging into 'main' frequently. On occasion,
the tip of 'main' is merged into a new 'release' branch. Thus, the
first-parent history of 'release' is long and completely separate
from the commit history of 'main'. To reach the queue_has_nonstale()
exit condition, we'd need to walk the entire history.

However, if we focus on the single-side condition you are proposing,
we can stop walking once everything in the queue that is reachable
form 'main' is also reachable from that top merge-base.
>>> If this reasoning is correct, then the walk only terminates after
>>> merge-base candidates have either been processed or marked STALE,
>>> and the counterexample should produce [B] rather than [B, C].
>> That's the correct distinction: we need the set [B] and not [B,C]
>> but we need to discover that B can reach C to remove it from the
>> result set.
> 
> Yes, and I think that part works since we visit them in generational order,
> so B can invalidate C before C is reached.
> 
>> I think there is potential merit in "switching walk modes" to DFS
>> when all queued commits have both P1 and P2, but it comes with a
>> lot of complications. So tread carefully if you go down this road.
>>
> 
> On the DFS point: I may be misunderstanding the suggestion, but my current
> approach depends quite heavily on generation ordering. The reason the
> STALE propagation is safe is that, in the finite-generation region,
> descendants are processed before ancestors. If we switch to DFS, I think we
> would lose that ordering property unless the DFS is constrained in some
> additional way.
My thought was focused on the case of "all queued commits have P1 and P2"
and then we could determine which should be non-stale using DFS focused
only on the current queued set.

But I think your single-sided approach is a better way to get the gains
that you want. I think that case is much more likely to occur.

Thanks for your persistence in working on this through my
misunderstanding.

Thanks,
-Stolee


^ permalink raw reply

* Re: t5563-simple-http-auth failures with v2.55.0-rc0
From: Matthew John Cheetham @ 2026-06-12 15:42 UTC (permalink / raw)
  To: Todd Zullinger, git@vger.kernel.org
In-Reply-To: <20260611210456.XYfhytSL@teonanacatl.net>

On 2026-06-11 22:04, Todd Zullinger wrote:

> Hi,
> 
> I tested the freshly-tagged 2.55.0-rc0 and noticed some new
> failures on the in-progress Fedora 45 (AKA Rawhide) for
> t5563.18 (http.emptyAuth=auto attempts Negotiate before
> credential_fill) which was added in 9b1630b972 (t5563: add
> tests for http.emptyAuth with Negotiate, 2026-04-16).
> 
> I notice that Fedora 44 (where the tests all pass) has
> curl-8.18.0 while Fedora 45 has curl-8.21.0-rc2.  The
> version of httpd is the same between them, FWIW.  I didn't
> compare other package differences; it could be something
> else entirely.

Thanks for the report. The failure is not in Git, it is a libcurl
behaviour change, and there is already an open upstream issue:

   https://github.com/curl/curl/issues/21943
   "Negotiate ignored with --anyauth" (Dan Fandrich, 2026-06-10)

Dan also bisected it to the same commit I had locally,
`8f71d0fde515` ("creds: hold credentials", curl PR #21548).

His report describes the regression at the `curl(1)` level (`curl
--anyauth -u : ...` no longer attempts Negotiate); t5563 test 18 is
the same regression observed through `http.emptyAuth=auto`, which
under the hood is the same `CURLOPT_USERPWD=":"` pattern.

Dan also notes a workaround: replacing `-u :` with `-u
literally:anything` (any non-blank username, real or fake) puts
libcurl back on the Negotiate path. That suggests a small Git-side
escape hatch is possible if we want to unblock people running
against current libcurl while we wait for an upstream fix; I have
not tried it yet and would want to be sure it does not have side
effects on other auth schemes before proposing it.

Daniel Stenberg has acknowledged the curl issue but has not yet
posted a fix. I will follow curl#21943 and, if the upstream answer
is "the new behaviour is intended", come back here with a proposal
for what Git should do about `http.emptyAuth` and test 18.

Thanks,
Matthew


^ permalink raw reply

* Re: [PATCH v3] update-ref: add --rename option
From: Junio C Hamano @ 2026-06-12 15:41 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git
In-Reply-To: <aiugat0gvprSX5yr@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> A slight tangent: this is part of why I really don't like commands that
> determine their mode via flags: you now have to worry about every
> combination of flags and whether they even make sense. With subcommands
> we at least only have to worry about the set of flags that directly
> apply to that given subcommand.
>
> Makes me wonder whether I should have a look at extending git-refs(1)
> further:
>
>     git refs delete <ref> [<oldvalue>]
>     git refs update <ref> <newvalue> [<oldvalue>]
>     git refs rename <ref> <oldname> <newname>
>
> I always wanted to do this eventually so that we have one top-level
> command that knows how to do "everything refs".

That may indeed be a better direction to go, but isn't update-ref
the "everything refs" command already?


^ permalink raw reply

* Re: [PATCH v2 3/3] environment: move trust_executable_bit into repo_config_values
From: Junio C Hamano @ 2026-06-12 15:21 UTC (permalink / raw)
  To: Christian Couder
  Cc: Tian Yuchen, git, ps, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <CAP8UFD0X48BJcjLrr8mY0x3A03NSEN35G7jrvdvvp7Qm5PYAdw@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

> On Wed, Jun 10, 2026 at 11:37 AM Tian Yuchen <cat@malon.dev> wrote:
>
>> +/*
>> + * Getters for the `repo_trust_executable_bit` fields of `struct repo_config_values`.
>
> s/Getters/Getter/
> s/fields/field/
> s/repo_trust_executable_bit/trust_executable_bit/
>
>> + * They check `repo->gitdir` to prevent calling repo_config_values()
>> + * before the configuration is loaded or in bare environments.
>> + */
>> +int repo_trust_executable_bit(struct repository *repo);
>
> Thanks.

Thanks.  

A hopefully small and final reroll is in order, and
<21f74852-2209-4d77-94f4-b2b9412eb8e0@malon.dev> has already
promised one.


^ permalink raw reply

* Re: [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Kristofer Karlsson @ 2026-06-12 15:21 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: git
In-Reply-To: <8d0902ca-98b7-44a4-a23b-51de44ab6daa@gmail.com>

On Fri, 12 Jun 2026 at 17:04, Derrick Stolee <stolee@gmail.com> wrote:
> > So the actual halt condition would be:
> >
> >     no non-stale P1|P2 candidates in the queue
> >     AND (no pure-P1 OR no pure-P2)
>
> And since STALE is added only after both P1 and P2 bits, the two
> conditions are identical to how queue_has_nonstale() terminates the
> loop.

No, I think this part is different. I can demonstrate with an example queue
state: [P1, stale, P1, stale, stale]
With the old code, the non-stale tracker would consider this to be non-stale
since it still has two P1 commits to process.
My new approach would instead consider that a valid halt state - we
can't find any new merge-bases at that point.

> > If this reasoning is correct, then the walk only terminates after
> > merge-base candidates have either been processed or marked STALE,
> > and the counterexample should produce [B] rather than [B, C].
> That's the correct distinction: we need the set [B] and not [B,C]
> but we need to discover that B can reach C to remove it from the
> result set.

Yes, and I think that part works since we visit them in generational order,
so B can invalidate C before C is reached.

> I think there is potential merit in "switching walk modes" to DFS
> when all queued commits have both P1 and P2, but it comes with a
> lot of complications. So tread carefully if you go down this road.
>

On the DFS point: I may be misunderstanding the suggestion, but my current
approach depends quite heavily on generation ordering. The reason the
STALE propagation is safe is that, in the finite-generation region,
descendants are processed before ancestors. If we switch to DFS, I think we
would lose that ordering property unless the DFS is constrained in some
additional way.

So I think I may not fully understand the DFS idea, and I am not sure if
that type of optimization would be orthogonal to tweaking the halt condition
or not.

Thanks,
Kristofer

^ permalink raw reply

* Re: [ANNOUNCE] Git v2.55.0-rc0
From: Junio C Hamano @ 2026-06-12 15:14 UTC (permalink / raw)
  To: rsbecker; +Cc: git
In-Reply-To: <065e01dcfa75$ade00690$09a013b0$@nexbridge.com>

<rsbecker@nexbridge.com> writes:

> On June 11, 2026 11:32 AM, Junio wrote:
>> An early preview release Git v2.55.0-rc0 is now available for testing at the usual
>> places.  It is comprised of 397 non-merge commits since v2.54.0, contributed by
>> 70 people, 22 of which are new faces [*].
>
> Cargo is not available everywhere. Build is not possible on NonStop.
>
> cargo build  --release
> /usr/coreutils/bin/bash: cargo: command not found
> Makefile:3021: recipe for target 'target/release/libgitcore.a' failed
> make: *** [target/release/libgitcore.a] Error 127
>
> Is there a way around this?

I see this in the Makefile that you may or may not have read.  Does
it work?

# Define NO_RUST if you want to disable features and subsystems written in Rust
# from being compiled into Git. For now, Rust is still an optional feature of
# the build process. With Git 3.0 though, Rust will always be enabled.

^ permalink raw reply

* Re: [PATCH v3 0/3] doc: config: fix AsciiDoc glitches
From: Junio C Hamano @ 2026-06-12 15:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Tuomas Ahola, git, Kristoffer Haugsbakk, Jean-Noël Avila
In-Reply-To: <20260612045329.GA593075@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Thu, Jun 11, 2026 at 07:19:43PM +0300, Tuomas Ahola wrote:
>
>> Tuomas Ahola (3):
>>   doc: config: terminate runaway lists
>>   doc: config/sideband: fix description list delimiter
>>   doc: git-config: escape erroneous highlight markup
>
> Thanks, this v3 looks good to me.

Yup this one nicely sidesteps the yucky \# thing, which is very
good.

Thanks.

^ permalink raw reply

* Re: [PATCH v3 0/3] doc: config: fix AsciiDoc glitches
From: Junio C Hamano @ 2026-06-12 15:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Tuomas Ahola, git, Kristoffer Haugsbakk, Jean-Noël Avila
In-Reply-To: <20260612045329.GA593075@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Thu, Jun 11, 2026 at 07:19:43PM +0300, Tuomas Ahola wrote:
>
>> Tuomas Ahola (3):
>>   doc: config: terminate runaway lists
>>   doc: config/sideband: fix description list delimiter
>>   doc: git-config: escape erroneous highlight markup
>
> Thanks, this v3 looks good to me.

Yup this one nicely sidesteps the yucky \# thing, which is very
good.

Thanks.

^ permalink raw reply

* Re: [PATCH v3 0/3] doc: config: fix AsciiDoc glitches
From: Junio C Hamano @ 2026-06-12 15:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Tuomas Ahola, git, Kristoffer Haugsbakk, Jean-Noël Avila
In-Reply-To: <20260612045329.GA593075@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Thu, Jun 11, 2026 at 07:19:43PM +0300, Tuomas Ahola wrote:
>
>> Tuomas Ahola (3):
>>   doc: config: terminate runaway lists
>>   doc: config/sideband: fix description list delimiter
>>   doc: git-config: escape erroneous highlight markup
>
> Thanks, this v3 looks good to me.

Yup this one nicely sidesteps the yucky \# thing, which is very
good.

Thanks.

^ permalink raw reply

* Re: [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Derrick Stolee @ 2026-06-12 15:04 UTC (permalink / raw)
  To: Kristofer Karlsson; +Cc: git
In-Reply-To: <CAL71e4OmPzpCXh-zZ8NsT6L4zVKnXV1gqiFZ2w0XgMJhD=LArQ@mail.gmail.com>

On 6/12/2026 10:32 AM, Kristofer Karlsson wrote:
> The required condition must then not be simply "one side exhausted".
> The walk must also continue while non-stale P1|P2 commits remain in the
> queue, since those still need STALE propagation - they are still
> merge-base candidates.
> 
> So the actual halt condition would be:
> 
>     no non-stale P1|P2 candidates in the queue
>     AND (no pure-P1 OR no pure-P2)

And since STALE is added only after both P1 and P2 bits, the two
conditions are identical to how queue_has_nonstale() terminates the
loop. 
> If this reasoning is correct, then the walk only terminates after
> merge-base candidates have either been processed or marked STALE,
> and the counterexample should produce [B] rather than [B, C].
That's the correct distinction: we need the set [B] and not [B,C]
but we need to discover that B can reach C to remove it from the
result set.

I think there is potential merit in "switching walk modes" to DFS
when all queued commits have both P1 and P2, but it comes with a
lot of complications. So tread carefully if you go down this road.

Thanks,
-Stolee


^ permalink raw reply

* RE: [ANNOUNCE] Git v2.55.0-rc0
From: rsbecker @ 2026-06-12 15:03 UTC (permalink / raw)
  To: rsbecker, 'Junio C Hamano', git
In-Reply-To: <065e01dcfa75$ade00690$09a013b0$@nexbridge.com>

On June 12, 2026 10:14 AM, I wrote:
> On June 11, 2026 11:32 AM, Junio wrote:
> > An early preview release Git v2.55.0-rc0 is now available for testing
> > at the usual places.  It is comprised of 397 non-merge commits since
> > v2.54.0, contributed by
> > 70 people, 22 of which are new faces [*].
> 
> Cargo is not available everywhere. Build is not possible on NonStop.
> 
> cargo build  --release
> /usr/coreutils/bin/bash: cargo: command not found
> Makefile:3021: recipe for target 'target/release/libgitcore.a' failed
> make: *** [target/release/libgitcore.a] Error 127
> 
> Is there a way around this?

Note: cargo is part of Rust. This dependency was only supposed to be added
as of git 3.0. If I may be polite and ask that this dependency be removed for
v2.55.0-rc1.

While my team is trying to do the Rust port for NonStop, it is not a quick
effort, and we expect no earlier than 2027 - given that the team is made up
entirely of volunteers.

Thank you,
Randall


^ permalink raw reply

* Re: [PATCH v2] commit-reach: remove get_reachable_subset()
From: Junio C Hamano @ 2026-06-12 14:41 UTC (permalink / raw)
  To: Weijie Yuan
  Cc: Derrick Stolee, Kristofer Karlsson via GitGitGadget, git,
	Kristofer Karlsson, Patrick Steinhardt
In-Reply-To: <aivQv5FkTEWDn22i@wyuan.org>

Weijie Yuan <wy@wyuan.org> writes:

> On Thu, Jun 11, 2026 at 10:48:18AM -0700, Junio C Hamano wrote:
>> I wonder if we should talk about it in the SubmittingPatches and/or
>> MyFirstContribution document?
>
> Hi, I think it might be a good idea to cover these details in
> MyFirstContribution, then cross-reference them from the part of
> SubmittingPatches that discusses sending a new version.

Sorry to be nitpicky, but the above is omitting too much from your
quote.  "it" in "talk about it" is totally unclear to a reader who
haven't seen the message you are replying to.

> Also, for the part about sending a new version, I wonder whether it
> would be better to summarize and fold in Patrick's explanation here,
> thank you Patrick:

Yup, that is a great example.

> From: Patrick Steinhardt <ps@pks.im>
> Message-ID: <aietF4BX1Ewt3cpG@pks.im>

^ permalink raw reply

* [PATCH] Add a test about broken notes handling on rebase
From: Uwe Kleine-König @ 2026-06-12 14:39 UTC (permalink / raw)
  To: git

When a commit disappears during rebase because the patch content is
already there (but not by the same patch in which case the commit would
be skipped) the notes of that disappearing commit still survives and is
added to the (rebased) parent of the disappearing commit.

So with the commit graph

 A -- B -- C
  `
   `-BD

where BD includes the changes done in B, when rebasing C on top of BD,
the note for B should disappear and not be added to BD.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
Hello,

this is a behaviour of git that really bothers me when working on big
patch series. I use notes to track the Message-Id of the patches when I
send them out. Then when rebasing to a newer upstream version, the
tracking gets confused because the Message-Id notes end up on commits
that were not sent out yet (or I got two Message-Ids in them).

I reported that already back in 2023[1], but obviously not in a way that
resulted in a fix. So I'm trying again with a patch that adds a failing
test.

Best regards
Uwe

[1] https://lore.kernel.org/git/20230530092155.3zbb5uxa7eisdzxb@pengutronix.de/

 t/meson.build           |  1 +
 t/t3322-notes-rebase.sh | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 t/t3322-notes-rebase.sh

diff --git a/t/meson.build b/t/meson.build
index c5832fee0535..6927bd9c794f 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -358,6 +358,7 @@ integration_tests = [
   't3311-notes-merge-fanout.sh',
   't3320-notes-merge-worktrees.sh',
   't3321-notes-stripspace.sh',
+  't3322-notes-rebase.sh',
   't3400-rebase.sh',
   't3401-rebase-and-am-rename.sh',
   't3402-rebase-merge.sh',
diff --git a/t/t3322-notes-rebase.sh b/t/t3322-notes-rebase.sh
new file mode 100644
index 000000000000..64c40a523b50
--- /dev/null
+++ b/t/t3322-notes-rebase.sh
@@ -0,0 +1,35 @@
+#!/bin/sh
+
+test_description='Test notes on rebase'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	git init &&
+	echo A > A &&
+	git add A &&
+	git commit -m A &&
+	git branch branch &&
+	echo B > B &&
+	git add B &&
+	git commit -m B &&
+	git notes add -m "This is B" @ &&
+	echo C > C &&
+	git add C &&
+	git commit -m C &&
+	git checkout branch &&
+	echo B > B &&
+	echo D > D &&
+	git add B D &&
+	git commit -m BD
+'
+
+test_expect_success 'rebase B + C on top of BD' '
+	git rebase @ master
+'
+
+test_expect_failure 'assert there is no note on BD' '
+	git notes show branch
+'
+
+test_done

base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
-- 
2.47.3


^ permalink raw reply related

* Re: [RFC] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Kristofer Karlsson @ 2026-06-12 14:32 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: git
In-Reply-To: <0b3f7429-a4fb-4f7a-bf7b-5a0edeb1db52@gmail.com>

On 6/12/2026 2:52 PM, Derrick Stolee wrote:

> The STALE bit is pushed from commits that have bits for both sides
> of the merge. This isn't something that we can learn from just
> walking each side: we need some amount of walking within the
> intersection.
>
> This doesn't matter if we are looking for a single merge base, but
> when we want the full set of independent merge bases, then the STALE
> bit becomes very important.

Thank you for the quick and detailed response and your counterexample
graph is exactly the right thing to worry about.

>    A    X
>   /| __/|
>  | |/   |
>  | B    |
>  | |    |
> ..........
>  | | __/
>   \|/
>    C
>
> In this example, B can reach C through some long list of commits.
> This makes B (and X) have much higher generation number than C.
> After exhausting both sides of A...X, we have B and C in the queue
> with both side bits and neither are stale. But we need to walk
> from B to C to discover that C should be stale.

I think your response helped me identify a mistake in how I described
the halt condition.

The required condition must then not be simply "one side exhausted".
The walk must also continue while non-stale P1|P2 commits remain in the
queue, since those still need STALE propagation - they are still
merge-base candidates.

So the actual halt condition would be:

    no non-stale P1|P2 candidates in the queue
    AND (no pure-P1 OR no pure-P2)

In your example, B and C are both non-stale P1|P2 commits after
both sides are exhausted. Therefore the walk continues. When B is
processed it propagates STALE toward C through the d-chain, and
because the finite-generation region is processed in descending
generation order, that propagation reaches C before C is popped.

If this reasoning is correct, then the walk only terminates after
merge-base candidates have either been processed or marked STALE,
and the counterexample should produce [B] rather than [B, C].

Thanks,
Kristofer

^ permalink raw reply

* Re: [PATCH 6/7] fetch: add configuration option fetch.followRemoteHEAD
From: Junio C Hamano @ 2026-06-12 14:17 UTC (permalink / raw)
  To: Matt Hunter; +Cc: git
In-Reply-To: <20260612055947.1499497-7-m@lfurio.us>

Matt Hunter <m@lfurio.us> writes:

I haven't been following the discussion, so I will not comment on
the idea, i.e., if it makes sense to add such a new option and
configuration, but if we were to add such a thing, I have some
comments on the mechanics.

By the way, do not call a "configuration variable" a "configuration option".
Let's keep the vocabulary forcused without using random synonyms.

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 3cc7efdd83a0..a21bb82274d4 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -103,6 +103,7 @@ static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
>  
>  struct fetch_config {
>  	enum display_format display_format;
> +	enum follow_remote_head_settings follow_remote_head;
>  	int all;
>  	int prune;
>  	int prune_tags;
> @@ -173,6 +174,22 @@ static int git_fetch_config(const char *k, const char *v,
>  			    "fetch.output", v);
>  	}
>  
> +	if (!strcmp(k, "fetch.followremotehead")) {
> +		if (!v)
> +			return config_error_nonbool(k);
> +		else if (!strcasecmp(v, "never"))
> +			fetch_config->follow_remote_head = FOLLOW_REMOTE_NEVER;
> +		else if (!strcasecmp(v, "create"))
> +			fetch_config->follow_remote_head = FOLLOW_REMOTE_CREATE;
> +		else if (!strcasecmp(v, "warn"))
> +			fetch_config->follow_remote_head = FOLLOW_REMOTE_WARN;
> +		else if (!strcasecmp(v, "always"))
> +			fetch_config->follow_remote_head = FOLLOW_REMOTE_ALWAYS;
> +		else
> +			die(_("invalid value for '%s': '%s'"),
> +				"fetch.followRemoteHEAD", v);
> +	}

I think these uses of strcasecmp() are unnecessary and actively
harms end-user experience.  This is especially true because the
value given to remote.<name>.followRemoteHEAD is case sensitive.

Instead of saying "if you want X to happen, set this variable to
'create'", you have to say "'create', or any other case variations
thereof like 'CrEAte'" somehow, for very dubious gain to the end
users.  If you use strcmp(), and document only all lowercase forms,
it would guarantee to avoid confusing a newbie who read the variable
to be set to 'never' on one blog and 'Never' on another and wonder
if 'NEVER' would work or not.

Admittedly values to some existing configuration variables may be
parsed case insensitively but we should aim to fix the mistake in
the longer term, and we should certainly not add more of them.

Is it sensible to die() here?  If you are fetching from somewhere
without keeping a set of remote-tracking branches for it (i.e., a
single shot "git fetch https://github.com/gitster/git master"), you
do not care what garbage value is in fetch.followRemoteHEAD.
Perhaps the version of Git that is slightly newer than the version
that ships with this patch defined new valid values that this patch
does not know about, and such a user who is doing a single-shot
fetch may have that setting to help them working with their usual
non-single shot repositories, but they use a newer version of Git
for such regular work, and they are using slightly old version of
Git to perform this single-shot fetch.  The point is that the
configured value will *NOT* be used for such a user, and dying only
because this piece of code does not understand the configuration that
will not be used is of dubious value.

^ permalink raw reply

* RE: [ANNOUNCE] Git v2.55.0-rc0
From: rsbecker @ 2026-06-12 14:13 UTC (permalink / raw)
  To: 'Junio C Hamano', git
In-Reply-To: <xmqqik7pqeiq.fsf@gitster.g>

On June 11, 2026 11:32 AM, Junio wrote:
> An early preview release Git v2.55.0-rc0 is now available for testing at the usual
> places.  It is comprised of 397 non-merge commits since v2.54.0, contributed by
> 70 people, 22 of which are new faces [*].

Cargo is not available everywhere. Build is not possible on NonStop.

cargo build  --release
/usr/coreutils/bin/bash: cargo: command not found
Makefile:3021: recipe for target 'target/release/libgitcore.a' failed
make: *** [target/release/libgitcore.a] Error 127

Is there a way around this?


^ permalink raw reply

* Re: [PATCH 6/7] fetch: add configuration option fetch.followRemoteHEAD
From: Matt Hunter @ 2026-06-12 14:00 UTC (permalink / raw)
  To: git
In-Reply-To: <20260612055947.1499497-7-m@lfurio.us>

On Fri Jun 12, 2026 at 1:55 AM EDT, Matt Hunter wrote:
> diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
> index eb9c8a3c4884..761bf4ba7d14 100644
> --- a/Documentation/config/remote.adoc
> +++ b/Documentation/config/remote.adoc
> @@ -157,15 +157,12 @@ Blank values signal to ignore all previous values, allowing a reset of
>  the list from broader config scenarios.
>  
>  remote.<name>.followRemoteHEAD::
> -	How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
> -	when fetching using the configured refspecs of a remote.
> -	The default value is "create", which will create `remotes/<name>/HEAD`
> -	if it exists on the remote, but not locally; this will not touch an
> -	already existing local reference. Setting it to "warn" will print
> -	a message if the remote has a different value than the local one;
> -	in case there is no local reference, it behaves like "create".
> -	A variant on "warn" is "warn-if-not-$branch", which behaves like
> -	"warn", but if `HEAD` on the remote is `$branch` it will be silent.
> -	Setting it to "always" will silently update `remotes/<name>/HEAD` to
> -	the value on the remote.  Finally, setting it to "never" will never
> -	change or create the local reference.
> +	When fetching this remote using its default refspec, this option determines
> +	how to handle differences between the remote's `HEAD` and the local
> +	`remotes/<name>/HEAD` symbolic-ref.  Overrides the setting for
> +	`fetch.followRemoteHEAD`.  See `fetch.followRemoteHEAD` for a description of
> +	accepted values.
> ++
> +In addition to the values supported by `fetch.followRemoteHEAD`, this option may
> +also take on the value "warn-if-not-`$branch`", which behaves like "warn", but
> +ignores the warning if the remote's `HEAD` is `remotes/<name>/$branch`.

In hindsight, I'm wondering if $branch ought to be stylized as <branch>
to match the rest of the docs.  Thoughts?

^ permalink raw reply

* [PATCH v4 2/2] graph: indent visual root in graph
From: Pablo Sabater @ 2026-06-12 13:48 UTC (permalink / raw)
  To: git
  Cc: ayu.chandekar, chandrapratap3519, christian.couder, gitster,
	jltobler, karthik.188, peff, phillip.wood, siddharthasthana31,
	Pablo Sabater
In-Reply-To: <20260612-ps-pre-commit-indent-v4-0-e8492037ebae@gmail.com>

When rendering a graph, if the history contains multiple "visual roots",
actual roots or commits that look like roots (i.e. have their parents
filtered out) can end up being vertically adjacent to unrelated commits,
falsely appearing to be related.

A fix for this issue was already attempted [1] a while ago.

This happens because the commits fill the space from left to right and
when a visual root ends, its column becomes free for the following
commit even if they are not related. Once this happens the unrelated
commit is rendered below the visual root. Because there is no special
character or way to identify when a visual root is rendered making the
graph confusing.

By indenting the visual roots when there are still commits to show the
vertical adjacency can be avoided.

Add is_visual_root flag to git_graph making it visible in all graph states,
give graph_update() a new function, graph_is_visual_root() to know if the
current commit is a visual root and set is_visual_root.
The different handled cases are:

- If a visual root has children: similar to GRAPH_PRE_COMMIT state when
  octopus merges need space, an edge row needs to be printed to connect
  the child with the indented visual root. A new state GRAPH_PRE_ROOT is
  needed to connect the child with the visual root:

    * child of the visual root
     \ GRAPH_PRE_ROOT
      * visual root indented

- If a visual root is child-less we can skip GRAPH_PRE_ROOT state and
  render the indented commit directly.

      * visual root indented
    * unrelated commit

- If two or more visual roots are adjacent: by having a lookahead to the
  next commit that will be rendered, if the next commit is also a visual
  root and we are on a visual root, meaning two visual root adjacent in
  the history, the top one can omit the indent, making the one below to
  indent only once, if there are more adjacent visual commits, the
  indentation will increase for each adjacent one, cascading.

    * visual root
      * visual root
        * visual root
    * last commit

  Even if the last commit is a root, because there is nothing that will be
  rendered below we can omit the indentation on purpose.

The lookahead is not completely reliable, on graphs with filtered parents,
the walker when processing the current commit it will simplify its
parents by removing the ones that won't be shown, (They have the
TREESAME flag when filtering by path for example), but it doesn't act
for the grandparents or the next commit if it is unrelated until we move
to the next.

For example given

  A visual root
  B child
  C parent of B, visual root FILTERED
  D last commit

We would expect

  A
    B
  D

When processing A, for the walker and the information at the renderer, B
is still a child of C, as B parent, hasn't been removed yet. This makes
cascade to not trigger as the lookahead fails to detect if the next
commit will be a visual root.
Once at B, its parent has been removed and has become a visual root, and
it just adds its indent to the one left by A. We end up with an extra
indent:

    A
      B
  D

The output isn't broken as unrelated commits are successfully separated
by indentation, but an indent level should have been avoided.

Create a new test file for graph indentations test called
't4218-log-graph-indentation.sh'.

The filtered parents edge case is documented as a NEEDSWORK on the
lookahead function and it has its own 'test_expect_failure' at 't4218'.

[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/

Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c                          | 262 ++++++++++++++++++++++
 t/meson.build                    |   1 +
 t/t4218-log-graph-indentation.sh | 455 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 718 insertions(+)

diff --git a/graph.c b/graph.c
index 842282685f..e0d1e2a510 100644
--- a/graph.c
+++ b/graph.c
@@ -60,12 +60,23 @@ struct column {
 	 * index into column_colors.
 	 */
 	unsigned short color;
+	/*
+	 * Marks if a commit is a non-first parent of a merge. These columns are
+	 * already visually connected to the merge commit and do not need
+	 * indentation.
+	 *
+	 * The first parent is the one that inherits the column and it can need
+	 * indentation if turns out to be a visual root and there's still
+	 * commits to render.
+	 */
+	unsigned is_merge_parent:1;
 };
 
 enum graph_state {
 	GRAPH_PADDING,
 	GRAPH_SKIP,
 	GRAPH_PRE_COMMIT,
+	GRAPH_PRE_ROOT,
 	GRAPH_COMMIT,
 	GRAPH_POST_MERGE,
 	GRAPH_COLLAPSING
@@ -315,6 +326,48 @@ struct git_graph {
 	 * diff_output_prefix_callback().
 	 */
 	struct strbuf prefix_buf;
+
+	/*
+	 * If a commit is a visual root, we need to indent it to prevent
+	 * unrelated commits from being vertically adjacent to it.
+	 */
+	unsigned is_visual_root:1;
+
+	/*
+	 * Indentation increases for each visual root adjacent to another visual
+	 * root, making visual root commits indentation cascade.
+	 */
+	unsigned int visual_root_depth;
+
+	/*
+	 * When a visual root is adjacent to other visual roots, the first one
+	 * can avoid indentation and the rest cascades, increasing the indentation
+	 * for each one.
+	 */
+	unsigned visual_root_cascade:1;
+
+	/*
+	 * Set when the current commit was already present in graph->columns
+	 * before being processed.
+	 */
+	unsigned commit_in_columns:1;
+};
+
+struct graph_lookahead_flags {
+	/*
+	 * Set when there will be a commit after the current one that will be
+	 * rendered.
+	 */
+	unsigned int is_next_visible:1;
+	/*
+	 * Set when the next visible commit is candidate to be a visual root.
+	 */
+	unsigned int is_next_visual_root:1;
+	/*
+	 * Set when the next visible commit will be rendered under the current
+	 * commit.
+	 */
+	unsigned int next_has_column:1;
 };
 
 static inline int graph_needs_truncation(struct git_graph *graph, int lane)
@@ -388,6 +441,8 @@ struct git_graph *graph_init(struct rev_info *opt)
 	graph->num_columns = 0;
 	graph->num_new_columns = 0;
 	graph->mapping_size = 0;
+	graph->visual_root_depth = 0;
+	graph->visual_root_cascade = 0;
 	/*
 	 * Start the column color at the maximum value, since we'll
 	 * always increment it for the first commit we output.
@@ -561,6 +616,11 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
 					  struct commit *commit,
 					  int idx)
 {
+	/*
+	 * Get the initial merge_layout before it's modified to know if this
+	 * is a merge.
+	 */
+	int initial_merge_layout = graph->merge_layout;
 	int i = graph_find_new_column_by_commit(graph, commit);
 	int mapping_idx;
 
@@ -572,6 +632,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
 		i = graph->num_new_columns++;
 		graph->new_columns[i].commit = commit;
 		graph->new_columns[i].color = graph_find_commit_color(graph, commit);
+		graph->new_columns[i].is_merge_parent = 0;
 	}
 
 	if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -610,6 +671,12 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
 	}
 
 	graph->mapping[mapping_idx] = i;
+
+	/*
+	 * Mark non-first parents of a merge.
+	 */
+	if (graph->num_parents > 1 && initial_merge_layout >= 0 && idx > -1)
+		graph->new_columns[i].is_merge_parent = 1;
 }
 
 static void graph_update_columns(struct git_graph *graph)
@@ -701,10 +768,20 @@ static void graph_update_columns(struct git_graph *graph)
 			if (graph->num_parents == 0)
 				graph->width += 2;
 		} else {
+			int j;
 			graph_insert_into_new_columns(graph, col_commit, -1);
+			/*
+			 * This column is not the current commit, but we need to
+			 * propagate the flag until the commit is processed.
+			 */
+			j = graph_find_new_column_by_commit(graph, col_commit);
+			if (j >= 0 && graph->columns[i].is_merge_parent)
+				graph->new_columns[j].is_merge_parent = 1;
 		}
 	}
 
+	graph->commit_in_columns = is_commit_in_columns;
+
 	/*
 	 * If graph_max_lanes is set, cap the width
 	 */
@@ -763,9 +840,135 @@ static int graph_needs_pre_commit_line(struct git_graph *graph)
 	       graph->expansion_row < graph_num_expansion_rows(graph);
 }
 
+/*
+ * A commit can be a visual root when:
+ * - It has no parents.
+ *
+ * - It has parents but they are all filtered out and
+ *   commit->parents arrives NULL.
+ *
+ * - It is not a boundary commit. Boundary commits also have no visible
+ *   parents, but they are not selected as visual roots because they cannot
+ *.  cause the ambiguity of being vertically adjacent because:
+ *
+ *   1. A boundary only appears because an included commit is its child.
+ *      Children are always above, and the renderer draws an edge down to
+ *      the boundary from that child. Rather than starting a column like a
+ *      visual root would do, it inherits its child column.
+ *
+ *   2. Included commits cannot appear below a boundary. Boundaries are
+ *      ancestors of the exclusion point; if an included commit were an
+ *      ancestor of the boundary it would be excluded and not rendered.
+ *      Boundaries therefore always sink to the bottom.
+ */
+static int graph_is_visual_root_candidate(struct commit *c)
+{
+	return c->parents == NULL && !(c->object.flags & BOUNDARY);
+}
+
+static int graph_is_visual_root(struct git_graph *graph,
+				struct graph_lookahead_flags *flags)
+{
+	/*
+	 * This must be only called for the current commit as graph contains
+	 * the state for the current commit only.
+	 *
+	 * To check if a commit is a visual root, call graph_is_visual_root_candidate()
+	 * but we won't know if it is really a visual root until we get to the
+	 * next commit state.
+	 *
+	 * The current commit is an actual visual root if it is a candidate and
+	 * the commit is not a non-first parent of a merge.
+	 *
+	 *   *
+	 *   |\
+	 *   | *    <- it is a visual root candidate but it shouldn't be indented
+	 *   *         because it is already connected by an edge.
+	 *   ^         if commit_in_columns && is_merge_parent means the commit
+	 *   |         was put by a merge and is connected.
+	 *   |
+	 *   `-------- if !is_next_visible means we're on the last commit, avoid
+	 *             indentation unless the one before is a visual root, then
+	 *             we need to differentiate from the one above.
+	 *
+	 * If next_has_columns means that the next commit has
+	 * already a column, so it will not be rendered below, the
+	 * current commit has to act as the last commit and omit
+	 * indentation.
+	 */
+	return graph_is_visual_root_candidate(graph->commit) &&
+	       !(graph->commit_in_columns &&
+		 graph->columns[graph->commit_index].is_merge_parent) &&
+	       flags->is_next_visible &&
+	       (!flags->next_has_column || graph->visual_root_depth > 0);
+}
+
+/*
+ * Iterates the commits queue searching for the next visible commit, once found
+ * sets visibleness and visual-root flags.
+ * Knowing if the next commit is also a visual root avoids redundant indentations
+ *
+ * NEEDSWORK: The queue is actively being modified by the walker, for each commit
+ * its parents and itself get simplified and their flags set, but for the next
+ * unrelated commit or the grandparents they are not simplified yet, which means
+ * that a commit whose parents are all filtered will not be marked as a visual
+ * root candidate at the lookahead.
+ * This causes the lookahead to fail, failing to set the cascade flag to avoid
+ * redundant indentations.
+ * See 'test_expect_failure' at t4218-log-graph-indentation.sh.
+ */
+static void graph_peek_next_visible(struct git_graph *graph,
+				    struct graph_lookahead_flags *flags)
+{
+	struct commit_list *cl;
+
+	flags->is_next_visible = 0;
+	flags->is_next_visual_root = 0;
+	flags->next_has_column = 0;
+
+	for (cl = graph->revs->commits; cl; cl = cl->next) {
+		if (get_commit_action(graph->revs, cl->item) != commit_show)
+			continue;
+		flags->is_next_visible = 1;
+		flags->next_has_column = graph_find_new_column_by_commit(graph, cl->item) >= 0;
+		/*
+		 * We do not need graph->commit_in_columns or is_merge_parent,
+		 * because we only need to know whether the next one might be a
+		 * visual root, affecting the current commit where the cascade
+		 * would have to be set and the first visual root not indented.
+		 *
+		 * It will set next_is_visual_root to true for merge parents that
+		 * graph_is_visual_root() would return false, but if the next is
+		 * a merge parent, the current commit is the child and cannot
+		 * be a visual root and therefore having no effect.
+		 */
+		if (!graph_is_visual_root_candidate(cl->item))
+			return;
+
+		/*
+		 * The next visible commit is a visual root candidate, but
+		 * only set cascade if it's not the last commit to be rendered.
+		 */
+		for (cl = cl->next; cl; cl = cl->next) {
+			if (get_commit_action(graph->revs, cl->item) != commit_show)
+				continue;
+			flags->is_next_visual_root = 1;
+			return;
+		}
+		return;
+	}
+}
+
+static int graph_needs_pre_root_line(struct git_graph *graph)
+{
+	return graph->commit_in_columns && graph->is_visual_root &&
+	       graph->num_columns > 0 && !graph->visual_root_cascade;
+}
+
 void graph_update(struct git_graph *graph, struct commit *commit)
 {
 	struct commit_list *parent;
+	struct graph_lookahead_flags flags;
 
 	/*
 	 * Set the new commit
@@ -796,6 +999,23 @@ void graph_update(struct git_graph *graph, struct commit *commit)
 	 */
 	graph_update_columns(graph);
 
+	graph_peek_next_visible(graph, &flags);
+
+	graph->is_visual_root = graph_is_visual_root(graph, &flags);
+
+	if (graph->is_visual_root) {
+		/*
+		 * If next is a visual root we can omit the indent for the first
+		 * visual root and start cascading.
+		 */
+		if (!graph->visual_root_depth && flags.is_next_visual_root)
+			graph->visual_root_cascade = 1;
+		graph->visual_root_depth++;
+	} else {
+		graph->visual_root_depth = 0;
+		graph->visual_root_cascade = 0;
+	}
+
 	graph->expansion_row = 0;
 
 	/*
@@ -813,11 +1033,16 @@ void graph_update(struct git_graph *graph, struct commit *commit)
 	 * room for it.  We need to do this only if there is a branch row
 	 * (or more) to the right of this commit.
 	 *
+	 * If it is a visual root, we need to print an extra row to
+	 * connect the indentation.
+	 *
 	 * If there are less than 3 parents, we can immediately print the
 	 * commit line.
 	 */
 	if (graph->state != GRAPH_PADDING)
 		graph->state = GRAPH_SKIP;
+	else if (graph_needs_pre_root_line(graph))
+		graph->state = GRAPH_PRE_ROOT;
 	else if (graph_needs_pre_commit_line(graph))
 		graph->state = GRAPH_PRE_COMMIT;
 	else
@@ -1065,6 +1290,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 
 		if (col_commit == graph->commit) {
 			seen_this = 1;
+			if (graph->is_visual_root) {
+				int depth = graph->visual_root_depth;
+				/*
+				 * Each visual column is 2 characters wide.
+				 * Omit the indentation for the first visual
+				 * root in cascade mode.
+				 */
+				int padding = (depth - graph->visual_root_cascade) * 2;
+				graph_line_addchars(line, ' ', padding);
+				graph->width += padding;
+			}
 			graph_output_commit_char(graph, line);
 
 			if (graph_needs_truncation(graph, i)) {
@@ -1436,6 +1672,29 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 		graph_update_state(graph, GRAPH_PADDING);
 }
 
+static void graph_output_pre_root_line(struct git_graph *graph, struct graph_line *line)
+{
+	/*
+	 * This function adds a row before a visual root, to connect the
+	 * branch to the indented commit. It should only be called on a
+	 * visual root.
+	 */
+	assert(graph->is_visual_root);
+
+	for (size_t i = 0; i < graph->num_columns; i++) {
+		struct column *col = &graph->columns[i];
+		if (col->commit == graph->commit) {
+			graph_line_addch(line, ' ');
+			graph_line_write_column(line, col, '\\');
+		} else {
+			graph_line_write_column(line, col, '|');
+		}
+		graph_line_addch(line, ' ');
+	}
+
+	graph_update_state(graph, GRAPH_COMMIT);
+}
+
 int graph_next_line(struct git_graph *graph, struct strbuf *sb)
 {
 	int shown_commit_line = 0;
@@ -1461,6 +1720,9 @@ int graph_next_line(struct git_graph *graph, struct strbuf *sb)
 	case GRAPH_PRE_COMMIT:
 		graph_output_pre_commit_line(graph, &line);
 		break;
+	case GRAPH_PRE_ROOT:
+		graph_output_pre_root_line(graph, &line);
+		break;
 	case GRAPH_COMMIT:
 		graph_output_commit_line(graph, &line);
 		shown_commit_line = 1;
diff --git a/t/meson.build b/t/meson.build
index c5832fee05..17037a8465 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -576,6 +576,7 @@ integration_tests = [
   't4215-log-skewed-merges.sh',
   't4216-log-bloom.sh',
   't4217-log-limit.sh',
+  't4218-log-graph-indentation.sh',
   't4252-am-options.sh',
   't4253-am-keep-cr-dos.sh',
   't4254-am-corrupt.sh',
diff --git a/t/t4218-log-graph-indentation.sh b/t/t4218-log-graph-indentation.sh
new file mode 100755
index 0000000000..f1c9584ba5
--- /dev/null
+++ b/t/t4218-log-graph-indentation.sh
@@ -0,0 +1,455 @@
+#!/bin/sh
+
+test_description='git log --graph visual root indentations'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
+
+check_graph_with_description () {
+	cat >expect &&
+	lib_test_cmp_graph --format="%s%ndescription%nsecond-line" "$@"
+}
+
+create_orphan () {
+	git checkout --orphan "$1" &&
+	{ git rm -rf . || true; }
+}
+
+test_expect_success 'single root commit is not indented' '
+	create_orphan _1 && test_commit 1_A &&
+	lib_test_check_graph _1 <<-\EOF
+	* 1_A
+	EOF
+'
+
+test_expect_success 'visual root indented before unrelated branch' '
+	create_orphan _2 && test_commit 2_A && test_commit 2_B &&
+	create_orphan _3 && test_commit 3_A &&
+	lib_test_check_graph _2 _3 <<-\EOF
+	  * 3_A
+	* 2_B
+	* 2_A
+	EOF
+'
+
+test_expect_success 'visual root indentation with --left-right' '
+	lib_test_check_graph --left-right _2..._3 <<-\EOF
+	  > 3_A
+	< 2_B
+	< 2_A
+	EOF
+'
+
+# A better case of why indentation is still needed with '--left-right' flag is
+# that unrelated branches can be on the same side, so it's needed to
+# differentiate visual roots on the same side.
+test_expect_success 'visual root indentation with --left-right having unrelated commits on the same side' '
+	lib_test_check_graph --left-right _2..._3 _1 <<-\EOF
+	  > 3_A
+	< 2_B
+	 \
+	  < 2_A
+	> 1_A
+	EOF
+'
+
+test_expect_success 'visual root indents the description also' '
+	check_graph_with_description _2 _3 <<-\EOF
+	  * 3_A
+	    description
+	    second-line
+	* 2_B
+	| description
+	| second-line
+	* 2_A
+	  description
+	  second-line
+	EOF
+'
+
+test_expect_success 'indented visual root parent gets connected to its child' '
+	create_orphan _4 && test_commit 4_A && test_commit 4_B &&
+	create_orphan _5 && test_commit 5_A && test_commit 5_B &&
+	lib_test_check_graph _4 _5<<-\EOF
+	* 5_B
+	 \
+	  * 5_A
+	* 4_B
+	* 4_A
+	EOF
+'
+
+test_expect_success 'indented visual root parent gets connected to its child with description' '
+	check_graph_with_description _4 _5 <<-\EOF
+	* 5_B
+	| description
+	| second-line
+	 \
+	  * 5_A
+	    description
+	    second-line
+	* 4_B
+	| description
+	| second-line
+	* 4_A
+	  description
+	  second-line
+	EOF
+'
+
+test_expect_success 'visual roots cascade and last root does not' '
+	create_orphan _7 && test_commit 7_A && test_commit 7_B &&
+	create_orphan _8 && test_commit 8_A &&
+	create_orphan _9 && test_commit 9_A &&
+	create_orphan _10 && test_commit 10_A &&
+	lib_test_check_graph _7 _8 _9 _10  <<-\EOF
+	* 10_A
+	  * 9_A
+	    * 8_A
+	* 7_B
+	* 7_A
+	EOF
+'
+
+test_expect_success 'last root does not cascade' '
+	lib_test_check_graph _8 _9 _10 <<-\EOF
+	* 10_A
+	  * 9_A
+	* 8_A
+	EOF
+'
+
+test_expect_success 'merge parents are roots between them but they do not indent' '
+	create_orphan _11 && test_commit 11_A &&
+	create_orphan _12 && test_commit 12_A &&
+	create_orphan _13 && test_commit 13_A &&
+	git checkout _11 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _11 -p _12 -p _13 -m 11_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph _11 <<-\EOF
+	*-.   11_octopus
+	|\ \
+	| | * 13_A
+	| * 12_A
+	* 11_A
+	EOF
+'
+
+# The last parent of a merge can be indented if nothing related to it needs to
+# be rendered after, if it's another visual root, merge parent must not get
+# indented but rather activate cascading.
+test_expect_success 'merge then unrelated visual root and unrelated branch' '
+	create_orphan _16 && test_commit 16_A && test_commit 16_B &&
+	create_orphan _17 && test_commit 17_A &&
+	create_orphan _18 && test_commit 18_A &&
+	create_orphan _19 && test_commit 19_A &&
+	create_orphan _20 && test_commit 20_A &&
+	git checkout _18 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _18 -p _19 -p _20 -m 18_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph _18 _17 _16 <<-\EOF
+	*-.   18_octopus
+	|\ \
+	| | * 20_A
+	| * 19_A
+	* 18_A
+	  * 17_A
+	* 16_B
+	* 16_A
+	EOF
+'
+
+# The last commit root does not get indented, if the next thing after the root
+# merge parent is the last commit, indent the merge parent.
+test_expect_success 'merge then unrelated root indents merge parent' '
+	lib_test_check_graph _18 _17 <<-\EOF
+	*-.   18_octopus
+	|\ \
+	| | * 20_A
+	| * 19_A
+	 \
+	  * 18_A
+	* 17_A
+	EOF
+'
+
+test_expect_success 'merge then unrelated branch indents merge parent' '
+	lib_test_check_graph _18 _16 <<-\EOF
+	*-.   18_octopus
+	|\ \
+	| | * 20_A
+	| * 19_A
+	 \
+	  * 18_A
+	* 16_B
+	* 16_A
+	EOF
+'
+
+test_expect_success 'two-parent merge of orphans' '
+	create_orphan _21 && test_commit 21_A &&
+	create_orphan _22 && test_commit 22_A &&
+	git checkout _21 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _21 -p _22 -m 21_merge) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph _21 <<-\EOF
+	*   21_merge
+	|\
+	| * 22_A
+	* 21_A
+	EOF
+'
+
+test_expect_success 'commit with filtered parent becomes a visual root' '
+	create_orphan _23 &&
+	echo test >other.txt &&
+	git add other.txt &&
+	git commit -m "23_A" &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "23_B" &&
+	create_orphan _24 &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "24_A" &&
+	lib_test_check_graph _23 _24 -- foo.txt <<-\EOF
+	  * 23_B
+	* 24_A
+	EOF
+'
+
+# The walker simplifies the commit for the current one and its parents, removing
+# the filtered parents, but it doesn't go one step ahead, this causes some edge
+# cases with the lookahead.
+# Given A (orphan), the walker only processes A, and when we lookahead for B
+# (child of C) even tho C will be filtered, it hasn't been simplified yet, so we
+# don't see B as a visual root, therefore cascade indentation isn't applied to A.
+# (cascade indentation starts the indentation at the second visual root, to avoid
+# redundant indentation). So A gets an extra indent, and once B is processed,
+# when rendering it, C has been removed, B is a visual root and as the last commit
+# isn't considered a visual root as it cannot have unrelated commits below it,
+# cascading isn't also applied, giving B another indent.
+#
+# The final result is an extra indent for A and B:
+#
+#	  A
+#	    B
+#	D
+#
+# This will happen for any case where we find ourselves with the next commit
+# being a unrelated child of a parent the will be filtered.
+#
+# instead of the expected:
+test_expect_failure 'filtered parent cascading edge case' '
+	create_orphan _25 &&
+	git rm -rf . &&
+	echo test >other.txt &&
+	git add other.txt &&
+	git commit -m "C-filtered" &&
+
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "B (child of filtered)" &&
+
+	create_orphan _26 &&
+	git rm -rf . &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "A (visual root)" &&
+
+	create_orphan _27 &&
+	git rm -rf . &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "D (last)" &&
+
+	lib_test_check_graph _25 _26 _27 -- foo.txt <<-\EOF
+	* A (visual root)
+	  * B (child of filtered)
+	* D (last)
+	EOF
+'
+
+test_expect_failure 'multiple filtered parents in sequence' '
+	create_orphan _44 &&
+	git rm -rf . &&
+	echo a >other.txt && git add other.txt && git commit -m "44_F" &&
+	echo b >foo.txt && git add foo.txt && git commit -m "44_C" &&
+
+	create_orphan _45 &&
+	git rm -rf . &&
+	echo c >other.txt && git add other.txt && git commit -m "45_F" &&
+	echo d >foo.txt && git add foo.txt && git commit -m "45_C" &&
+
+	create_orphan _46 &&
+	git rm -rf . &&
+	echo e >foo.txt && git add foo.txt && git commit -m "46_A" &&
+
+	lib_test_check_graph _44 _45 _46 -- foo.txt <<-\EOF
+	* 46_A
+	  * 45_C
+	* 44_C
+	EOF
+'
+
+test_expect_failure 'real orphan root followed by child of filtered parent' '
+	create_orphan _47 &&
+	git rm -rf . &&
+	echo a >foo.txt && git add foo.txt && git commit -m "47_A" &&
+
+	create_orphan _48 &&
+	git rm -rf . &&
+	echo b >other.txt && git add other.txt && git commit -m "48_filtered" &&
+	echo c >foo.txt && git add foo.txt && git commit -m "48_B" &&
+
+	create_orphan _49 &&
+	git rm -rf . &&
+	echo d >foo.txt && git add foo.txt && git commit -m "49_last" &&
+
+	lib_test_check_graph _47 _48 _49 -- foo.txt <<-\EOF
+	* 47_A
+	  * 48_B
+	* 49_last
+	EOF
+'
+
+# This tests prove why there is no need to have indentation for boundary
+# commits.
+#
+# Boundary commits rather than starting a column they 'inherit' the one of
+# its child so there will always be an edge that connects it removing the
+# ambiguity.
+test_expect_success 'unrelated boundaries are not ambiguous' '
+	create_orphan _28 && test_commit 28_A && test_commit 28_B &&
+	test_commit 28_C &&
+	create_orphan _29 && test_commit 29_A && test_commit 29_B &&
+	lib_test_check_graph --boundary 28_A.._28 29_A.._29 <<-\EOF
+	* 29_B
+	| * 28_C
+	| * 28_B
+	| o 28_A
+	o 29_A
+	EOF
+'
+
+# Same structure as t6016
+test_expect_success 'boundary commits big test' '
+	# 3 commits on branch _30
+	create_orphan _30 &&
+	test_commit 30_A &&
+	test_commit 30_B &&
+	test_commit 30_C &&
+
+	# 2 commits on branch _31, started from 30_A
+	git checkout -b _31 30_A &&
+	test_commit 31_A &&
+	test_commit 31_B &&
+
+	# 2 commits on branch _32, started from 30_B
+	git checkout -b _32 30_B &&
+	test_commit 32_A &&
+	test_commit 32_B &&
+
+	# Octopus merge _31 and _32 into -30
+	git checkout _30 &&
+	git merge _31 _32 -m 30_D &&
+	git tag 30_D &&
+	test_commit 30_E &&
+
+	# More commits on _32, then merge _32 into _30
+	git checkout _32 &&
+	test_commit 32_C &&
+	test_commit 32_D &&
+	git checkout _30 &&
+	git merge -s ours _32 -m 30_F &&
+	git tag 30_F &&
+	test_commit 30_G &&
+	lib_test_check_graph --boundary _30 _31 _32 ^32_C <<-\EOF
+	* 30_G
+	*   30_F
+	|\
+	| * 32_D
+	* | 30_E
+	| |
+	|  \
+	*-. \   30_D
+	|\ \ \
+	| * | | 31_B
+	| * | | 31_A
+	* | | | 30_C
+	o | | | 30_B
+	|/ / /
+	o / / 30_A
+	 / /
+	| o 32_C
+	|/
+	o 32_B
+	EOF
+'
+
+# Filter by --first-parent and then forcing the filtered parents to be shown.
+test_expect_success '--first-parent flag with the filtered parents' '
+	(
+	create_orphan _35 && test_commit 35_A && test_commit 35_B &&
+	create_orphan _36 && test_commit 36_A &&
+	create_orphan _37 && test_commit 37_A &&
+	git checkout _35 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _35 -p _36 -p _37 -m 35_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph --first-parent _35 _36 _37 <<-\EOF
+	* 35_octopus
+	| * 37_A
+	|   * 36_A
+	* 35_B
+	* 35_A
+	EOF
+	)
+'
+
+test_expect_success '--first-parent with filtered parents but one has a child' '
+	(
+	create_orphan _38 && test_commit 38_A && test_commit 38_B &&
+	create_orphan _39 && test_commit 39_A &&
+	create_orphan _40 && test_commit 40_A && test_commit 40_B &&
+	git checkout _38 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _38 -p _39 -p _40 -m 38_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph --first-parent _38 _39 _40 <<-\EOF
+	* 38_octopus
+	| * 40_B
+	| * 40_A
+	|   * 39_A
+	* 38_B
+	* 38_A
+	EOF
+	)
+'
+
+test_expect_success '--first-parent with filtered parents but both have childs' '
+	(
+	create_orphan _41 && test_commit 41_A && test_commit 41_B &&
+	create_orphan _42 && test_commit 42_A && test_commit 42_B &&
+	create_orphan _43 && test_commit 43_A && test_commit 43_B &&
+	git checkout _41 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _41 -p _42 -p _43 -m 41_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph --first-parent _41 _42 _43 <<-\EOF
+	* 41_octopus
+	| * 43_B
+	|  \
+	|   * 43_A
+	| * 42_B
+	| * 42_A
+	* 41_B
+	* 41_A
+	EOF
+	)
+'
+
+test_done

-- 
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