On Fri, Aug 19, 2022 at 2:22 AM Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
>
> Hi Elijah,
>
> On Fri, 19 Aug 2022, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@xxxxxxxxx>
> >
> > Commit 66b209b86a ("merge-ort: implement CE_SKIP_WORKTREE handling with
> > conflicted entries", 2021-03-20) added some code for merge-ort to handle
> > conflicted and skip_worktree entries in general. Included in this was
> > an ugly hack for dealing with present-despite-skipped entries and a
> > testcase (t6428.2) specific to that hack, since at that time users could
> > accidentally get files into that state when using a sparse checkout.
> >
> > However, with the merging of 82386b4496 ("Merge branch
> > 'en/present-despite-skipped'", 2022-03-09), that class of problems was
> > addressed globally and in a much cleaner way. As such, the
> > present-despite-skipped hack in merge-ort is no longer needed and can
> > simply be removed.
> >
> > No additional testcase is needed here; t6428.2 was written to test the
> > necessary functionality and is being kept. The fact that this test
> > continues to pass despite the code being removed shows that the extra
> > code is no longer necessary.
> >
> > Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> > ---
> > merge-ort: remove code obsoleted by other changes
> >
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1302%2Fnewren%2Fnuke-present-despite-skipped-hack-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1302/newren/nuke-present-despite-skipped-hack-v1
> > Pull-Request: https://github.com/git/git/pull/1302
> >
> > merge-ort.c | 22 ++--------------------
> > 1 file changed, 2 insertions(+), 20 deletions(-)
>
> Nice!
>
> Since I have been in the `merge-ort` code quite a bit as of late, I deem
> myself familiar enough with the code to dare offering my ACK.
Thanks for taking a look.
> What is _particularly_ nice is that this patch removes an `lstat()` call
> that was a bit of a concern for me when using `merge-ort` in a
> worktree-less scenario. After some reasoning about the code, it turned out
> that it is not hit in that use case, nevertheless it is much easier to
> reason about `lstat()` calls that simply are not in the code.
Oh cool, glad there are other benefits too!
