diff mbox series

Strange diff-index with fsmonitor, submodules

Message ID 20230829005606.136615-1-jonathantanmy@google.com (mailing list archive)
State New, archived
Headers show
Series Strange diff-index with fsmonitor, submodules | expand

Commit Message

Jonathan Tan Aug. 29, 2023, 12:56 a.m. UTC
There is a strange interaction where diff-index not only produces
different results when run with and without fsmonitor, but produces
different results for 2 entries that as far as I can tell, should behave
identically (sibling files in the same directory - file_11 and file_12,
and both of these filenames are only mentioned once each in the entire
test).

You can see this with this patch:

error: last command exited with $?=1
not ok 61 - submodule always visited

Notice that with fsmonitor, diff-index reports a "D" line that is not
present when fsmonitor is off. To add to that, it only reports "D" for
file_11 when I would expect that if it reported file_11, it would report
file_12 as well.

I'll continue investigating this myself, but does anyone know what is
going on?

Comments

Junio C Hamano Aug. 29, 2023, 1:20 a.m. UTC | #1
Jonathan Tan <jonathantanmy@google.com> writes:

> There is a strange interaction where diff-index not only produces
> different results when run with and without fsmonitor, but produces
> different results for 2 entries that as far as I can tell, should behave
> identically (sibling files in the same directory - file_11 and file_12,
> and both of these filenames are only mentioned once each in the entire
> test).

Picking those who had non-clean-up changes to fsmonitor out of the
output from

    $ git shortlog -n --since=2.year --no-merges ':(glob)**/*fsmonitor*'

and adding them to CC: for their attention.

> You can see this with this patch:
>
> diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
> index 0c241d6c14..e9e5e32016 100755
> --- a/t/t7527-builtin-fsmonitor.sh
> +++ b/t/t7527-builtin-fsmonitor.sh
> @@ -809,6 +809,11 @@ my_match_and_clean () {
>                 status --porcelain=v2 >actual.without &&
>         test_cmp actual.with actual.without &&
>  
> +       git -C super --no-optional-locks diff-index --name-status HEAD >actual.with &&
> +       git -C super --no-optional-locks -c core.fsmonitor=false \
> +               diff-index --name-status HEAD >actual.without &&
> +       test_cmp actual.with actual.without &&
> +
>         git -C super/dir_1/dir_2/sub reset --hard &&
>         git -C super/dir_1/dir_2/sub clean -d -f
>  }
> @@ -837,6 +842,7 @@ test_expect_success 'submodule always visited' '
>         # some dirt in the submodule and confirm matching output.
>  
>         # Completely clean status.
> +       echo Now running for clean status &&
>         my_match_and_clean &&
>
> and this is the output:
>
> ++ echo Now running for clean status
> Now running for clean status
> ++ my_match_and_clean
> ++ git -C super --no-optional-locks status --porcelain=v2
> ++ git -C super --no-optional-locks -c core.fsmonitor=false status --porcelain=v2
> ++ test_cmp actual.with actual.without
> ++ test 2 -ne 2
> ++ eval 'diff -u' '"$@"'
> +++ diff -u actual.with actual.without
> ++ git -C super --no-optional-locks diff-index --name-status HEAD
> ++ git -C super --no-optional-locks -c core.fsmonitor=false diff-index --name-status HEAD
> ++ test_cmp actual.with actual.without
> ++ test 2 -ne 2
> ++ eval 'diff -u' '"$@"'
> +++ diff -u actual.with actual.without
> --- actual.with	2023-08-29 00:39:26
> +++ actual.without	2023-08-29 00:39:26
> @@ -1 +0,0 @@
> -D	dir_1/file_11
> error: last command exited with $?=1
> not ok 61 - submodule always visited
>
> Notice that with fsmonitor, diff-index reports a "D" line that is not
> present when fsmonitor is off. To add to that, it only reports "D" for
> file_11 when I would expect that if it reported file_11, it would report
> file_12 as well.
>
> I'll continue investigating this myself, but does anyone know what is
> going on?
Jeff Hostetler Aug. 29, 2023, 12:45 p.m. UTC | #2
On 8/28/23 9:20 PM, Junio C Hamano wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
> 
>> There is a strange interaction where diff-index not only produces
>> different results when run with and without fsmonitor, but produces
>> different results for 2 entries that as far as I can tell, should behave
>> identically (sibling files in the same directory - file_11 and file_12,
>> and both of these filenames are only mentioned once each in the entire
>> test).
...
>> I'll continue investigating this myself, but does anyone know what is
>> going on?

Just a quick ACK. I'll take a look.

Thanks
Jeff
Jeff Hostetler Aug. 29, 2023, 4:57 p.m. UTC | #3
On 8/28/23 8:56 PM, Jonathan Tan wrote:
> There is a strange interaction where diff-index not only produces
> different results when run with and without fsmonitor, but produces
> different results for 2 entries that as far as I can tell, should behave
> identically (sibling files in the same directory - file_11 and file_12,
> and both of these filenames are only mentioned once each in the entire
> test).
> 
> You can see this with this patch:

Jonathan, what platform are you seeing this on?

Jeff
Jonathan Tan Aug. 29, 2023, 5:01 p.m. UTC | #4
Jeff Hostetler <git@jeffhostetler.com> writes:
> 
> 
> On 8/28/23 8:56 PM, Jonathan Tan wrote:
> > There is a strange interaction where diff-index not only produces
> > different results when run with and without fsmonitor, but produces
> > different results for 2 entries that as far as I can tell, should behave
> > identically (sibling files in the same directory - file_11 and file_12,
> > and both of these filenames are only mentioned once each in the entire
> > test).
> > 
> > You can see this with this patch:
> 
> Jonathan, what platform are you seeing this on?
> 
> Jeff

Ah, I should have mentioned this. This is on Mac OS X.
diff mbox series

Patch

diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 0c241d6c14..e9e5e32016 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -809,6 +809,11 @@  my_match_and_clean () {
                status --porcelain=v2 >actual.without &&
        test_cmp actual.with actual.without &&
 
+       git -C super --no-optional-locks diff-index --name-status HEAD >actual.with &&
+       git -C super --no-optional-locks -c core.fsmonitor=false \
+               diff-index --name-status HEAD >actual.without &&
+       test_cmp actual.with actual.without &&
+
        git -C super/dir_1/dir_2/sub reset --hard &&
        git -C super/dir_1/dir_2/sub clean -d -f
 }
@@ -837,6 +842,7 @@  test_expect_success 'submodule always visited' '
        # some dirt in the submodule and confirm matching output.
 
        # Completely clean status.
+       echo Now running for clean status &&
        my_match_and_clean &&

and this is the output:

++ echo Now running for clean status
Now running for clean status
++ my_match_and_clean
++ git -C super --no-optional-locks status --porcelain=v2
++ git -C super --no-optional-locks -c core.fsmonitor=false status --porcelain=v2
++ test_cmp actual.with actual.without
++ test 2 -ne 2
++ eval 'diff -u' '"$@"'
+++ diff -u actual.with actual.without
++ git -C super --no-optional-locks diff-index --name-status HEAD
++ git -C super --no-optional-locks -c core.fsmonitor=false diff-index --name-status HEAD
++ test_cmp actual.with actual.without
++ test 2 -ne 2
++ eval 'diff -u' '"$@"'
+++ diff -u actual.with actual.without
--- actual.with	2023-08-29 00:39:26
+++ actual.without	2023-08-29 00:39:26
@@ -1 +0,0 @@ 
-D	dir_1/file_11