git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Nguyen Thai Ngoc Duy" <pclouds@gmail.com>,
	"Stefan Beller" <sbeller@google.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Ramsay Jones" <ramsay@ramsayjones.plus.com>,
	"Jeff King" <peff@peff.net>,
	"Karsten Blees" <karsten.blees@gmail.com>,
	"Matthieu Moy" <Matthieu.Moy@grenoble-inp.fr>,
	"Christian Couder" <chriscool@tuxfamily.org>
Subject: [PATCH v3 08/49] builtin/apply: introduce 'struct apply_state' to start libifying
Date: Tue, 24 May 2016 10:10:45 +0200	[thread overview]
Message-ID: <20160524081126.16973-9-chriscool@tuxfamily.org> (raw)
In-Reply-To: <20160524081126.16973-1-chriscool@tuxfamily.org>

Currently commands that want to use the apply functionality have to launch
a "git apply" process which can be bad for performance.

Let's start libifying the apply functionality and to do that we first need
to get rid of the global variables in "builtin/apply.c".

This patch introduces "struct apply_state" into which all the previously
global variables will be moved. A new parameter called "state" that is a
pointer to the "apply_state" structure will come at the beginning of the
helper functions that need it and will be passed around the call chain.

To start let's move the "prefix" and "prefix_length" global variables into
"struct apply_state".

Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/apply.c | 94 ++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 56 insertions(+), 38 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index c911e4e..ae068e7 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -21,6 +21,11 @@
 #include "ll-merge.h"
 #include "rerere.h"
 
+struct apply_state {
+	const char *prefix;
+	int prefix_length;
+};
+
 /*
  *  --check turns on checking that the working tree matches the
  *    files that are being modified, but doesn't apply the patch
@@ -30,8 +35,6 @@
  *  --index updates the cache as well.
  *  --cached updates only the cache without ever touching the working tree.
  */
-static const char *prefix;
-static int prefix_length = -1;
 static int newfd = -1;
 
 static int unidiff_zero;
@@ -748,7 +751,7 @@ static int count_slashes(const char *cp)
  * Given the string after "--- " or "+++ ", guess the appropriate
  * p_value for the given patch.
  */
