From: Ben Peart <peartben@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, benpeart@microsoft.com,
christian.couder@gmail.com, larsxschneider@gmail.com
Subject: [PATCH v5 7/8] sub-process: move sub-process functions into separate files
Date: Fri, 7 Apr 2017 08:03:53 -0400 [thread overview]
Message-ID: <20170407120354.17736-8-benpeart@microsoft.com> (raw)
In-Reply-To: <20170407120354.17736-1-benpeart@microsoft.com>
Move the sub-proces functions into sub-process.h/c. Add documentation
for the new module in Documentation/technical/api-sub-process.txt
Signed-off-by: Ben Peart <benpeart@microsoft.com>
---
Documentation/technical/api-sub-process.txt | 54 +++++++++++++
Makefile | 1 +
convert.c | 119 +---------------------------
sub-process.c | 116 +++++++++++++++++++++++++++
sub-process.h | 46 +++++++++++
5 files changed, 218 insertions(+), 118 deletions(-)
create mode 100644 Documentation/technical/api-sub-process.txt
create mode 100644 sub-process.c
create mode 100644 sub-process.h
diff --git a/Documentation/technical/api-sub-process.txt b/Documentation/technical/api-sub-process.txt
new file mode 100644
index 0000000000..eb5005aa72
--- /dev/null
+++ b/Documentation/technical/api-sub-process.txt
@@ -0,0 +1,54 @@
+sub-process API
+===============
+
+The sub-process API makes it possible to run background sub-processes
+that should run until the git command exits and communicate with it
+through stdin and stdout. This reduces the overhead of having to fork
+a new process each time it needs to be communicated with.
+
+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
+---------------
+
+* `struct subprocess_entry`
+
+The sub-process structure. Members should not be accessed directly.
+
+Types
+-----
+
+'int(*subprocess_start_fn)(struct subprocess_entry *entry)'::
+
+ User-supplied function to initialize the sub-process. This is
+ typically used to negoiate the interface version and capabilities.
+
+
+Functions
+---------
+
+`subprocess_start`::
+
+ Start a subprocess and add it to the subprocess hashmap.
+
+`subprocess_stop`::
+
+ Kill a subprocess and remove it from the subprocess hashmap.
+
+`subprocess_find_entry`::
+
+ Find a subprocess in the subprocess hashmap.
+
+`subprocess_get_child_process`::
+
+ Get the underlying `struct child_process` from a subprocess.
+
+`subprocess_read_status`::
+
+ Helper function to read packets looking for the last "status=<foo>"
+ key/value pair.
diff --git a/Makefile b/Makefile
index 9f8b35ad41..add945b560 100644
--- a/Makefile
+++ b/Makefile
@@ -838,6 +838,7 @@ LIB_OBJS += streaming.o
LIB_OBJS += string-list.o
LIB_OBJS += submodule.o
LIB_OBJS += submodule-config.o
+LIB_OBJS += sub-process.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += tempfile.o
diff --git a/convert.c b/convert.c
index 235a6a5279..baa41da760 100644
--- a/convert.c
+++ b/convert.c
@@ -4,6 +4,7 @@
#include "quote.h"
#include "sigchain.h"
#include "pkt-line.h"
+#include "sub-process.h"
/*
* convert.c - convert a file when checking it out and checking it in.
@@ -496,86 +497,11 @@ static int apply_single_file_filter(const char *path, const char *src, size_t le
#define CAP_CLEAN (1u<<0)
#define CAP_SMUDGE (1u<<1)
-struct subprocess_entry {
- struct hashmap_entry ent; /* must be the first member! */
- const char *cmd;
- struct child_process process;
-};
-
struct cmd2process {
struct subprocess_entry subprocess; /* must be the first member! */
unsigned int supported_capabilities;
};
-static int subprocess_map_initialized;
-static struct hashmap subprocess_map;
-
-static int cmd2process_cmp(const struct subprocess_entry *e1,
- const struct subprocess_entry *e2,
- const void *unused)
-{
- return strcmp(e1->cmd, e2->cmd);
-}
-
-static struct subprocess_entry *subprocess_find_entry(const char *cmd)
-{
- struct subprocess_entry key;
-
- if (!subprocess_map_initialized) {
- subprocess_map_initialized = 1;
- hashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);
- return NULL;
- }
-
- hashmap_entry_init(&key, strhash(cmd));
- key.cmd = cmd;
- return hashmap_get(&subprocess_map, &key, NULL);
-}
-
-static void subprocess_read_status(int fd, struct strbuf *status)
-{
- struct strbuf **pair;
- char *line;
- for (;;) {
- line = packet_read_line(fd, NULL);
- if (!line)
- break;
- pair = strbuf_split_str(line, '=', 2);
- if (pair[0] && pair[0]->len && pair[1]) {
- /* the last "status=<foo>" line wins */
- if (!strcmp(pair[0]->buf, "status=")) {
- strbuf_reset(status);
- strbuf_addbuf(status, pair[1]);
- }
- }
- strbuf_list_free(pair);
- }
-}
-
-static void subprocess_stop(struct subprocess_entry *entry)
-{
- if (!entry)
- return;
-
- entry->process.clean_on_exit = 0;
- kill(entry->process.pid, SIGTERM);
- finish_command(&entry->process);
-
- hashmap_remove(&subprocess_map, entry, NULL);
- free(entry);
-}
-
-static void subprocess_exit_handler(struct child_process *process)
-{
- sigchain_push(SIGPIPE, SIG_IGN);
- /* Closing the pipe signals the subprocess to initiate a shutdown. */
- close(process->in);
- close(process->out);
- sigchain_pop(SIGPIPE);
- /* Finish command will wait until the shutdown is complete. */
- finish_command(process);
-}
-
static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
{
int err;
@@ -639,49 +565,6 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
return err;
}
-typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);
-int subprocess_start(struct subprocess_entry *entry, const char *cmd,
- subprocess_start_fn startfn)
-{
- int err;
- struct child_process *process;
- const char *argv[] = { cmd, NULL };
-
- if (!subprocess_map_initialized) {
- subprocess_map_initialized = 1;
- hashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);
- }
-
- entry->cmd = cmd;
- process = &entry->process;
-
- child_process_init(process);
- process->argv = argv;
- process->use_shell = 1;
- process->in = -1;
- process->out = -1;
- process->clean_on_exit = 1;
- process->clean_on_exit_handler = subprocess_exit_handler;
-
- err = start_command(process);
- if (err) {
- error("cannot fork to run subprocess '%s'", cmd);
- return err;
- }
-
- hashmap_entry_init(entry, strhash(cmd));
-
- err = startfn(entry);
- if (err) {
- error("initialization for subprocess '%s' failed", cmd);
- subprocess_stop(entry);
- return err;
- }
-
- hashmap_add(&subprocess_map, entry);
- return 0;
-}
-
static int apply_multi_file_filter(const char *path, const char *src, size_t len,
int fd, struct strbuf *dst, const char *cmd,
const unsigned int wanted_capability)
diff --git a/sub-process.c b/sub-process.c
new file mode 100644
index 0000000000..60bb650012
--- /dev/null
+++ b/sub-process.c
@@ -0,0 +1,116 @@
+/*
+ * Generic implementation of background process infrastructure.
+ */
+#include "sub-process.h"
+#include "sigchain.h"
+#include "pkt-line.h"
+
+static int subprocess_map_initialized;
+static struct hashmap subprocess_map;
+
+static int cmd2process_cmp(const struct subprocess_entry *e1,
+ const struct subprocess_entry *e2,
+ const void *unused)
+{
+ return strcmp(e1->cmd, e2->cmd);
+}
+
+struct subprocess_entry *subprocess_find_entry(const char *cmd)
+{
+ struct subprocess_entry key;
+
+ if (!subprocess_map_initialized) {
+ subprocess_map_initialized = 1;
+ hashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);
+ return NULL;
+ }
+
+ hashmap_entry_init(&key, strhash(cmd));
+ key.cmd = cmd;
+ return hashmap_get(&subprocess_map, &key, NULL);
+}
+
+void subprocess_read_status(int fd, struct strbuf *status)
+{
+ struct strbuf **pair;
+ char *line;
+ for (;;) {
+ line = packet_read_line(fd, NULL);
+ if (!line)
+ break;
+ pair = strbuf_split_str(line, '=', 2);
+ if (pair[0] && pair[0]->len && pair[1]) {
+ /* the last "status=<foo>" line wins */
+ if (!strcmp(pair[0]->buf, "status=")) {
+ strbuf_reset(status);
+ strbuf_addbuf(status, pair[1]);
+ }
+ }
+ strbuf_list_free(pair);
+ }
+}
+
+void subprocess_stop(struct subprocess_entry *entry)
+{
+ if (!entry)
+ return;
+
+ entry->process.clean_on_exit = 0;
+ kill(entry->process.pid, SIGTERM);
+ finish_command(&entry->process);
+
+ hashmap_remove(&subprocess_map, entry, NULL);
+}
+
+static void subprocess_exit_handler(struct child_process *process)
+{
+ sigchain_push(SIGPIPE, SIG_IGN);
+ /* Closing the pipe signals the subprocess to initiate a shutdown. */
+ close(process->in);
+ close(process->out);
+ sigchain_pop(SIGPIPE);
+ /* Finish command will wait until the shutdown is complete. */
+ finish_command(process);
+}
+
+int subprocess_start(struct subprocess_entry *entry, const char *cmd,
+ subprocess_start_fn startfn)
+{
+ int err;
+ struct child_process *process;
+ const char *argv[] = { cmd, NULL };
+
+ if (!subprocess_map_initialized) {
+ subprocess_map_initialized = 1;
+ hashmap_init(&subprocess_map, (hashmap_cmp_fn)cmd2process_cmp, 0);
+ }
+
+ entry->cmd = cmd;
+ process = &entry->process;
+
+ child_process_init(process);
+ process->argv = argv;
+ process->use_shell = 1;
+ process->in = -1;
+ process->out = -1;
+ process->clean_on_exit = 1;
+ process->clean_on_exit_handler = subprocess_exit_handler;
+
+ err = start_command(process);
+ if (err) {
+ error("cannot fork to run subprocess '%s'", cmd);
+ return err;
+ }
+
+ hashmap_entry_init(entry, strhash(cmd));
+
+ err = startfn(entry);
+ if (err) {
+ error("initialization for subprocess '%s' failed", cmd);
+ subprocess_stop(entry);
+ return err;
+ }
+
+ hashmap_add(&subprocess_map, entry);
+ return 0;
+}
diff --git a/sub-process.h b/sub-process.h
new file mode 100644
index 0000000000..0cf1760a0a
--- /dev/null
+++ b/sub-process.h
@@ -0,0 +1,46 @@
+#ifndef SUBPROCESS_H
+#define SUBPROCESS_H
+
+#include "git-compat-util.h"
+#include "hashmap.h"
+#include "run-command.h"
+
+/*
+ * Generic implementation of background process infrastructure.
+ * See Documentation/technical/api-background-process.txt.
+ */
+
+ /* data structures */
+
+struct subprocess_entry {
+ struct hashmap_entry ent; /* must be the first member! */
+ const char *cmd;
+ struct child_process process;
+};
+
+/* subprocess functions */
+
+typedef int(*subprocess_start_fn)(struct subprocess_entry *entry);
+int subprocess_start(struct subprocess_entry *entry, const char *cmd,
+ subprocess_start_fn startfn);
+
+void subprocess_stop(struct subprocess_entry *entry);
+
+struct subprocess_entry *subprocess_find_entry(const char *cmd);
+
+/* subprocess helper functions */
+
+static inline struct child_process *subprocess_get_child_process(
+ struct subprocess_entry *entry)
+{
+ return &entry->process;
+}
+
+/*
+ * Helper function that will read packets looking for "status=<foo>"
+ * key/value pairs and return the value from the last "status" packet
+ */
+
+void subprocess_read_status(int fd, struct strbuf *status);
+
+#endif
--
2.12.0.windows.1.31.g1548525701.dirty
next prev parent reply other threads:[~2017-04-07 12:04 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-07 12:03 [PATCH v5 0/8] refactor the filter process code into a reusable module Ben Peart
2017-04-07 12:03 ` [PATCH v5 1/8] pkt-line: add packet_read_line_gently() Ben Peart
2017-04-09 19:34 ` Lars Schneider
2017-04-07 12:03 ` [PATCH v5 2/8] convert: move packet_write_list() into pkt-line as packet_writel() Ben Peart
2017-04-09 19:43 ` Lars Schneider
2017-04-07 12:03 ` [PATCH v5 3/8] convert: Split start_multi_file_filter into two separate functions Ben Peart
2017-04-09 19:56 ` Lars Schneider
2017-04-11 16:16 ` Jeff King
2017-04-11 19:29 ` Lars Schneider
2017-04-11 19:37 ` Jeff King
2017-04-11 20:01 ` Lars Schneider
2017-04-11 20:05 ` Jeff King
2017-04-20 17:27 ` Ben Peart
2017-04-07 12:03 ` [PATCH v5 4/8] convert: Separate generic structures and variables from the filter specific ones Ben Peart
2017-04-10 10:18 ` Lars Schneider
2017-04-17 3:31 ` Junio C Hamano
2017-04-18 16:38 ` Ben Peart
2017-04-19 1:23 ` Junio C Hamano
2017-04-20 17:24 ` Ben Peart
2017-04-07 12:03 ` [PATCH v5 5/8] convert: Update generic functions to only use generic data structures Ben Peart
2017-04-10 12:05 ` Lars Schneider
2017-04-07 12:03 ` [PATCH v5 6/8] convert: rename reusable sub-process functions Ben Peart
2017-04-10 12:11 ` Lars Schneider
2017-04-07 12:03 ` Ben Peart [this message]
2017-04-10 12:41 ` [PATCH v5 7/8] sub-process: move sub-process functions into separate files Lars Schneider
2017-04-07 12:03 ` [PATCH v5 8/8] convert: Update subprocess_read_status to not die on EOF Ben Peart
2017-04-10 12:48 ` Lars Schneider
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170407120354.17736-8-benpeart@microsoft.com \
--to=peartben@gmail.com \
--cc=benpeart@microsoft.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=larsxschneider@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).