From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>,
jrnieder@gmail.com, peartben@gmail.com
Subject: [PATCH v2 2/2] sub-process: refactor handshake to common function
Date: Tue, 25 Jul 2017 11:29:38 -0700 [thread overview]
Message-ID: <e47344b6e4bce2a038ba62abb158ec720221a96c.1501007300.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1501007300.git.jonathantanmy@google.com>
In-Reply-To: <cover.1501007300.git.jonathantanmy@google.com>
Refactor, into a common function, the version and capability negotiation
done when invoking a long-running process as a clean or smudge filter.
This will be useful for other Git code that needs to interact similarly
with a long-running process.
As you can see in the change to t0021, this commit changes the error
message reported when the long-running process does not introduce itself
with the expected "server"-terminated line. Originally, the error
message reports that the filter "does not support filter protocol
version 2", differentiating between the old single-file filter protocol
and the new multi-file filter protocol - I have updated it to something
more generic and useful.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
convert.c | 61 ++++-----------------------------
pkt-line.c | 19 -----------
pkt-line.h | 2 --
sub-process.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++
sub-process.h | 26 ++++++++++++++
t/t0021-conversion.sh | 2 +-
6 files changed, 128 insertions(+), 76 deletions(-)
diff --git a/convert.c b/convert.c
index deaf0ba7b..ec8ecc2ea 100644
--- a/convert.c
+++ b/convert.c
@@ -512,62 +512,15 @@ static struct hashmap subprocess_map;
static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
{
- int err;
+ static int versions[] = {2, 0};
+ static struct subprocess_capability capabilities[] = {
+ {"clean", CAP_CLEAN}, {"smudge", CAP_SMUDGE}, {NULL, 0}
+ };
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;
-
- 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;
-
- err = packet_writel(process->in, "capability=clean", "capability=smudge", NULL);
-
- 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;
- if (!strcmp(cap_name, "clean")) {
- entry->supported_capabilities |= CAP_CLEAN;
- } else if (!strcmp(cap_name, "smudge")) {
- entry->supported_capabilities |= CAP_SMUDGE;
- } 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;
+ return subprocess_handshake(subprocess, "git-filter-", versions, NULL,
+ capabilities,
+ &entry->supported_capabilities);
}
static int apply_multi_file_filter(const char *path, const char *src, size_t len,
diff --git a/pkt-line.c b/pkt-line.c
index 9d845ecc3..7db911957 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -171,25 +171,6 @@ int packet_write_fmt_gently(int fd, const char *fmt, ...)
return status;
}
-int packet_writel(int fd, const char *line, ...)
-{
- va_list args;
- int err;
- va_start(args, line);
- for (;;) {
- if (!line)
- break;
- if (strlen(line) > LARGE_PACKET_DATA_MAX)
- return -1;
- err = packet_write_fmt_gently(fd, "%s\n", line);
- if (err)
- return err;
- line = va_arg(args, const char*);
- }
- va_end(args);
- return packet_flush_gently(fd);
-}
-
static int packet_write_gently(const int fd_out, const char *buf, size_t size)
{
static char packet_write_buffer[LARGE_PACKET_MAX];
diff --git a/pkt-line.h b/pkt-line.h
index 450183b64..66ef610fc 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -25,8 +25,6 @@ void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
int packet_flush_gently(int fd);
int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
-LAST_ARG_MUST_BE_NULL
-int packet_writel(int fd, const char *line, ...);
int write_packetized_from_fd(int fd_in, int fd_out);
int write_packetized_from_buf(const char *src_in, size_t len, int fd_out);
diff --git a/sub-process.c b/sub-process.c
index a3cfab1a9..1a3f39bdf 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -105,3 +105,97 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co
hashmap_add(hashmap, entry);
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 version_scratch;
+ unsigned int capabilities_scratch;
+ struct child_process *process = &entry->process;
+ int i;
+ char *line;
+ const char *p;
+
+ if (!chosen_version)
+ chosen_version = &version_scratch;
+ if (!supported_capabilities)
+ supported_capabilities = &capabilities_scratch;
+
+ sigchain_push(SIGPIPE, SIG_IGN);
+
+ if (packet_write_fmt_gently(process->in, "%sclient\n",
+ welcome_prefix)) {
+ error("Could not write client identification");
+ goto error;
+ }
+ for (i = 0; versions[i]; i++) {
+ if (packet_write_fmt_gently(process->in, "version=%d\n",
+ versions[i])) {
+ error("Could not write requested version");
+ goto error;
+ }
+ }
+ if (packet_flush_gently(process->in))
+ goto error;
+
+ if (!(line = packet_read_line(process->out, NULL)) ||
+ !skip_prefix(line, welcome_prefix, &p) ||
+ strcmp(p, "server")) {
+ error("Unexpected line '%s', expected %sserver",
+ line ? line : "<flush packet>", welcome_prefix);
+ goto error;
+ }
+ if (!(line = packet_read_line(process->out, NULL)) ||
+ !skip_prefix(line, "version=", &p) ||
+ strtol_i(p, 10, chosen_version)) {
+ error("Unexpected line '%s', expected version",
+ line ? line : "<flush packet>");
+ goto error;
+ }
+ for (i = 0; versions[i]; i++) {
+ if (versions[i] == *chosen_version)
+ goto version_found;
+ }
+ error("Version %d not supported", *chosen_version);
+ goto error;
+version_found:
+ if ((line = packet_read_line(process->out, NULL))) {
+ error("Unexpected line '%s', expected flush", line);
+ goto error;
+ }
+
+ for (i = 0; capabilities[i].name; i++) {
+ if (packet_write_fmt_gently(process->in, "capability=%s\n",
+ capabilities[i].name)) {
+ error("Could not write requested capability");
+ goto error;
+ }
+ }
+ if (packet_flush_gently(process->in))
+ goto error;
+
+ while ((line = packet_read_line(process->out, NULL))) {
+ if (!skip_prefix(line, "capability=", &p))
+ continue;
+
+ for (i = 0; capabilities[i].name; i++) {
+ if (!strcmp(p, capabilities[i].name)) {
+ *supported_capabilities |= capabilities[i].flag;
+ goto capability_found;
+ }
+ }
+ warning("external filter requested unsupported filter capability '%s'",
+ p);
+capability_found:
+ ;
+ }
+
+ sigchain_pop(SIGPIPE);
+ return 0;
+error:
+ sigchain_pop(SIGPIPE);
+ return 1;
+}
diff --git a/sub-process.h b/sub-process.h
index 9e6975b5e..28863fabc 100644
--- a/sub-process.h
+++ b/sub-process.h
@@ -29,6 +29,16 @@ struct subprocess_entry {
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. */
@@ -62,6 +72,22 @@ static inline struct child_process *subprocess_get_child_process(
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
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 161f56044..d191a7228 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -697,7 +697,7 @@ test_expect_success PERL 'invalid process filter must fail (and not hang!)' '
cp "$TEST_ROOT/test.o" test.r &&
test_must_fail git add . 2>git-stderr.log &&
- grep "does not support filter protocol version" git-stderr.log
+ grep "expected git-filter-server" git-stderr.log
)
'
--
2.14.0.rc0.400.g1c36432dff-goog
next prev parent reply other threads:[~2017-07-25 18:29 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-24 21:38 [PATCH] sub-process: refactor handshake to common function Jonathan Tan
2017-07-24 22:21 ` Jonathan Nieder
2017-07-25 14:38 ` Ben Peart
2017-07-25 17:53 ` Jonathan Tan
2017-07-25 18:29 ` [PATCH v2 0/2] " Jonathan Tan
2017-07-25 18:29 ` [PATCH v2 1/2] Documentation: migrate sub-process docs to header Jonathan Tan
2017-07-25 20:18 ` Brandon Williams
2017-07-25 18:29 ` Jonathan Tan [this message]
2017-07-25 20:28 ` [PATCH v2 2/2] sub-process: refactor handshake to common function Brandon Williams
2017-07-25 22:25 ` Junio C Hamano
2017-07-26 16:52 ` [PATCH] " Lars Schneider
2017-07-26 18:14 ` Junio C Hamano
2017-07-26 18:17 ` [PATCH for NEXT v3 0/2] " Jonathan Tan
2017-07-26 19:48 ` Junio C Hamano
2017-07-29 16:26 ` Junio C Hamano
2017-07-26 18:17 ` [PATCH for NEXT v3 1/2] Documentation: migrate sub-process docs to header Jonathan Tan
2017-07-26 18:17 ` [PATCH for NEXT v3 2/2] sub-process: refactor handshake to common function Jonathan Tan
2017-08-06 19:58 ` Lars Schneider
2017-08-07 17:21 ` Jonathan Tan
2017-08-07 17:51 ` Lars Schneider
2017-08-07 18:17 ` Jonathan Tan
2017-08-07 18:29 ` Ben Peart
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=e47344b6e4bce2a038ba62abb158ec720221a96c.1501007300.git.jonathantanmy@google.com \
--to=jonathantanmy@google.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
--cc=peartben@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).