Git development
 help / color / mirror / Atom feed
* BUG: git commit -a crashes with "unable to stat" during unresolved merge
@ 2026-03-31 21:53 Nick Golden
  2026-04-01 17:05 ` [PATCH] read-cache: disable renames in add_files_to_cache blindmansion
  2026-04-01 19:00 ` [PATCH v2] " Nick Golden
  0 siblings, 2 replies; 5+ messages in thread
From: Nick Golden @ 2026-03-31 21:53 UTC (permalink / raw)
  To: git

Hello,

I found a reproducible bug in `git commit -a`.

With an unresolved merge conflict present, if a different tracked file
has been deleted from the working tree, `git commit -a` can crash
with:

    fatal: unable to stat 'bystander.txt': No such file or directory

I can reproduce this reliably with Git 2.53.0 on macOS in a fresh repository.

To reproduce:

1. Create two tracked files with identical content.
2. Create a merge conflict on one of them.
3. Delete the other tracked file from the working tree.
4. Run `git commit -a -m test`.

Expected:

Git should either stage the deletion and then stop because of the unresolved
merge, or refuse the commit because of the unresolved merge, but not
crash trying to stat the deleted path.

Actual:

Git aborts with:

    fatal: unable to stat 'bystander.txt': No such file or directory

Reproducer script follows.

Possible cause:

This appears to involve rename detection during `add_files_to_cache()`: the
deleted file gets paired with the unmerged path, and a later index update tries
to `stat()` the deleted path.

Thanks,

Nick Golden
nreesegolden@gmail.com

---8<---
#!/bin/sh
set -eu

repro_dir="$(mktemp -d "${TMPDIR:-/tmp}/git-commit-a-rename-crash.XXXXXX")"
echo "Working in: $repro_dir"
cd "$repro_dir"

git init -b main
git config user.name "Test User"
git config user.email "test@example.com"

i=1
while [ "$i" -le 100 ]; do
    printf 'line %s: shared content that is identical across both files\n' "$i"
    i=$((i + 1))
done > conflict.txt

cp conflict.txt bystander.txt

git add conflict.txt bystander.txt
git commit -m "initial"

git checkout -b feature
perl -0pi -e 's/line 50:.*$/line 50: FEATURE BRANCH CHANGE/m' conflict.txt
git add conflict.txt
git commit -m "feature"

git checkout main
perl -0pi -e 's/line 50:.*$/line 50: MAIN BRANCH CHANGE/m' conflict.txt
git add conflict.txt
git commit -m "main"

git merge feature || true
rm bystander.txt

set +e
output="$(git commit -a -m test 2>&1)"
status=$?
set -e

printf '%s\n' "$output"
printf 'exit status: %s\n' "$status"

case "$output" in
    *"fatal: unable to stat 'bystander.txt': No such file or directory"*)
        echo "Bug reproduced."
        exit 0
        ;;
    *)
        echo "Did not reproduce the expected failure."
        exit 1
        ;;
esac

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] read-cache: disable renames in add_files_to_cache
  2026-03-31 21:53 BUG: git commit -a crashes with "unable to stat" during unresolved merge Nick Golden
