On 01/21, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@xxxxxxxxx> writes:
>
> > On 01/19, Junio C Hamano wrote:
> >> Thomas Gummerer <t.gummerer@xxxxxxxxx> writes:
> >>
> >> > 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().
> >>
> >> This was a bit painful change, given that some changes in flight do
> >> add new callsites to read_index_from() and they got the function
> >> changed under their feet.
> >
> > Sorry about that. Is there any way to make such a change less painful
> > in the future?
>
> One way is to do for read_index_from() what you did for the existing
> users of read_cache_from(). Introduce a _new_ helper that will not
> be known for any existing topics in flight, and use that to make the
> existing API a thin wrapper around it.
I'll do that next time. My worries were just that the
'read_index_from()' API is broken with split index, which is what this
series fixes. It would be easy to introduce new breakages if we're
not careful.
> I _think_ I got it right with evil merge, so unless this fix needs
> to be delayed for extended period of time for whatever reason while
> any more new callers of the function appears (which is unlikely), we
> should be OK ;-)
Thanks! As mentioned there's one call that was added that the evil
merge didn't get quite right, for which I sent the diff below in my
previous email. But I'm happy to fix that on top once this series
goes to master or next if that's preferred.
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6a49f9e628..4d86a3574f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -612,7 +612,8 @@ static void validate_no_submodules(const struct worktree *wt)
struct index_state istate = {0};
int i, found_submodules = 0;
- if (read_index_from(&istate, worktree_git_path(wt, "index"), get_git_dir()) > 0) {
+ if (read_index_from(&istate, worktree_git_path(wt, "index"),
+ get_worktree_git_dir(wt)) > 0) {
for (i = 0; i < istate.cache_nr; i++) {
struct cache_entry *ce = istate.cache[i];
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index b3105eaaed..8faf61bbf5 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -90,6 +90,16 @@ test_expect_success 'move main worktree' '
test_must_fail git worktree move . def
'
+test_expect_success 'move worktree with split index' '
+ git worktree add test &&
+ (
+ cd test &&
+ test_commit file &&
+ git update-index --split-index
+ ) &&
+ git worktree move test test-destination
+'
+
test_expect_success 'remove main worktree' '
test_must_fail git worktree remove .
'
> Thanks.
