@matheusbsbs / matheus-bagliano / commits / 4bddd98311

split-index: don't write cache tree with null oid entries

In a96d3cc3f6 ("cache-tree: reject entries with null sha1", 2017-04-21) we made sure that broken cache entries do not get propagated to new trees. Part of that was making sure not to re-use an existing cache tree that includes a null oid. It did so by dropping the cache tree in 'do_write_index()' if one of the entries contains a null oid. In split index mode however, there are two invocations to 'do_write_index()', one for the shared index and one for the split index. The cache tree is only written once, to the split index. As we only loop through the elements that are effectively being written by the current invocation, that may not include the entry with a null oid in the split index (when it is already written to the shared index), where we write the cache tree. Therefore in split index mode we may still end up writing the cache tree, even though there is an entry with a null oid in the index. Fix this by checking for null oids in prepare_to_write_split_index, where we loop the entries of the shared index as well as the entries for the split index. This fixes t7009 with GIT_TEST_SPLIT_INDEX. Also add a new test that's more specifically showing the problem. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Thomas Gummerer committed Jan 7, 2018 at 22:30 UTC 4bddd98311f249e9ed8a9b6379c7a097201612d1

4 files changed +24 -2

cache.h

+2 -1

@@ -340,7 +340,8 @@ struct index_state {

340 struct split_index *split_index;

341 struct cache_time timestamp;

342 unsigned name_hash_initialized : 1,

343 - initialized : 1;

343 + initialized : 1,

344 + drop_cache_tree : 1;

345 struct hashmap name_hash;

346 struct hashmap dir_hash;

347 unsigned char sha1[20];

read-cache.c

+1 -1

@@ -2199,7 +2199,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,

2199 struct stat st;

2200 struct ondisk_cache_entry_extended ondisk;

2201 struct strbuf previous_name_buf = STRBUF_INIT, *previous_name;

2202 - int drop_cache_tree = 0;

2202 + int drop_cache_tree = istate->drop_cache_tree;

2203

2204 for (i = removed = extended = 0; i < entries; i++) {

2205 if (cache[i]->ce_flags & CE_REMOVE)

split-index.c

@@ -238,6 +238,8 @@ void prepare_to_write_split_index(struct index_state *istate)

238 ALLOC_GROW(entries, nr_entries+1, nr_alloc);

239 entries[nr_entries++] = ce;

240 }

241 + if (is_null_oid(&ce->oid))

242 + istate->drop_cache_tree = 1;

243 }

244 }

245

t/t1700-split-index.sh

+19

@@ -400,4 +400,23 @@ done <<\EOF

400 0642 -rw-r---w-

401 EOF

402

403 +test_expect_success 'writing split index with null sha1 does not write cache tree' '

404 + git config core.splitIndex true &&

405 + git config splitIndex.maxPercentChange 0 &&

406 + git commit -m "commit" &&

407 + {

408 + git ls-tree HEAD &&

409 + printf "160000 commit $_z40\\tbroken\\n"

410 + } >broken-tree &&

411 + echo "add broken entry" >msg &&

412 +

413 + tree=$(git mktree <broken-tree) &&

414 + test_tick &&

415 + commit=$(git commit-tree $tree -p HEAD <msg) &&

416 + git update-ref HEAD "$commit" &&

417 + GIT_ALLOW_NULL_SHA1=1 git reset --hard &&

418 + (test-dump-cache-tree >cache-tree.out || true) &&

419 + test_line_count = 0 cache-tree.out

420 +'

421 +

422 test_done