RE: sb/submodule-move-nested breaks t7411 under GIT_FSMONITOR_TEST

From: Ben Peart <ben.peart@microsoft.com>
Date: Sep 6, 2018, 19:03 UTC
Message-ID: <MW2PR2101MB0970C7744A6FA72940DCB252F4010@MW2PR2101MB0970.namprd21.prod.outlook.com>
In-Reply-To: <CAGZ79kYKzrRXy+GUCpMN3jpo4MvcpGvBFvEkTjrsy85XJb0K2A@mail.gmail.com>

Show 69 quoted lines

> -----Original Message-----
> From: Stefan Beller <sbeller@google.com>
> Sent: Thursday, September 6, 2018 12:57 PM
> To: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Cc: Junio C Hamano <gitster@pobox.com>; git <git@vger.kernel.org>; Ben
> Peart <Ben.Peart@microsoft.com>
> Subject: Re: sb/submodule-move-nested breaks t7411 under
> GIT_FSMONITOR_TEST
> 
> > > Will debug further.
> >
> > I spotted this again after testing the split index (see
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpublic-
> inbox.org%2Fgit%2F87va7ireuu.fsf%40evledraar.gmail.com%2F&amp;data=0
> 2%7C01%7CBen.Peart%40microsoft.com%7C27c901f198a24e5a045a08d6141
> 9ce42%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636718498401
> 788697&amp;sdata=JfavHtKafiJRAVnvMXU3nw1RI27G4OFxs3Ymt7STbvM%3
> D&amp;reserved=0) and
> > was testing the fsmonitor test mode as well.
> >
> > So gentle *poke*: Did you get anywhere with debugging this? It's still
> > failing on "master" now.
> 
> I started looking into this again, help would be appreciated, as I do not
> quite understand the fsmonitor part.
> 
> The error is in the setup, where we have "git mv a b"
> 
> GIT_TRACE_FSMONITOR=1 GIT_TRACE=1
> GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all
> ./t7411-submodule-config.sh -d -i -v -x
> ++ git mv a b
> trace: built-in: git mv a b
> read fsmonitor extension successful
> add fsmonitor
> refresh fsmonitor
> trace: run_command: cd '/u/git/t/trash
> directory.t7411-submodule-config/super'; /u/git/t/t7519/fsmonitor-all
> 1 1536252819824793728
> fsmonitor process '/u/git/t/t7519/fsmonitor-all' returned success
> mark_fsmonitor_clean '.gitmodules'
> write fsmonitor extension successful
> 
> or with more trace_printfs littered through the code
> (https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub
> .com%2Fstefanbeller%2Fgit%2Ftree%2Fsubmodule_fsmoitor_debug&amp;d
> ata=02%7C01%7CBen.Peart%40microsoft.com%7C27c901f198a24e5a045a08
> d61419ce42%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6367184
> 98401788697&amp;sdata=poAUFlVDkoNP56t1hmdxHHyChrH05cut1iSVt70yJ
> eo%3D&amp;reserved=0)
> 
> trace: built-in: git mv a b
> read fsmonitor extension successful
> add fsmonitor
> refresh fsmonitor
> trace: run_command: cd '/u/git/t/trash
> directory.t7411-submodule-config/super'; /u/git/t/t7519/fsmonitor-all
> 1 1536252497951329341
> fsmonitor process '/u/git/t/t7519/fsmonitor-all' returned success
> need to stage .gitmodules
> calling add_file_to_index
> 
> 
> I suspect that the FSMONITOR API is handled wrongly by the part of git-mv
> that writes out the .gitmodules file (if needed) and the .git/index (as needed).
> 
> Ben, do you have an idea?
>

I'll take a look as soon as I can (and at the other fsmonitor test issue Ævar sent email about) but it may be a few days before I get a chance.

I haven't had a chance to look into this yet but here are a couple of SWAG's I'd start with:

I wonder if there is a missing call to mark_fsmonitor_invalid() in the "git mv" codepath somewhere.

refresh_fsmonitor() only runs once per git command - is it possible that "git mv" is triggering it early, making a change, and then updating the index with stale fsmonitor data?

Sorry, I'll look as soon as I can.

Ben

> Thanks,
> Stefan