@ 2026-04-01 17:05 ` blindmansion
  2026-04-01 18:16   ` Junio C Hamano
  2026-04-01 19:00 ` [PATCH v2] " Nick Golden
  1 sibling, 1 reply; 5+ messages in thread
From: blindmansion @ 2026-04-01 17:05 UTC (permalink / raw)
  To: git; +Cc: blindmansion

add_files_to_cache() refreshes the index from worktree changes and does
not need rename detection. When unmerged entries and a deleted stage-0
path are present together, rename detection can pair them and rewrite an
unmerged diff pair to point at the deleted path.

That later makes "git commit -a" and "git add -u" try to stat the
deleted path and die with "unable to stat". Disable rename detection in
this callback-driven staging path and add a regression test covering the
crash.
---
 read-cache.c          |  1 +
 t/t2200-add-update.sh | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/read-cache.c b/read-cache.c
index 5049f9b..d938abc 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -4049,6 +4049,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
 	rev.diffopt.flags.override_submodule_config = 1;
+	rev.diffopt.detect_rename = 0; /* staging worktree changes does not need renames */
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 
 	/*
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 06e83d3..56c7e55 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -200,6 +200,45 @@ test_expect_success 'add -u resolves unmerged paths' '
 	test_cmp expect actual
 '
 
+test_expect_success 'add -u avoids rename pairing on unmerged paths' '
+	test_create_repo rename-crash &&
+	(
+		cd rename-crash &&
+		test_seq 1 100 |
+		sed "s/.*/line &: shared content that is identical across both files/" >conflict.txt &&
+		cp conflict.txt bystander.txt &&
+		git add conflict.txt bystander.txt &&
+		git commit -m "initial: two files with identical content" &&
+		main_branch=$(git symbolic-ref --short HEAD) &&
+		git checkout -b feature &&
+		perl -pe '\''s/^line 50:.*/line 50: FEATURE BRANCH CHANGE/'\'' \
+			conflict.txt >conflict.txt.tmp &&
+		mv conflict.txt.tmp conflict.txt &&
+		git add conflict.txt &&
+		git commit -m "feature: modify line 50" &&
+		git checkout "$main_branch" &&
+		perl -pe '\''s/^line 50:.*/line 50: MAIN BRANCH CHANGE/'\'' \
+			conflict.txt >conflict.txt.tmp &&
+		mv conflict.txt.tmp conflict.txt &&
+		git add conflict.txt &&
+		git commit -m "main: modify line 50 differently" &&
+		test_must_fail git merge feature &&
+		rm bystander.txt &&
+		git add -u >out 2>err &&
+		test_must_be_empty out &&
+		test_must_be_empty err &&
+		git ls-files -u >actual &&
+		test_must_be_empty actual &&
+		git ls-files bystander.txt >actual &&
+		test_must_be_empty actual &&
+		echo conflict.txt >expect &&
+		git ls-files conflict.txt >actual &&
+		test_cmp expect actual &&
+		git diff-files --name-only >actual &&
+		test_must_be_empty actual
+	)
+'
+
 test_expect_success '"add -u non-existent" should fail' '
 	test_must_fail git add -u non-existent &&
 	git ls-files >actual &&
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] read-cache: disable renames in add_files_to_cache
  2026-04-01 17:05 ` [PATCH] read-cache: disable renames in add_files_to_cache blindmansion
@ 2026-04-01 18:16   ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2026-04-01 18:16 UTC (permalink / raw)
  To: blindmansion; +Cc: git

blindmansion <blindmansion@gmail.com> writes:

> add_files_to_cache() refreshes the index from worktree changes and does
> not need rename detection. When unmerged entries and a deleted stage-0
> path are present together, rename detection can pair them and rewrite an
> unmerged diff pair to point at the deleted path.
>
> That later makes "git commit -a" and "git add -u" try to stat the
> deleted path and die with "unable to stat". Disable rename detection in
> this callback-driven staging path and add a regression test covering the
> crash.
> ---

Thanks for trying to help.

 * Documentation/SubmittingPatches::[[sign-off]]
 * Documentation/SubmittingPatches::[[real-name]]

Other than that, the contents of the proposed log message looks
perfect, which is rarely seen in a patch from somebody whose patch
we haven't seen around here yet.

> diff --git a/read-cache.c b/read-cache.c
> index 5049f9b..d938abc 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -4049,6 +4049,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
>  	rev.diffopt.format_callback = update_callback;
>  	rev.diffopt.format_callback_data = &data;
>  	rev.diffopt.flags.override_submodule_config = 1;
> +	rev.diffopt.detect_rename = 0; /* staging worktree changes does not need renames */
>  	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
>  

