[PATCH v2 01/16] Add data structures and basic functions for commit trailers
We will use a doubly linked list to store all information
about trailers and their configuration.
This way we can easily remove or add trailers to or from
trailer lists while traversing the lists in either direction.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Makefile | 1 +
trailer.c | 47 +++
2 files changed, 48 insertions(+)
create mode 100644 trailer.c
diff --git a/Makefile b/Makefile
index b4af1e2..ec90feb 100644
--- a/Makefile
+++ b/Makefile
@@ -871,6 +871,7 @@ LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
LIB_OBJS += trace.o
+LIB_OBJS += trailer.o
LIB_OBJS += transport.o
LIB_OBJS += transport-helper.o
LIB_OBJS += tree-diff.o
diff --git a/trailer.c b/trailer.c
new file mode 100644
index 000..d80d047
--- /dev/null
+++ b/trailer.c
@@ -0,0 +1,47 @@
+#include cache.h
+/*
+ * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
+ */
+
+enum action_where { WHERE_AFTER, WHERE_BEFORE };
+enum action_if_exist { EXIST_ADD_IF_DIFFERENT, EXIST_ADD_IF_DIFFERENT_NEIGHBOR,
+ EXIST_ADD, EXIST_OVERWRITE, EXIST_DO_NOTHING };
+enum action_if_missing { MISSING_ADD, MISSING_DO_NOTHING };
+
+struct conf_info {
+ char *name;
+ char *key;
+ char *command;
+ enum action_where where;
+ enum action_if_exist if_exist;
+ enum action_if_missing if_missing;
+};
+
+struct trailer_item {
+ struct trailer_item *previous;
+ struct trailer_item *next;
+ const char *token;
+ const char *value;
+ struct conf_info *conf;
+};
+
+static int same_token(struct trailer_item *a, struct trailer_item *b, int
alnum_len)
+{
+ return !strncasecmp(a-token, b-token, alnum_len);
+}
+
+static int same_value(struct trailer_item *a, struct trailer_item *b)
+{
+ return !strcasecmp(a-value, b-value);
+}
+
+static int same_trailer(struct trailer_item *a, struct trailer_item *b, int
alnum_len)
+{
+ return same_token(a, b, alnum_len) same_value(a, b);
+}
+
+/* Get the length of buf from its beginning until its last alphanumeric
character */
+static size_t alnum_len(const char *buf, size_t len) {
+ while (--len = 0 !isalnum(buf[len]));
+ return len + 1;
+}
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/16] Add interpret-trailers builtin
This patch series implements a new command:
git interpret-trailers
and an infrastructure to process trailers that can be reused,
for example in commit.c.
1) Rationale:
This command should help with RFC 822 style headers, called
trailers, that are found at the end of commit messages.
(Note that these headers do not follow and are not intended to
follow many rules that are in RFC 822. For example they do not
follow the line breaking rules, the encoding rules and probably
many other rules.)
For a long time, these trailers have become a de facto standard
way to add helpful information into commit messages.
Until now git commit has only supported the well known
Signed-off-by: trailer, that is used by many projects like
the Linux kernel and Git.
It is better to implement features for these trailers first in a
new command rather than in builtin/commit.c, because this way the
prepare-commit-msg and commit-msg hooks can reuse this command.
2) Current state:
Currently the usage string of this command is:
git interpret-trailers [--trim-empty] [--infile=file] [token[=value]...]
The following features are implemented:
- the result is printed on stdout
- the [token[=value]...] arguments are interpreted
- a commit message passed using the --infile=file option is
interpreted
- (new) if --infile is not used a commit message is read from stdin
- the trailer.token.key options in the config are interpreted
- the trailer.token.where options are interpreted
- the trailer.token.ifExist options are interpreted
- the trailer.token.ifMissing options are interpreted
- (new) the trailer.token.command config works
- (new) GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL} env variables
can be used in commands
- there are a lot of tests
The following features are planned but not yet implemented:
- some documentation
- integration with git commit
Possible improvements:
- support GIT_COMMIT_PROTO env variable in commands
3) Notes:
* I used sed -e 's/ Z$/ /' -\EOF in the tests as suggested by Junio.
* I added many commits on top of the previous series, but of course they
can be squashed.
Christian Couder (16):
Add data structures and basic functions for commit trailers
trailer: process trailers from file and arguments
trailer: read and process config information
trailer: process command line trailer arguments
strbuf: add strbuf_isspace()
trailer: parse trailers from input file
trailer: put all the processing together and print
trailer: add interpret-trailers command
trailer: add tests for git interpret-trailers
trailer: if no input file is passed, read from stdin
trailer: add new_trailer_item() function
strbuf: add strbuf_replace()
trailer: execute command from 'trailer.name.command'
trailer: add tests for trailer command
trailer: set author and committer env variables
trailer: add tests for commands using env variables
.gitignore| 1 +
Makefile | 2 +
builtin.h | 1 +
builtin/interpret-trailers.c | 36 +++
git.c | 1 +
strbuf.c | 14 +
strbuf.h | 4 +
t/t7513-interpret-trailers.sh | 262 +
trailer.c | 639 ++
trailer.h | 6 +
10 files changed, 966 insertions(+)
create mode 100644 builtin/interpret-trailers.c
create mode 100755 t/t7513-interpret-trailers.sh
create mode 100644 trailer.c
create mode 100644 trailer.h
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 16/16] trailer: add tests for commands using env variables
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 20 1 file changed, 20 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 2d50b7a..00894a8 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -223,6 +223,26 @@ test_expect_success 'with simple command' ' test_cmp expected actual ' +test_expect_success 'with command using commiter information' ' + git config trailer.sign.ifExist addIfDifferent + git config trailer.sign.command echo \\$GIT_COMMITTER_NAME \$GIT_COMMITTER_EMAIL\ + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: C O Mitter commit...@example.com\n expected + git interpret-trailers review: fix=22 complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using author information' ' + git config trailer.sign.key Signed-off-by: + git config trailer.sign.where after + git config trailer.sign.ifExist addIfDifferentNeighbor + git config trailer.sign.command echo \\$GIT_AUTHOR_NAME \$GIT_AUTHOR_EMAIL\ + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor aut...@example.com\n expected + git interpret-trailers review: fix=22 complex_message actual + test_cmp expected actual +' + test_expect_success 'setup a commit' ' echo Content of the first commit. a.txt git add a.txt -- 1.8.5.2.201.gacc5987 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 07/16] trailer: put all the processing together and print
This patch adds the process_trailers() function that
calls all the previously added processing functions
and then prints the results on the standard output.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 40
1 file changed, 40 insertions(+)
diff --git a/trailer.c b/trailer.c
index 36eb1f8..d2a4427 100644
--- a/trailer.c
+++ b/trailer.c
@@ -48,6 +48,26 @@ static size_t alnum_len(const char *buf, size_t len) {
return len + 1;
}
+static void print_tok_val(const char *tok, const char *val)
+{
+ char c = tok[strlen(tok) - 1];
+ if (isalnum(c))
+ printf(%s: %s\n, tok, val);
+ else if (isspace(c) || c == '#')
+ printf(%s%s\n, tok, val);
+ else
+ printf(%s %s\n, tok, val);
+}
+
+static void print_all(struct trailer_item *first, int trim_empty)
+{
+ struct trailer_item *item;
+ for (item = first; item; item = item-next) {
+ if (!trim_empty || strlen(item-value) 0)
+ print_tok_val(item-token, item-value);
+ }
+}
+
static void add_arg_to_infile(struct trailer_item *infile_tok,
struct trailer_item *arg_tok)
{
@@ -502,3 +522,23 @@ static void process_input_file(const char *infile,
add_trailer_item(infile_tok_first, infile_tok_last, new);
}
}
+
+void process_trailers(const char *infile, int trim_empty, int argc, const char
**argv)
+{
+ struct trailer_item *infile_tok_first = NULL;
+ struct trailer_item *infile_tok_last = NULL;
+ struct trailer_item *arg_tok_first;
+
+ git_config(git_trailer_config, NULL);
+
+ /* Print the non trailer part of infile */
+ if (infile) {
+ process_input_file(infile, infile_tok_first, infile_tok_last);
+ }
+
+ arg_tok_first = process_command_line_args(argc, argv);
+
+ process_trailers_lists(infile_tok_first, infile_tok_last,
arg_tok_first);
+
+ print_all(infile_tok_first, trim_empty);
+}
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 06/16] trailer: parse trailers from input file
This patch reads trailers from an input file, parses
them and puts the result into a doubly linked list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 62 ++
1 file changed, 62 insertions(+)
diff --git a/trailer.c b/trailer.c
index bb1fcfb..36eb1f8 100644
--- a/trailer.c
+++ b/trailer.c
@@ -440,3 +440,65 @@ static struct trailer_item *process_command_line_args(int
argc, const char **arg
return arg_tok_first;
}
+
+static struct strbuf **read_input_file(const char *infile)
+{
+ struct strbuf sb = STRBUF_INIT;
+
+ if (strbuf_read_file(sb, infile, 0) 0)
+ die_errno(_(could not read input file '%s'), infile);
+
+ return strbuf_split(sb, '\n');
+}
+
+/*
+ * Return the the (0 based) index of the first trailer line
+ * or the line count if there are no trailers.
+ */
+static int find_trailer_start(struct strbuf **lines)
+{
+ int count, start, empty = 1;
+
+ /* Get the line count */
+ for (count = 0; lines[count]; count++);
+
+ /*
+* Get the start of the trailers by looking starting from the end
+* for a line with only spaces before lines with one ':'.
+*/
+ for (start = count - 1; start = 0; start--) {
+ if (strbuf_isspace(lines[start])) {
+ if (empty)
+ continue;
+ return start + 1;
+ }
+ if (strchr(lines[start]-buf, ':')) {
+ if (empty)
+ empty = 0;
+ continue;
+ }
+ return count;
+ }
+
+ return empty ? count : start + 1;
+}
+
+static void process_input_file(const char *infile,
+ struct trailer_item **infile_tok_first,
+ struct trailer_item **infile_tok_last)
+{
+ struct strbuf **lines = read_input_file(infile);
+ int start = find_trailer_start(lines);
+ int i;
+
+ /* Print non trailer lines as is */
+ for (i = 0; lines[i] i start; i++) {
+ printf(%s, lines[i]-buf);
+ }
+
+ /* Parse trailer lines */
+ for (i = start; lines[i]; i++) {
+ struct trailer_item *new = create_trailer_item(lines[i]-buf);
+ add_trailer_item(infile_tok_first, infile_tok_last, new);
+ }
+}
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 10/16] trailer: if no input file is passed, read from stdin
It is simpler and more natural if the git interpret-trailers
is made a filter as its output already goes to sdtout.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/interpret-trailers.c | 2 +-
t/t7513-interpret-trailers.sh | 7 +++
trailer.c | 15 +--
3 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index f79bffa..37237f7 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -23,7 +23,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const
char *prefix)
struct option options[] = {
OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty
trailers)),
- OPT_FILENAME(0, infile, infile, N_(use message from file)),
+ OPT_FILENAME(0, infile, infile, N_(use message from file,
instead of stdin)),
OPT_END()
};
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 8be333c..f5ef81f 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -205,4 +205,11 @@ test_expect_success 'using ifMissing = doNothing' '
test_cmp expected actual
'
+test_expect_success 'with input from stdin' '
+ cat complex_message_body expected
+ printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Junio\nAcked-by=
Peff\nReviewed-by: \nSigned-off-by: \n expected
+ git interpret-trailers review: fix=53 cc=Linus ack: Junio
fix=22 bug: 42 ack: Peff complex_message actual
+ test_cmp expected actual
+'
+
test_done
diff --git a/trailer.c b/trailer.c
index d2a4427..9026337 100644
--- a/trailer.c
+++ b/trailer.c
@@ -465,8 +465,13 @@ static struct strbuf **read_input_file(const char *infile)
{
struct strbuf sb = STRBUF_INIT;
- if (strbuf_read_file(sb, infile, 0) 0)
- die_errno(_(could not read input file '%s'), infile);
+ if (infile) {
+ if (strbuf_read_file(sb, infile, 0) 0)
+ die_errno(_(could not read input file '%s'), infile);
+ } else {
+ if (strbuf_read(sb, fileno(stdin), 0) 0)
+ die_errno(_(could not read from stdin));
+ }
return strbuf_split(sb, '\n');
}
@@ -531,10 +536,8 @@ void process_trailers(const char *infile, int trim_empty,
int argc, const char *
git_config(git_trailer_config, NULL);
- /* Print the non trailer part of infile */
- if (infile) {
- process_input_file(infile, infile_tok_first, infile_tok_last);
- }
+ /* Print the non trailer part of infile (or stdin if infile is NULL) */
+ process_input_file(infile, infile_tok_first, infile_tok_last);
arg_tok_first = process_command_line_args(argc, argv);
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 14/16] trailer: add tests for trailer command
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 27 +++ 1 file changed, 27 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index f5ef81f..2d50b7a 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -212,4 +212,31 @@ test_expect_success 'with input from stdin' ' test_cmp expected actual ' +test_expect_success 'with simple command' ' + git config trailer.sign.key Signed-off-by: + git config trailer.sign.where after + git config trailer.sign.ifExist addIfDifferentNeighbor + git config trailer.sign.command echo \A U Thor aut...@example.com\ + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor aut...@example.com\n expected + git interpret-trailers review: fix=22 complex_message actual + test_cmp expected actual +' + +test_expect_success 'setup a commit' ' + echo Content of the first commit. a.txt + git add a.txt + git commit -m Add file a.txt +' + +test_expect_success 'with command using $ARG' ' + git config trailer.fix.ifExist overwrite + git config trailer.fix.command git log -1 --oneline --format=\%h (%s)\ --abbrev-commit --abbrev=14 \$ARG + FIXED=$(git log -1 --oneline --format=%h (%s) --abbrev-commit --abbrev=14 HEAD) + cat complex_message_body expected + printf Fixes: $FIXED\nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor aut...@example.com\n expected + git interpret-trailers review: fix=HEAD complex_message actual + test_cmp expected actual +' + test_done -- 1.8.5.2.201.gacc5987 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 13/16] trailer: execute command from 'trailer.name.command'
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 57 +
1 file changed, 57 insertions(+)
diff --git a/trailer.c b/trailer.c
index 43a3735..549d381 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,4 +1,5 @@
#include cache.h
+#include run-command.h
/*
* Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
*/
@@ -12,11 +13,14 @@ struct conf_info {
char *name;
char *key;
char *command;
+ unsigned command_uses_arg : 1;
enum action_where where;
enum action_if_exist if_exist;
enum action_if_missing if_missing;
};
+#define TRAILER_ARG_STRING $ARG
+
struct trailer_item {
struct trailer_item *previous;
struct trailer_item *next;
@@ -368,6 +372,7 @@ static int git_trailer_config(const char *conf_key, const
char *value, void *cb)
if (conf-command)
warning(_(more than one %s), orig_conf_key);
conf-command = xstrdup(value);
+ conf-command_uses_arg = !!strstr(conf-command,
TRAILER_ARG_STRING);
} else if (type == TRAILER_WHERE) {
if (set_where(conf, value))
warning(_(unknown value '%s' for key '%s'),
value, orig_conf_key);
@@ -400,6 +405,45 @@ static void parse_trailer(struct strbuf *tok, struct
strbuf *val, const char *tr
}
}
+static int read_from_command(struct child_process *cp, struct strbuf *buf)
+{
+ if (run_command(cp))
+ return error(running trailer command '%s' failed,
cp-argv[0]);
+ if (strbuf_read(buf, cp-out, 1024) 1)
+ return error(reading from trailer command '%s' failed,
cp-argv[0]);
+ strbuf_trim(buf);
+ return 0;
+}
+
+static const char *apply_command(const char *command, const char *arg)
+{
+ struct strbuf cmd = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ struct child_process cp;
+ const char *argv[] = {NULL, NULL};
+ const char *result = ;
+
+ strbuf_addstr(cmd, command);
+ if (arg)
+ strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
+
+ argv[0] = cmd.buf;
+ memset(cp, 0, sizeof(cp));
+ cp.argv = argv;
+ cp.env = local_repo_env;
+ cp.no_stdin = 1;
+ cp.out = -1;
+ cp.use_shell = 1;
+
+ if (read_from_command(cp, buf))
+ strbuf_release(buf);
+ else
+ result = strbuf_detach(buf, NULL);
+
+ strbuf_release(cmd);
+ return result;
+}
+
static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
const char* tok, const char* val)
{
@@ -409,6 +453,8 @@ static struct trailer_item *new_trailer_item(struct
trailer_item *conf_item,
if (conf_item) {
new-conf = conf_item-conf;
new-token = xstrdup(conf_item-conf-key);
+ if (conf_item-conf-command_uses_arg || !val)
+ new-value = apply_command(conf_item-conf-command,
val);
} else {
new-conf = xcalloc(sizeof(struct conf_info), 1);
new-token = tok;
@@ -459,12 +505,23 @@ static struct trailer_item *process_command_line_args(int
argc, const char **arg
int i;
struct trailer_item *arg_tok_first = NULL;
struct trailer_item *arg_tok_last = NULL;
+ struct trailer_item *item;
for (i = 0; i argc; i++) {
struct trailer_item *new = create_trailer_item(argv[i]);
add_trailer_item(arg_tok_first, arg_tok_last, new);
}
+ /* Add conf commands that don't use $ARG */
+ for (item = first_conf_item; item; item = item-next)
+ {
+ if (item-conf-command !item-conf-command_uses_arg)
+ {
+ struct trailer_item *new = new_trailer_item(item, NULL,
NULL);
+ add_trailer_item(arg_tok_first, arg_tok_last, new);
+ }
+ }
+
return arg_tok_first;
}
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 09/16] trailer: add tests for git interpret-trailers
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 208 ++ 1 file changed, 208 insertions(+) create mode 100755 t/t7513-interpret-trailers.sh diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh new file mode 100755 index 000..8be333c --- /dev/null +++ b/t/t7513-interpret-trailers.sh @@ -0,0 +1,208 @@ +#!/bin/sh +# +# Copyright (c) 2013 Christian Couder +# + +test_description='git interpret-trailers' + +. ./test-lib.sh + +cat basic_message 'EOF' +subject + +body +EOF + +cat complex_message_body 'EOF' +my subject + +my body which is long +and contains some special +chars like : = ? ! + +EOF + +# We want one trailing space at the end of each line. +# Let's use sed to make sure that these spaces are not removed +# by any automatic tool. +sed -e 's/ Z$/ /' complex_message_trailers -\EOF +Fixes: Z +Acked-by: Z +Reviewed-by: Z +Signed-off-by: Z +EOF + +test_expect_success 'without config' ' + printf ack: Peff\nReviewed-by: \nAcked-by: Johan\n expected + git interpret-trailers ack = Peff Reviewed-by Acked-by: Johan actual + test_cmp expected actual +' + +test_expect_success '--trim-empty without config' ' + printf ack: Peff\nAcked-by: Johan\n expected + git interpret-trailers --trim-empty ack = Peff Reviewed-by Acked-by: Johan sob: actual + test_cmp expected actual +' + +test_expect_success 'with config setup' ' + git config trailer.ack.key Acked-by: + printf Acked-by: Peff\n expected + git interpret-trailers --trim-empty ack = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by :Peff actual + test_cmp expected actual +' + +test_expect_success 'with config setup and = sign' ' + git config trailer.ack.key Acked-by= + printf Acked-by= Peff\n expected + git interpret-trailers --trim-empty ack = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by= Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by : Peff actual + test_cmp expected actual +' + +test_expect_success 'with config setup and # sign' ' + git config trailer.bug.key Bug # + printf Bug #42\n expected + git interpret-trailers --trim-empty bug = 42 actual + test_cmp expected actual +' + +test_expect_success 'with commit basic message' ' + git interpret-trailers --infile basic_message actual + test_cmp basic_message actual +' + +test_expect_success 'with commit complex message' ' + cat complex_message_body complex_message_trailers complex_message + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \n expected + git interpret-trailers --infile complex_message actual + test_cmp expected actual +' + +test_expect_success 'with commit complex message and args' ' + cat complex_message_body expected + printf Fixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: \nSigned-off-by: \nBug #42\n expected + git interpret-trailers --infile complex_message ack: Peff bug: 42 actual + test_cmp expected actual +' + +test_expect_success 'with commit complex message, args and --trim-empty' ' + cat complex_message_body expected + printf Acked-by= Peff\nBug #42\n expected + git interpret-trailers --trim-empty --infile complex_message ack: Peff bug: 42 actual + test_cmp expected actual +' + +test_expect_success 'using where = before' ' + git config trailer.bug.where before + cat complex_message_body expected + printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: \nSigned-off-by: \n expected + git interpret-trailers --infile complex_message ack: Peff bug: 42 actual + test_cmp expected actual +' + +test_expect_success 'using where = before for a token in the middle of infile' ' + git config trailer.review.key Reviewed-by: + git config trailer.review.where before + cat complex_message_body expected + printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: Johan\nReviewed-by: \nSigned-off-by: \n expected + git interpret-trailers --infile complex_message ack: Peff bug: 42 review: Johan actual + test_cmp expected actual +' + +test_expect_success 'using where = before and --trim-empty' ' + cat complex_message_body expected + printf Bug #46\nBug #42\nAcked-by= Peff\nReviewed-by: Johan\n expected + git interpret-trailers --infile complex_message --trim-empty ack: Peff bug: 42 review: Johan Bug: 46 actual + test_cmp expected actual +' + +test_expect_success 'the default is ifExist = addIfDifferent' ' + cat complex_message_body expected +
[PATCH v2 11/16] trailer: add new_trailer_item() function
This is a small refactoring to prepare for the next steps.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 31 +++
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/trailer.c b/trailer.c
index 9026337..43a3735 100644
--- a/trailer.c
+++ b/trailer.c
@@ -400,11 +400,27 @@ static void parse_trailer(struct strbuf *tok, struct
strbuf *val, const char *tr
}
}
+static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
+const char* tok, const char* val)
+{
+ struct trailer_item *new = xcalloc(sizeof(struct trailer_item), 1);
+ new-value = val;
+
+ if (conf_item) {
+ new-conf = conf_item-conf;
+ new-token = xstrdup(conf_item-conf-key);
+ } else {
+ new-conf = xcalloc(sizeof(struct conf_info), 1);
+ new-token = tok;
+ }
+
+ return new;
+}
+
static struct trailer_item *create_trailer_item(const char *string)
{
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
- struct trailer_item *new;
parse_trailer(tok, val, string);
@@ -416,21 +432,12 @@ static struct trailer_item *create_trailer_item(const
char *string)
{
if (!strncasecmp(tok.buf, item-conf-key, tok_alnum_len) ||
!strncasecmp(tok.buf, item-conf-name, tok_alnum_len)) {
- new = xcalloc(sizeof(struct trailer_item), 1);
- new-conf = item-conf;
- new-token = xstrdup(item-conf-key);
- new-value = strbuf_detach(val, NULL);
strbuf_release(tok);
- return new;
+ return new_trailer_item(item, NULL, strbuf_detach(val,
NULL));
}
}
- new = xcalloc(sizeof(struct trailer_item), 1);
- new-conf = xcalloc(sizeof(struct conf_info), 1);
- new-token = strbuf_detach(tok, NULL);
- new-value = strbuf_detach(val, NULL);
-
- return new;
+ return new_trailer_item(NULL, strbuf_detach(tok, NULL),
strbuf_detach(val, NULL));;
}
static void add_trailer_item(struct trailer_item **first,
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 05/16] strbuf: add strbuf_isspace()
This helper function checks if a strbuf
contains only space chars or not.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
strbuf.c | 7 +++
strbuf.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/strbuf.c b/strbuf.c
index 83caf4a..2124bb8 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -124,6 +124,13 @@ void strbuf_ltrim(struct strbuf *sb)
sb-buf[sb-len] = '\0';
}
+int strbuf_isspace(struct strbuf *sb)
+{
+ char *b;
+ for (b = sb-buf; *b isspace(*b); b++);
+ return !*b;
+}
+
struct strbuf **strbuf_split_buf(const char *str, size_t slen,
int terminator, int max)
{
diff --git a/strbuf.h b/strbuf.h
index 73e80ce..02bff3a 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -42,6 +42,7 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t
len) {
extern void strbuf_trim(struct strbuf *);
extern void strbuf_rtrim(struct strbuf *);
extern void strbuf_ltrim(struct strbuf *);
+extern int strbuf_isspace(struct strbuf *);
extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
/*
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 08/16] trailer: add interpret-trailers command
This patch adds the git interpret-trailers command.
This command uses the previously added process_trailers()
function in trailer.c.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
.gitignore | 1 +
Makefile | 1 +
builtin.h| 1 +
builtin/interpret-trailers.c | 36
git.c| 1 +
trailer.h| 6 ++
6 files changed, 46 insertions(+)
create mode 100644 builtin/interpret-trailers.c
create mode 100644 trailer.h
diff --git a/.gitignore b/.gitignore
index b5f9def..c870ada 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
/git-index-pack
/git-init
/git-init-db
+/git-interpret-trailers
/git-instaweb
/git-log
/git-ls-files
diff --git a/Makefile b/Makefile
index ec90feb..a91465e 100644
--- a/Makefile
+++ b/Makefile
@@ -935,6 +935,7 @@ BUILTIN_OBJS += builtin/hash-object.o
BUILTIN_OBJS += builtin/help.o
BUILTIN_OBJS += builtin/index-pack.o
BUILTIN_OBJS += builtin/init-db.o
+BUILTIN_OBJS += builtin/interpret-trailers.o
BUILTIN_OBJS += builtin/log.o
BUILTIN_OBJS += builtin/ls-files.o
BUILTIN_OBJS += builtin/ls-remote.o
diff --git a/builtin.h b/builtin.h
index d4afbfe..30f4c30 100644
--- a/builtin.h
+++ b/builtin.h
@@ -71,6 +71,7 @@ extern int cmd_hash_object(int argc, const char **argv, const
char *prefix);
extern int cmd_help(int argc, const char **argv, const char *prefix);
extern int cmd_index_pack(int argc, const char **argv, const char *prefix);
extern int cmd_init_db(int argc, const char **argv, const char *prefix);
+extern int cmd_interpret_trailers(int argc, const char **argv, const char
*prefix);
extern int cmd_log(int argc, const char **argv, const char *prefix);
extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
extern int cmd_ls_files(int argc, const char **argv, const char *prefix);
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
new file mode 100644
index 000..f79bffa
--- /dev/null
+++ b/builtin/interpret-trailers.c
@@ -0,0 +1,36 @@
+/*
+ * Builtin git interpret-trailers
+ *
+ * Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
+ *
+ */
+
+#include cache.h
+#include builtin.h
+#include parse-options.h
+#include strbuf.h
+#include trailer.h
+
+static const char * const git_interpret_trailers_usage[] = {
+ N_(git interpret-trailers [--trim-empty] [--infile=file]
[token[=value]...]),
+ NULL
+};
+
+int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
+{
+ const char *infile = NULL;
+ int trim_empty = 0;
+
+ struct option options[] = {
+ OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty
trailers)),
+ OPT_FILENAME(0, infile, infile, N_(use message from file)),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_interpret_trailers_usage, 0);
+
+ process_trailers(infile, trim_empty, argc, argv);
+
+ return 0;
+}
diff --git a/git.c b/git.c
index 3799514..1420b58 100644
--- a/git.c
+++ b/git.c
@@ -383,6 +383,7 @@ static void handle_internal_command(int argc, const char
**argv)
{ index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
{ init, cmd_init_db },
{ init-db, cmd_init_db },
+ { interpret-trailers, cmd_interpret_trailers, RUN_SETUP },
{ log, cmd_log, RUN_SETUP },
{ ls-files, cmd_ls_files, RUN_SETUP },
{ ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY },
diff --git a/trailer.h b/trailer.h
new file mode 100644
index 000..9db4459
--- /dev/null
+++ b/trailer.h
@@ -0,0 +1,6 @@
+#ifndef TRAILER_H
+#define TRAILER_H
+
+void process_trailers(const char *infile, int trim_empty, int argc, const char
**argv);
+
+#endif /* TRAILER_H */
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 12/16] strbuf: add strbuf_replace()
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
strbuf.c | 7 +++
strbuf.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/strbuf.c b/strbuf.c
index 2124bb8..e45e513 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -197,6 +197,13 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t
len,
strbuf_setlen(sb, sb-len + dlen - len);
}
+void strbuf_replace(struct strbuf *sb, const char *a, const char *b)
+{
+ char *ptr = strstr(sb-buf, a);
+ if (ptr)
+ strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b));
+}
+
void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)
{
strbuf_splice(sb, pos, 0, data, len);
diff --git a/strbuf.h b/strbuf.h
index 02bff3a..38faf70 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -111,6 +111,9 @@ extern void strbuf_remove(struct strbuf *, size_t pos,
size_t len);
extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,
const void *, size_t);
+/* first occurence of a replaced with b */
+extern void strbuf_replace(struct strbuf *, const char *a, const char *b);
+
extern void strbuf_add_commented_lines(struct strbuf *out, const char *buf,
size_t size);
extern void strbuf_add(struct strbuf *, const void *, size_t);
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 02/16] trailer: process trailers from file and arguments
This patch implements the logic that process trailers
from file and arguments.
At the beginning trailers from file are in their own
infile_tok doubly linked list, and trailers from
arguments are in their own arg_tok doubly linked list.
The lists are traversed and when an arg_tok should be
applied, it is removed from its list and inserted
into the infile_tok list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 189 ++
1 file changed, 189 insertions(+)
diff --git a/trailer.c b/trailer.c
index d80d047..d88de3f 100644
--- a/trailer.c
+++ b/trailer.c
@@ -45,3 +45,192 @@ static size_t alnum_len(const char *buf, size_t len) {
while (--len = 0 !isalnum(buf[len]));
return len + 1;
}
+
+static void add_arg_to_infile(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok)
+{
+ if (arg_tok-conf-where == WHERE_AFTER) {
+ arg_tok-next = infile_tok-next;
+ infile_tok-next = arg_tok;
+ arg_tok-previous = infile_tok;
+ if (arg_tok-next)
+ arg_tok-next-previous = arg_tok;
+ } else {
+ arg_tok-previous = infile_tok-previous;
+ infile_tok-previous = arg_tok;
+ arg_tok-next = infile_tok;
+ if (arg_tok-previous)
+ arg_tok-previous-next = arg_tok;
+ }
+}
+
+static int check_if_different(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len, int check_all)
+{
+ enum action_where where = arg_tok-conf-where;
+ do {
+ if (!infile_tok)
+ return 1;
+ if (same_trailer(infile_tok, arg_tok, alnum_len))
+ return 0;
+ /*
+* if we want to add a trailer after another one,
+* we have to check those before this one
+*/
+ infile_tok = (where == WHERE_AFTER) ? infile_tok-previous :
infile_tok-next;
+ } while (check_all);
+ return 1;
+}
+
+static void apply_arg_if_exist(struct trailer_item *infile_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len)
+{
+ switch(arg_tok-conf-if_exist) {
+ case EXIST_DO_NOTHING:
+ free(arg_tok);
+ break;
+ case EXIST_OVERWRITE:
+ free((char *)infile_tok-value);
+ infile_tok-value = xstrdup(arg_tok-value);
+ free(arg_tok);
+ break;
+ case EXIST_ADD:
+ add_arg_to_infile(infile_tok, arg_tok);
+ break;
+ case EXIST_ADD_IF_DIFFERENT:
+ if (check_if_different(infile_tok, arg_tok, alnum_len, 1))
+ add_arg_to_infile(infile_tok, arg_tok);
+ else
+ free(arg_tok);
+ break;
+ case EXIST_ADD_IF_DIFFERENT_NEIGHBOR:
+ if (check_if_different(infile_tok, arg_tok, alnum_len, 0))
+ add_arg_to_infile(infile_tok, arg_tok);
+ else
+ free(arg_tok);
+ break;
+ }
+}
+
+static void remove_from_list(struct trailer_item *item,
+struct trailer_item **first)
+{
+ if (item-next)
+ item-next-previous = item-previous;
+ if (item-previous)
+ item-previous-next = item-next;
+ else
+ *first = item-next;
+}
+
+static struct trailer_item *remove_first(struct trailer_item **first)
+{
+ struct trailer_item *item = *first;
+ *first = item-next;
+ if (item-next) {
+ item-next-previous = NULL;
+ item-next = NULL;
+ }
+ return item;
+}
+
+static void process_infile_tok(struct trailer_item *infile_tok,
+ struct trailer_item **arg_tok_first,
+ enum action_where where)
+{
+ struct trailer_item *arg_tok;
+ struct trailer_item *next_arg;
+
+ int tok_alnum_len = alnum_len(infile_tok-token,
strlen(infile_tok-token));
+ for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+ next_arg = arg_tok-next;
+ if (same_token(infile_tok, arg_tok, tok_alnum_len)
+ arg_tok-conf-where == where) {
+ /* Remove arg_tok from list */
+ remove_from_list(arg_tok, arg_tok_first);
+ /* Apply arg */
+ apply_arg_if_exist(infile_tok, arg_tok, tok_alnum_len);
+ /*
+* If arg has been added to infile,
+* then we need to process it too now.
+*/
+ if ((where == WHERE_AFTER ?
[PATCH v2 15/16] trailer: set author and committer env variables
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 30 +-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/trailer.c b/trailer.c
index 549d381..3b21f57 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,6 @@
#include cache.h
#include run-command.h
+#include argv-array.h
/*
* Copyright (c) 2013 Christian Couder chrisc...@tuxfamily.org
*/
@@ -415,14 +416,40 @@ static int read_from_command(struct child_process *cp,
struct strbuf *buf)
return 0;
}
+static void setup_ac_env(struct argv_array *env, const char *ac_name, const
char *ac_mail, const char *(*read)(int))
+{
+ if (!getenv(ac_name) || !getenv(ac_mail)) {
+ struct ident_split ident;
+ const char *namebuf, *mailbuf;
+ int namelen, maillen;
+ const char *ac_info = read(IDENT_NO_DATE);
+
+ if (split_ident_line(ident, ac_info, strlen(ac_info)))
+ return;
+
+ namelen = ident.name_end - ident.name_begin;
+ namebuf = ident.name_begin;
+
+ maillen = ident.mail_end - ident.mail_begin;
+ mailbuf = ident.mail_begin;
+
+ argv_array_pushf(env, %s=%.*s, ac_name, namelen, namebuf);
+ argv_array_pushf(env, %s=%.*s, ac_mail, maillen, mailbuf);
+ }
+}
+
static const char *apply_command(const char *command, const char *arg)
{
+ struct argv_array env = ARGV_ARRAY_INIT;
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
struct child_process cp;
const char *argv[] = {NULL, NULL};
const char *result = ;
+ setup_ac_env(env, GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL,
git_author_info);
+ setup_ac_env(env, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL,
git_committer_info);
+
strbuf_addstr(cmd, command);
if (arg)
strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
@@ -430,7 +457,7 @@ static const char *apply_command(const char *command, const
char *arg)
argv[0] = cmd.buf;
memset(cp, 0, sizeof(cp));
cp.argv = argv;
- cp.env = local_repo_env;
+ cp.env = env.argv;
cp.no_stdin = 1;
cp.out = -1;
cp.use_shell = 1;
@@ -441,6 +468,7 @@ static const char *apply_command(const char *command, const
char *arg)
result = strbuf_detach(buf, NULL);
strbuf_release(cmd);
+ argv_array_clear(env);
return result;
}
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 04/16] trailer: process command line trailer arguments
This patch parses the trailer command line arguments
and put the result into an arg_tok doubly linked
list.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 77 +++
1 file changed, 77 insertions(+)
diff --git a/trailer.c b/trailer.c
index e7d8244..bb1fcfb 100644
--- a/trailer.c
+++ b/trailer.c
@@ -363,3 +363,80 @@ static int git_trailer_config(const char *conf_key, const
char *value, void *cb)
}
return 0;
}
+
+static void parse_trailer(struct strbuf *tok, struct strbuf *val, const char
*trailer)
+{
+ char *end = strchr(trailer, '=');
+ if (!end)
+ end = strchr(trailer, ':');
+ if (end) {
+ strbuf_add(tok, trailer, end - trailer);
+ strbuf_trim(tok);
+ strbuf_addstr(val, end + 1);
+ strbuf_trim(val);
+ } else {
+ strbuf_addstr(tok, trailer);
+ strbuf_trim(tok);
+ }
+}
+
+static struct trailer_item *create_trailer_item(const char *string)
+{
+ struct strbuf tok = STRBUF_INIT;
+ struct strbuf val = STRBUF_INIT;
+ struct trailer_item *new;
+
+ parse_trailer(tok, val, string);
+
+ int tok_alnum_len = alnum_len(tok.buf, tok.len);
+
+ /* Lookup if the token matches something in the config */
+ struct trailer_item *item;
+ for (item = first_conf_item; item; item = item-next)
+ {
+ if (!strncasecmp(tok.buf, item-conf-key, tok_alnum_len) ||
+ !strncasecmp(tok.buf, item-conf-name, tok_alnum_len)) {
+ new = xcalloc(sizeof(struct trailer_item), 1);
+ new-conf = item-conf;
+ new-token = xstrdup(item-conf-key);
+ new-value = strbuf_detach(val, NULL);
+ strbuf_release(tok);
+ return new;
+ }
+ }
+
+ new = xcalloc(sizeof(struct trailer_item), 1);
+ new-conf = xcalloc(sizeof(struct conf_info), 1);
+ new-token = strbuf_detach(tok, NULL);
+ new-value = strbuf_detach(val, NULL);
+
+ return new;
+}
+
+static void add_trailer_item(struct trailer_item **first,
+struct trailer_item **last,
+struct trailer_item *new)
+{
+ if (!*last) {
+ *first = new;
+ *last = new;
+ } else {
+ (*last)-next = new;
+ new-previous = *last;
+ *last = new;
+ }
+}
+
+static struct trailer_item *process_command_line_args(int argc, const char
**argv)
+{
+ int i;
+ struct trailer_item *arg_tok_first = NULL;
+ struct trailer_item *arg_tok_last = NULL;
+
+ for (i = 0; i argc; i++) {
+ struct trailer_item *new = create_trailer_item(argv[i]);
+ add_trailer_item(arg_tok_first, arg_tok_last, new);
+ }
+
+ return arg_tok_first;
+}
--
1.8.5.2.201.gacc5987
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 03/16] trailer: read and process config information
This patch implements reading the configuration
to get trailer information, and then processing
it and storing it in a doubly linked list.
The config information is stored in the list
whose first item is pointed to by:
static struct trailer_item *first_conf_item;
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
trailer.c | 129 ++
1 file changed, 129 insertions(+)
diff --git a/trailer.c b/trailer.c
index d88de3f..e7d8244 100644
--- a/trailer.c
+++ b/trailer.c
@@ -25,6 +25,8 @@ struct trailer_item {
struct conf_info *conf;
};
+static struct trailer_item *first_conf_item;
+
static int same_token(struct trailer_item *a, struct trailer_item *b, int
alnum_len)
{
return !strncasecmp(a-token, b-token, alnum_len);
@@ -234,3 +236,130 @@ static void process_trailers_lists(struct trailer_item
**infile_tok_first,
apply_arg_if_missing(infile_tok_first, infile_tok_last,
arg_tok);
}
}
+
+static int set_where(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(after, value)) {
+ item-where = WHERE_AFTER;
+ } else if (!strcasecmp(before, value)) {
+ item-where = WHERE_BEFORE;
+ } else
+ return 1;
+ return 0;
+}
+
+static int set_if_exist(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(addIfDifferent, value)) {
+ item-if_exist = EXIST_ADD_IF_DIFFERENT;
+ } else if (!strcasecmp(addIfDifferentNeighbor, value)) {
+ item-if_exist = EXIST_ADD_IF_DIFFERENT_NEIGHBOR;
+ } else if (!strcasecmp(add, value)) {
+ item-if_exist = EXIST_ADD;
+ } else if (!strcasecmp(overwrite, value)) {
+ item-if_exist = EXIST_OVERWRITE;
+ } else if (!strcasecmp(doNothing, value)) {
+ item-if_exist = EXIST_DO_NOTHING;
+ } else
+ return 1;
+ return 0;
+}
+
+static int set_if_missing(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(doNothing, value)) {
+ item-if_missing = MISSING_DO_NOTHING;
+ } else if (!strcasecmp(add, value)) {
+ item-if_missing = MISSING_ADD;
+ } else
+ return 1;
+ return 0;
+}
+
+enum trailer_info_type { TRAILER_VALUE, TRAILER_COMMAND, TRAILER_WHERE,
+TRAILER_IF_EXIST, TRAILER_IF_MISSING };
+
+static int set_name_and_type(const char *conf_key, const char *suffix,
+enum trailer_info_type type,
+char **pname, enum trailer_info_type *ptype)
+{
+ int ret = ends_with(conf_key, suffix);
+ if (ret) {
+ *pname = xstrndup(conf_key, strlen(conf_key) - strlen(suffix));
+ *ptype = type;
+ }
+ return ret;
+}
+
+static struct trailer_item *get_conf_item(char *name)
+{
+ struct trailer_item *item;
+ struct trailer_item *previous;
+
+ /* Look up item with same name */
+ for (previous = NULL, item = first_conf_item;
+item;
+previous = item, item = item-next)
+ {
+ if (!strcasecmp(item-conf-name, name))
+ return item;
+ }
+
+ /* Item does not already exists, create it */
+ item = xcalloc(sizeof(struct trailer_item), 1);
+ item-conf = xcalloc(sizeof(struct conf_info), 1);
+ item-conf-name = xstrdup(name);
+
+ if (!previous) {
+ first_conf_item = item;
+ } else {
+ previous-next = item;
+ item-previous = previous;
+ }
+
+ return item;
+}
+
+static int git_trailer_config(const char *conf_key, const char *value, void
*cb)
+{
+ if (starts_with(conf_key, trailer.)) {
+ const char *orig_conf_key = conf_key;
+ struct trailer_item *item;
+ struct conf_info *conf;
+ char *name;
+ enum trailer_info_type type;
+
+ conf_key += 8;
+ if (!set_name_and_type(conf_key, .key, TRAILER_VALUE, name,
type)
+ !set_name_and_type(conf_key, .command, TRAILER_COMMAND,
name, type)
+ !set_name_and_type(conf_key, .where, TRAILER_WHERE,
name, type)
+ !set_name_and_type(conf_key, .ifexist, TRAILER_IF_EXIST,
name, type)
+ !set_name_and_type(conf_key, .ifmissing,
TRAILER_IF_MISSING, name, type))
+ return 0;
+
+ item = get_conf_item(name);
+ conf = item-conf;
+
+ if (type == TRAILER_VALUE) {
+ if (conf-key)
+ warning(_(more than one %s), orig_conf_key);
+ conf-key = xstrdup(value);
+ } else if (type == TRAILER_COMMAND) {
+ if (conf-command)
+ warning(_(more than one %s),
Re: [Question] Usercase about git clone
Hi, On Fri, Jan 17, 2014 at 4:53 PM, Wang Shilong wangshilong1...@gmail.com wrote: Hello everyone, I have a question about command 'git clone' If i clone a repo from remote, and if i run command: # git remote show origin It will output origin's url, however, this is what i want, Is it really what you want or not? i just want to clone codes, but keep everything else unchanged, for example branches and they url…. There are urls for remote not for branches. Did you look at the --mirror option? Do you want something like the mirror option but with a working directory? How can i implement such functions by 'git clone'….I think this is really helpful because i really don't want to reset my branches' url every one… Again there are no urls for branches. Could you explain better what you would like exactly? Best, Christian. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP v2 08/14] read-cache: add GIT_TEST_FORCE_WATCHER for testing
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
This can be used to force watcher on when running the test
suite.
git-file-watcher processes are not automatically cleaned up after each
test. So after running the test suite you'll be left with plenty
git-file-watcher processes that should all end after about a minute.
Probably not a very good idea, especially in noninteractive use? E.g.,
a bisection through the test suite or parallel test runs on different
commits may exhaust the available processes and/or memory.
Each test should make an effort to clean up all watchers before
terminating.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
read-cache.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index 5dae9eb..a1245d4 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1544,7 +1544,12 @@ static void validate_watcher(struct index_state
*istate, const char *path)
}
if (autorun_watcher == -1) {
- git_config(watcher_config, NULL);
+ if (getenv(GIT_TEST_FORCE_WATCHER)) {
+ watch_lowerlimit = 0;
+ recent_limit = 0;
+ autorun_watcher = 1;
+ } else
+ git_config(watcher_config, NULL);
if (autorun_watcher == -1)
autorun_watcher = 0;
}
--
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP v2 00/14] inotify support
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: read-cache: save trailing sha-1 read-cache: new extension to mark what file is watched read-cache: connect to file watcher read-cache: ask file watcher to watch files read-cache: put some limits on file watching read-cache: get modified file list from file watcher read-cache: add config to start file watcher automatically read-cache: add GIT_TEST_FORCE_WATCHER for testing file-watcher: add --shutdown and --log options file-watcher: automatically quit file-watcher: support inotify file-watcher: exit when cwd is gone pkt-line.c: increase buffer size to 8192 t1301: exclude sockets from file permission check I never got the last three patches, did you send them? Also, this doesn't cleanly apply anywhere at my end. Can you push it somewhere for easier experimentation? This is getting in better shape. Still wondering if the design is right, so documentation, tests and some error cases are still neglected. I have not addressed Jonathan's and Jeff's comments in this reroll, but I haven't forgotten them yet. The test suite seems to be fine when file-watcher is forced on with GIT_TEST_FORCE_WATCHER set.. I tried to figure out whether there were any corners you are painting it into, but doing so without a clear overview of the protocol makes my head spin. So here's what I gather from the patches (I would have started from the final result, but see above). The slow path, before the watcher is ready, works like: spawn watcher send hello $index_sha1 receive hello mark most files CE_WATCHED send watch $paths for each CE_WATCHED the paths are actually a pkt-line-encoded series of paths get back fine $n (n = number of watches processed) do work as usual (including lstat()) The fast path: load CE_WATCHED from index send hello $index_sha1 receive hello $index_sha1 send status get back new $path for each changed file again aggregate mark files that are new as ~CE_WATCHED files that are CE_WATCHED can skip their lstat(), since they are unchanged On index writing (both paths): save CE_WATCHED in index extension send bye $index_sha1 send forget $index_sha1 $path for every path that is known to be changed So I think the watcher protocol can be specified roughly as: Definitions: The watcher is a separate process that maintains a set of _watched paths_. Git uses the commands below to add or remove paths from this set. In addition it maintains a set of _changed paths_, which is a subset of the watched paths. Git occasionally queries the changed paths. If at any time after a path P was added to the watched set, P has or may have changed, it MUST be added to the changed set. Note that a path being unchanged under this definition does NOT mean that it is unchanged in the index as per `git diff`. It may have been changed before the watch was issued! Therefore, Git MUST test whether the path started out unchanged using lstat() AFTER it was added to the watched set. Handshake:: On connecting, git sends hello magic. magic is an opaque string that identifies the state of the index. The watcher MUST respond with hello previous_magic, where previous_magic is obtained from the bye command (below). If magic != previous_magic, the watcher MUST reset state for this repository. Watch:: Git sends watch pathlist where the pathlist consists of a series of 4-digit lengths and literal pathnames (as in the pkt-line format) for each path it is interested in. The watcher MUST respond with fine n after processing a message that contained n paths. The watcher MUST add each path to the watched set and remove it from the changed set (no error if it was already watched, or not changed). Status:: Git sends status. The watcher MUST respond with its current changed set. This uses the format new pathlist, with pathlist formatted as for the watch command. Bye:: Git sends bye magic to indicate it has finished writing the index. The watcher must store magic so as to use it as the previous_magic in a hello response. Forget:: Git sends forget pathlist, with the pathlist formatted as for the watch command. The watcher SHOULD remove each path in pathlist from its watched set. Did I get that approximately straight? Perhaps you can fix up as needed and then turn it into the documentation for the protocol. There are several points about this that I don't like: * What does the datagram socket buy us? You already do a lot of pkt-line work for the really big messages, so why not just use pkt-line everywhere and a streaming socket? * The handshake should have room for capabilities, so that we can extend the protocol. * The hello/hello handshake is confusing, and probably would allow you to point two watchers at each other without them noticing. Make it hello/ready, or
Re: [PATCH/WIP v2 05/14] read-cache: put some limits on file watching
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
watch_entries() is a lot of computation and could trigger a lot more
lookups in file-watcher. Normally after the first set of watches are
in place, we do not need to update often. Moreover if the number of
entries is small, the overhead of file watcher may actually slow git
down.
This patch only allows to update watches if the number of watchable
files is over a limit (and there are new files added if this is not
the first time). Measurements on Core i5-2520M and Linux 3.7.6, about
920 lstat() take 1ms. Somewhere between 2^16 and 2^17 lstat calls that
it starts to take longer than 100ms. 2^16 is chosen at the minimum
limit to start using file watcher.
Recently updated files are not considered watchable because they are
likely to be updated again soon, not worth the ping-pong game with
file watcher. The default limit 30min is just a random value.
But then a fresh clone of a big repository would not get any benefit
from the watcher?
Not yet sure how to best handle this.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/config.txt | 9 +
cache.h | 1 +
read-cache.c | 44
3 files changed, 46 insertions(+), 8 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a405806..e394399 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1038,6 +1038,15 @@ difftool.tool.cmd::
difftool.prompt::
Prompt before each invocation of the diff tool.
+filewatcher.minfiles::
+ Start watching files if the number of watchable files are
+ above this limit. Default value is 65536.
+
+filewatcher.recentlimit::
+ Files that are last updated within filewatcher.recentlimit
+ seconds from now are not considered watchable. Default value
+ is 1800 (30 minutes).
+
fetch.recurseSubmodules::
This option can be either set to a boolean value or to 'on-demand'.
Setting it to a boolean changes the behavior of fetch and pull to
diff --git a/cache.h b/cache.h
index 0d1..bcec29b 100644
--- a/cache.h
+++ b/cache.h
@@ -278,6 +278,7 @@ struct index_state {
struct cache_tree *cache_tree;
struct cache_time timestamp;
unsigned name_hash_initialized : 1,
+ update_watches : 1,
initialized : 1;
struct hashmap name_hash;
struct hashmap dir_hash;
diff --git a/read-cache.c b/read-cache.c
index 21c3207..406834a 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -38,6 +38,8 @@ static struct cache_entry *refresh_cache_entry(struct
cache_entry *ce, int reall
#define CACHE_EXT_WATCH 0x57415443 /* WATC */
struct index_state the_index;
+static int watch_lowerlimit = 65536;
+static int recent_limit = 1800;
static void set_index_entry(struct index_state *istate, int nr, struct
cache_entry *ce)
{
@@ -1014,6 +1016,7 @@ int add_index_entry(struct index_state *istate, struct
cache_entry *ce, int opti
(istate-cache_nr - pos - 1) * sizeof(ce));
set_index_entry(istate, pos, ce);
istate-cache_changed = 1;
+ istate-update_watches = 1;
return 0;
}
@@ -1300,13 +1303,14 @@ static void read_watch_extension(struct index_state
*istate, uint8_t *data,
unsigned long sz)
{
int i;
- if ((istate-cache_nr + 7) / 8 != sz) {
+ if ((istate-cache_nr + 7) / 8 + 1 != sz) {
error(invalid 'WATC' extension);
return;
}
for (i = 0; i istate-cache_nr; i++)
if (data[i / 8] (1 (i % 8)))
istate-cache[i]-ce_flags |= CE_WATCHED;
+ istate-update_watches = data[sz - 1];
}
static int read_index_extension(struct index_state *istate,
@@ -1449,6 +1453,19 @@ static struct cache_entry *create_from_disk(struct
ondisk_cache_entry *ondisk,
return ce;
}
+static int watcher_config(const char *var, const char *value, void *data)
+{
+ if (!strcmp(var, filewatcher.minfiles)) {
+ watch_lowerlimit = git_config_int(var, value);
+ return 0;
+ }
+ if (!strcmp(var, filewatcher.recentlimit)) {
+ recent_limit = git_config_int(var, value);
+ return 0;
+ }
+ return 0;
+}
+
static void validate_watcher(struct index_state *istate, const char *path)
{
int i;
@@ -1458,6 +1475,7 @@ static void validate_watcher(struct index_state
*istate, const char *path)
return;
}
+ git_config(watcher_config, NULL);
istate-watcher = connect_watcher(path);
if (istate-watcher != -1) {
struct strbuf sb = STRBUF_INIT;
@@ -1478,6 +1496,7 @@ static void validate_watcher(struct index_state
*istate, const char *path)
istate-cache[i]-ce_flags = ~CE_WATCHED;
Re: [PATCH/WIP v2 02/14] read-cache: new extension to mark what file is watched
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: If an entry is watched, git lets an external program decide if the entry is modified or not. It's more like --assume-unchanged, but designed to be controlled by machine. We are running out of on-disk ce_flags, so instead of extending on-disk entry format again, watched flags are in-core only and stored as extension instead. I wonder if this would be a good use-case for EWAH bitmaps? Presumably most users would end up having only a few large ranges of files that are being watched. Quite possibly most users would watch *all* files. -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Two janitorial patches for builtin/blame.c
This is more a warmup than anything else: I'm actually doing a quite more involved rewrite of git-blame right now. But it's been a long time since I sent patches for Git, so I'm starting out with something reasonably uncontroversial. Patch 1 is a no-brainer: maintaining reverse links is not worth the trouble if you are not going to use them. Now one can be conservative and say but git-blame is awfully inefficient and maybe we'll need them in a more efficient version. I can answer this with no: the kind of stuff that would make things more efficient does not require backlinks. Patch 2 is a bit more tricky: basically its contention is that I understand some implications of the code better than its author appeared to do. Which is somewhat optimistic. Since my followup work depends on my understanding this correctly, it's better to make sure of that by handing in a nicely isolated patch for review. It's conceivable that my understanding of the commit-util cache is not fully satisfactory. I don't use it in my followup work anyway, but it still would be nice to get this code path cleaned out in advance. I don't expect measurable performance improvements from these two patches: their main purpose is to get some cruft out of the way so that the heavy-duty patches actually dealing with the performance sinks will be a bit more focused. And of course, getting the ball rolling again. David Kastrup (2): builtin/blame.c: struct blame_entry does not need a prev link Eliminate same_suspect function in builtin/blame.c builtin/blame.c | 38 ++ 1 file changed, 10 insertions(+), 28 deletions(-) -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] Eliminate same_suspect function in builtin/blame.c
Since the origin pointers are interned and reference-counted, comparing
the pointers rather than the content is enough. The only uninterned
origins are cached values kept in commit-util, but same_suspect is not
called on them.
---
builtin/blame.c | 25 -
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 2195595..ead6148 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -255,15 +255,6 @@ struct scoreboard {
int *lineno;
};
-static inline int same_suspect(struct origin *a, struct origin *b)
-{
- if (a == b)
- return 1;
- if (a-commit != b-commit)
- return 0;
- return !strcmp(a-path, b-path);
-}
-
static void sanity_check_refcnt(struct scoreboard *);
/*
@@ -276,7 +267,7 @@ static void coalesce(struct scoreboard *sb)
struct blame_entry *ent, *next;
for (ent = sb-ent; ent (next = ent-next); ent = next) {
- if (same_suspect(ent-suspect, next-suspect)
+ if (ent-suspect == next-suspect
ent-guilty == next-guilty
ent-s_lno + ent-num_lines == next-s_lno) {
ent-num_lines += next-num_lines;
@@ -735,7 +726,7 @@ static int find_last_in_target(struct scoreboard *sb,
struct origin *target)
int last_in_target = -1;
for (e = sb-ent; e; e = e-next) {
- if (e-guilty || !same_suspect(e-suspect, target))
+ if (e-guilty || e-suspect != target)
continue;
if (last_in_target e-s_lno + e-num_lines)
last_in_target = e-s_lno + e-num_lines;
@@ -755,7 +746,7 @@ static void blame_chunk(struct scoreboard *sb,
struct blame_entry *e;
for (e = sb-ent; e; e = e-next) {
- if (e-guilty || !same_suspect(e-suspect, target))
+ if (e-guilty || e-suspect != target)
continue;
if (same = e-s_lno)
continue;
@@ -985,7 +976,7 @@ static int find_move_in_parent(struct scoreboard *sb,
while (made_progress) {
made_progress = 0;
for (e = sb-ent; e; e = e-next) {
- if (e-guilty || !same_suspect(e-suspect, target) ||
+ if (e-guilty || e-suspect != target ||
ent_score(sb, e) blame_move_score)
continue;
find_copy_in_blob(sb, e, parent, split, file_p);
@@ -1020,14 +1011,14 @@ static struct blame_list *setup_blame_list(struct
scoreboard *sb,
for (e = sb-ent, num_ents = 0; e; e = e-next)
if (!e-scanned !e-guilty
- same_suspect(e-suspect, target)
+ e-suspect == target
min_score ent_score(sb, e))
num_ents++;
if (num_ents) {
blame_list = xcalloc(num_ents, sizeof(struct blame_list));
for (e = sb-ent, i = 0; e; e = e-next)
if (!e-scanned !e-guilty
- same_suspect(e-suspect, target)
+ e-suspect == target
min_score ent_score(sb, e))
blame_list[i++].ent = e;
}
@@ -1171,7 +1162,7 @@ static void pass_whole_blame(struct scoreboard *sb,
origin-file.ptr = NULL;
}
for (e = sb-ent; e; e = e-next) {
- if (!same_suspect(e-suspect, origin))
+ if (e-suspect != origin)
continue;
origin_incref(porigin);
origin_decref(e-suspect);
@@ -1560,7 +1551,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
/* Take responsibility for the remaining entries */
for (ent = sb-ent; ent; ent = ent-next)
- if (same_suspect(ent-suspect, suspect))
+ if (ent-suspect == suspect)
found_guilty_entry(ent);
origin_decref(suspect);
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] builtin/blame.c: struct blame_entry does not need a prev link
---
builtin/blame.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index e44a6bb..2195595 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -197,7 +197,6 @@ static void drop_origin_blob(struct origin *o)
* scoreboard structure, sorted by the target line number.
*/
struct blame_entry {
- struct blame_entry *prev;
struct blame_entry *next;
/* the first line of this group in the final image;
@@ -282,8 +281,6 @@ static void coalesce(struct scoreboard *sb)
ent-s_lno + ent-num_lines == next-s_lno) {
ent-num_lines += next-num_lines;
ent-next = next-next;
- if (ent-next)
- ent-next-prev = ent;
origin_decref(next-suspect);
free(next);
ent-score = 0;
@@ -534,7 +531,7 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
prev = ent;
/* prev, if not NULL, is the last one that is below e */
- e-prev = prev;
+
if (prev) {
e-next = prev-next;
prev-next = e;
@@ -543,8 +540,6 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
e-next = sb-ent;
sb-ent = e;
}
- if (e-next)
- e-next-prev = e;
}
/*
@@ -555,14 +550,12 @@ static void add_blame_entry(struct scoreboard *sb, struct
blame_entry *e)
*/
static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
{
- struct blame_entry *p, *n;
+ struct blame_entry *n;
- p = dst-prev;
n = dst-next;
origin_incref(src-suspect);
origin_decref(dst-suspect);
memcpy(dst, src, sizeof(*src));
- dst-prev = p;
dst-next = n;
dst-score = 0;
}
@@ -2502,8 +2495,6 @@ parse_done:
ent-suspect = o;
ent-s_lno = bottom;
ent-next = next;
- if (next)
- next-prev = ent;
origin_incref(o);
}
origin_decref(o);
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] safe_create_leading_directories(): on Windows, \ can separate path components
On Sun, Jan 19, 2014 at 12:40 AM, Michael Haggerty mhag...@alum.mit.edu wrote: This patch applies on top of v3 of mh/safe-create-leading-directories. The only logical change from Sebastian's patch is that this version restores the original slash character rather than always restoring it to '/' (as suggested by Junio). Thanks Michael. This is very similar to Junio's current merge conflict resultion in pu between your original series and my patch (c2fec83). I like this patch of yours slightly better, however, as it uses a while instead of a for loop and the more conclusive slash_character than the was_slash variable name. -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/17] safe_create_leading_directories(): add explicit slash pointer
On Tue, Jan 7, 2014 at 6:41 PM, Junio C Hamano gits...@pobox.com wrote: Let me know if you would like me to merge or rebase the is_dir_sep() changes into this patch series. I'd want SSchuberth and windows folks to be at least aware of this series and preferrably see that they offer inputs to this series, making their is_dir_sep() change just one step in this series. That way I have one less series to worry about ;-) Thanks Junio for pointing out Michael's series to me and resolving the initial merge conflict. However, as written in my reply to Michael's mail of today, I'd prefer to take Michael's patch that applies cleanly on top of v3 of his mh/safe-create-leading-directories instead of your merge conflict resolution. -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP v2 00/14] inotify support
On Mon, Jan 20, 2014 at 12:04 AM, Thomas Rast t...@thomasrast.ch wrote: I never got the last three patches, did you send them? Also, this doesn't cleanly apply anywhere at my end. Can you push it somewhere for easier experimentation? Sorry I rebased it on top of kb/fast-hashmap but never got around to actually using hash tables. The code is here (and on top of master) https://github.com/pclouds/git.git file-watcher So I think the watcher protocol can be specified roughly as: Definitions: The watcher is a separate process that maintains a set of _watched paths_. Git uses the commands below to add or remove paths from this set. In addition it maintains a set of _changed paths_, which is a subset of the watched paths. Git occasionally queries the changed paths. If at any time after a path P was added to the watched set, P has or may have changed, it MUST be added to the changed set. Note that a path being unchanged under this definition does NOT mean that it is unchanged in the index as per `git diff`. It may have been changed before the watch was issued! Therefore, Git MUST test whether the path started out unchanged using lstat() AFTER it was added to the watched set. Correct. Handshake:: On connecting, git sends hello magic. magic is an opaque string that identifies the state of the index. The watcher MUST respond with hello previous_magic, where previous_magic is obtained from the bye command (below). If magic != previous_magic, the watcher MUST reset state for this repository. In addition, git must reset itself (i.e. clear all CE_WATCHED flags) because nothing can be trusted at this point. ... Did I get that approximately straight? Perhaps you can fix up as needed and then turn it into the documentation for the protocol. Will do (and probably steal some of your text above). There are several points about this that I don't like: * What does the datagram socket buy us? You already do a lot of pkt-line work for the really big messages, so why not just use pkt-line everywhere and a streaming socket? With datagram sockets I did not have to maintain the connected sockets, which made it somewhat simpler to handle so far. The default SO_SNDBUF goes up to 212k, so we can send up to that amount without blocking. With current pkt-line we send up to 64k in watch message then we have to wait for fine, which results in more context switches. But I think we can extend pkt-line's length field to 5 hex digit to cover this. Streaming sockets are probably the way to go for per-user daemon, so we can identify a socket with a repo. * The handshake should have room for capabilities, so that we can extend the protocol. Yeah. One more point for streaming sockets. With datagram sockets it's harder to define a session and thus hard to add capabilities. * The hello/hello handshake is confusing, and probably would allow you to point two watchers at each other without them noticing. Make it hello/ready, or some other unambiguous choice. OK * I took some liberty and tried to fully define the transitions between the sets. So even though I'm not sure this is currently handled, I made it well-defined to issue watch for a path that is in the changed set. Yes that should avoid races. The path can be removed from watched set later after git acknowledges it. * bye is confusing, because in practice git issues forgets after bye. The best I can come up with is setstate, I'm sure you have better ideas. Originally it was forget, forget, forget then bye. But with that order, if git crashes before sending bye we could lose info in changed set so the order was changed but I did not update the command name. There's also the problem of ordering guarantees between the socket and inotify. I haven't found any, so I would conservatively assume that the socket messages may in fact arrive before inotify, which is a race in the current code. E.g., in the sequence 'touch foo; git status' the daemon may see socketinotify hello... status new empty list touch foo I think a clever way to handle this would be to add a new command: Wait:: This command serves synchronization. Git creates a file of its choice in $GIT_DIR/watch (say, `.git/watch/wait.random`). Then it sends wait path. The watcher MUST block until it has processed all change notifications up to and including path. So wait.random inotify event functions as a barrier. Nice. As far as inotify corner-cases go, the only one I'm aware of is directory renames. I suspect we'll have to watch directories all the way up to the repository root to reliably detect when this happens. Not sure how to best handle this. Perhaps we should declare Git completely agnostic wrt such issues, and behind the scenes issue all watches up to the root even if we don't need
Re: [PATCH/WIP v2 08/14] read-cache: add GIT_TEST_FORCE_WATCHER for testing
On Mon, Jan 20, 2014 at 12:04 AM, Thomas Rast t...@thomasrast.ch wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: This can be used to force watcher on when running the test suite. git-file-watcher processes are not automatically cleaned up after each test. So after running the test suite you'll be left with plenty git-file-watcher processes that should all end after about a minute. Probably not a very good idea, especially in noninteractive use? E.g., a bisection through the test suite or parallel test runs on different commits may exhaust the available processes and/or memory. I think we run out of inotify resources before hitting process/memory limits. At least that's the case when running tests in parallel. Each test should make an effort to clean up all watchers before terminating. For now it's hard to do this correctly. Maybe once we get the multi-repo file watcher, we could launch one watcher per trash directory and cleaning up would be easier. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP v2 05/14] read-cache: put some limits on file watching
On Mon, Jan 20, 2014 at 12:06 AM, Thomas Rast t...@thomasrast.ch wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: watch_entries() is a lot of computation and could trigger a lot more lookups in file-watcher. Normally after the first set of watches are in place, we do not need to update often. Moreover if the number of entries is small, the overhead of file watcher may actually slow git down. This patch only allows to update watches if the number of watchable files is over a limit (and there are new files added if this is not the first time). Measurements on Core i5-2520M and Linux 3.7.6, about 920 lstat() take 1ms. Somewhere between 2^16 and 2^17 lstat calls that it starts to take longer than 100ms. 2^16 is chosen at the minimum limit to start using file watcher. Recently updated files are not considered watchable because they are likely to be updated again soon, not worth the ping-pong game with file watcher. The default limit 30min is just a random value. But then a fresh clone of a big repository would not get any benefit from the watcher? Not yet sure how to best handle this. Gaahh, perhaps limit the number of unwatchable recent files to a hundred or so in addition to time limit. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/WIP v2 02/14] read-cache: new extension to mark what file is watched
On Mon, Jan 20, 2014 at 12:06 AM, Thomas Rast t...@thomasrast.ch wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: If an entry is watched, git lets an external program decide if the entry is modified or not. It's more like --assume-unchanged, but designed to be controlled by machine. We are running out of on-disk ce_flags, so instead of extending on-disk entry format again, watched flags are in-core only and stored as extension instead. I wonder if this would be a good use-case for EWAH bitmaps? Presumably most users would end up having only a few large ranges of files that are being watched. Quite possibly most users would watch *all* files. Oh yeah. I edited my commit message locally to this a while ago On webkit.git with 182k entries, that's 364k more to be SHA-1'd on current index versions, compared to 22k in this format (and even less when jk/pack-bitmap ewah graduates and we can use ewah compression) -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] improve git svn performance Hi, I am trying to improve git svn's performance according to some profiling data.As the data showed,_rev_list subroutine and rebuild subroutine are consuming a l
From: linzj li...@ucweb.com
---
perl/Git/SVN.pm | 63 -
1 file changed, 49 insertions(+), 14 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 5273ee8..3cd1c8f 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1599,27 +1599,36 @@ sub tie_for_persistent_memoization {
my %lookup_svn_merge_cache;
my %check_cherry_pick_cache;
my %has_no_changes_cache;
+ my %_rev_list_cache;
tie_for_persistent_memoization(\%lookup_svn_merge_cache,
- $cache_path/lookup_svn_merge);
+ $cache_path/lookup_svn_merge);
memoize 'lookup_svn_merge',
- SCALAR_CACHE = 'FAULT',
- LIST_CACHE = ['HASH' = \%lookup_svn_merge_cache],
- ;
+ SCALAR_CACHE = 'FAULT',
+ LIST_CACHE = ['HASH' =
\%lookup_svn_merge_cache],
+ ;
tie_for_persistent_memoization(\%check_cherry_pick_cache,
- $cache_path/check_cherry_pick);
+ $cache_path/check_cherry_pick);
memoize 'check_cherry_pick',
- SCALAR_CACHE = 'FAULT',
- LIST_CACHE = ['HASH' = \%check_cherry_pick_cache],
- ;
+ SCALAR_CACHE = 'FAULT',
+ LIST_CACHE = ['HASH' =
\%check_cherry_pick_cache],
+ ;
tie_for_persistent_memoization(\%has_no_changes_cache,
- $cache_path/has_no_changes);
+ $cache_path/has_no_changes);
memoize 'has_no_changes',
- SCALAR_CACHE = ['HASH' = \%has_no_changes_cache],
- LIST_CACHE = 'FAULT',
- ;
+ SCALAR_CACHE = ['HASH' =
\%has_no_changes_cache],
+ LIST_CACHE = 'FAULT',
+ ;
+
+ tie_for_persistent_memoization(\%_rev_list_cache,
+ $cache_path/_rev_list);
+ memoize '_rev_list',
+ SCALAR_CACHE = 'FAULT',
+ LIST_CACHE = ['HASH' = \%_rev_list_cache],
+ ;
+
}
sub unmemoize_svn_mergeinfo_functions {
@@ -1629,6 +1638,7 @@ sub tie_for_persistent_memoization {
Memoize::unmemoize 'lookup_svn_merge';
Memoize::unmemoize 'check_cherry_pick';
Memoize::unmemoize 'has_no_changes';
+ Memoize::unmemoize '_rev_list';
}
sub clear_memoized_mergeinfo_caches {
@@ -1959,11 +1969,25 @@ sub rebuild_from_rev_db {
unlink $path or croak unlink: $!;
}
+#define a global associate map to record rebuild status
+my %rebuildStatus;
+#define a global associate map to record rebuild verify status
+my %rebuildVerifyStatus;
+
sub rebuild {
my ($self) = @_;
my $map_path = $self-map_path;
my $partial = (-e $map_path ! -z $map_path);
- return unless ::verify_ref($self-refname.'^0');
+my $verifyKey = $self-refname.'^0';
+if (! exists $rebuildVerifyStatus{$verifyKey} || ! defined
$rebuildVerifyStatus{$verifyKey} ) {
+my $verifyResult = ::verify_ref($verifyKey);
+if ($verifyResult) {
+$rebuildVerifyStatus{$verifyKey} = 1;
+}
+}
+if (! exists $rebuildVerifyStatus{$verifyKey}) {
+return;
+}
if (!$partial ($self-use_svm_props || $self-no_metadata)) {
my $rev_db = $self-rev_db_path;
$self-rebuild_from_rev_db($rev_db);
@@ -1977,10 +2001,21 @@ sub rebuild {
print Rebuilding $map_path ...\n if (!$partial);
my ($base_rev, $head) = ($partial ? $self-rev_map_max_norebuild(1) :
(undef, undef));
+my $keyValue = ($head ? $head.. : ) . $self-refname;
+if (exists $rebuildStatus{$keyValue}) {
+print Done rebuilding $map_path\n if (!$partial || !$head);
+my $rev_db_path = $self-rev_db_path;
+if (-f $self-rev_db_path) {
+unlink $self-rev_db_path or croak unlink: $!;
+}
+$self-unlink_rev_db_symlink;
+ return;
+}
my ($log, $ctx) =
command_output_pipe(qw/rev-list --pretty=raw --reverse/,
- ($head ? $head.. : ) . $self-refname,
+ $keyValue,
'--');
+$rebuildStatus{$keyValue} = 1;
my $metadata_url = $self-metadata_url;
remove_username($metadata_url);
my $svn_uuid = $self-rewrite_uuid || $self-ra_uuid;
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to
[PATCH] improve git svn performance
From: linzj li...@ucweb.com
Hi,
I am trying to improve git svn's performance according to some profiling
data.As the data showed,_rev_list subroutine and rebuild subroutine are
consuming a large proportion of time.So I improve _rev_list's performance by
memoize its results,and avoid subprocess invocation by memoize rebuild
subroutine's key data.Here's my patch:
---
perl/Git/SVN.pm | 63 -
1 file changed, 49 insertions(+), 14 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 5273ee8..3cd1c8f 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1599,27 +1599,36 @@ sub tie_for_persistent_memoization {
my %lookup_svn_merge_cache;
my %check_cherry_pick_cache;
my %has_no_changes_cache;
+ my %_rev_list_cache;
tie_for_persistent_memoization(\%lookup_svn_merge_cache,
- $cache_path/lookup_svn_merge);
+ $cache_path/lookup_svn_merge);
memoize 'lookup_svn_merge',
- SCALAR_CACHE = 'FAULT',
- LIST_CACHE = ['HASH' = \%lookup_svn_merge_cache],
- ;
+ SCALAR_CACHE = 'FAULT',
+ LIST_CACHE = ['HASH' =
\%lookup_svn_merge_cache],
+ ;
tie_for_persistent_memoization(\%check_cherry_pick_cache,
- $cache_path/check_cherry_pick);
+ $cache_path/check_cherry_pick);
memoize 'check_cherry_pick',
- SCALAR_CACHE = 'FAULT',
- LIST_CACHE = ['HASH' = \%check_cherry_pick_cache],
- ;
+ SCALAR_CACHE = 'FAULT',
+ LIST_CACHE = ['HASH' =
\%check_cherry_pick_cache],
+ ;
tie_for_persistent_memoization(\%has_no_changes_cache,
- $cache_path/has_no_changes);
+ $cache_path/has_no_changes);
memoize 'has_no_changes',
- SCALAR_CACHE = ['HASH' = \%has_no_changes_cache],
- LIST_CACHE = 'FAULT',
- ;
+ SCALAR_CACHE = ['HASH' =
\%has_no_changes_cache],
+ LIST_CACHE = 'FAULT',
+ ;
+
+ tie_for_persistent_memoization(\%_rev_list_cache,
+ $cache_path/_rev_list);
+ memoize '_rev_list',
+ SCALAR_CACHE = 'FAULT',
+ LIST_CACHE = ['HASH' = \%_rev_list_cache],
+ ;
+
}
sub unmemoize_svn_mergeinfo_functions {
@@ -1629,6 +1638,7 @@ sub tie_for_persistent_memoization {
Memoize::unmemoize 'lookup_svn_merge';
Memoize::unmemoize 'check_cherry_pick';
Memoize::unmemoize 'has_no_changes';
+ Memoize::unmemoize '_rev_list';
}
sub clear_memoized_mergeinfo_caches {
@@ -1959,11 +1969,25 @@ sub rebuild_from_rev_db {
unlink $path or croak unlink: $!;
}
+#define a global associate map to record rebuild status
+my %rebuildStatus;
+#define a global associate map to record rebuild verify status
+my %rebuildVerifyStatus;
+
sub rebuild {
my ($self) = @_;
my $map_path = $self-map_path;
my $partial = (-e $map_path ! -z $map_path);
- return unless ::verify_ref($self-refname.'^0');
+my $verifyKey = $self-refname.'^0';
+if (! exists $rebuildVerifyStatus{$verifyKey} || ! defined
$rebuildVerifyStatus{$verifyKey} ) {
+my $verifyResult = ::verify_ref($verifyKey);
+if ($verifyResult) {
+$rebuildVerifyStatus{$verifyKey} = 1;
+}
+}
+if (! exists $rebuildVerifyStatus{$verifyKey}) {
+return;
+}
if (!$partial ($self-use_svm_props || $self-no_metadata)) {
my $rev_db = $self-rev_db_path;
$self-rebuild_from_rev_db($rev_db);
@@ -1977,10 +2001,21 @@ sub rebuild {
print Rebuilding $map_path ...\n if (!$partial);
my ($base_rev, $head) = ($partial ? $self-rev_map_max_norebuild(1) :
(undef, undef));
+my $keyValue = ($head ? $head.. : ) . $self-refname;
+if (exists $rebuildStatus{$keyValue}) {
+print Done rebuilding $map_path\n if (!$partial || !$head);
+my $rev_db_path = $self-rev_db_path;
+if (-f $self-rev_db_path) {
+unlink $self-rev_db_path or croak unlink: $!;
+}
+$self-unlink_rev_db_symlink;
+ return;
+}
my ($log, $ctx) =
command_output_pipe(qw/rev-list --pretty=raw --reverse/,
- ($head ? $head.. : ) . $self-refname,
+
