summaryrefslogtreecommitdiff
path: root/t
diff options
context:
space:
mode:
authorLibravatar Jeff King <peff@peff.net>2018-07-11 10:14:06 -0400
committerLibravatar Junio C Hamano <gitster@pobox.com>2018-07-11 12:12:37 -0700
commit3506dc944558b7d88544408b312e795458383195 (patch)
treec6c1c9ed2c4ba2baf23b1acb3610a416f4bf2853 /t
parentGit 2.16.4 (diff)
downloadtgif-3506dc944558b7d88544408b312e795458383195.tar.xz
has_uncommitted_changes(): fall back to empty tree
If has_uncommitted_changes() can't resolve HEAD (e.g., because it's unborn or corrupt), then we end up calling run_diff_index() with an empty revs.pending array. This causes a segfault, as run_diff_index() blindly looks at the first pending item. Fixing this raises a question of fault: should run_diff_index() handle this case, or is the caller wrong to pass an empty pending list? Looking at the other callers of run_diff_index(), they handle this in one of three ways: - they resolve the object themselves, and avoid doing the diff if it's not valid - they resolve the object themselves, and fall back to the empty tree - they use setup_revisions(), which will die() if the object isn't valid Since this is the only broken caller, that argues that the fix should go there. Falling back to the empty tree makes sense here, as we'd claim uncommitted changes if and only if the index is non-empty. This may be a little funny in the case of corruption (the corrupt HEAD probably _isn't_ empty), but: - we don't actually know the reason here that HEAD didn't resolve (the much more likely case is that we have an unborn HEAD, in which case the empty tree comparison is the right thing) - this matches how other code, like "git diff", behaves While we're thinking about it, let's add an assertion to run_diff_index(). It should always be passed a single object, and as this bug shows, it's easy to get it wrong (and an assertion is easier to hunt down than a segfault, or a quietly ignored extra tree). Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 't')
-rwxr-xr-xt/t5520-pull.sh12
1 files changed, 12 insertions, 0 deletions
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 59c4b778d3..68aa5f0340 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -618,6 +618,18 @@ test_expect_success 'pull --rebase fails on unborn branch with staged changes' '
)
'
+test_expect_success 'pull --rebase fails on corrupt HEAD' '
+ test_when_finished "rm -rf corrupt" &&
+ git init corrupt &&
+ (
+ cd corrupt &&
+ test_commit one &&
+ obj=$(git rev-parse --verify HEAD | sed "s#^..#&/#") &&
+ rm -f .git/objects/$obj &&
+ test_must_fail git pull --rebase
+ )
+'
+
test_expect_success 'setup for detecting upstreamed changes' '
mkdir src &&
(cd src &&