The change itself looks sensible, but it makes me wonder if there
are other very similar glitches lying around.

> diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> index 06e83d3..56c7e55 100755
> --- a/t/t2200-add-update.sh
> +++ b/t/t2200-add-update.sh
> @@ -200,6 +200,45 @@ test_expect_success 'add -u resolves unmerged paths' '
>  	test_cmp expect actual
>  '

I didn't see too many questionable steps in the new test, other than
the mixed use of "perl" plus "sed".

 - Can't we do this without relying on "perl"?  Also, why does those
   invocations of Perl insist on using single quotes, which is more
   awkward to write in our test framework, where double quotes should
   work equally well?

 - Can't the sample line be shortened so that patch fits comfortably
   on 80-column terminals?

 - Insisting on "add -u" showing nothing on its standard error
   stream makes the test more fragile than it is necessary.  We
   cannot dictate that nobody in the future will add progress
   eye-candy to the codepath and some folks run the test on overly
   loaded machine.  Checking that the command exits with status 0 is
   a good thing to do (which is already done here) and should be
   sufficient in this case.

 - Wouldn't running a single "git ls-files" with pathspec and
   comparing it with expected list of contents in the index make it
   more clear what is being tested and expected in the last part of
   the test where we see no bystander.txt is mentioned and
   conflict.txt is mentioned there?  I.e., something like

		git ls-files bystander.txt conflict.txt >actual &&
		cat >expect <<\-EOF &&
		conflict.txt
		EOF
		test_cmp expect actual

