On Tue, Jun 5, 2018 at 12:14 AM, Elijah Newren <newren@xxxxxxxxx> wrote: > On Sun, Jun 3, 2018 at 8:19 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> Elijah Newren <newren@xxxxxxxxx> writes: >> >>> `git merge-recursive` does a three-way merge between user-specified trees >>> base, head, and remote. Since the user is allowed to specify head, we can >>> not necesarily assume that head == HEAD. >>> >>> We modify index_has_changes() to take an extra argument specifying the >>> tree to compare the index to. If NULL, it will compare to HEAD. We then >>> use this from merge-recursive to make sure we compare to the >>> user-specified head. >>> >>> Signed-off-by: Elijah Newren <newren@xxxxxxxxx> >>> --- >>> >>> I'm really unsure where the index_has_changes() declaration should go; >>> I stuck it in tree.h, but is there a better spot? >> >> I think I saw you tried to lift an assumption that we're always >> working on the_index in a separate patch recently. Should that >> logic apply also to this part of the codebase? IOW, shouldn't >> index_has_changes() take a pointer to istate (as opposed to a >> function that uses the implicit the_index that should be named as >> "cache_has_changes()" or something?) >> >> I tend to think this function as part of the larger read-cache.c >> family whose definitions are in cache.h and accompanied by macros >> that are protected by NO_THE_INDEX_COMPATIBILITY_MACROS so if we >> were to move it elsewhere, I'd keep the header part as-is and >> implementation to read-cache.c to keep it together with the family, >> but I do not see a huge issue with the current placement, either. > > That's good point; the goal to lift assumptions on the_index should > probably also apply here. I'll make the change. > (And it was actually Duy's patch that I was reviewing, but close > enough.) I'll take a look at moving it to read-cache.c as well. Making it not depend on the_index will require changes to make diff-lib.c not depend on the_index first, so this is going to have to wait for Duy's changes mentioned at https://public-inbox.org/git/CACsJy8Ba74iSPf4_zFxuV=_uNJgL6Z2QunOvAvi3qab-6EWi5g@xxxxxxxxxxxxxx/. I'll re-roll this series on top of Duy's when it comes out.
