FSMonitor by dscho · Pull Request #273 · microsoft/git

dscho · 2020-06-01T20:08:34Z

This PR supersedes #256, and is rebased on top of vfs-2.27.0.

On purpose, it merges the FSMonitor branch thicket that is based on Git v2.27.0, to maintain easy "upstreamability".

In 897d68e (Makefile: use curl-config --cflags, 2020-03-26), we taught the build process to use `curl-config --cflags` to make sure that it can find cURL's headers. In the MSVC build, this is completely bogus because we're running in a Git for Windows SDK whose `curl-config` supports the _GCC_ build. Let's just ignore each and every `-I<path>` option where `<path>` points to GCC/Clang specific headers. Reported by Jeff Hostetler in #275. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Teach packet_write_gently() to use a stack buffer rather than a static buffer when composing the packet line message. This helps get us ready for threaded operations. Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>

So far, the (possibly indirect) callers of `get_packet_data()` can ask that function to return an error instead of `die()`ing upon end-of-file. However, when we call this function in a long-running daemon, we absolutely want the daemon to live, still, even if there was a read error when one random read failed. So let's introduce an explicit option to tell the packet reader machinery to please be nice and only return an error. This change prepares pkt-line for the internal fsmonitor (which is precisely such a daemon). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

…uf() This function currently has only one caller: `apply_multi_file_filter()` in `convert.c`. That caller wants a flush packet to be written after writing the payload. However, we are about to introduce a user that wants to write many packets before a final flush packet, so let's extend this function to prepare for that scenario. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The `read_packetized_to_strbuf()` function reads packets into a strbuf until a flush packet has been received. So far, it has only one caller: `apply_multi_file_filter()` in `convert.c`. This caller really only needs the `PACKET_READ_GENTLE_ON_EOF` option to be passed to `packet_read()` (which makes sense in the scenario where packets should be read until a flush packet is received). We are about to introduce a caller that wants to pass other options through to `packet_read()`, so let's extend the function signature accordingly. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Fix the MSVC build

sluongng · 2020-06-14T19:04:11Z

Few questions:

Do you guys have any intention to upstream this to Git Core?
I see that aside from Window and MacOS, there is also Linux support? Does that mean this patch works on Linux? This concern arose from this being a fork from git-for-windows/git.

dscho · 2020-06-15T00:20:09Z

Do you guys have any intention to upstream this to Git Core?

Yes.

See #277 for the first part. We're still working on it.

I see that aside from Window and MacOS, there is also Linux support? Does that mean this patch works on Linux? This concern arose from this being a fork from git-for-windows/git.

Let me clarify one big misunderstanding: Git for Windows' main branch compiles on Windows, macOS and Linux, and passes the test suite. This has always been our aim. Almost all of the patches are meant for upstream (with the obvious exceptions as the changed README.md which reflects the nature of the fork).