> +test_expect_success 'add -u avoids rename pairing on unmerged paths' '
> +	test_create_repo rename-crash &&
> +	(
> +		cd rename-crash &&
> +		test_seq 1 100 |
> +		sed "s/.*/line &: shared content that is identical across both files/" >conflict.txt &&
> +		cp conflict.txt bystander.txt &&
> +		git add conflict.txt bystander.txt &&
> +		git commit -m "initial: two files with identical content" &&
> +		main_branch=$(git symbolic-ref --short HEAD) &&
> +		git checkout -b feature &&
> +		perl -pe '\''s/^line 50:.*/line 50: FEATURE BRANCH CHANGE/'\'' \
> +			conflict.txt >conflict.txt.tmp &&
> +		mv conflict.txt.tmp conflict.txt &&
> +		git add conflict.txt &&
> +		git commit -m "feature: modify line 50" &&
> +		git checkout "$main_branch" &&
> +		perl -pe '\''s/^line 50:.*/line 50: MAIN BRANCH CHANGE/'\'' \
> +			conflict.txt >conflict.txt.tmp &&
> +		mv conflict.txt.tmp conflict.txt &&
> +		git add conflict.txt &&
> +		git commit -m "main: modify line 50 differently" &&
> +		test_must_fail git merge feature &&
> +		rm bystander.txt &&
> +		git add -u >out 2>err &&
> +		test_must_be_empty out &&
> +		test_must_be_empty err &&
> +		git ls-files -u >actual &&
> +		test_must_be_empty actual &&
> +		git ls-files bystander.txt >actual &&
> +		test_must_be_empty actual &&
> +		echo conflict.txt >expect &&
> +		git ls-files conflict.txt >actual &&
> +		test_cmp expect actual &&
> +		git diff-files --name-only >actual &&
> +		test_must_be_empty actual
> +	)
> +'
> +
>  test_expect_success '"add -u non-existent" should fail' '
>  	test_must_fail git add -u non-existent &&
>  	git ls-files >actual &&

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] read-cache: disable renames in add_files_to_cache
  2026-03-31 21:53 BUG: git commit -a crashes with "unable to stat" during unresolved merge Nick Golden
  2026-04-01 17:05 ` [PATCH] read-cache: disable renames in add_files_to_cache blindmansion
@ 2026-04-01 19:00 ` Nick Golden
  2026-04-01 19:08   ` Blind Mansion
  1 sibling, 1 reply; 5+ messages in thread
From: Nick Golden @ 2026-04-01 19:00 UTC (permalink / raw)
  To: git; +Cc: gitster, Nick Golden

add_files_to_cache() refreshes the index from worktree changes and does
not need rename detection. When unmerged entries and a deleted stage-0
path are present together, rename detection can pair them and rewrite an
unmerged diff pair to point at the deleted path.

That later makes "git commit -a" and "git add -u" try to stat the
deleted path and die with "unable to stat". Disable rename detection in
this callback-driven staging path and add a regression test covering the
crash.

Signed-off-by: Nick Golden <blindmansion@gmail.com>
---
Changes since v1:
- use sed instead of perl in the regression test
- shorten the sample line content
- stop asserting that stderr is empty
- simplify the final ls-files check
- use real name and add Signed-off-by

 read-cache.c          |  1 +
 t/t2200-add-update.sh | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/read-cache.c b/read-cache.c
index 5049f9b..d938abc 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -4049,6 +4049,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
 	rev.diffopt.format_callback = update_callback;
 	rev.diffopt.format_callback_data = &data;
 	rev.diffopt.flags.override_submodule_config = 1;
+	rev.diffopt.detect_rename = 0; /* staging worktree changes does not need renames */
 	rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
 
 	/*
diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index 06e83d3..0a96655 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -200,6 +200,44 @@ test_expect_success 'add -u resolves unmerged paths' '
 	test_cmp expect actual
 '
 
+test_expect_success 'add -u avoids rename pairing on unmerged paths' '
+	test_create_repo rename-crash &&
+	(
+		cd rename-crash &&
+		test_seq 1 100 |
+		sed "s/.*/line &: same text/" >conflict.txt &&
+		cp conflict.txt bystander.txt &&
+		git add conflict.txt bystander.txt &&
+		git commit -m "initial: two files with identical content" &&
+		main_branch=$(git symbolic-ref --short HEAD) &&
+		git checkout -b feature &&
+		sed "s/^line 50:.*/line 50: FEATURE/" \
+			conflict.txt >conflict.txt.tmp &&
+		mv conflict.txt.tmp conflict.txt &&
+		git add conflict.txt &&
+		git commit -m "feature: modify line 50" &&
+		git checkout "$main_branch" &&
+		sed "s/^line 50:.*/line 50: MAIN/" \
+			conflict.txt >conflict.txt.tmp &&
+		mv conflict.txt.tmp conflict.txt &&
+		git add conflict.txt &&
+		git commit -m "main: modify line 50 differently" &&
+		test_must_fail git merge feature &&
+		rm bystander.txt &&
+		git add -u >out &&
+		test_must_be_empty out &&
+		git ls-files -u >actual &&
+		test_must_be_empty actual &&
+		git ls-files bystander.txt conflict.txt >actual &&
+		cat >expect <<-\EOF &&
+		conflict.txt
+		EOF
+		test_cmp expect actual &&
+		git diff-files --name-only >actual &&
+		test_must_be_empty actual
+	)
+'
+
 test_expect_success '"add -u non-existent" should fail' '
 	test_must_fail git add -u non-existent &&
 	git ls-files >actual &&
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] read-cache: disable renames in add_files_to_cache
  2026-04-01 19:00 ` [PATCH v2] " Nick Golden
