write_index: optionally allow broken null sha1s

Commit 4337b58 (do not write null sha1s to on-disk index,
2012-07-28) added a safety check preventing git from writing
null sha1s into the index. The intent was to catch errors in
other parts of the code that might let such an entry slip
into the index (or worse, a tree).

Some existing repositories may have invalid trees that
contain null sha1s already, though.  Until 4337b58, a common
way to clean this up would be to use git-filter-branch's
index-filter to repair such broken entries.  That now fails
when filter-branch tries to write out the index.

Introduce a GIT_ALLOW_NULL_SHA1 environment variable to
relax this check and make it easier to recover from such a
history.

It is tempting to not involve filter-branch in this commit
at all, and instead require the user to manually invoke

	GIT_ALLOW_NULL_SHA1=1 git filter-branch ...

to perform an index-filter on a history with trees with null
sha1s.  That would be slightly safer, but requires some
specialized knowledge from the user.  So let's set the
GIT_ALLOW_NULL_SHA1 variable automatically when checking out
the to-be-filtered trees.  Advice on using filter-branch to
remove such entries already exists on places like
stackoverflow, and this patch makes it Just Work again on
recent versions of git.

Further commands that touch the index will still notice and
fail, unless they actually remove the broken entries.  A
filter-branch whose filters do not touch the index at all
will not error out (since we complain of the null sha1 only
on writing, not when making a tree out of the index), but
this is acceptable, as we still print a loud warning, so the
problem is unlikely to go unnoticed.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index ac2a005fdb23c48d8451188ffd7b1c8194b0295f..98e8fe43d2fdde1b76e8f10f4c9473e23c8f347c 100755 (executable)
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -283,11 +283,12 @@ while read commit parents; do
 
        case "$filter_subdir" in
        "")
-               git read-tree -i -m $commit
+               GIT_ALLOW_NULL_SHA1=1 git read-tree -i -m $commit
                ;;
        *)
                # The commit may not have the subdirectory at all
-               err=$(git read-tree -i -m $commit:"$filter_subdir" 2>&1) || {
+               err=$(GIT_ALLOW_NULL_SHA1=1 \
+                     git read-tree -i -m $commit:"$filter_subdir" 2>&1) || {
                        if ! git rev-parse -q --verify $commit:"$filter_subdir"
                        then
                                rm -f "$GIT_INDEX_FILE"

diff --git a/read-cache.c b/read-cache.c
index d97e3bf7cdb025de06ecf339059c38976355d8a5..835070e24c45fd277ca75e2c4caeee84d71deb03 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -1800,8 +1800,17 @@ int write_index(struct index_state *istate, int newfd)
                        continue;
                if (!ce_uptodate(ce) && is_racy_timestamp(istate, ce))
                        ce_smudge_racily_clean_entry(ce);
-               if (is_null_sha1(ce->sha1))
-                       return error("cache entry has null sha1: %s", ce->name);
+               if (is_null_sha1(ce->sha1)) {
+                       static const char msg[] = "cache entry has null sha1: %s";
+                       static int allow = -1;
+
+                       if (allow < 0)
+                               allow = git_env_bool("GIT_ALLOW_NULL_SHA1", 0);
+                       if (allow)
+                               warning(msg, ce->name);
+                       else
+                               return error(msg, ce->name);
+               }
                if (ce_write_entry(&c, newfd, ce, previous_name) < 0)
                        return -1;
        }

diff --git a/t/t7009-filter-branch-null-sha1.sh b/t/t7009-filter-branch-null-sha1.sh
new file mode 100755 (executable)
index 0000000..a997f7a
--- /dev/null
+++ b/t/t7009-filter-branch-null-sha1.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='filter-branch removal of trees with null sha1'
+. ./test-lib.sh
+
+test_expect_success 'setup: base commits' '
+       test_commit one &&
+       test_commit two &&
+       test_commit three
+'
+
+test_expect_success 'setup: a commit with a bogus null sha1 in the tree' '
+       {
+               git ls-tree HEAD &&
+               printf "160000 commit $_z40\\tbroken\\n"
+       } >broken-tree
+       echo "add broken entry" >msg &&
+
+       tree=$(git mktree <broken-tree) &&
+       test_tick &&
+       commit=$(git commit-tree $tree -p HEAD <msg) &&
+       git update-ref HEAD "$commit"
+'
+
+# we have to make one more commit on top removing the broken
+# entry, since otherwise our index does not match HEAD (and filter-branch will
+# complain). We could make the index match HEAD, but doing so would involve
+# writing a null sha1 into the index.
+test_expect_success 'setup: bring HEAD and index in sync' '
+       test_tick &&
+       git commit -a -m "back to normal"
+'
+
+test_expect_success 'filter commands are still checked' '
+       test_must_fail git filter-branch \
+               --force --prune-empty \
+               --index-filter "git rm --cached --ignore-unmatch three.t"
+'
+
+test_expect_success 'removing the broken entry works' '
+       echo three >expect &&
+       git filter-branch \
+               --force --prune-empty \
+               --index-filter "git rm --cached --ignore-unmatch broken" &&
+       git log -1 --format=%s >actual &&
+       test_cmp expect actual
+'
+
+test_done