So yes, there is experimental Linux support (with one known bug: when auto-spawning the daemon, the process is sometimes daemonize()d too fast, so that the caller cannot connect to the just-spawned daemon. This will need to be fixed somehow, but we're tackling other problems first.

derrickstolee · 2020-06-15T08:36:25Z

I see that aside from Window and MacOS, there is also Linux support? Does that mean this patch works on Linux? This concern arose from this being a fork from git-for-windows/git.

Let me clarify one big misunderstanding: Git for Windows' main branch compiles on Windows, macOS and Linux, and passes the test suite. This has always been our aim. Almost all of the patches are meant for upstream (with the obvious exceptions as the changed README.md which reflects the nature of the fork).

We have also started releasing .deb packages at https://github.com/microsoft/git/releases

For the upcoming `fsmonitor--daemon` command, we need some way to send a command from some Git process to that daemon, and to receive an answer. In other words, we need to support Inter-Process Communication. By nature, Inter-Process Communication requires OS-specific implementations. Instead of using full-blown TCP/UDP socket communication (which comes with all kinds of challenges such as: restricting access permissions, collisions when multiple instances might want to listen on the same port, etc), we use named pipes on Windows, and plan on implementing a backend based on Unix sockets for Linux/Unix later. For now, we use a super-simple protocol: identified by a path into .git/, the daemon listens for clients, reads one command, then sends the answer and closes the connection. To avoid redundant coding patterns, we use the `pkt-line` protocol that we just libified to be able to use here: we really, really do not want our simple IPC client to take down the entire Git process with it when it could not talk to `fsmonitor--daemon`, but would rather instead fall back to a full scan of all tracked files. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

We previously implemented simple Inter-Process Communication (IPC) on Windows using named pipes. Create an equivalent implementation for Unix-based platforms using Unix sockets. Note that there are a variety of differences between Win32 Named Pipes and Unix sockets, for example: - a Win32 Named Pipe vanishes once the listener handle is closed. Unix sockets do not just disappear, even if the process that created them exited. - Win32 Named Pipes exist in a name space parallel to the actual file system. In contrast, Unix sockets live in the same name space, i.e. a Unix socket cannot have the same path as an existing file and vice versa. - Win32 Named Pipes have two communication modes: "message" and "byte". The former allows senders to define the size of messages to be sent; the receiving end will not receive partial messages. Unix sockets only operate in the mode equivalent to "byte". - When a listener accepts a client via a Win32 Named Pipe, subsequent communication will use the same handle to the same pipe, i.e. it can become a bit tricky to keep the communication to several concurrent clients apart from one another. When a listener on a Unix socket accepts a new client, it gets handed a new file descriptor specific to that client. The tests in `t0052-simple-ipc.sh` will automatically start running on Unix platforms with this change due to the response from `test-tool simple-ipc SUPPORTS_SIMPLE_IPC` changing automatically with the change here in `simple-ipc.h`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This adds a very rudimentary test, just to make sure that the simple Inter-Process Communication (where supported) works as intended (and maybe also serving as an example how the command listener is supposed to be implemented). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

prevent multiple listeners associated with the same path Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Kevin Willford <Kevin.Willford@microsoft.com>

TODO details here Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>

Just _starting_ to fix up the Unix Sockets part to match the new internal simple IPC API. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This topic branch implements a framework for a simple Inter-Process Communication API, using Unix sockets on Linux/Unix/macOS, and Named Pipes on Windows. This prepares for the upcoming patch series to add a built-in, Git-aware FSMonitor. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

The idea is to provide an efficient default implementation of the `fsmonitor` hook for the major platforms, starting on the platform that benefits from it most: Windows (because there is no native `stat()`, it has to be emulated, which means that refreshing the Git index is a rather hefty operation on that platform). Originally, the intention of the `fsmonitor` extension was quite obviously to use Facebook's `watchman`. However, it turned out that it is just too hard to make it work reliably on Windows: none of the core `watchman` developers has access to a Windows machine, and it is even non-trivial to make the project build on Windows (which is not helped by the hierarchy of build dependencies). Besides: when it comes to Git's `fsmonitor` hook, `watchman` does both too much (it allows pretty complicated queries, which we do not need in the `fsmonitor` hook) and too little (`watchman` is totally unaware of Git's internals, e.g. cannot use the `untracked` extension to initialize its state efficiently upon start-up, therefore the first `git status` is still slow, even if `watchman` is used via the `fsmonitor`). Given all these considerations, it makes sense to "roll our own". This commit merely adds the command without any functionality; Over the course of the next commits we will implement it in bits and pieces. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

This patch adds a scaffolding for an internal Git-aware FSMonitor, in preparation for adding platform-specific backends. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

By nature, the backends for the fsmonitor daemon need to be platform-specific: there is no common API (not even a POSIX one) for monitoring directories for changes. This patch adds the first such backend: a Windows one, based on the Win32 API function `ReadDirectoryChangesW()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>