@ 2026-04-01 19:08   ` Blind Mansion
  0 siblings, 0 replies; 5+ messages in thread
From: Blind Mansion @ 2026-04-01 19:08 UTC (permalink / raw)
  To: git; +Cc: gitster

Thanks for the review. I encountered this bug while doing differential
testing against another implementation, and enjoyed learning more about
this process. I have sent v2 with the metadata fix and test
cleanups you noted.

I also audited nearby callback-driven index refresh/staging paths and did
not find another obvious instance of this specific pattern outside
add_files_to_cache().


On Wed, Apr 1, 2026 at 3:00 PM Nick Golden <blindmansion@gmail.com> wrote:
>
> add_files_to_cache() refreshes the index from worktree changes and does
> not need rename detection. When unmerged entries and a deleted stage-0
> path are present together, rename detection can pair them and rewrite an
> unmerged diff pair to point at the deleted path.
>
> That later makes "git commit -a" and "git add -u" try to stat the
> deleted path and die with "unable to stat". Disable rename detection in
> this callback-driven staging path and add a regression test covering the
> crash.
>
> Signed-off-by: Nick Golden <blindmansion@gmail.com>
> ---
> Changes since v1:
> - use sed instead of perl in the regression test
> - shorten the sample line content
> - stop asserting that stderr is empty
> - simplify the final ls-files check
> - use real name and add Signed-off-by
>
>  read-cache.c          |  1 +
>  t/t2200-add-update.sh | 38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
>
> diff --git a/read-cache.c b/read-cache.c
> index 5049f9b..d938abc 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -4049,6 +4049,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
>         rev.diffopt.format_callback = update_callback;
>         rev.diffopt.format_callback_data = &data;
>         rev.diffopt.flags.override_submodule_config = 1;
> +       rev.diffopt.detect_rename = 0; /* staging worktree changes does not need renames */
>         rev.max_count = 0; /* do not compare unmerged paths with stage #2 */
>
>         /*
> diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
> index 06e83d3..0a96655 100755
> --- a/t/t2200-add-update.sh
> +++ b/t/t2200-add-update.sh
> @@ -200,6 +200,44 @@ test_expect_success 'add -u resolves unmerged paths' '
>         test_cmp expect actual
>  '
>
> +test_expect_success 'add -u avoids rename pairing on unmerged paths' '
> +       test_create_repo rename-crash &&
> +       (
> +               cd rename-crash &&
> +               test_seq 1 100 |
> +               sed "s/.*/line &: same text/" >conflict.txt &&
> +               cp conflict.txt bystander.txt &&
> +               git add conflict.txt bystander.txt &&
> +               git commit -m "initial: two files with identical content" &&
> +               main_branch=$(git symbolic-ref --short HEAD) &&
> +               git checkout -b feature &&
> +               sed "s/^line 50:.*/line 50: FEATURE/" \
> +                       conflict.txt >conflict.txt.tmp &&
> +               mv conflict.txt.tmp conflict.txt &&
> +               git add conflict.txt &&
> +               git commit -m "feature: modify line 50" &&
> +               git checkout "$main_branch" &&
> +               sed "s/^line 50:.*/line 50: MAIN/" \
> +                       conflict.txt >conflict.txt.tmp &&
> +               mv conflict.txt.tmp conflict.txt &&
> +               git add conflict.txt &&
> +               git commit -m "main: modify line 50 differently" &&
> +               test_must_fail git merge feature &&
> +               rm bystander.txt &&
> +               git add -u >out &&
> +               test_must_be_empty out &&
> +               git ls-files -u >actual &&
> +               test_must_be_empty actual &&
> +               git ls-files bystander.txt conflict.txt >actual &&
> +               cat >expect <<-\EOF &&
> +               conflict.txt
> +               EOF
> +               test_cmp expect actual &&
> +               git diff-files --name-only >actual &&
> +               test_must_be_empty actual
> +       )
> +'
> +
>  test_expect_success '"add -u non-existent" should fail' '
>         test_must_fail git add -u non-existent &&
>         git ls-files >actual &&
> --
> 2.53.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-01 19:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-31 21:53 BUG: git commit -a crashes with "unable to stat" during unresolved merge Nick Golden
2026-04-01 17:05 ` [PATCH] read-cache: disable renames in add_files_to_cache blindmansion
2026-04-01 18:16   ` Junio C Hamano
2026-04-01 19:00 ` [PATCH v2] " Nick Golden
2026-04-01 19:08   ` Blind Mansion

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