Re: Bug in index-helper/watchman?

...and somehow I forgot to CC git@vger, grrrr....

On Thu, Jul 14, 2016 at 5:14 PM, Duy Nguyen [off-list ref] wrote:

Bug reports should always be in public (so people are aware of it)
unless it's security exploit of course.

I have not read this mail yet, but I will in a couple of hours, hopefully.

On Thu, Jul 14, 2016 at 12:11 AM, Ben Peart [off-list ref] wrote:

quoted

I’ve been chasing down an issue where it looks like the untracked cache
logic doesn’t work correctly in the index-helper/watchman patch series.
It’s also entirely possible that I’m just missing something so feel free to
correct my misconceptions.



Ultimately, it appears that none of the untracked cache directories are
getting flagged as invalid from the data in the watchman extension.  I
believe this is happening because untracked->root doesn’t get initialized
until validate_untracked_cache is called from read_directory.  This causes
all calls to lookup_untracked to return NULL so the dir->valid flag is never
set to zero in mark_untracked_invalid.  See the call stacks and sequence
below for details:





cmd_status at builtin/commit.c:1362

              status_init_config (s=0x6667a0 <s>, fn=0x432790
<git_status_config>) at builtin/commit.c:187

                             gitmodules_config () at submodule.c:196

                                           read_index (istate=0x693860
<the_index>) at read-cache.c:1442

                                                          read_index_from
(istate=0x693860 <the_index>, path=0x2ea3c58 ".git/index") at
read-cache.c:1849


do_read_index


read_index_extension


read_watchman_ext


mark_untracked_invalid


find_untracked_cache_dir


lookup_untracked


if (!dir)


return NULL;



              wt_status_collect (s=0x6667a0 <s>) at wt-status.c:627

                             wt_status_collect_untracked (s=0x6667a0 <s>) at
wt-status.c:593

                                           fill_directory (dir=0xbafab0,
pathspec=0x6667b8 <s+24>) at dir.c:191

                                                          read_directory
(dir=0xbafab0, path=0x61cac3 <atat+155> "", len=0, pathspec=0x6667b8 <s+24>)
at dir.c:2009


validate_untracked_cache at dir.c:1903


if (!dir->untracked->root)


read_directory_recursive


open_cached_dir


valid_cached_dir


read_directory_recursive


open_cached_dir


valid_cached_dir


if (dir->untracked->use_watchman)





If I’m reading this correctly, one potential fix is to move the logic that
loops through the directories calling mark_untracked_invalid to between the
call to validate_untracked_cache and the call to read_directory_recursive.
I wonder if there is another simpler/better fix.



Thoughts?



Ben

--
Duy

-- 
Duy