git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] commit-reach: parse commits in the given repository

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 23, 2026, 12:49 UTC
Message-ID
<arPK8phxWv1pNG_m@pks.im>
In-Reply-To
<20260916134632.1424829-1-orestisflo@gmail.com>
On Wed, Sep 16, 2026 at 03:46:31PM +0200, Orestis Floros wrote:
Show 33 quoted lines
> `can_all_from_reach()` and `can_all_from_reach_with_flag()` parse the
> commits they walk in `the_repository`, even though their caller may be
> working in a different repository. `repo_is_descendant_of()` is such a
> caller: it is told which repository to work in, but as soon as
> generation numbers are enabled it hands the commits over to
> `can_all_from_reach()`, which then parses them elsewhere.
> 
> This breaks merging a superproject whose submodule pointer advanced on
> both sides. merge-ort resolves it by calling `repo_in_merge_bases()` on
> the submodule, and with a commit-graph in both the superproject and the
> submodule the merge dies:
> 
>     $ git merge side
>     fatal: invalid commit position. commit-graph is likely corrupt
> 
> `merge_submodule()` looks the submodule commits up in the submodule, so
> walking their ancestry pulls in parents whose commit-graph position was
> recorded while reading the submodule's commit-graph. The walk then
> parses those parents in `the_repository`, where the recorded position
> indexes the superproject's commit-graph instead: `fill_commit_graph_info()`
> dies when the position is out of bounds, and quietly returns another
> commit's date, generation and parents when it is not.
> 
> The latter used to be the only symptom. Before bb5da75d61 (commit: use
> commit graph in `lookup_commit_reference_gently()`, 2026-02-16) the
> initial lookup did not record commit-graph positions, so the walk simply
> failed to find the submodule commits in the superproject:
> 
>     error: Could not read <commit>
>     Failed to merge submodule sub (commits don't follow merge-base)
> 
> Pass the repository into both functions. git-fetch-pack(1) and
> git-upload-pack(1) keep passing `the_repository`.
Thanks for the nice explanation.
> diff --git a/commit-reach.c b/commit-reach.c
> index 5df471a313..3d579d8f7f 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c

I'm always a fan of removing this implicit dependency. Doubly so if it actually fixes a bug.

Show 11 quoted lines
> diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
> index a564758f52..afb484b963 100755
> --- a/t/t6437-submodule-merge.sh
> +++ b/t/t6437-submodule-merge.sh
> @@ -517,4 +517,43 @@ test_expect_success 'merging should fail with no merge base' '
>  	)
>  '
>  
> +test_expect_success 'setup for commit-graphs in superproject and submodule' '
> +	git init commit-graph &&
> +	(cd commit-graph &&

I wanted to complain about formatting at first, but I see that you simply follow the preexisting style in this file. So I guess this is okay.

I also double-checked that the test indeed catches the bug.
So overall, this looks good to me. Thanks!
Patrick
Previous: Junio C Hamano
Message 6 of 6 in “[BUG] "commit graph is likely corrupt" on git rebase”
  1. Florian SchmidtJul 31, 2026
  2. Patrick SteinhardtAug 10, 2026
  3. commit-reach: parse commits in the given repositoryOrestis Floros, Sep 16, 2026
  4. Kristofer KarlssonSep 16, 2026
  5. Junio C HamanoSep 16, 2026
  6. Patrick SteinhardtSep 23, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.