Re: [PATCH v12 04/20] index-helper: new daemon for caching index and related stuff

On Thu, May 19, 2016 at 11:45 PM, David Turner [off-list ref] wrote:

 .gitignore                             |   1 +
 Documentation/git-index-helper.txt     |  50 ++++++
 Makefile                               |   5 +
 cache.h                                |  11 ++
 contrib/completion/git-completion.bash |   1 +
 git-compat-util.h                      |   1 +
 index-helper.c                         | 280 +++++++++++++++++++++++++++++++++
 read-cache.c                           | 125 +++++++++++++--
 t/t7900-index-helper.sh                |  23 +++
 9 files changed, 488 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/git-index-helper.txt
 create mode 100644 index-helper.c
 create mode 100755 t/t7900-index-helper.sh

We still need to classify this new command in command-list.txt.
purehelpers probably the easiest choice.

+NOTES
+-----
+
+$GIT_DIR/index-helper.sock a Unix domain socket that the daemon reads
+commands from.  The directory will also contain files named
+"shm-index-<SHA1>".  These are used as backing stores for shared
+memory.  Normally the daemon will clean up these files when it exits
+or when they are no longer relevant.  But if it crashes, some objects
+could remain there and they can be safely deleted with "rm"
+command. The following commands are used to control the daemon:
+
+"refresh"::
+       Reread the index.
+
+"poke":
+       Let the daemon know the index is to be read. It keeps the
+       daemon alive longer, unless `--exit-after=0` is used.
+
+All commands and replies are terminated by a NUL byte.

I think it's "all commands are replies are in pkt-line format" now.

We need unix-socket.c too, which is only built when NO_UNIX_SOCKETS
are not defined. We don't have any platform where mmap is supported
but unix socket is not, do we?

(a couple of minutes later, reading to read-cache.c..) well
poke_daemon() calls unix_stream_connect() without any #ifdef, if
NO_UNIX_SOCKETS is set, then it would lead to undefined reference
error.

+static void set_socket_blocking_flag(int fd, int make_nonblocking)
+{
+       int flags;
+
+       flags = fcntl(fd, F_GETFL, NULL);
+
+       if (flags < 0)
+               die(_("fcntl failed"));
+
+       if (make_nonblocking)
+               flags |= O_NONBLOCK;
+       else
+               flags &= ~O_NONBLOCK;
+
+       if (fcntl(fd, F_SETFL, flags) < 0)
+               die(_("fcntl failed"));
+}
...
+               client_fd = accept(fd, NULL, NULL);
+               if (client_fd < 0)
+                       /*
+                        * An error here is unlikely -- it probably
+                        * indicates that the connecting process has
+                        * already dropped the connection.
+                        */
+                       continue;
+
+               /*
+                * Our connection to the client is blocking since a client
+                * can always be killed by SIGINT or similar.
+                */
+               set_socket_blocking_flag(client_fd, 0);

Out of curiosity, do we really need this? I thought default behavior
was always blocking (and checked linux kernel, it seemed to agree with
me). Maybe for extra safety because other OSes may default to
something else?

+static int poke_daemon(struct index_state *istate,
+                      const struct stat *st, int refresh_cache)
+{
+       int fd;
+       int ret = 0;
+       const char *socket_path;
+
+       /* if this is from index-helper, do not poke itself (recursively) */
+       if (istate->to_shm)
+               return 0;
+
+       socket_path = git_path("index-helper.sock");
+       if (!socket_path)
+               return -1;

This "if" is later removed. But you can remove it now if you re-roll,
git_path() must not fail or we are screwed up long before this.
-- 
Duy