git/compat/fsmonitor, branch jch

global: trivial conversions to fix `-Wsign-compare` warnings

2024-12-06T11:20:04Z

We have a bunch of loops which iterate up to an unsigned boundary using a signed index, which generates warnigs because we compare a signed and unsigned value in the loop condition. Address these sites for trivial cases and enable `-Wsign-compare` warnings for these code units. This patch only adapts those code units where we can drop the `DISABLE_SIGN_COMPARE_WARNINGS` macro in the same step. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: mark code units that generate warnings with `-Wsign-compare`

2024-12-06T11:20:02Z

Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

Merge branch 'ak/typofixes'

2024-10-25T18:02:04Z

Typofixes. * ak/typofixes: t: fix typos t/helper: fix a typo t/perf: fix typos t/unit-tests: fix typos contrib: fix typos compat: fix typos

Merge branch 'jk/fsmonitor-event-listener-race-fix'

2024-10-15T20:56:43Z

On macOS, fsmonitor can fall into a race condition that results in a client waiting forever to be notified for an event that have already happened. This problem has been corrected. * jk/fsmonitor-event-listener-race-fix: fsmonitor: initialize fs event listener before accepting clients simple-ipc: split async server initialization and running

compat: fix typos

2024-10-10T20:31:12Z

Fix typos and grammar. Reported-by: Patrick Steinhardt Signed-off-by: Andrew Kreimer Signed-off-by: Junio C Hamano

fsmonitor: initialize fs event listener before accepting clients

2024-10-08T19:03:56Z

There's a racy hang in fsmonitor on macOS that we sometimes see in CI. When we serve a client, what's supposed to happen is: 1. The client thread calls with_lock__wait_for_cookie() in which we create a cookie file and then wait for a pthread_cond event 2. The filesystem event listener sees the cookie file creation, does some internal book-keeping, and then triggers the pthread_cond. But there's a problem: we start the listener that accepts client threads before we start the fs event thread. So it's possible for us to accept a client which creates the cookie file and starts waiting before the fs event thread is initialized, and we miss those filesystem events entirely. That leaves the client thread hanging forever. In CI, the symptom is that t9210 (which is testing scalar, which always enables fsmonitor under the hood) may hang forever in "scalar clone". It is waiting on "git fetch" which is waiting on the fsmonitor daemon. The race happens more frequently under load, but you can trigger it predictably with a sleep like this, which delays the start of the fs event thread: --- a/compat/fsmonitor/fsm-listen-darwin.c +++ b/compat/fsmonitor/fsm-listen-darwin.c @@ -510,6 +510,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) FSEventStreamSetDispatchQueue(data->stream, data->dq); data->stream_scheduled = 1; + sleep(1); if (!FSEventStreamStart(data->stream)) { error(_("Failed to start the FSEventStream")); goto force_error_stop_without_loop; One solution might be to reverse the order of initialization: start the fs event thread before we start the thread listening for clients. But the fsmonitor code explicitly does it in the opposite direction. The fs event thread wants to refer to the ipc_server_data struct, so we need it to be initialized first. A further complication is that we need a signal from the fs event thread that it is actually ready and listening. And those details happen within backend-specific fsmonitor code, whereas the initialization is in the shared code. So instead, let's use the ipc_server init/start split added in the previous commit. The generic fsmonitor code will init the ipc_server but _not_ start it, leaving that to the backend specific code, which now needs to call ipc_server_start_async() at the right time. For macOS, that is right after we start the FSEventStream that you can see in the diff above. It's not clear to me if Windows suffers from the same problem (and we simply don't trigger it in CI), or if it is immune. Regardless, the obvious place to start accepting clients there is right after we've established the ReadDirectoryChanges watch. This makes the hangs go away in our macOS CI environment, even when compiled with the sleep() above. Helped-by: Koji Nakamaru Signed-off-by: Jeff King Acked-by: Koji Nakamaru Signed-off-by: Junio C Hamano

global: prepare for hiding away repo-less config functions

2024-08-13T17:01:05Z

We're about to hide config functions that implicitly depend on `the_repository` behind the `USE_THE_REPOSITORY_VARIABLE` macro. This will uncover a bunch of dependents that transitively relied on the global variable, but didn't define the macro yet. Adapt them such that we define the macro to prepare for this change. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

compat/fsmonitor: fix socket path in networked SHA256 repos

2024-06-14T17:26:34Z

The IPC socket used by the fsmonitor on Darwin is usually contained in the Git repository itself. When the repository is hosted on a networked filesystem though, we instead create the socket path in the user's home directory or the socket directory. In that case, we derive the path by hashing the repository path. But while we always use SHA1 to hash the repository path, we then end up using `hash_to_hex()` to append the computed hash to the socket path. This is wrong because `hash_to_hex()` uses the hash algorithm configured in `the_repository`, which may not be SHA1. The consequence is that we may append uninitialized bytes to the path when operating in a SHA256 repository. Fix this bug by using `hash_to_hex_algop()` with SHA1. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

fsmonitor--daemon.h: remove unnecessary includes

2023-12-26T20:04:32Z

The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano

fsmonitor/darwin: mark unused parameters in system callback

2023-09-18T22:56:15Z

We pass fsevent_callback() to the system FSEventStreamCreate() function as a callback. So we must match the expected function signature, even though we don't care about all of the parameters. Mark the unused ones to satisfy -Wunused-parameter. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano