read-cache: fix reading the shared index for other repos

read_index_from() takes a path argument for the location of the index file. For reading the shared index in split index mode however it just ignores that path argument, and reads it from the gitdir of the current repository. This works as long as an index in the_repository is read. Once that changes, such as when we read the index of a submodule, or of a different working tree than the current one, the gitdir of the_repository will no longer contain the appropriate shared index, and git will fail to read it. For example t3007-ls-files-recurse-submodules.sh was broken with GIT_TEST_SPLIT_INDEX set in 188dce131f ("ls-files: use repository object", 2017-06-22), and t7814-grep-recurse-submodules.sh was also broken in a similar manner, probably by introducing struct repository there, although I didn't track down the exact commit for that. be489d02d2 ("revision.c: --indexed-objects add objects from all worktrees", 2017-08-23) breaks with split index mode in a similar manner, not erroring out when it can't read the index, but instead carrying on with pruning, without taking the index of the worktree into account. Fix this by passing an additional gitdir parameter to read_index_from, to indicate where it should look for and read the shared index from. read_cache_from() defaults to using the gitdir of the_repository. As it is mostly a convenience macro, having to pass get_git_dir() for every call seems overkill, and if necessary users can have more control by using read_index_from(). Helped-by: Brandon Williams <bmwill@google.com> 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 a125a223347a8d43fffc1b7ec2bec93d88ec17b7
5 files changed +20 -15
cache-tree.c
+1 -1
@@ -608,7 +608,7 @@ int write_index_as_tree(unsigned char *sha1, struct index_state *index_state, co
608
609 newfd = hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
610
611 - entries = read_index_from(index_state, index_path);
611 + entries = read_index_from(index_state, index_path, get_git_dir());
612 if (entries < 0) {
613 ret = WRITE_TREE_UNREADABLE_INDEX;
614 goto out;
cache.h
+3 -2
@@ -364,7 +364,7 @@ extern void free_name_hash(struct index_state *istate);
364 #define active_cache_tree (the_index.cache_tree)
365
366 #define read_cache() read_index(&the_index)
367 -#define read_cache_from(path) read_index_from(&the_index, (path))
367 +#define read_cache_from(path) read_index_from(&the_index, (path), (get_git_dir()))
368 #define read_cache_preload(pathspec) read_index_preload(&the_index, (pathspec))
369 #define is_cache_unborn() is_index_unborn(&the_index)
370 #define read_cache_unmerged() read_index_unmerged(&the_index)
@@ -599,7 +599,8 @@ extern int read_index(struct index_state *);
599 extern int read_index_preload(struct index_state *, const struct pathspec *pathspec);
600 extern int do_read_index(struct index_state *istate, const char *path,
601 int must_exist); /* for testting only! */
602 -extern int read_index_from(struct index_state *, const char *path);
602 +extern int read_index_from(struct index_state *, const char *path,
603 + const char *gitdir);
604 extern int is_index_unborn(struct index_state *);
605 extern int read_index_unmerged(struct index_state *);
606 #define COMMIT_LOCK (1 << 0)
read-cache.c
+13 -10
@@ -1568,7 +1568,7 @@ int hold_locked_index(struct lock_file *lk, int lock_flags)
1568
1569 int read_index(struct index_state *istate)
1570 {
1571 - return read_index_from(istate, get_index_file());
1571 + return read_index_from(istate, get_index_file(), get_git_dir());
1572 }
1573
1574 static struct cache_entry *cache_entry_from_ondisk(struct ondisk_cache_entry *ondisk,
@@ -1824,20 +1824,19 @@ unmap:
1824 * This way, shared index can be removed if they have not been used
1825 * for some time.
1826 */
1827 -static void freshen_shared_index(char *base_sha1_hex, int warn)
1827 +static void freshen_shared_index(const char *shared_index, int warn)
1828 {
1829 - char *shared_index = git_pathdup("sharedindex.%s", base_sha1_hex);
1829 if (!check_and_freshen_file(shared_index, 1) && warn)
1830 warning("could not freshen shared index '%s'", shared_index);
1832 - free(shared_index);
1831 }
1832
1835 -int read_index_from(struct index_state *istate, const char *path)
1833 +int read_index_from(struct index_state *istate, const char *path,
1834 + const char *gitdir)
1835 {
1836 struct split_index *split_index;
1837 int ret;
1838 char *base_sha1_hex;
1840 - const char *base_path;
1839 + char *base_path;
1840
1841 /* istate->initialized covers both .git/index and .git/sharedindex.xxx */
1842 if (istate->initialized)
@@ -1857,16 +1856,17 @@ int read_index_from(struct index_state *istate, const char *path)
1856 split_index->base = xcalloc(1, sizeof(*split_index->base));
1857
1858 base_sha1_hex = sha1_to_hex(split_index->base_sha1);
1860 - base_path = git_path("sharedindex.%s", base_sha1_hex);
1859 + base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_sha1_hex);
1860 ret = do_read_index(split_index->base, base_path, 1);
1861 if (hashcmp(split_index->base_sha1, split_index->base->sha1))
1862 die("broken index, expect %s in %s, got %s",
1863 base_sha1_hex, base_path,
1864 sha1_to_hex(split_index->base->sha1));
1865
1867 - freshen_shared_index(base_sha1_hex, 0);
1866 + freshen_shared_index(base_path, 0);
1867 merge_base_index(istate);
1868 post_read_index_from(istate);
1869 + free(base_path);
1870 return ret;
1871 }
1872
@@ -2521,8 +2521,11 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
2521 ret = write_split_index(istate, lock, flags);
2522
2523 /* Freshen the shared index only if the split-index was written */
2524 - if (!ret && !new_shared_index)
2525 - freshen_shared_index(sha1_to_hex(si->base_sha1), 1);
2524 + if (!ret && !new_shared_index) {
2525 + const char *shared_index = git_path("sharedindex.%s",
2526 + sha1_to_hex(si->base_sha1));
2527 + freshen_shared_index(shared_index, 1);
2528 + }
2529
2530 return ret;
2531 }
repository.c
+1 -1
@@ -229,5 +229,5 @@ int repo_read_index(struct repository *repo)
229 if (!repo->index)
230 repo->index = xcalloc(1, sizeof(*repo->index));
231
232 - return read_index_from(repo->index, repo->index_file);
232 + return read_index_from(repo->index, repo->index_file, repo->gitdir);
233 }
revision.c
+2 -1
@@ -1343,7 +1343,8 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
1343 continue; /* current index already taken care of */
1344
1345 if (read_index_from(&istate,
1346 - worktree_git_path(wt, "index")) > 0)
1346 + worktree_git_path(wt, "index"),
1347 + get_worktree_git_dir(wt)) > 0)
1348 do_add_index_objects_to_pending(revs, &istate);
1349 discard_index(&istate);
1350 }