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 <pclouds@gmail.com> wrote:
Show 125 quoted lines
> 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 <peartben@gmail.com> wrote:
>> 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