Re: [PATCH v11 00/12] fsmonitor: implement filesystem change listener for Linux

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

>> > So please take a bit more time to work feedback into your patch series
>> > before sending out the next version. It's totally fine to wait a couple
>> > days between iterations.
>> >
>> > Also, could you please clarify whether the patch series has been written
>> > by AI and if so, which parts of it are?
>> >
>> > Thanks!
>> >
>> > Patrick
>> 
>> The topic went totally silent after this message; is this topic
>> still viable?
>
> I didn't have the capacity recently to focus on this patch series.
>
> Also, I think we need to be extra careful with merging this patch series
> as it's written mostly by AI as far as I understood. So I don't quite
> feel comfortable to ACK this series by myself, as I'm not sure to what
> degree the code is understood and reasoned about by anyone.
>
> I'm not against merging it in general, but if we want to merge it I
> would like to have at least one more thorough review by somebody who
> understands this area before I feel comfortable committing to it and
> maintaining the code going forward.

I share that exact feeling.  After all, the section recently added
to SubmittingPatches about AI-generated code was to address this
kind of patches.

In any case, when this topic is merged to 'seen', it seem to break
linux-TEST-vars CI job, which essentially runs the build and test
with these settings:

        export OPENSSL_SHA1_UNSAFE=YesPlease
        export GIT_TEST_SPLIT_INDEX=yes
        export GIT_TEST_FULL_IN_PACK_ARRAY=true
        export GIT_TEST_OE_SIZE=10
        export GIT_TEST_OE_DELTA_SIZE=5
        export GIT_TEST_COMMIT_GRAPH=1
        export GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1
        export GIT_TEST_MULTI_PACK_INDEX=1
        export GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL=1
        export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
        export GIT_TEST_NO_WRITE_REV_INDEX=1
        export GIT_TEST_CHECKOUT_WORKERS=2
        export GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL=1

and t9210 fails with this topic merged (abfd972), and succeeds
without (4b8355c6f8).

  https://github.com/git/git/actions/runs/23775508256/job/69276399578#step:10:562
  https://github.com/git/git/actions/runs/23777158230/job/69281498657    

I also reproduced the breakage locally on a debian-testing derivative.

Help to diagnose and pinpoint the bad interactions with topics in
flight is greatly appreciated.


! [abfd972] Merge branch 'pt/fsmonitor-linux' into seen
 ! [4b8355c6f8] Merge branch 'ps/receive-pack-updateinstead-in-worktree' into seen
--
-  [abfd972] Merge branch 'pt/fsmonitor-linux' into seen
+  [abfd972^2] fsmonitor: convert shown khash to strset in do_handle_client
+  [abfd972^2^] fsmonitor: add tests for Linux
+  [abfd972^2~2] fsmonitor: add timeout to daemon stop command
+  [abfd972^2~3] fsmonitor: close inherited file descriptors and detach in daemon
+  [abfd972^2~4] run-command: add close_fd_above_stderr option
+  [abfd972^2~5] fsmonitor: implement filesystem change listener for Linux
+  [abfd972^2~6] fsmonitor: rename fsm-settings-darwin.c to fsm-settings-unix.c
+  [abfd972^2~7] fsmonitor: rename fsm-ipc-darwin.c to fsm-ipc-unix.c
+  [abfd972^2~8] fsmonitor: use pthread_cond_timedwait for cookie wait
+  [abfd972^2~9] compat/win32: add pthread_cond_timedwait
+  [abfd972^2~10] fsmonitor: fix hashmap memory leak in fsmonitor_run_daemon
+  [abfd972^2~11] fsmonitor: fix khash memory leak in do_handle_client
-- [4b8355c6f8] Merge branch 'ps/receive-pack-updateinstead-in-worktree' into seen