Re: [PATCH v4 0/7] Avoid multiple recursive calls for same path in read_directory_recursive()

From: Elijah Newren <newren@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Martin Melka" <martin.melka@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Samuel Lijin" <sxlijin@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH v4 0/7] Avoid multiple recursive calls for same path in read_directory_recursive()
Date: Sat, 28 Mar 2020 10:33:10 -0700	[thread overview]
Message-ID: <CABPp-BFXWx_MPL1Z0-X5hHf0N1T_13TEbAhVTKh6-atO1+g90Q@mail.gmail.com> (raw)
In-Reply-To: <ad7c7cd7-2f2e-d406-6a1f-9dd7f155dc10@gmail.com>

On Fri, Mar 27, 2020 at 6:13 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 3/26/2020 5:27 PM, Elijah Newren via GitGitGadget wrote:
> > This series provides some "modest" speedups (see last commit message), and
> > should allow 'git status --ignored' to complete in a more reasonable
> > timeframe for Martin Melka (see
> > https://lore.kernel.org/git/CANt4O2L_DZnMqVxZzTBMvr=BTWqB6L0uyORkoN_yMHLmUX7yHw@mail.gmail.com/
> > ).
> >
> > Changes since v3:
> >
> >  * Turns out I was wrong about the untracked cache stuff and had some bugs
> >    around untracked directories with nothing bug ignored sub-entries.
> >  * First patch now is no longer a change of expectation of the untracked
> >    cache, but some more thorough testing/verification in that test that
> >    helped explain my misunderstanding and uncover the bug in my refactor.
> >  * Corrected the check_only and stop_at_first_file logic in the last patch
> >    and added a big comment explaining how/why it all works. Also stopped
> >    disabling part of the untracked cache in the same patch, and undid all
> >    the changes to t7063 in that patch.
> >
> > Stuff still missing from v4:
> >
> >  * I didn't make the DIR_KEEP_UNTRACKED_CONTENTS changes I mentioned in
> >    https://lore.kernel.org/git/CABPp-BEQ5s=+6Rnb-A+pdEaoPXxfo-hMSegSe1eai=RE74A3Og@mail.gmail.com/
> >    which I think would make the code cleaner & clearer. I guess I'm leaving
> >    that for future work.
> >
> > As per the commit message of the final patch, this series has some risk.
> > Extra eyes would be greatly appreciated; one pair already helped me find one
> > bug.
>
> I'm glad that I could help you discover mixed expectations. This pair of eyes
> is now satisfied with this series to the extent I can check it.
>
> Adding the previous patch to our microsoft/git fork pass the functional tests
> in Scalar and VFS for Git, for what it's worth:
>
> [1] https://github.com/microsoft/scalar/pull/358
> [2] https://github.com/microsoft/VFSForGit/pull/1646

Thanks, that helps.

An update of my own for this series: Based on Felipe's reported
bash-completion issue I was modifying commands to try out a number of
other things and discovered some cases that can trigger the die("git
ls-files: internal error - directory entry not superset of prefix")
message from ls-files so there's still some fixes I need to make.
Will send an update when I've got it.