On Fri, Jul 20, 2018 at 3:05 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Elijah Newren <newren@xxxxxxxxx> writes: > >> Ah, okay, that's helpful. So, if there are conflicts, it should be >> free to clear the skip_worktree flag. Since merge-recursive calls >> add_cacheinfo() for all entries it needs to update, which deletes the >> old cache entry and just makes new ones, we get that for free. > > Correct. > >> And conversely, if a file-level merge succeeds without conflicts then >> it clearly doesn't "need to materialize a working tree file", so it >> should NOT clear the skip_worktree flag for that path. > > That is not at all implied by what I wrote, though. > > If it can be done without too much effort, then it certainly is > nicer to keep the sparseness when we do not have to materialize the > working tree file. But at least in my mind, if it needs too many > special cases, hacks, and conditionals, then it is not worth the > complexity---if it is easier to write a correct code by allowing Git > to populate working tree files, it is perfectly fine to do so. > > In a sense, the sparse checkout "feature" itself is a hack by > itself, and that is why I think this part should be "best effort" as > well. That's good to know, but I don't think we can back out easily: - Clearing the skip_worktree bit: no big deal, as you mention above - Avoiding working tree updates when merge doesn't change them: very desirable[1] - Doing both: whoops [1] https://public-inbox.org/git/CA+55aFzLZ3UkG5svqZwSnhNk75=fXJRkvU1m_RHBG54NOoaZPA@xxxxxxxxxxxxxx/ I don't want to regress the bug Linus reported, so to fix Ben's issue, when we detect that a path's contents/mode won't be modified by the merge, we can either: - Update the working tree file if the original cache entry had the skip_worktree flag set - Mark the new cache entry as skip_worktree if the original cache entry had the skip_worktree flag set Both should be about the same amount of work; the first seems weird and confusing for future readers of the code. The second makes sense, but probably should be accompanied with a note in the code about how there are other codepaths that could consider skip_worktree too. I'll see if I can put something together, but I have family flying in tomorrow, and then am out on vacation Mon-Sat next week, so...