-static int guess_p_value(const char *nameline)
+static int guess_p_value(struct apply_state *state, const char *nameline)
 {
 	char *name, *cp;
 	int val = -1;
@@ -761,17 +764,17 @@ static int guess_p_value(const char *nameline)
 	cp = strchr(name, '/');
 	if (!cp)
 		val = 0;
-	else if (prefix) {
+	else if (state->prefix) {
 		/*
 		 * Does it begin with "a/$our-prefix" and such?  Then this is
 		 * very likely to apply to our directory.
 		 */
-		if (!strncmp(name, prefix, prefix_length))
-			val = count_slashes(prefix);
+		if (!strncmp(name, state->prefix, state->prefix_length))
+			val = count_slashes(state->prefix);
 		else {
 			cp++;
-			if (!strncmp(cp, prefix, prefix_length))
-				val = count_slashes(prefix) + 1;
+			if (!strncmp(cp, state->prefix, state->prefix_length))
+				val = count_slashes(state->prefix) + 1;
 		}
 	}
 	free(name);
@@ -858,7 +861,10 @@ static int has_epoch_timestamp(const char *nameline)
  * files, we can happily check the index for a match, but for creating a
  * new file we should try to match whatever "patch" does. I have no idea.
  */
-static void parse_traditional_patch(const char *first, const char *second, struct patch *patch)
+static void parse_traditional_patch(struct apply_state *state,
+				    const char *first,
+				    const char *second,
+				    struct patch *patch)
 {
 	char *name;
 
@@ -866,8 +872,8 @@ static void parse_traditional_patch(const char *first, const char *second, struc
 	second += 4;	/* skip "+++ " */
 	if (!p_value_known) {
 		int p, q;
-		p = guess_p_value(first);
-		q = guess_p_value(second);
+		p = guess_p_value(state, first);
+		q = guess_p_value(state, second);
 		if (p < 0) p = q;
 		if (0 <= p && p == q) {
 			state_p_value = p;
@@ -1429,7 +1435,11 @@ static int parse_fragment_header(const char *line, int len, struct fragment *fra
 	return offset;
 }
 
-static int find_header(const char *line, unsigned long size, int *hdrsize, struct patch *patch)
+static int find_header(struct apply_state *state,
+		       const char *line,
+		       unsigned long size,
+		       int *hdrsize,
+		       struct patch *patch)
 {
 	unsigned long offset, len;
 
@@ -1506,7 +1516,7 @@ static int find_header(const char *line, unsigned long size, int *hdrsize, struc
 			continue;
 
 		/* Ok, we'll consider it a patch */
-		parse_traditional_patch(line, line+len, patch);
+		parse_traditional_patch(state, line, line+len, patch);
 		*hdrsize = len + nextlen;
 		state_linenr += 2;
 		return offset;
@@ -1913,21 +1923,21 @@ static int parse_binary(char *buffer, unsigned long size, struct patch *patch)
 	return used;
 }
 
-static void prefix_one(char **name)
+static void prefix_one(struct apply_state *state, char **name)
 {
 	char *old_name = *name;
 	if (!old_name)
 		return;
-	*name = xstrdup(prefix_filename(prefix, prefix_length, *name));
+	*name = xstrdup(prefix_filename(state->prefix, state->prefix_length, *name));
 	free(old_name);
 }
 
-static void prefix_patch(struct patch *p)
+static void prefix_patch(struct apply_state *state, struct patch *p)
 {
-	if (!prefix || p->is_toplevel_relative)
+	if (!state->prefix || p->is_toplevel_relative)
 		return;
-	prefix_one(&p->new_name);
-	prefix_one(&p->old_name);
+	prefix_one(state, &p->new_name);
+	prefix_one(state, &p->old_name);
 }
 
 /*
@@ -1944,16 +1954,16 @@ static void add_name_limit(const char *name, int exclude)
 	it->util = exclude ? NULL : (void *) 1;
 }
 
-static int use_patch(struct patch *p)
+static int use_patch(struct apply_state *state, struct patch *p)
 {
 	const char *pathname = p->new_name ? p->new_name : p->old_name;
 	int i;
 
 	/* Paths outside are not touched regardless of "--include" */
-	if (0 < prefix_length) {
+	if (0 < state->prefix_length) {
 		int pathlen = strlen(pathname);
-		if (pathlen <= prefix_length ||
-		    memcmp(prefix, pathname, prefix_length))
+		if (pathlen <= state->prefix_length ||
+		    memcmp(state->prefix, pathname, state->prefix_length))
 			return 0;
 	}
 
@@ -1980,17 +1990,17 @@ static int use_patch(struct patch *p)
  * Return the number of bytes consumed, so that the caller can call us
  * again for the next patch.
  */
-static int parse_chunk(char *buffer, unsigned long size, struct patch *patch)
+static int parse_chunk(struct apply_state *state, char *buffer, unsigned long size, struct patch *patch)
 {
 	int hdrsize, patchsize;
-	int offset = find_header(buffer, size, &hdrsize, patch);
+	int offset = find_header(state, buffer, size, &hdrsize, patch);
 
 	if (offset < 0)
 		return offset;
 
-	prefix_patch(patch);
+	prefix_patch(state, patch);
 
-	if (!use_patch(patch))
+	if (!use_patch(state, patch))
 		patch->ws_rule = 0;
 	else
 		patch->ws_rule = whitespace_rule(patch->new_name
@@ -4367,7 +4377,10 @@ static struct lock_file lock_file;
 #define INACCURATE_EOF	(1<<0)
 #define RECOUNT		(1<<1)
 
-static int apply_patch(int fd, const char *filename, int options)
+static int apply_patch(struct apply_state *state,
+		       int fd,
+		       const char *filename,
+		       int options)
 {
 	size_t offset;
 	struct strbuf buf = STRBUF_INIT; /* owns the patch text */
@@ -4384,14 +4397,14 @@ static int apply_patch(int fd, const char *filename, int options)
 		patch = xcalloc(1, sizeof(*patch));
 		patch->inaccurate_eof = !!(options & INACCURATE_EOF);
 		patch->recount =  !!(options & RECOUNT);
-		nr = parse_chunk(buf.buf + offset, buf.len - offset, patch);
+		nr = parse_chunk(state, buf.buf + offset, buf.len - offset, patch);
 		if (nr < 0) {
 			free_patch(patch);
 			break;
 		}
 		if (apply_in_reverse)
 			reverse_patches(patch);
-		if (use_patch(patch)) {
+		if (use_patch(state, patch)) {
 			patch_stats(patch);
 			*listp = patch;
 			listp = &patch->next;
@@ -4517,6 +4530,7 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 	int force_apply = 0;
 	int options = 0;
 	int read_stdin = 1;
+	struct apply_state state;
 
 	const char *whitespace_option = NULL;
 
@@ -4589,15 +4603,17 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 		OPT_END()
 	};
 
-	prefix = prefix_;
-	prefix_length = prefix ? strlen(prefix) : 0;
+	memset(&state, 0, sizeof(state));
+	state.prefix = prefix_;
+	state.prefix_length = state.prefix ? strlen(state.prefix) : 0;
+
 	git_apply_config();
 	if (apply_default_whitespace)
 		parse_whitespace_option(apply_default_whitespace);
 	if (apply_default_ignorewhitespace)
 		parse_ignorewhitespace_option(apply_default_ignorewhitespace);
 
-	argc = parse_options(argc, argv, prefix, builtin_apply_options,
+	argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
 			apply_usage, 0);
 
 	if (apply_with_reject && threeway)
@@ -4628,23 +4644,25 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)
 		int fd;
 
 		if (!strcmp(arg, "-")) {
-			errs |= apply_patch(0, "<stdin>", options);
+			errs |= apply_patch(&state, 0, "<stdin>", options);
 			read_stdin = 0;
 			continue;
-		} else if (0 < prefix_length)
-			arg = prefix_filename(prefix, prefix_length, arg);
+		} else if (0 < state.prefix_length)
+			arg = prefix_filename(state.prefix,
+					      state.prefix_length,
+					      arg);
 
 		fd = open(arg, O_RDONLY);
 		if (fd < 0)
 			die_errno(_("can't open patch '%s'"), arg);
 		read_stdin = 0;
 		set_default_whitespace_mode(whitespace_option);
-		errs |= apply_patch(fd, arg, options);
+		errs |= apply_patch(&state, fd, arg, options);
 		close(fd);
 	}
 	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
-		errs |= apply_patch(0, "<stdin>", options);
+		errs |= apply_patch(&state, 0, "<stdin>", options);
 	if (whitespace_error) {
 		if (squelch_whitespace_errors &&
 		    squelch_whitespace_errors < whitespace_error) {
-- 
2.8.3.443.gaeee61e

  parent reply	other threads:[~2016-05-24  8:12 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-24  8:10 [PATCH v3 00/49] libify apply and use lib in am, part 1 Christian Couder
2016-05-24  8:10 ` [PATCH v3 01/49] builtin/apply: make gitdiff_verify_name() return void Christian Couder
2016-05-24  8:10 ` [PATCH v3 02/49] builtin/apply: avoid parameter shadowing 'p_value' global Christian Couder
2016-05-24  8:10 ` [PATCH v3 03/49] builtin/apply: avoid parameter shadowing 'linenr' global Christian Couder
2016-05-24  8:10 ` [PATCH v3 04/49] builtin/apply: avoid local variable shadowing 'len' parameter Christian Couder
2016-05-24  8:10 ` [PATCH v3 05/49] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment() Christian Couder
2016-05-24  8:10 ` [PATCH v3 06/49] builtin/apply: move 'options' variable into cmd_apply() Christian Couder
2016-05-24  8:10 ` [PATCH v3 07/49] builtin/apply: move 'read_stdin' global " Christian Couder
2016-05-24  8:10 ` Christian Couder [this message]
2016-05-24  8:10 ` [PATCH v3 09/49] builtin/apply: move 'state' init into init_apply_state() Christian Couder
2016-05-24  8:10 ` [PATCH v3 10/49] builtin/apply: move 'unidiff_zero' global into 'struct apply_state' Christian Couder
2016-05-24  8:10 ` [PATCH v3 11/49] builtin/apply: move 'check' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 12/49] builtin/apply: move 'check_index' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 13/49] builtin/apply: move 'apply_in_reverse' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 14/49] builtin/apply: move 'apply_with_reject' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 15/49] builtin/apply: move 'apply_verbosely' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 16/49] builtin/apply: move 'update_index' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 17/49] builtin/apply: move 'allow_overlap' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 18/49] builtin/apply: move 'cached' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 19/49] builtin/apply: move 'diffstat' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 20/49] builtin/apply: move 'numstat' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 21/49] builtin/apply: move 'summary' " Christian Couder
2016-05-24  8:10 ` [PATCH v3 22/49] builtin/apply: move 'threeway' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 23/49] builtin/apply: move 'no_add' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 24/49] builtin/apply: move 'unsafe_paths' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 25/49] builtin/apply: move 'line_termination' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 26/49] builtin/apply: move 'fake_ancestor' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 27/49] builtin/apply: move 'p_context' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 28/49] builtin/apply: move 'apply' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 29/49] builtin/apply: move 'patch_input_file' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 30/49] builtin/apply: move 'limit_by_name' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 31/49] builtin/apply: move 'has_include' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 32/49] builtin/apply: move 'p_value' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 33/49] builtin/apply: move 'p_value_known' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 34/49] builtin/apply: move 'root' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 35/49] builtin/apply: move 'whitespace_error' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 36/49] builtin/apply: move 'whitespace_option' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 37/49] builtin/apply: remove whitespace_option arg from set_default_whitespace_mode() Christian Couder
2016-05-24  8:11 ` [PATCH v3 38/49] builtin/apply: move 'squelch_whitespace_errors' into 'struct apply_state' Christian Couder
2016-05-24  8:11 ` [PATCH v3 39/49] builtin/apply: move 'applied_after_fixing_ws' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 40/49] builtin/apply: move 'ws_error_action' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 41/49] builtin/apply: move 'ws_ignore_action' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 42/49] builtin/apply: move 'max_change' and 'max_len' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 43/49] builtin/apply: move 'state_linenr' global " Christian Couder
2016-05-24  8:11 ` [PATCH v3 44/49] builtin/apply: move 'fn_table' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 45/49] builtin/apply: move 'symlink_changes' " Christian Couder
2016-05-24  8:11 ` [PATCH v3 46/49] builtin/apply: move 'state' check into check_apply_state() Christian Couder
2016-05-24  8:11 ` [PATCH v3 47/49] builtin/apply: move applying patches into apply_all_patches() Christian Couder
2016-05-24  8:11 ` [PATCH v3 48/49] builtin/apply: move 'lock_file' global into 'struct apply_state' Christian Couder
2016-06-01 17:23   ` Junio C Hamano
2016-06-03  9:42     ` Christian Couder
2016-05-24  8:11 ` [PATCH v3 49/49] builtin/apply: move 'newfd' " Christian Couder
2016-05-24  8:59 ` [PATCH v3 00/49] libify apply and use lib in am, part 1 Christian Couder

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=20160524081126.16973-9-chriscool@tuxfamily.org \
    --to=christian.couder@gmail.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karsten.blees@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sbeller@google.com \
    --cc=sunshine@sunshineco.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).