Merge branch 'jt/subprocess-handshake'
authorJunio C Hamano <gitster@pobox.com>
Fri, 11 Aug 2017 20:27:05 +0000 (13:27 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 11 Aug 2017 20:27:05 +0000 (13:27 -0700)
Code cleanup.

* jt/subprocess-handshake:
sub-process: refactor handshake to common function
Documentation: migrate sub-process docs to header

1  2 
convert.c
sub-process.c
sub-process.h
diff --combined convert.c
index dbdbb24e4d37e1f4d4db8e479d48125c90a071a4,4936fcc26b907ca6338ef74bcadc03079b3a3075..1012462e3c9c114ad1b2374a2ae27568ace57bac
+++ b/convert.c
@@@ -513,78 -513,17 +513,17 @@@ static struct hashmap subprocess_map
  
  static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
  {
-       int err, i;
-       struct cmd2process *entry = (struct cmd2process *)subprocess;
-       struct string_list cap_list = STRING_LIST_INIT_NODUP;
-       char *cap_buf;
-       const char *cap_name;
-       struct child_process *process = &subprocess->process;
-       const char *cmd = subprocess->cmd;
-       static const struct {
-               const char *name;
-               unsigned int cap;
-       } known_caps[] = {
+       static int versions[] = {2, 0};
+       static struct subprocess_capability capabilities[] = {
                { "clean",  CAP_CLEAN  },
                { "smudge", CAP_SMUDGE },
                { "delay",  CAP_DELAY  },
+               { NULL, 0 }
        };
-       sigchain_push(SIGPIPE, SIG_IGN);
-       err = packet_writel(process->in, "git-filter-client", "version=2", NULL);
-       if (err)
-               goto done;
-       err = strcmp(packet_read_line(process->out, NULL), "git-filter-server");
-       if (err) {
-               error("external filter '%s' does not support filter protocol version 2", cmd);
-               goto done;
-       }
-       err = strcmp(packet_read_line(process->out, NULL), "version=2");
-       if (err)
-               goto done;
-       err = packet_read_line(process->out, NULL) != NULL;
-       if (err)
-               goto done;
-       for (i = 0; i < ARRAY_SIZE(known_caps); ++i) {
-               err = packet_write_fmt_gently(
-                       process->in, "capability=%s\n", known_caps[i].name);
-               if (err)
-                       goto done;
-       }
-       err = packet_flush_gently(process->in);
-       if (err)
-               goto done;
-       for (;;) {
-               cap_buf = packet_read_line(process->out, NULL);
-               if (!cap_buf)
-                       break;
-               string_list_split_in_place(&cap_list, cap_buf, '=', 1);
-               if (cap_list.nr != 2 || strcmp(cap_list.items[0].string, "capability"))
-                       continue;
-               cap_name = cap_list.items[1].string;
-               i = ARRAY_SIZE(known_caps) - 1;
-               while (i >= 0 && strcmp(cap_name, known_caps[i].name))
-                       i--;
-               if (i >= 0)
-                       entry->supported_capabilities |= known_caps[i].cap;
-               else
-                       warning("external filter '%s' requested unsupported filter capability '%s'",
-                       cmd, cap_name);
-               string_list_clear(&cap_list, 0);
-       }
- done:
-       sigchain_pop(SIGPIPE);
-       return err;
+       struct cmd2process *entry = (struct cmd2process *)subprocess;
+       return subprocess_handshake(subprocess, "git-filter", versions, NULL,
+                                   capabilities,
+                                   &entry->supported_capabilities);
  }
  
  static void handle_filter_error(const struct strbuf *filter_status,
@@@ -625,7 -564,8 +564,7 @@@ static int apply_multi_file_filter(cons
  
        if (!subprocess_map_initialized) {
                subprocess_map_initialized = 1;
 -              hashmap_init(&subprocess_map, (hashmap_cmp_fn) cmd2process_cmp,
 -                           NULL, 0);
 +              hashmap_init(&subprocess_map, cmd2process_cmp, NULL, 0);
                entry = NULL;
        } else {
                entry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd);
diff --combined sub-process.c
index 6cbffa44064f6e2040be496ad509a57db648e554,86de8d7bfbcd781b50571f867f0669c84093eca3..6edb97c1c68c063216bda710d5880b1d08de27da
@@@ -6,13 -6,10 +6,13 @@@
  #include "pkt-line.h"
  
  int cmd2process_cmp(const void *unused_cmp_data,
 -                  const struct subprocess_entry *e1,
 -                  const struct subprocess_entry *e2,
 +                  const void *entry,
 +                  const void *entry_or_key,
                    const void *unused_keydata)
  {
 +      const struct subprocess_entry *e1 = entry;
 +      const struct subprocess_entry *e2 = entry_or_key;
 +
        return strcmp(e1->cmd, e2->cmd);
  }
  
@@@ -108,3 -105,107 +108,107 @@@ int subprocess_start(struct hashmap *ha
        hashmap_add(hashmap, entry);
        return 0;
  }
+ static int handshake_version(struct child_process *process,
+                            const char *welcome_prefix, int *versions,
+                            int *chosen_version)
+ {
+       int version_scratch;
+       int i;
+       char *line;
+       const char *p;
+       if (!chosen_version)
+               chosen_version = &version_scratch;
+       if (packet_write_fmt_gently(process->in, "%s-client\n",
+                                   welcome_prefix))
+               return error("Could not write client identification");
+       for (i = 0; versions[i]; i++) {
+               if (packet_write_fmt_gently(process->in, "version=%d\n",
+                                           versions[i]))
+                       return error("Could not write requested version");
+       }
+       if (packet_flush_gently(process->in))
+               return error("Could not write flush packet");
+       if (!(line = packet_read_line(process->out, NULL)) ||
+           !skip_prefix(line, welcome_prefix, &p) ||
+           strcmp(p, "-server"))
+               return error("Unexpected line '%s', expected %s-server",
+                            line ? line : "<flush packet>", welcome_prefix);
+       if (!(line = packet_read_line(process->out, NULL)) ||
+           !skip_prefix(line, "version=", &p) ||
+           strtol_i(p, 10, chosen_version))
+               return error("Unexpected line '%s', expected version",
+                            line ? line : "<flush packet>");
+       if ((line = packet_read_line(process->out, NULL)))
+               return error("Unexpected line '%s', expected flush", line);
+       /* Check to make sure that the version received is supported */
+       for (i = 0; versions[i]; i++) {
+               if (versions[i] == *chosen_version)
+                       break;
+       }
+       if (!versions[i])
+               return error("Version %d not supported", *chosen_version);
+       return 0;
+ }
+ static int handshake_capabilities(struct child_process *process,
+                                 struct subprocess_capability *capabilities,
+                                 unsigned int *supported_capabilities)
+ {
+       int i;
+       char *line;
+       for (i = 0; capabilities[i].name; i++) {
+               if (packet_write_fmt_gently(process->in, "capability=%s\n",
+                                           capabilities[i].name))
+                       return error("Could not write requested capability");
+       }
+       if (packet_flush_gently(process->in))
+               return error("Could not write flush packet");
+       while ((line = packet_read_line(process->out, NULL))) {
+               const char *p;
+               if (!skip_prefix(line, "capability=", &p))
+                       continue;
+               for (i = 0;
+                    capabilities[i].name && strcmp(p, capabilities[i].name);
+                    i++)
+                       ;
+               if (capabilities[i].name) {
+                       if (supported_capabilities)
+                               *supported_capabilities |= capabilities[i].flag;
+               } else {
+                       warning("external filter requested unsupported filter capability '%s'",
+                               p);
+               }
+       }
+       return 0;
+ }
+ int subprocess_handshake(struct subprocess_entry *entry,
+                        const char *welcome_prefix,
+                        int *versions,
+                        int *chosen_version,
+                        struct subprocess_capability *capabilities,
+                        unsigned int *supported_capabilities)
+ {
+       int retval;
+       struct child_process *process = &entry->process;
+       sigchain_push(SIGPIPE, SIG_IGN);
+       retval = handshake_version(process, welcome_prefix, versions,
+                                  chosen_version) ||
+                handshake_capabilities(process, capabilities,
+                                       supported_capabilities);
+       sigchain_pop(SIGPIPE);
+       return retval;
+ }
diff --combined sub-process.h
index 8cd07a59ab4344fdc9a5b35b201a075aa7c06423,caa91a9b92f2256a33d3330b1084a205bef98632..49701998c9bd663073244e64c4edd7e80991a3ef
@@@ -6,41 -6,88 +6,88 @@@
  #include "run-command.h"
  
  /*
-  * Generic implementation of background process infrastructure.
-  * See: Documentation/technical/api-sub-process.txt
+  * The sub-process API makes it possible to run background sub-processes
+  * for the entire lifetime of a Git invocation. If Git needs to communicate
+  * with an external process multiple times, then this can reduces the process
+  * invocation overhead. Git and the sub-process communicate through stdin and
+  * stdout.
+  *
+  * The sub-processes are kept in a hashmap by command name and looked up
+  * via the subprocess_find_entry function.  If an existing instance can not
+  * be found then a new process should be created and started.  When the
+  * parent git command terminates, all sub-processes are also terminated.
+  *
+  * This API is based on the run-command API.
   */
  
   /* data structures */
  
+ /* Members should not be accessed directly. */
  struct subprocess_entry {
        struct hashmap_entry ent; /* must be the first member! */
        const char *cmd;
        struct child_process process;
  };
  
+ struct subprocess_capability {
+       const char *name;
+       /*
+        * subprocess_handshake will "|=" this value to supported_capabilities
+        * if the server reports that it supports this capability.
+        */
+       unsigned int flag;
+ };
  /* subprocess functions */
  
+ /* Function to test two subprocess hashmap entries for equality. */
  extern int cmd2process_cmp(const void *unused_cmp_data,
 -                         const struct subprocess_entry *e1,
 -                         const struct subprocess_entry *e2,
 +                         const void *e1,
 +                         const void *e2,
                           const void *unused_keydata);
  
+ /*
+  * User-supplied function to initialize the sub-process.  This is
+  * typically used to negotiate the interface version and capabilities.
+  */
  typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);
+ /* Start a subprocess and add it to the subprocess hashmap. */
  int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, const char *cmd,
                subprocess_start_fn startfn);
  
+ /* Kill a subprocess and remove it from the subprocess hashmap. */
  void subprocess_stop(struct hashmap *hashmap, struct subprocess_entry *entry);
  
+ /* Find a subprocess in the subprocess hashmap. */
  struct subprocess_entry *subprocess_find_entry(struct hashmap *hashmap, const char *cmd);
  
  /* subprocess helper functions */
  
+ /* Get the underlying `struct child_process` from a subprocess. */
  static inline struct child_process *subprocess_get_child_process(
                struct subprocess_entry *entry)
  {
        return &entry->process;
  }
  
+ /*
+  * Perform the version and capability negotiation as described in the "Long
+  * Running Filter Process" section of the gitattributes documentation using the
+  * given requested versions and capabilities. The "versions" and "capabilities"
+  * parameters are arrays terminated by a 0 or blank struct.
+  *
+  * This function is typically called when a subprocess is started (as part of
+  * the "startfn" passed to subprocess_start).
+  */
+ int subprocess_handshake(struct subprocess_entry *entry,
+                        const char *welcome_prefix,
+                        int *versions,
+                        int *chosen_version,
+                        struct subprocess_capability *capabilities,
+                        unsigned int *supported_capabilities);
  /*
   * Helper function that will read packets looking for "status=<foo>"
   * key/value pairs and return the value from the last "status" packet