[PATCH v11 07/11] 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- .gitignore | 1 + Makefile | 1 + builtin.h| 1 + builtin/interpret-trailers.c | 44 git.c| 1 + 5 files changed, 48 insertions(+) create mode 100644 builtin/interpret-trailers.c 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 c47c110..8ca0065 100644 --- a/builtin.h +++ b/builtin.h @@ -73,6 +73,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..46838d2 --- /dev/null +++ b/builtin/interpret-trailers.c @@ -0,0 +1,44 @@ +/* + * Builtin git interpret-trailers + * + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + * + */ + +#include cache.h +#include builtin.h +#include parse-options.h +#include string-list.h +#include trailer.h + +static const char * const git_interpret_trailers_usage[] = { + N_(git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...]), + NULL +}; + +int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) +{ + int trim_empty = 0; + struct string_list trailers = STRING_LIST_INIT_DUP; + + struct option options[] = { + OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty trailers)), + OPT_STRING_LIST(0, trailer, trailers, N_(trailer), + N_(trailer(s) to add)), + OPT_END() + }; + + argc = parse_options(argc, argv, prefix, options, +git_interpret_trailers_usage, 0); + + if (argc) { + int i; + for (i = 0; i argc; i++) + process_trailers(argv[i], trim_empty, trailers); + } else + process_trailers(NULL, trim_empty, trailers); + + string_list_clear(trailers, 0); + + return 0; +} diff --git a/git.c b/git.c index 7cf2953..63a03eb 100644 --- a/git.c +++ b/git.c @@ -380,6 +380,7 @@ static struct cmd_struct commands[] = { { 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 }, -- 1.9.1.636.g20d5f34 -- 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 v11 10/11] trailer: add tests for commands in config file
And add a few other tests for some special cases. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 124 ++ 1 file changed, 124 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 4506e18..9aae721 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -415,4 +415,128 @@ test_expect_success 'using ifMissing = doNothing' ' 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \A U Thor aut...@example.com\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: Z + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=22 \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using commiter information' ' + git config trailer.sign.ifExists addIfDifferent + git config trailer.sign.command echo \\$GIT_COMMITTER_NAME \$GIT_COMMITTER_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: Z + Signed-off-by: Z + Signed-off-by: C O Mitter commit...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \\$GIT_AUTHOR_NAME \$GIT_AUTHOR_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: Z + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists 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 + sed -e s/ Z\$/ / expected -EOF + Fixes: $FIXED + Acked-by= Z + Reviewed-by: Z + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with failing command using $ARG' ' + git config trailer.fix.ifExists overwrite + git config trailer.fix.command false \$ARG + cat complex_message_body expected + sed -e s/ Z\$/ / expected -EOF + Fixes: Z + Acked-by= Z + Reviewed-by: Z + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with empty tokens' ' + cat expected -EOF + + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer : --trailer :test actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with command but no key' ' + git config --unset trailer.sign.key + cat expected -EOF + + sign: A U Thor aut...@example.com + EOF + git interpret-trailers actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with no command and no key' ' + git config --unset trailer.review.key + cat expected -EOF + + review: Junio + sign: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review:Junio actual -EOF + EOF + test_cmp expected actual +' + test_done
[PATCH v11 01/11] trailer: add data structures and basic functions
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 Signed-off-by: Junio C Hamano gits...@pobox.com --- Makefile | 1 + trailer.c | 49 + 2 files changed, 50 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..db93a63 --- /dev/null +++ b/trailer.c @@ -0,0 +1,49 @@ +#include cache.h +/* + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + */ + +enum action_where { WHERE_AFTER, WHERE_BEFORE }; +enum action_if_exists { EXISTS_ADD_IF_DIFFERENT, EXISTS_ADD_IF_DIFFERENT_NEIGHBOR, + EXISTS_ADD, EXISTS_OVERWRITE, EXISTS_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_exists if_exists; + 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 - 1])) + len--; + return len; +} -- 1.9.1.636.g20d5f34 -- 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 v11 06/11] 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 58 ++ trailer.h | 6 ++ 2 files changed, 64 insertions(+) create mode 100644 trailer.h diff --git a/trailer.c b/trailer.c index 4ca9157..feadd3a 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,6 @@ #include cache.h #include string-list.h +#include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -69,6 +70,26 @@ static void free_trailer_item(struct trailer_item *item) free(item); } +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_input_list(struct trailer_item *in_tok, struct trailer_item *arg_tok) { @@ -625,3 +646,40 @@ static int process_input_file(struct strbuf **lines, return patch_start; } + +static void free_all(struct trailer_item **first) +{ + while (*first) { + struct trailer_item *item = remove_first(first); + free_trailer_item(item); + } +} + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers) +{ + struct trailer_item *in_tok_first = NULL; + struct trailer_item *in_tok_last = NULL; + struct trailer_item *arg_tok_first; + struct strbuf **lines; + int patch_start; + + git_config(git_trailer_config, NULL); + + lines = read_input_file(file); + + /* Print the lines before the trailers */ + patch_start = process_input_file(lines, in_tok_first, in_tok_last); + + arg_tok_first = process_command_line_args(trailers); + + process_trailers_lists(in_tok_first, in_tok_last, arg_tok_first); + + print_all(in_tok_first, trim_empty); + + free_all(in_tok_first); + + /* Print the lines after the trailers as is */ + print_lines(lines, patch_start, INT_MAX); + + strbuf_list_free(lines); +} diff --git a/trailer.h b/trailer.h new file mode 100644 index 000..8eb25d5 --- /dev/null +++ b/trailer.h @@ -0,0 +1,6 @@ +#ifndef TRAILER_H +#define TRAILER_H + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers); + +#endif /* TRAILER_H */ -- 1.9.1.636.g20d5f34 -- 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 v11 09/11] trailer: execute command from 'trailer.name.command'
Let the user specify a command that will give on its standard output the value to use for the specified trailer. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 65 +++ 1 file changed, 65 insertions(+) diff --git a/trailer.c b/trailer.c index feadd3a..4d32b42 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,7 @@ #include cache.h #include string-list.h +#include run-command.h +#include string-list.h #include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org @@ -14,11 +16,14 @@ struct conf_info { char *name; char *key; char *command; + unsigned command_uses_arg : 1; enum action_where where; enum action_if_exists if_exists; enum action_if_missing if_missing; }; +#define TRAILER_ARG_STRING $ARG + struct trailer_item { struct trailer_item *previous; struct trailer_item *next; @@ -60,6 +65,13 @@ static inline int contains_only_spaces(const char *str) return !*s; } +static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b) +{ + const char *ptr = strstr(sb-buf, a); + if (ptr) + strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b)); +} + static void free_trailer_item(struct trailer_item *item) { free(item-conf.name); @@ -403,6 +415,7 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb) if (conf-command) warning(_(more than one %s), conf_key); conf-command = xstrdup(value); + conf-command_uses_arg = !!strstr(conf-command, TRAILER_ARG_STRING); break; case TRAILER_WHERE: if (set_where(conf, value)) @@ -439,6 +452,45 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra return 0; } +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); + result = xstrdup(); + } else + result = strbuf_detach(buf, NULL); + + strbuf_release(cmd); + return result; +} static void duplicate_conf(struct conf_info *dst, struct conf_info *src) { @@ -469,6 +521,10 @@ static struct trailer_item *new_trailer_item(struct trailer_item *conf_item, duplicate_conf(new-conf, conf_item-conf); new-token = xstrdup(token_from_item(conf_item)); free(tok); + if (conf_item-conf.command_uses_arg || !val) { + new-value = apply_command(conf_item-conf.command, val); + free(val); + } } else new-token = tok; @@ -530,12 +586,21 @@ static struct trailer_item *process_command_line_args(struct string_list *traile struct trailer_item *arg_tok_first = NULL; struct trailer_item *arg_tok_last = NULL; struct string_list_item *tr; + struct trailer_item *item; for_each_string_list_item(tr, trailers) { struct trailer_item *new = create_trailer_item(tr-string); 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.9.1.636.g20d5f34 -- 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 v10 11/12] Documentation: add documentation for 'git interpret-trailers'
From: Junio C Hamano gits...@pobox.com Christian Couder chrisc...@tuxfamily.org writes: +Help add RFC 822-like headers, called 'trailers', at the end of the +otherwise free-form part of a commit message. I think it is somewhat misleading to use the word headers like that. 'trailers' look similar to RFC-822-headers but they come at the end. The sentence however reads as if they are headers that look like RFC 822. Perhaps shuffling words like so: Help adding 'trailers' lines, that look similar to RFC 822 e-mail headers, at the end of the ... would make it less confusing. Ok, I made this change in v11. +Some configuration variables control the way the `token` arguments are +applied to the message and the way any existing trailer in the message +is changed. They also make it possible to automatically add some +trailers. + +By default, a 'token=value' or 'token:value' argument will be added +only if no trailer with the same (token, value) pair is already in the +message. The 'token' and 'value' parts will be trimmed to remove +starting and trailing whitespace, and the resulting trimmed 'token' +and 'value' will appear in the message like this: + + +token: value + Mental note: this does assume that the final output for the 'token' is to have a line label that is followed by a colon :, SP and the value. And the natural way to express that on the command line would be to say token: value, I would think, but let's just read on. +Note that 'trailers' 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. s/that are in RFC 822/for RFC 822 headers/. s/line breaking/line folding/. (see RFC 822, 3.1.1) Ok, it's in v11 too. +OPTIONS +--- +--trim-empty:: +If the 'value' part of any trailer contains only whitespace, +the whole trailer will be removed from the resulting message. + +CONFIGURATION VARIABLES +--- + +trailer.token.key:: +This 'key' will be used instead of 'token' in the As `key` is something that is typed literally, it should be typeset as `key` in the descriptive text. Ok, I used `key` in v11. I think other manpages spell the placeholder as `token` (or 'token', I am not sure which...). I found mostly token, so I used that in v11. +trailer. After some alphanumeric characters, it can contain +some non alphanumeric characters like ':', '=' or '#' that will +be used instead of ':' to separate the token from the value in +the trailer, though the default ':' is more standard. I assume that this is for things like bug #538 and the configuration would say something like: [trailer bug] key = bug # For completeness (of this example), the bog-standard s-o-b would look like Signed-off-by: Christian Couder chrisc...@tuxfamily.org and the configuration for it that spell the redundant key would be: [trailer Signed-off-by] key = Signed-off-by: Yeah, but you can use the following instead: [trailer s-o-b] key = Signed-off-by: The token and the key can be different. Am I reading the intention correctly? Yeah, I think so. That is, when trailer.token.key is not defined, the value defaults to token: (with one SP after the label and colon), Yes. and when it is defined, the value can come directly after it. The value can come directly after the key, only if the key ends with '#'. If it ends with something else, except spaces, one SP will be added between the key and the value. Yeah, I made '#' special in the hope that it would be more compatible with GitHub and other services that might also use '#'. Thanks, 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 v10 11/12] Documentation: add documentation for 'git interpret-trailers'
From: Michael Haggerty mhag...@alum.mit.edu On 04/08/2014 01:35 PM, Christian Couder wrote: The trailers are recognized in the input commit message using the following rules: - only lines that contains a ':' are considered trailers, - the trailer lines must all be next to each other, - after them it's only possible to have some lines that contain only spaces, - before them there must be at least one line with only spaces Thanks for all the explanation. I think that most/all of this information should be included in the documentation. Ok, I included the above rules in v11, but maybe not other pieces of information that you might have wanted. +OPTIONS +--- +--trim-empty:: + If the 'value' part of any trailer contains only whitespace, + the whole trailer will be removed from the resulting message. Does this apply to existing trailers, new trailers, or both? Both. If it applies to existing trailers, then it seems a bit dangerous, in the sense that the command might end up changing trailers that are unrelated to the one that the command is trying to add. The command is not just for adding trailers. But there could be an option to just trim trailers that are added. Maybe that should be the *only* behavior of this option. Maybe there should be a trailer.token.trimEmpty config option. One possible usage of the git interpret-trailers command that was discussed in the early threads was the following: 1) You have a commit message template like the following: - ***SUBJECT*** ***MESSAGE*** Fixes: Cc: Cc: Reviewed-by: Reviewed-by: Signed-off-by: - 2) The user add some information when committing: $ git commit --trailer Fixes:534 --trailer Signed-off-by: Michael mhag...@alum.mit.edu 3) git interpret-trailers is used automatically by git commit without --trim-empty, and it is passed the --trailer arguments and the commit message template, so the user is shown the result which is for example the following: - ***SUBJECT*** ***MESSAGE*** Fixes: 534 Cc: Cc: Reviewed-by: Reviewed-by: Signed-off-by: Michael mhag...@alum.mit.edu - 4) The user adds some information and the resulting message is for example: - Doing foo and bar And also baz. Fixes: 534 Cc: Cc: Peff p...@peff.net Reviewed-by: Junio gits...@pobox.com Reviewed-by: Signed-off-by: Michael mhag...@alum.mit.edu - 5) Then a post commit hook automatically uses git interpret-trailers --trim-empty on the result, so the commit message is eventually the following: - Doing foo and bar And also baz. Fixes: 534 Cc: Peff p...@peff.net Reviewed-by: Junio gits...@pobox.com Signed-off-by: Michael mhag...@alum.mit.edu - So I think it could be very useful to have --trim-empty work on all the trailers, not just those passed as arguments. If a commit message containing trailer lines with separators other than ':' is input to the program, will it recognize them as trailer lines? No, '=' and '#' are not supported in the input message, only in the output. Do such trailer lines have to have the same separator as the one listed in this configuration setting to be recognized? No they need to have ':' as a separator. The reason why only ':' is supported is because it is the cannonical trailer separator and it could create problems with many input messages if other separators where supported. Maybe we could detect a special line like the following: # TRAILERS START in the input message and consider everyhting after that line as trailers. In this case it would be ok to accept other separators. It would be ugly to have to use such a line. I think it would be preferable to be more restrictive about trailer separators than to require something like this. The code is already very restrictive about trailer separators. From what you've said above, it sounds like your code might get confused with the following input commit message: This is the human-readable comment Foo: bar Fixes #123 Plugh: xyzzy It seems to me that none of these lines would be accepted as trailers, because they include a non-trailer Fixes line (non-trailer in the sense that it doesn't use a colon separator). Yeah, they would not be accepted because the code is very restrictive. The following would be accepted: Foo: bar Fixes: 123 Plugh: xyzzy I suppose that there is some compelling reason to allow non-colon separators here. If not, it seems like it adds a lot of complexity and should maybe be omitted, or limited to only a few specific separators. Yeah, but in the early threads concerning this subject, someone said that GitHub for example uses bug #XXX. I will have a look again. Yes, that's true: GitHub recognizes strings like fixes #33 but not if there is an intervening colon like in fixes: #33. OTOH GitHub recognizes
[PATCH v1 4/4] replace: add --edit option
From: Jeff King p...@peff.net This allows you to run: git replace --edit SHA1 to get dumped in an editor with the contents of the object for SHA1. The result is then read back in and used as a replace object for SHA1. The writing/reading is type-aware, so you get to edit ls-tree output rather than the binary tree format. Missing documentation and tests. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 112 +- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/builtin/replace.c b/builtin/replace.c index af40129..3da1bae 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -12,6 +12,7 @@ #include builtin.h #include refs.h #include parse-options.h +#include run-command.h static const char * const git_replace_usage[] = { N_(git replace [-f] object replacement), @@ -176,6 +177,107 @@ static int replace_object(const char *object_ref, const char *replace_ref, int f return replace_object_sha1(object_ref, object, replace_ref, repl, force); } +/* + * Write the contents of the object named by sha1 to the file filename, + * pretty-printed for human editing based on its type. + */ +static void export_object(const unsigned char *sha1, const char *filename) +{ + const char *argv[] = { --no-replace-objects, cat-file, -p, NULL, NULL }; + struct child_process cmd = { argv }; + int fd; + + fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0666); + if (fd 0) + die_errno(unable to open %s for writing, filename); + + argv[3] = sha1_to_hex(sha1); + cmd.git_cmd = 1; + cmd.out = fd; + + if (run_command(cmd)) + die(cat-file reported failure); + + close(fd); +} + +/* + * Read a previously-exported (and possibly edited) object back from filename, + * interpreting it as type, and writing the result to the object database. + * The sha1 of the written object is returned via sha1. + */ +static void import_object(unsigned char *sha1, enum object_type type, + const char *filename) +{ + int fd; + + fd = open(filename, O_RDONLY); + if (fd 0) + die_errno(unable to open %s for reading, filename); + + if (type == OBJ_TREE) { + const char *argv[] = { mktree, NULL }; + struct child_process cmd = { argv }; + struct strbuf result = STRBUF_INIT; + + cmd.argv = argv; + cmd.git_cmd = 1; + cmd.in = fd; + cmd.out = -1; + + if (start_command(cmd)) + die(unable to spawn mktree); + + if (strbuf_read(result, cmd.out, 41) 0) + die_errno(unable to read from mktree); + close(cmd.out); + + if (finish_command(cmd)) + die(mktree reported failure); + if (get_sha1_hex(result.buf, sha1) 0) + die(mktree did not return an object name); + + strbuf_release(result); + } else { + struct stat st; + int flags = HASH_FORMAT_CHECK | HASH_WRITE_OBJECT; + + if (fstat(fd, st) 0) + die_errno(unable to fstat %s, filename); + if (index_fd(sha1, fd, st, type, NULL, flags) 0) + die(unable to write object to database); + /* index_fd close()s fd for us */ + } + + /* +* No need to close(fd) here; both run-command and index-fd +* will have done it for us. +*/ +} + +static int edit_and_replace(const char *object_ref, int force) +{ + char *tmpfile = git_pathdup(REPLACE_EDITOBJ); + enum object_type type; + unsigned char old[20], new[20]; + + if (get_sha1(object_ref, old) 0) + die(Not a valid object name: '%s', object_ref); + + type = sha1_object_info(old, NULL); + if (type 0) + die(unable to get object type for %s, sha1_to_hex(old)); + + export_object(old, tmpfile); + if (launch_editor(tmpfile, NULL, NULL) 0) + die(editing object file failed); + import_object(new, type, tmpfile); + + free(tmpfile); + + return replace_object_sha1(object_ref, old, replacement, new, force); +} + int cmd_replace(int argc, const char **argv, const char *prefix) { int force = 0; @@ -184,11 +286,13 @@ int cmd_replace(int argc, const char **argv, const char *prefix) MODE_UNSPECIFIED = 0, MODE_LIST, MODE_DELETE, + MODE_EDIT, MODE_REPLACE } cmdmode = MODE_UNSPECIFIED; struct option options[] = { OPT_CMDMODE('l', list, cmdmode, N_(list replace refs), MODE_LIST), OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs
[PATCH v1 2/4] replace: use OPT_CMDMODE to handle modes
From: Jeff King p...@peff.net By using OPT_CMDMODE, the mutual exclusion between modes is taken care of for us. It also makes it easy for us to maintain a single variable with the mode, which makes its intent more clear. We can use a single switch() to make sure we have covered all of the modes. This ends up breaking even in code size, but the win will be much bigger when we start adding more modes. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 49 + 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 28db96f..29cf699 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -168,11 +168,17 @@ static int replace_object(const char *object_ref, const char *replace_ref, int cmd_replace(int argc, const char **argv, const char *prefix) { - int list = 0, delete = 0, force = 0; + int force = 0; const char *format = NULL; + enum { + MODE_UNSPECIFIED = 0, + MODE_LIST, + MODE_DELETE, + MODE_REPLACE + } cmdmode = MODE_UNSPECIFIED; struct option options[] = { - OPT_BOOL('l', list, list, N_(list replace refs)), - OPT_BOOL('d', delete, delete, N_(delete replace refs)), + OPT_CMDMODE('l', list, cmdmode, N_(list replace refs), MODE_LIST), + OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs), MODE_DELETE), OPT_BOOL('f', force, force, N_(replace the ref if it exists)), OPT_STRING(0, format, format, N_(format), N_(use this format)), OPT_END() @@ -182,42 +188,37 @@ int cmd_replace(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0); - if (!list !delete) - if (!argc) - list = 1; + if (!cmdmode) + cmdmode = argc ? MODE_REPLACE : MODE_LIST; - if (list delete) - usage_msg_opt(-l and -d cannot be used together, - git_replace_usage, options); - - if (format !list) + if (format cmdmode != MODE_LIST) usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); - if (force (list || delete)) - usage_msg_opt(-f cannot be used with -d or -l, + if (force cmdmode != MODE_REPLACE) + usage_msg_opt(-f only makes sense when writing a replacement, git_replace_usage, options); - /* Delete refs */ - if (delete) { + switch (cmdmode) { + case MODE_DELETE: if (argc 1) usage_msg_opt(-d needs at least one argument, git_replace_usage, options); return for_each_replace_name(argv, delete_replace_ref); - } - /* Replace object */ - if (!list argc) { + case MODE_REPLACE: if (argc != 2) usage_msg_opt(bad number of arguments, git_replace_usage, options); return replace_object(argv[0], argv[1], force); - } - /* List refs, even if list is not set */ - if (argc 1) - usage_msg_opt(only one pattern can be given with -l, - git_replace_usage, options); + case MODE_LIST: + if (argc 1) + usage_msg_opt(only one pattern can be given with -l, + git_replace_usage, options); + return list_replace_refs(argv[0], format); - return list_replace_refs(argv[0], format); + default: + die(BUG: invalid cmdmode %d, (int)cmdmode); + } } -- 1.9.1.636.g20d5f34 -- 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 v1 3/4] replace: factor object resolution out of replace_object
From: Jeff King p...@peff.net As we add new options that operate on objects before replacing them, we'll want to be able to feed raw sha1s straight into replace_object. Split replace_object into the object-resolution part and the actual replacement. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 26 ++ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 29cf699..af40129 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -123,19 +123,17 @@ static int delete_replace_ref(const char *name, const char *ref, return 0; } -static int replace_object(const char *object_ref, const char *replace_ref, - int force) +static int replace_object_sha1(const char *object_ref, + unsigned char object[20], + const char *replace_ref, + unsigned char repl[20], + int force) { - unsigned char object[20], prev[20], repl[20]; + unsigned char prev[20]; enum object_type obj_type, repl_type; char ref[PATH_MAX]; struct ref_lock *lock; - if (get_sha1(object_ref, object)) - die(Failed to resolve '%s' as a valid ref., object_ref); - if (get_sha1(replace_ref, repl)) - die(Failed to resolve '%s' as a valid ref., replace_ref); - if (snprintf(ref, sizeof(ref), refs/replace/%s, sha1_to_hex(object)) sizeof(ref) - 1) @@ -166,6 +164,18 @@ static int replace_object(const char *object_ref, const char *replace_ref, return 0; } +static int replace_object(const char *object_ref, const char *replace_ref, int force) +{ + unsigned char object[20], repl[20]; + + if (get_sha1(object_ref, object)) + die(Failed to resolve '%s' as a valid ref., object_ref); + if (get_sha1(replace_ref, repl)) + die(Failed to resolve '%s' as a valid ref., replace_ref); + + return replace_object_sha1(object_ref, object, replace_ref, repl, force); +} + int cmd_replace(int argc, const char **argv, const char *prefix) { int force = 0; -- 1.9.1.636.g20d5f34 -- 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 v1 1/4] replace: refactor command-mode determination
From: Jeff King p...@peff.net The git-replace command has three modes: listing, deleting, and replacing. The first two are selected explicitly. If none is selected, we fallback to listing when there are no arguments, and replacing otherwise. Let's figure out up front which operation we are going to do, before getting into the application logic. That lets us simplify our option checks (e.g., we currently have to check whether a useless --force is given both along with an explicit list, as well as with an implicit one). This saves some lines, makes the logic easier to follow, and will facilitate further cleanups. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index b62420a..28db96f 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -182,12 +182,16 @@ int cmd_replace(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0); + if (!list !delete) + if (!argc) + list = 1; + if (list delete) usage_msg_opt(-l and -d cannot be used together, git_replace_usage, options); - if (format delete) - usage_msg_opt(--format and -d cannot be used together, + if (format !list) + usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); if (force (list || delete)) @@ -207,9 +211,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc != 2) usage_msg_opt(bad number of arguments, git_replace_usage, options); - if (format) - usage_msg_opt(--format cannot be used when not listing, - git_replace_usage, options); return replace_object(argv[0], argv[1], force); } @@ -217,9 +218,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc 1) usage_msg_opt(only one pattern can be given with -l, git_replace_usage, options); - if (force) - usage_msg_opt(-f needs some arguments, - git_replace_usage, options); return list_replace_refs(argv[0], format); } -- 1.9.1.636.g20d5f34 -- 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 v1 0/4] replace: add option to edit a Git object
This patch series comes from what Peff sent in the following thread: http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528 I added the following fixes: - add strbuf_release(result); in import_object(); this was suggested by Eric Sunshine - use MODE_LIST instead of MODE_DELETE if no arguments are passed; this makes the test suite pass - add --no-replace-objects when calling git cat-file in export_object(); so that we edit the original object if an object is already replaced I am not happy with the fact that if the user doesn't modify the object when editing it, then a replace ref can still be created that points to the original object. I think something should be done to avoid that. Once that is fixed, I plan to add some tests and documentation, but I wanted first to let you know that I am looking at this. Jeff King (4): replace: refactor command-mode determination replace: use OPT_CMDMODE to handle modes replace: factor object resolution out of replace_object replace: add --edit option builtin/replace.c | 189 -- 1 file changed, 154 insertions(+), 35 deletions(-) -- 1.9.1.636.g20d5f34 -- 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
[RFC/PATCH 1/2] trailer: fix to ignore any line starting with '#'
It looks like the commit-msg hook is passed a commit message that can contain lines starting with a '#'. Those comment lines will be removed from the commit message after the hook is run. If we want git interpret-trailers to be able to process commit messages correctly in the commit-msg hook we need to ignore those lines. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 26 ++ trailer.c | 29 ++--- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 9aae721..aa63b1b 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -150,6 +150,32 @@ test_expect_success 'with 2 files arguments' ' test_cmp expected actual ' +test_expect_success 'with message that has comments' ' + cat basic_message message_with_comments + sed -e s/ Z\$/ / message_with_comments -\EOF + # comment + + # other comment + Cc: Z + # yet another comment + Reviewed-by: Johan + Reviewed-by: Z + # last comment + + EOF + cat basic_patch message_with_comments + cat basic_message expected + cat expected -\EOF + # comment + + Cc: Peff + Reviewed-by: Johan + EOF + cat basic_patch expected + git interpret-trailers --trim-empty --trailer Cc: Peff message_with_comments actual + test_cmp expected actual +' + test_expect_success 'with commit complex message and trailer args' ' cat complex_message_body expected sed -e s/ Z\$/ / expected -\EOF diff --git a/trailer.c b/trailer.c index 4d32b42..81b2c5c 100644 --- a/trailer.c +++ b/trailer.c @@ -644,10 +644,9 @@ static int find_patch_start(struct strbuf **lines, int count) /* * Return the (0 based) index of the first trailer line or count if * there are no trailers. Trailers are searched only in the lines from - * index (count - 1) down to index 0. The has_blank_line parameter - * tells if there is a blank line before the trailers. + * index (count - 1) down to index 0. */ -static int find_trailer_start(struct strbuf **lines, int count, int *has_blank_line) +static int find_trailer_start(struct strbuf **lines, int count) { int start, only_spaces = 1; @@ -656,10 +655,11 @@ static int find_trailer_start(struct strbuf **lines, int count, int *has_blank_l * for a line with only spaces before lines with one ':'. */ for (start = count - 1; start = 0; start--) { + if (lines[start]-buf[0] == '#') + continue; if (contains_only_spaces(lines[start]-buf)) { if (only_spaces) continue; - *has_blank_line = 1; return start + 1; } if (strchr(lines[start]-buf, ':')) { @@ -667,13 +667,20 @@ static int find_trailer_start(struct strbuf **lines, int count, int *has_blank_l only_spaces = 0; continue; } - *has_blank_line = start == count - 1 ? - 0 : contains_only_spaces(lines[start + 1]-buf); return count; } - *has_blank_line = only_spaces ? count 0 : 0; - return only_spaces ? count : start + 1; + return only_spaces ? count : 0; +} + +static int has_blank_line_before(struct strbuf **lines, int start) +{ + for (;start = 0; start--) { + if (lines[start]-buf[0] == '#') + continue; + return contains_only_spaces(lines[start]-buf); + } + return 0; } static void print_lines(struct strbuf **lines, int start, int end) @@ -688,19 +695,19 @@ static int process_input_file(struct strbuf **lines, struct trailer_item **in_tok_last) { int count = 0; - int patch_start, trailer_start, has_blank_line, i; + int patch_start, trailer_start, i; /* Get the line count */ while (lines[count]) count++; patch_start = find_patch_start(lines, count); - trailer_start = find_trailer_start(lines, patch_start, has_blank_line); + trailer_start = find_trailer_start(lines, patch_start); /* Print lines before the trailers as is */ print_lines(lines, 0, trailer_start); - if (!has_blank_line) + if (!has_blank_line_before(lines, trailer_start - 1)) printf(\n); /* Parse trailer lines */ -- 1.9.rc0.17.g651113e -- 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
[RFC/PATCH 0/2] fix and examples for git interpret-trailers
As I sent version 11 of the Add interpret-trailers builtin only a few days ago, I am sending now a short series on top instead of another full version. Christian Couder (2): trailer: fix to ignore any line starting with '#' trailer: add examples to the documentation Documentation/git-interpret-trailers.txt | 98 +++- t/t7513-interpret-trailers.sh| 26 + trailer.c| 29 ++ 3 files changed, 141 insertions(+), 12 deletions(-) -- 1.9.rc0.17.g651113e -- 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
[RFC/PATCH 2/2] trailer: add examples to the documentation
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-interpret-trailers.txt | 98 +++- 1 file changed, 97 insertions(+), 1 deletion(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 450ec54..42c2f71 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -134,9 +134,105 @@ If the command contains the `$ARG` string, this string will be replaced with the value part of an existing trailer with the same token, if any, before the command is launched. +EXAMPLES + + +* Configure a 'sign' trailer with a 'Signed-off-by: ' key, and then + add two of these trailers to a message: ++ + +$ git config trailer.sign.key Signed-off-by: +$ cat msg.txt +subject + +message +$ cat msg.txt | git interpret-trailers --trailer 'sign: Alice al...@example.com' --trailer 'sign: Bob b...@example.com' +subject + +message + +Signed-off-by: Alice al...@example.com +Signed-off-by: Bob b...@example.com + + +* Extract the last commit as a patch, and add a 'Cc' and a + 'Reviewed-by' trailer to it: ++ + +$ git format-patch -1 +0001-foo.patch +$ git interpret-trailers --trailer 'Cc: Alice al...@example.com' --trailer 'Reviewed-by: Bob b...@example.com' 0001-foo.patch 0001-bar.patch + + +* Configure a 'sign' trailer with a command to automatically add a + 'Signed-off-by: ' with the author information only if there is no + 'Signed-off-by: ' already, and show how it works: ++ + +$ git config trailer.sign.key Signed-off-by: +$ git config trailer.sign.ifmissing add +$ git config trailer.sign.ifexists doNothing +$ git config trailer.sign.command 'echo $(git config user.name) $(git config user.email)' +$ git interpret-trailers EOF + EOF + +Signed-off-by: Bob b...@example.com +$ git interpret-trailers EOF + Signed-off-by: Alice al...@example.com + EOF + +Signed-off-by: Alice al...@example.com + + +* Configure a 'fix' trailer with a command to show the subject of a + commit that is fixed, and show how it works: ++ + +$ git config trailer.fix.key Fixes # +$ git config trailer.fix.ifExists overwrite +$ git config trailer.fix.command git log -1 --oneline --format=\%h (%s)\ --abbrev-commit --abbrev=14 \$ARG +$ git interpret-trailers EOF + subject + + message + + fix: HEAD~2 + EOF +subject + +message + +Fixes #fe3187489d69c4 (subject of fixed commit) + + +* Configure a commit template with some trailers with empty values, + then configure a commit-msg hook that uses git interpret-trailers to + remove trailers with empty values and to add a 'git-version' + trailer: ++ + +$ cat commit_template.txt EOF + ***subject*** + + ***message*** + + Fixes: + Cc: + Reviewed-by: + Signed-off-by: + EOF +$ git config commit.template commit_template.txt +$ cat .git/hooks/commit-msg EOF +#!/bin/sh +git interpret-trailers --trim-empty --trailer git-version: \$(git describe) \$1 \$1.new +mv \$1.new \$1 +EOF +$ chmod +x .git/hooks/commit-msg + + SEE ALSO -linkgit:git-commit[1] +linkgit:git-commit[1], linkgit:git-format-patch[1], linkgit:git-config[1] GIT --- -- 1.9.rc0.17.g651113e -- 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: Recording the current branch on each commit?
From: Johan Herland jo...@herland.net Subject: Re: Recording the current branch on each commit? Date: Mon, 28 Apr 2014 01:39:26 +0200 On Sun, Apr 27, 2014 at 10:55 PM, Jeremy Morton ad...@game-point.net wrote: On 27/04/2014 20:33, Johan Herland wrote: On Sun, Apr 27, 2014 at 7:38 PM, Jeremy Mortonad...@game-point.net wrote: On 27/04/2014 10:09, Johan Herland wrote: As far as I can tell from that discussion, the general opposition to encoding the branch name as a structural part of the commit object is that, for some people's workflows, it would be unhelpful and/or misleading. Well fair enough then - why don't we make it a setting that is off by default, and can easily be switched on? That way the people for whom tagging the branch name would be useful have a very easy way to switch it on. Therefore, the most pragmatic and constructive thing to do at this point, is IMHO to work within the confines of the existing commit object structure. I actually believe using commit message trailers like Made-on-branch: frotz in addition to some helpful infrastructure (hooks, templates, git-interpret-trailers, etc.) should get you pretty much exactly what you want. And if this feature turns out to be extremely useful for a lot of users, we can certainly consider changing the commit object format in the future. OK, fair enough. So I guess what I'd like to see, then, is good built-in functionality in Git for these commit message trailers, so that they are very easy to turn on. I'd like to be able to tell co-developers to add a one-liner to their git config file rather than some post-commit script. I think this is what the interpret-trailers effort is about. Unfortunately I have not followed it closely enough to say if your use case is already covered by Christian's (CCed) work. Christian: With your current patch series, is it possible for Jeremy to configure interpret-trailers to automatically append a Made-on-branch: current_branch trailer whenever he creates a commit? Yes, it's possible. Yesterday, I sent the following patch: [RFC/PATCH 2/2] trailer: add examples to the documentation and it shows a commit-msg hook to do something like that: $ cat .git/hooks/commit-msg EOF #!/bin/sh git interpret-trailers --trim-empty --trailer git-version: \$(git describe) \$1 \$1.new mv \$1.new \$1 EOF $ chmod +x .git/hooks/commit-msg I think you just need to use the following if you want the branch instead of the git version: git interpret-trailers --trim-empty --trailer git-branch: \$(git name-rev --name-only HEAD) \$1 \$1.new It could even be simpler if there was an option (which has already been discussed) that made it possible to modify the file in place. This way one would not need the 'mv \$1.new \$1' command. 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: [RFC/PATCH 1/2] trailer: fix to ignore any line starting with '#'
On Mon, Apr 28, 2014 at 7:58 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 04/27/2014 10:12 PM, Christian Couder wrote: It looks like the commit-msg hook is passed a commit message that can contain lines starting with a '#'. Those comment lines will be removed from the commit message after the hook is run. If we want git interpret-trailers to be able to process commit messages correctly in the commit-msg hook we need to ignore those lines. Shouldn't this take into account the config setting core.commentchar? Yes, I will have a look at that. Thanks, 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 v10 11/12] Documentation: add documentation for 'git interpret-trailers'
On Tue, Apr 29, 2014 at 1:05 PM, Jeremy Morton ad...@game-point.net wrote: On 28/04/2014 17:37, Junio C Hamano wrote: Christian Couderchrisc...@tuxfamily.org writes: From: Junio C Hamanogits...@pobox.com Christian Couderchrisc...@tuxfamily.org writes: ... + trailer. After some alphanumeric characters, it can contain + some non alphanumeric characters like ':', '=' or '#' that will + be used instead of ':' to separate the token from the value in + the trailer, though the default ':' is more standard. I assume that this is for things like bug #538 and the configuration would say something like: [trailer bug] key = bug # For completeness (of this example), the bog-standard s-o-b would look like Signed-off-by: Christian Couderchrisc...@tuxfamily.org and the configuration for it that spell the redundant key would be: [trailer Signed-off-by] key = Signed-off-by: Yeah, but you can use the following instead: [trailer s-o-b] key = Signed-off-by: One thing I'm not quite understanding is where the Christian Couderchrisc...@tuxfamily.org bit comes from. So you've defined the trailer token and key, but interpret-trailers then needs to get the value it will give for the key from somewhere. Does it have to just be hardcoded in? We probably want some way to get various variables like current branch name, current git version, etc. So in the case of always adding a trailer for the branch that the commit was checked in to at the time (Developed-on, Made-on-branch, Author-branch, etc. [I think my favourite is Made-on-branch]), you'd want something like: [trailer m-o-b] key = Made-on-branch: value = $currentBranch ... resulting in the trailer (for example): Made-on-branch: pacman-minigame In the documentation patch, there is: trailer.token.command:: This option can be used to specify a shell command that will be used to automatically add or modify a trailer with the specified 'token'. When this option is specified, it is like if a special 'token=value' argument is added at the end of the command line, where 'value' will be given by the standard output of the specified command. If the command contains the `$ARG` string, this string will be replaced with the 'value' part of an existing trailer with the same token, if any, before the command is launched. That's why Something like the following should work if git commit automitically runs git interpret-trailers: [trailer m-o-b] key = Made-on-branch: command = git name-rev --name-only HEAD Also, if there were no current branch name because you're committing in a detached head state, it would be nice if you could have some logic to determine that, and instead write the trailer as: Made-on-branch: (detached HEAD: AB12CD34) You may need to write a small script for that. Then you just need the trailer.m-o-b.command config value to point to your script. ... or whatever. And also how about some logic to be able to say that if you're committing to the master branch, the trailer doesn't get inserted at all? You can script that too. 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 v10 11/12] Documentation: add documentation for 'git interpret-trailers'
From: Jeremy Morton ad...@game-point.net On 29/04/2014 12:47, Christian Couder wrote: Also, if there were no current branch name because you're committing in a detached head state, it would be nice if you could have some logic to determine that, and instead write the trailer as: Made-on-branch: (detached HEAD: AB12CD34) You may need to write a small script for that. Then you just need the trailer.m-o-b.command config value to point to your script. ... or whatever. And also how about some logic to be able to say that if you're committing to the master branch, the trailer doesn't get inserted at all? You can script that too. But it would be nicer if the logic were built-in, then you wouldn't have to share some script with your work colleagues. :-) The above logic is very specific to your workflow. For example some people might a Made-on-branch: trailer only when they are on real branches except dev and master. 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
[PATCH v2 06/10] replace: refactor checking ref validity
This will be useful in a following commit when we will want to check if the ref already exists before we let the user edit an object. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 31 --- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 0751804..3d6edaf 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -124,6 +124,25 @@ static int delete_replace_ref(const char *name, const char *ref, return 0; } +static void check_ref_valid(unsigned char object[20], + unsigned char prev[20], + char *ref, + int ref_size, + int force) +{ + if (snprintf(ref, ref_size, +refs/replace/%s, +sha1_to_hex(object)) ref_size - 1) + die(replace ref name too long: %.*s..., 50, ref); + if (check_refname_format(ref, 0)) + die('%s' is not a valid ref name., ref); + + if (read_ref(ref, prev)) + hashclr(prev); + else if (!force) + die(replace ref '%s' already exists, ref); +} + static int replace_object_sha1(const char *object_ref, unsigned char object[20], const char *replace_ref, @@ -135,13 +154,6 @@ static int replace_object_sha1(const char *object_ref, char ref[PATH_MAX]; struct ref_lock *lock; - if (snprintf(ref, sizeof(ref), -refs/replace/%s, -sha1_to_hex(object)) sizeof(ref) - 1) - die(replace ref name too long: %.*s..., 50, ref); - if (check_refname_format(ref, 0)) - die('%s' is not a valid ref name., ref); - obj_type = sha1_object_info(object, NULL); repl_type = sha1_object_info(repl, NULL); if (!force obj_type != repl_type) @@ -151,10 +163,7 @@ static int replace_object_sha1(const char *object_ref, object_ref, typename(obj_type), replace_ref, typename(repl_type)); - if (read_ref(ref, prev)) - hashclr(prev); - else if (!force) - die(replace ref '%s' already exists, ref); + check_ref_valid(object, prev, ref, sizeof(ref), force); lock = lock_any_ref_for_update(ref, prev, 0, NULL); if (!lock) -- 1.9.rc0.17.g651113e -- 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/10] replace: add option to edit a Git object
This patch series comes from what Peff sent in the following thread: http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528 The first 4 patches (1/4, 2/4, 3/4 and 4/4) didn't change since v1. I added 6 more small patches to add tests, documentation and a few small improvements. Christian Couder (6): replace: make sure --edit results in a different object replace: refactor checking ref validity replace: die early if replace ref already exists replace: add tests for --edit replace: add --edit to usage string Documentation: replace: describe new --edit option Jeff King (4): replace: refactor command-mode determination replace: use OPT_CMDMODE to handle modes replace: factor object resolution out of replace_object replace: add --edit option Documentation/git-replace.txt | 15 ++- builtin/replace.c | 225 +- t/t6050-replace.sh| 29 ++ 3 files changed, 223 insertions(+), 46 deletions(-) -- 1.9.rc0.17.g651113e -- 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/10] replace: make sure --edit results in a different object
It's a bad idea to create a replace ref for an object that points to the original object itself. That's why we have to check if the result from editing the original object is a different object and error out if it isn't. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/replace.c b/builtin/replace.c index 3da1bae..0751804 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -275,6 +275,9 @@ static int edit_and_replace(const char *object_ref, int force) free(tmpfile); + if (!hashcmp(old, new)) + return error(new object is the same as the old one: '%s', sha1_to_hex(old)); + return replace_object_sha1(object_ref, old, replacement, new, force); } -- 1.9.rc0.17.g651113e -- 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/10] replace: factor object resolution out of replace_object
From: Jeff King p...@peff.net As we add new options that operate on objects before replacing them, we'll want to be able to feed raw sha1s straight into replace_object. Split replace_object into the object-resolution part and the actual replacement. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 26 ++ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 29cf699..af40129 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -123,19 +123,17 @@ static int delete_replace_ref(const char *name, const char *ref, return 0; } -static int replace_object(const char *object_ref, const char *replace_ref, - int force) +static int replace_object_sha1(const char *object_ref, + unsigned char object[20], + const char *replace_ref, + unsigned char repl[20], + int force) { - unsigned char object[20], prev[20], repl[20]; + unsigned char prev[20]; enum object_type obj_type, repl_type; char ref[PATH_MAX]; struct ref_lock *lock; - if (get_sha1(object_ref, object)) - die(Failed to resolve '%s' as a valid ref., object_ref); - if (get_sha1(replace_ref, repl)) - die(Failed to resolve '%s' as a valid ref., replace_ref); - if (snprintf(ref, sizeof(ref), refs/replace/%s, sha1_to_hex(object)) sizeof(ref) - 1) @@ -166,6 +164,18 @@ static int replace_object(const char *object_ref, const char *replace_ref, return 0; } +static int replace_object(const char *object_ref, const char *replace_ref, int force) +{ + unsigned char object[20], repl[20]; + + if (get_sha1(object_ref, object)) + die(Failed to resolve '%s' as a valid ref., object_ref); + if (get_sha1(replace_ref, repl)) + die(Failed to resolve '%s' as a valid ref., replace_ref); + + return replace_object_sha1(object_ref, object, replace_ref, repl, force); +} + int cmd_replace(int argc, const char **argv, const char *prefix) { int force = 0; -- 1.9.rc0.17.g651113e -- 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/10] Documentation: replace: describe new --edit option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 0a02f70..37d872d 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -9,6 +9,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement +'git replace' [-f] --edit object 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -63,6 +64,14 @@ OPTIONS --delete:: Delete existing replace refs for the given objects. +--edit object:: + Launch an editor to let the user change the content of + object, then create a new object of the same type with the + changed content, and create a replace ref to replace object + with the new object. See linkgit:git-commit[1] and + linkgit:git-var[1] for details about how the editor will be + chosen. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or @@ -92,7 +101,9 @@ CREATING REPLACEMENT OBJECTS linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and linkgit:git-rebase[1], among other git commands, can be used to create -replacement objects from existing objects. +replacement objects from existing objects. The `--edit` option can +also be used with 'git replace' to create a replacement object by +editing an existing object. If you want to replace many blobs, trees or commits that are part of a string of commits, you may just want to create a replacement string of @@ -117,6 +128,8 @@ linkgit:git-filter-branch[1] linkgit:git-rebase[1] linkgit:git-tag[1] linkgit:git-branch[1] +linkgit:git-commit[1] +linkgit:git-var[1] linkgit:git[1] GIT -- 1.9.rc0.17.g651113e -- 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/10] replace: use OPT_CMDMODE to handle modes
From: Jeff King p...@peff.net By using OPT_CMDMODE, the mutual exclusion between modes is taken care of for us. It also makes it easy for us to maintain a single variable with the mode, which makes its intent more clear. We can use a single switch() to make sure we have covered all of the modes. This ends up breaking even in code size, but the win will be much bigger when we start adding more modes. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 49 + 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 28db96f..29cf699 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -168,11 +168,17 @@ static int replace_object(const char *object_ref, const char *replace_ref, int cmd_replace(int argc, const char **argv, const char *prefix) { - int list = 0, delete = 0, force = 0; + int force = 0; const char *format = NULL; + enum { + MODE_UNSPECIFIED = 0, + MODE_LIST, + MODE_DELETE, + MODE_REPLACE + } cmdmode = MODE_UNSPECIFIED; struct option options[] = { - OPT_BOOL('l', list, list, N_(list replace refs)), - OPT_BOOL('d', delete, delete, N_(delete replace refs)), + OPT_CMDMODE('l', list, cmdmode, N_(list replace refs), MODE_LIST), + OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs), MODE_DELETE), OPT_BOOL('f', force, force, N_(replace the ref if it exists)), OPT_STRING(0, format, format, N_(format), N_(use this format)), OPT_END() @@ -182,42 +188,37 @@ int cmd_replace(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0); - if (!list !delete) - if (!argc) - list = 1; + if (!cmdmode) + cmdmode = argc ? MODE_REPLACE : MODE_LIST; - if (list delete) - usage_msg_opt(-l and -d cannot be used together, - git_replace_usage, options); - - if (format !list) + if (format cmdmode != MODE_LIST) usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); - if (force (list || delete)) - usage_msg_opt(-f cannot be used with -d or -l, + if (force cmdmode != MODE_REPLACE) + usage_msg_opt(-f only makes sense when writing a replacement, git_replace_usage, options); - /* Delete refs */ - if (delete) { + switch (cmdmode) { + case MODE_DELETE: if (argc 1) usage_msg_opt(-d needs at least one argument, git_replace_usage, options); return for_each_replace_name(argv, delete_replace_ref); - } - /* Replace object */ - if (!list argc) { + case MODE_REPLACE: if (argc != 2) usage_msg_opt(bad number of arguments, git_replace_usage, options); return replace_object(argv[0], argv[1], force); - } - /* List refs, even if list is not set */ - if (argc 1) - usage_msg_opt(only one pattern can be given with -l, - git_replace_usage, options); + case MODE_LIST: + if (argc 1) + usage_msg_opt(only one pattern can be given with -l, + git_replace_usage, options); + return list_replace_refs(argv[0], format); - return list_replace_refs(argv[0], format); + default: + die(BUG: invalid cmdmode %d, (int)cmdmode); + } } -- 1.9.rc0.17.g651113e -- 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 01/10] replace: refactor command-mode determination
From: Jeff King p...@peff.net The git-replace command has three modes: listing, deleting, and replacing. The first two are selected explicitly. If none is selected, we fallback to listing when there are no arguments, and replacing otherwise. Let's figure out up front which operation we are going to do, before getting into the application logic. That lets us simplify our option checks (e.g., we currently have to check whether a useless --force is given both along with an explicit list, as well as with an implicit one). This saves some lines, makes the logic easier to follow, and will facilitate further cleanups. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index b62420a..28db96f 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -182,12 +182,16 @@ int cmd_replace(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0); + if (!list !delete) + if (!argc) + list = 1; + if (list delete) usage_msg_opt(-l and -d cannot be used together, git_replace_usage, options); - if (format delete) - usage_msg_opt(--format and -d cannot be used together, + if (format !list) + usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); if (force (list || delete)) @@ -207,9 +211,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc != 2) usage_msg_opt(bad number of arguments, git_replace_usage, options); - if (format) - usage_msg_opt(--format cannot be used when not listing, - git_replace_usage, options); return replace_object(argv[0], argv[1], force); } @@ -217,9 +218,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc 1) usage_msg_opt(only one pattern can be given with -l, git_replace_usage, options); - if (force) - usage_msg_opt(-f needs some arguments, - git_replace_usage, options); return list_replace_refs(argv[0], format); } -- 1.9.rc0.17.g651113e -- 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/10] replace: add --edit to usage string
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/replace.c b/builtin/replace.c index 4ee3d92..1bb491d 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -16,6 +16,7 @@ static const char * const git_replace_usage[] = { N_(git replace [-f] object replacement), + N_(git replace [-f] --edit object), N_(git replace -d object...), N_(git replace [--format=format] [-l [pattern]]), NULL -- 1.9.rc0.17.g651113e -- 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/10] replace: add tests for --edit
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 29 + 1 file changed, 29 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 719a116..7609174 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -318,6 +318,35 @@ test_expect_success 'test --format long' ' test_cmp expected actual ' +test_expect_success 'setup a fake editor' ' + cat fakeeditor -\EOF + #!/bin/sh + sed -e s/A U Thor/A fake Thor/ $1 $1.new + mv $1.new $1 + EOF + chmod +x fakeeditor +' + +test_expect_success '--edit with and without already replaced object' ' + GIT_EDITOR=./fakeeditor test_must_fail git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor git replace --force --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor + git replace -d $PARA3 + GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor +' + +test_expect_success '--edit and change nothing or command failed' ' + git replace -d $PARA3 + GIT_EDITOR=true test_must_fail git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor;false test_must_fail git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor +' + test_expect_success 'replace ref cleanup' ' test -n $(git replace) git replace -d $(git replace) -- 1.9.rc0.17.g651113e -- 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 v3 01/10] replace: refactor command-mode determination
From: Jeff King p...@peff.net The git-replace command has three modes: listing, deleting, and replacing. The first two are selected explicitly. If none is selected, we fallback to listing when there are no arguments, and replacing otherwise. Let's figure out up front which operation we are going to do, before getting into the application logic. That lets us simplify our option checks (e.g., we currently have to check whether a useless --force is given both along with an explicit list, as well as with an implicit one). This saves some lines, makes the logic easier to follow, and will facilitate further cleanups. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index b62420a..28db96f 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -182,12 +182,16 @@ int cmd_replace(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0); + if (!list !delete) + if (!argc) + list = 1; + if (list delete) usage_msg_opt(-l and -d cannot be used together, git_replace_usage, options); - if (format delete) - usage_msg_opt(--format and -d cannot be used together, + if (format !list) + usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); if (force (list || delete)) @@ -207,9 +211,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc != 2) usage_msg_opt(bad number of arguments, git_replace_usage, options); - if (format) - usage_msg_opt(--format cannot be used when not listing, - git_replace_usage, options); return replace_object(argv[0], argv[1], force); } @@ -217,9 +218,6 @@ int cmd_replace(int argc, const char **argv, const char *prefix) if (argc 1) usage_msg_opt(only one pattern can be given with -l, git_replace_usage, options); - if (force) - usage_msg_opt(-f needs some arguments, - git_replace_usage, options); return list_replace_refs(argv[0], format); } -- 1.9.rc0.17.g651113e -- 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 v3 02/10] replace: use OPT_CMDMODE to handle modes
From: Jeff King p...@peff.net By using OPT_CMDMODE, the mutual exclusion between modes is taken care of for us. It also makes it easy for us to maintain a single variable with the mode, which makes its intent more clear. We can use a single switch() to make sure we have covered all of the modes. This ends up breaking even in code size, but the win will be much bigger when we start adding more modes. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 49 + 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 28db96f..29cf699 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -168,11 +168,17 @@ static int replace_object(const char *object_ref, const char *replace_ref, int cmd_replace(int argc, const char **argv, const char *prefix) { - int list = 0, delete = 0, force = 0; + int force = 0; const char *format = NULL; + enum { + MODE_UNSPECIFIED = 0, + MODE_LIST, + MODE_DELETE, + MODE_REPLACE + } cmdmode = MODE_UNSPECIFIED; struct option options[] = { - OPT_BOOL('l', list, list, N_(list replace refs)), - OPT_BOOL('d', delete, delete, N_(delete replace refs)), + OPT_CMDMODE('l', list, cmdmode, N_(list replace refs), MODE_LIST), + OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs), MODE_DELETE), OPT_BOOL('f', force, force, N_(replace the ref if it exists)), OPT_STRING(0, format, format, N_(format), N_(use this format)), OPT_END() @@ -182,42 +188,37 @@ int cmd_replace(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_replace_usage, 0); - if (!list !delete) - if (!argc) - list = 1; + if (!cmdmode) + cmdmode = argc ? MODE_REPLACE : MODE_LIST; - if (list delete) - usage_msg_opt(-l and -d cannot be used together, - git_replace_usage, options); - - if (format !list) + if (format cmdmode != MODE_LIST) usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); - if (force (list || delete)) - usage_msg_opt(-f cannot be used with -d or -l, + if (force cmdmode != MODE_REPLACE) + usage_msg_opt(-f only makes sense when writing a replacement, git_replace_usage, options); - /* Delete refs */ - if (delete) { + switch (cmdmode) { + case MODE_DELETE: if (argc 1) usage_msg_opt(-d needs at least one argument, git_replace_usage, options); return for_each_replace_name(argv, delete_replace_ref); - } - /* Replace object */ - if (!list argc) { + case MODE_REPLACE: if (argc != 2) usage_msg_opt(bad number of arguments, git_replace_usage, options); return replace_object(argv[0], argv[1], force); - } - /* List refs, even if list is not set */ - if (argc 1) - usage_msg_opt(only one pattern can be given with -l, - git_replace_usage, options); + case MODE_LIST: + if (argc 1) + usage_msg_opt(only one pattern can be given with -l, + git_replace_usage, options); + return list_replace_refs(argv[0], format); - return list_replace_refs(argv[0], format); + default: + die(BUG: invalid cmdmode %d, (int)cmdmode); + } } -- 1.9.rc0.17.g651113e -- 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 v3 09/10] replace: add --edit to usage string
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/replace.c b/builtin/replace.c index 4ee3d92..1bb491d 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -16,6 +16,7 @@ static const char * const git_replace_usage[] = { N_(git replace [-f] object replacement), + N_(git replace [-f] --edit object), N_(git replace -d object...), N_(git replace [--format=format] [-l [pattern]]), NULL -- 1.9.rc0.17.g651113e -- 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 v3 10/10] Documentation: replace: describe new --edit option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 0a02f70..61461b9 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -9,6 +9,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement +'git replace' [-f] --edit object 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -63,6 +64,15 @@ OPTIONS --delete:: Delete existing replace refs for the given objects. +--edit object:: + Edit an object's content interactively. The existing content + for object is pretty-printed into a temporary file, an + editor is launched on the file, and the result is parsed to + create a new object of the same type as object. A + replacement ref is then created to replace object with the + newly created object. See linkgit:git-var[1] for details about + how the editor will be chosen. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or @@ -92,7 +102,9 @@ CREATING REPLACEMENT OBJECTS linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and linkgit:git-rebase[1], among other git commands, can be used to create -replacement objects from existing objects. +replacement objects from existing objects. The `--edit` option can +also be used with 'git replace' to create a replacement object by +editing an existing object. If you want to replace many blobs, trees or commits that are part of a string of commits, you may just want to create a replacement string of @@ -117,6 +129,8 @@ linkgit:git-filter-branch[1] linkgit:git-rebase[1] linkgit:git-tag[1] linkgit:git-branch[1] +linkgit:git-commit[1] +linkgit:git-var[1] linkgit:git[1] GIT -- 1.9.rc0.17.g651113e -- 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 v3 03/10] replace: factor object resolution out of replace_object
From: Jeff King p...@peff.net As we add new options that operate on objects before replacing them, we'll want to be able to feed raw sha1s straight into replace_object. Split replace_object into the object-resolution part and the actual replacement. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 26 ++ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 29cf699..af40129 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -123,19 +123,17 @@ static int delete_replace_ref(const char *name, const char *ref, return 0; } -static int replace_object(const char *object_ref, const char *replace_ref, - int force) +static int replace_object_sha1(const char *object_ref, + unsigned char object[20], + const char *replace_ref, + unsigned char repl[20], + int force) { - unsigned char object[20], prev[20], repl[20]; + unsigned char prev[20]; enum object_type obj_type, repl_type; char ref[PATH_MAX]; struct ref_lock *lock; - if (get_sha1(object_ref, object)) - die(Failed to resolve '%s' as a valid ref., object_ref); - if (get_sha1(replace_ref, repl)) - die(Failed to resolve '%s' as a valid ref., replace_ref); - if (snprintf(ref, sizeof(ref), refs/replace/%s, sha1_to_hex(object)) sizeof(ref) - 1) @@ -166,6 +164,18 @@ static int replace_object(const char *object_ref, const char *replace_ref, return 0; } +static int replace_object(const char *object_ref, const char *replace_ref, int force) +{ + unsigned char object[20], repl[20]; + + if (get_sha1(object_ref, object)) + die(Failed to resolve '%s' as a valid ref., object_ref); + if (get_sha1(replace_ref, repl)) + die(Failed to resolve '%s' as a valid ref., replace_ref); + + return replace_object_sha1(object_ref, object, replace_ref, repl, force); +} + int cmd_replace(int argc, const char **argv, const char *prefix) { int force = 0; -- 1.9.rc0.17.g651113e -- 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 v3 04/10] replace: add --edit option
From: Jeff King p...@peff.net This allows you to run: git replace --edit SHA1 to get dumped in an editor with the contents of the object for SHA1. The result is then read back in and used as a replace object for SHA1. The writing/reading is type-aware, so you get to edit ls-tree output rather than the binary tree format. Missing documentation and tests. Signed-off-by: Jeff King p...@peff.net Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 112 +- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/builtin/replace.c b/builtin/replace.c index af40129..3da1bae 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -12,6 +12,7 @@ #include builtin.h #include refs.h #include parse-options.h +#include run-command.h static const char * const git_replace_usage[] = { N_(git replace [-f] object replacement), @@ -176,6 +177,107 @@ static int replace_object(const char *object_ref, const char *replace_ref, int f return replace_object_sha1(object_ref, object, replace_ref, repl, force); } +/* + * Write the contents of the object named by sha1 to the file filename, + * pretty-printed for human editing based on its type. + */ +static void export_object(const unsigned char *sha1, const char *filename) +{ + const char *argv[] = { --no-replace-objects, cat-file, -p, NULL, NULL }; + struct child_process cmd = { argv }; + int fd; + + fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0666); + if (fd 0) + die_errno(unable to open %s for writing, filename); + + argv[3] = sha1_to_hex(sha1); + cmd.git_cmd = 1; + cmd.out = fd; + + if (run_command(cmd)) + die(cat-file reported failure); + + close(fd); +} + +/* + * Read a previously-exported (and possibly edited) object back from filename, + * interpreting it as type, and writing the result to the object database. + * The sha1 of the written object is returned via sha1. + */ +static void import_object(unsigned char *sha1, enum object_type type, + const char *filename) +{ + int fd; + + fd = open(filename, O_RDONLY); + if (fd 0) + die_errno(unable to open %s for reading, filename); + + if (type == OBJ_TREE) { + const char *argv[] = { mktree, NULL }; + struct child_process cmd = { argv }; + struct strbuf result = STRBUF_INIT; + + cmd.argv = argv; + cmd.git_cmd = 1; + cmd.in = fd; + cmd.out = -1; + + if (start_command(cmd)) + die(unable to spawn mktree); + + if (strbuf_read(result, cmd.out, 41) 0) + die_errno(unable to read from mktree); + close(cmd.out); + + if (finish_command(cmd)) + die(mktree reported failure); + if (get_sha1_hex(result.buf, sha1) 0) + die(mktree did not return an object name); + + strbuf_release(result); + } else { + struct stat st; + int flags = HASH_FORMAT_CHECK | HASH_WRITE_OBJECT; + + if (fstat(fd, st) 0) + die_errno(unable to fstat %s, filename); + if (index_fd(sha1, fd, st, type, NULL, flags) 0) + die(unable to write object to database); + /* index_fd close()s fd for us */ + } + + /* +* No need to close(fd) here; both run-command and index-fd +* will have done it for us. +*/ +} + +static int edit_and_replace(const char *object_ref, int force) +{ + char *tmpfile = git_pathdup(REPLACE_EDITOBJ); + enum object_type type; + unsigned char old[20], new[20]; + + if (get_sha1(object_ref, old) 0) + die(Not a valid object name: '%s', object_ref); + + type = sha1_object_info(old, NULL); + if (type 0) + die(unable to get object type for %s, sha1_to_hex(old)); + + export_object(old, tmpfile); + if (launch_editor(tmpfile, NULL, NULL) 0) + die(editing object file failed); + import_object(new, type, tmpfile); + + free(tmpfile); + + return replace_object_sha1(object_ref, old, replacement, new, force); +} + int cmd_replace(int argc, const char **argv, const char *prefix) { int force = 0; @@ -184,11 +286,13 @@ int cmd_replace(int argc, const char **argv, const char *prefix) MODE_UNSPECIFIED = 0, MODE_LIST, MODE_DELETE, + MODE_EDIT, MODE_REPLACE } cmdmode = MODE_UNSPECIFIED; struct option options[] = { OPT_CMDMODE('l', list, cmdmode, N_(list replace refs), MODE_LIST), OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs
[PATCH v3 00/10] replace: add option to edit a Git object
This patch series comes from what Peff sent in the following thread: http://thread.gmane.org/gmane.comp.version-control.git/243361/focus=243528 The only changes compared to v2 are in the test (8/10) and documentation patches (10/10). Thanks to Peff. Christian Couder (6): replace: make sure --edit results in a different object replace: refactor checking ref validity replace: die early if replace ref already exists replace: add tests for --edit replace: add --edit to usage string Documentation: replace: describe new --edit option Jeff King (4): replace: refactor command-mode determination replace: use OPT_CMDMODE to handle modes replace: factor object resolution out of replace_object replace: add --edit option Documentation/git-replace.txt | 16 ++- builtin/replace.c | 225 +- t/t6050-replace.sh| 27 + 3 files changed, 222 insertions(+), 46 deletions(-) -- 1.9.rc0.17.g651113e -- 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 v3 08/10] replace: add tests for --edit
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 27 +++ 1 file changed, 27 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 719a116..68b3cb2 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -318,6 +318,33 @@ test_expect_success 'test --format long' ' test_cmp expected actual ' +test_expect_success 'setup a fake editor' ' + write_script fakeeditor -\EOF + sed -e s/A U Thor/A fake Thor/ $1 $1.new + mv $1.new $1 + EOF +' + +test_expect_success '--edit with and without already replaced object' ' + test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor git replace --force --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor + git replace -d $PARA3 + GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor +' + +test_expect_success '--edit and change nothing or command failed' ' + git replace -d $PARA3 + test_must_fail env GIT_EDITOR=true git replace --edit $PARA3 + test_must_fail env GIT_EDITOR=./fakeeditor;false git replace --edit $PARA3 + GIT_EDITOR=./fakeeditor git replace --edit $PARA3 + git replace -l | grep $PARA3 + git cat-file commit $PARA3 | grep A fake Thor +' + test_expect_success 'replace ref cleanup' ' test -n $(git replace) git replace -d $(git replace) -- 1.9.rc0.17.g651113e -- 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 v3 06/10] replace: refactor checking ref validity
This will be useful in a following commit when we will want to check if the ref already exists before we let the user edit an object. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 31 --- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 0751804..3d6edaf 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -124,6 +124,25 @@ static int delete_replace_ref(const char *name, const char *ref, return 0; } +static void check_ref_valid(unsigned char object[20], + unsigned char prev[20], + char *ref, + int ref_size, + int force) +{ + if (snprintf(ref, ref_size, +refs/replace/%s, +sha1_to_hex(object)) ref_size - 1) + die(replace ref name too long: %.*s..., 50, ref); + if (check_refname_format(ref, 0)) + die('%s' is not a valid ref name., ref); + + if (read_ref(ref, prev)) + hashclr(prev); + else if (!force) + die(replace ref '%s' already exists, ref); +} + static int replace_object_sha1(const char *object_ref, unsigned char object[20], const char *replace_ref, @@ -135,13 +154,6 @@ static int replace_object_sha1(const char *object_ref, char ref[PATH_MAX]; struct ref_lock *lock; - if (snprintf(ref, sizeof(ref), -refs/replace/%s, -sha1_to_hex(object)) sizeof(ref) - 1) - die(replace ref name too long: %.*s..., 50, ref); - if (check_refname_format(ref, 0)) - die('%s' is not a valid ref name., ref); - obj_type = sha1_object_info(object, NULL); repl_type = sha1_object_info(repl, NULL); if (!force obj_type != repl_type) @@ -151,10 +163,7 @@ static int replace_object_sha1(const char *object_ref, object_ref, typename(obj_type), replace_ref, typename(repl_type)); - if (read_ref(ref, prev)) - hashclr(prev); - else if (!force) - die(replace ref '%s' already exists, ref); + check_ref_valid(object, prev, ref, sizeof(ref), force); lock = lock_any_ref_for_update(ref, prev, 0, NULL); if (!lock) -- 1.9.rc0.17.g651113e -- 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 v3 07/10] replace: die early if replace ref already exists
If a replace ref already exists for an object, it is much better for the user if we error out before we let the user edit the object, rather than after. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/builtin/replace.c b/builtin/replace.c index 3d6edaf..4ee3d92 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -268,7 +268,8 @@ static int edit_and_replace(const char *object_ref, int force) { char *tmpfile = git_pathdup(REPLACE_EDITOBJ); enum object_type type; - unsigned char old[20], new[20]; + unsigned char old[20], new[20], prev[20]; + char ref[PATH_MAX]; if (get_sha1(object_ref, old) 0) die(Not a valid object name: '%s', object_ref); @@ -277,6 +278,8 @@ static int edit_and_replace(const char *object_ref, int force) if (type 0) die(unable to get object type for %s, sha1_to_hex(old)); + check_ref_valid(old, prev, ref, sizeof(ref), force); + export_object(old, tmpfile); if (launch_editor(tmpfile, NULL, NULL) 0) die(editing object file failed); -- 1.9.rc0.17.g651113e -- 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
[RFC/PATCH] replace: add --graft option
The usage string for this option is: git replace [-f] --graft commit [parent...] First we create a new commit that is the same as commit except that its parents are [parents...] Then we create a replace ref that replace commit with the commit we just created. With this new option, it should be straightforward to convert grafts to replace refs, with something like: cat .git/info/grafts | while read line do git replace --graft $line; done Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 79 ++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/builtin/replace.c b/builtin/replace.c index 1bb491d..4b3705d 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -17,6 +17,7 @@ static const char * const git_replace_usage[] = { N_(git replace [-f] object replacement), N_(git replace [-f] --edit object), + N_(git replace [-f] --graft commit [parent...]), N_(git replace -d object...), N_(git replace [--format=format] [-l [pattern]]), NULL @@ -294,6 +295,71 @@ static int edit_and_replace(const char *object_ref, int force) return replace_object_sha1(object_ref, old, replacement, new, force); } +static int read_sha1_commit(const unsigned char *sha1, struct strbuf *dst) +{ + void *buf; + enum object_type type; + unsigned long size; + + buf = read_sha1_file(sha1, type, size); + if (!buf) + return error(_(cannot read object %s), sha1_to_hex(sha1)); + if (type != OBJ_COMMIT) { + free(buf); + return error(_(object %s is not a commit), sha1_to_hex(sha1)); + } + strbuf_attach(dst, buf, size, size + 1); + return 0; +} + +static int create_graft(int argc, const char **argv, int force) +{ + unsigned char old[20], new[20]; + const char *old_ref = argv[0]; + struct strbuf buf = STRBUF_INIT; + struct strbuf new_parents = STRBUF_INIT; + const char *parent_start, *parent_end; + int i; + + if (get_sha1(old_ref, old) 0) + die(_(Not a valid object name: '%s'), old_ref); + lookup_commit_or_die(old, old_ref); + if (read_sha1_commit(old, buf)) + die(_(Invalid commit: '%s'), old_ref); + + /* find existing parents */ + parent_start = buf.buf; + parent_start += 46; /* tree + hex sha1 + \n */ + parent_end = parent_start; + + while (starts_with(parent_end, parent )) + parent_end += 48; /* parent + hex sha1 + \n */ + + /* prepare new parents */ + for (i = 1; i argc; i++) { + unsigned char sha1[20]; + if (get_sha1(argv[i], sha1) 0) + die(_(Not a valid object name: '%s'), argv[i]); + lookup_commit_or_die(sha1, argv[i]); + strbuf_addf(new_parents, parent %s\n, sha1_to_hex(sha1)); + } + + /* replace existing parents with new ones */ + strbuf_splice(buf, parent_start - buf.buf, parent_end - parent_start, + new_parents.buf, new_parents.len); + + if (write_sha1_file(buf.buf, buf.len, commit_type, new)) + die(_(could not write replacement commit for: '%s'), old_ref); + + strbuf_release(new_parents); + strbuf_release(buf); + + if (!hashcmp(old, new)) + return error(new commit is the same as the old one: '%s', sha1_to_hex(old)); + + return replace_object_sha1(old_ref, old, replacement, new, force); +} + int cmd_replace(int argc, const char **argv, const char *prefix) { int force = 0; @@ -303,12 +369,14 @@ int cmd_replace(int argc, const char **argv, const char *prefix) MODE_LIST, MODE_DELETE, MODE_EDIT, + MODE_GRAFT, MODE_REPLACE } cmdmode = MODE_UNSPECIFIED; struct option options[] = { OPT_CMDMODE('l', list, cmdmode, N_(list replace refs), MODE_LIST), OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs), MODE_DELETE), OPT_CMDMODE('e', edit, cmdmode, N_(edit existing object), MODE_EDIT), + OPT_CMDMODE('g', graft, cmdmode, N_(change a commit's parents), MODE_GRAFT), OPT_BOOL('f', force, force, N_(replace the ref if it exists)), OPT_STRING(0, format, format, N_(format), N_(use this format)), OPT_END() @@ -325,7 +393,10 @@ int cmd_replace(int argc, const char **argv, const char *prefix) usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); - if (force cmdmode != MODE_REPLACE cmdmode != MODE_EDIT) + if (force + cmdmode != MODE_REPLACE + cmdmode != MODE_EDIT + cmdmode != MODE_GRAFT) usage_msg_opt(-f only makes sense when writing
Re: Sharing merge conflict resolution between multiple developers
Le 11 août 2014 07:50, Christian Couder christian.cou...@gmail.com a écrit : This should be possible using git imerge which is separate tool. Sorry I sent the above using the gmail app on my mobile phone and unfortunately I can't make it send plain text emails. (Emails which are not plain text are rejected by vger.kernel.org.) 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
[PATCH v13 03/11] trailer: read and process config information
Read the configuration to get trailer information, and then process it and store 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 185 ++ 1 file changed, 185 insertions(+) diff --git a/trailer.c b/trailer.c index 4940e06..2d391f3 100644 --- a/trailer.c +++ b/trailer.c @@ -272,3 +272,188 @@ static void process_trailers_lists(struct trailer_item **in_tok_first, 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 if (!strcasecmp(end, value)) + item-where = WHERE_END; + else if (!strcasecmp(start, value)) + item-where = WHERE_START; + else + return -1; + return 0; +} + +static int set_if_exists(struct conf_info *item, const char *value) +{ + if (!strcasecmp(addIfDifferent, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT; + else if (!strcasecmp(addIfDifferentNeighbor, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR; + else if (!strcasecmp(add, value)) + item-if_exists = EXISTS_ADD; + else if (!strcasecmp(replace, value)) + item-if_exists = EXISTS_REPLACE; + else if (!strcasecmp(doNothing, value)) + item-if_exists = EXISTS_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; +} + +static void duplicate_conf(struct conf_info *dst, struct conf_info *src) +{ + *dst = *src; + if (src-name) + dst-name = xstrdup(src-name); + if (src-key) + dst-key = xstrdup(src-key); + if (src-command) + dst-command = xstrdup(src-command); +} + +static struct trailer_item *get_conf_item(const 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); + duplicate_conf(item-conf, default_conf_info); + item-conf.name = xstrdup(name); + + if (!previous) + first_conf_item = item; + else { + previous-next = item; + item-previous = previous; + } + + return item; +} + +enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE, +TRAILER_IF_EXISTS, TRAILER_IF_MISSING }; + +static struct { + const char *name; + enum trailer_info_type type; +} trailer_config_items[] = { + { key, TRAILER_KEY }, + { command, TRAILER_COMMAND }, + { where, TRAILER_WHERE }, + { ifexists, TRAILER_IF_EXISTS }, + { ifmissing, TRAILER_IF_MISSING } +}; + +static int git_trailer_default_config(const char *conf_key, const char *value, void *cb) +{ + const char *trailer_item, *variable_name; + + if (!skip_prefix(conf_key, trailer., trailer_item)) + return 0; + + variable_name = strrchr(trailer_item, '.'); + if (!variable_name) { + if (!strcmp(trailer_item, where)) { + if (set_where(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifexists)) { + if (set_if_exists(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifmissing)) { + if (set_if_missing(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, separators)) { + separators = xstrdup(value); + } + } + return 0
[PATCH v13 05/11] trailer: parse trailers from file or stdin
Read trailers from a file or from stdin, parse the trailers and then put the result into a doubly linked list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 123 ++ 1 file changed, 123 insertions(+) diff --git a/trailer.c b/trailer.c index b9d3ed4..46e2fcb 100644 --- a/trailer.c +++ b/trailer.c @@ -64,6 +64,14 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b) return same_token(a, b) same_value(a, b); } +static inline int contains_only_spaces(const char *str) +{ + const char *s = str; + while (*s isspace(*s)) + s++; + return !*s; +} + static void free_trailer_item(struct trailer_item *item) { free(item-conf.name); @@ -582,3 +590,118 @@ static struct trailer_item *process_command_line_args(struct string_list *traile return arg_tok_first; } + +static struct strbuf **read_input_file(const char *file) +{ + struct strbuf **lines; + struct strbuf sb = STRBUF_INIT; + + if (file) { + if (strbuf_read_file(sb, file, 0) 0) + die_errno(_(could not read input file '%s'), file); + } else { + if (strbuf_read(sb, fileno(stdin), 0) 0) + die_errno(_(could not read from stdin)); + } + + lines = strbuf_split(sb, '\n'); + + strbuf_release(sb); + + return lines; +} + +/* + * Return the (0 based) index of the start of the patch or the line + * count if there is no patch in the message. + */ +static int find_patch_start(struct strbuf **lines, int count) +{ + int i; + + /* Get the start of the patch part if any */ + for (i = 0; i count; i++) { + if (starts_with(lines[i]-buf, ---)) + return i; + } + + return count; +} + +/* + * Return the (0 based) index of the first trailer line or count if + * there are no trailers. Trailers are searched only in the lines from + * index (count - 1) down to index 0. + */ +static int find_trailer_start(struct strbuf **lines, int count) +{ + int start, only_spaces = 1; + + /* +* Get the start of the trailers by looking starting from the end +* for a line with only spaces before lines with one separator. +*/ + for (start = count - 1; start = 0; start--) { + if (lines[start]-buf[0] == comment_line_char) + continue; + if (contains_only_spaces(lines[start]-buf)) { + if (only_spaces) + continue; + return start + 1; + } + if (strcspn(lines[start]-buf, separators) lines[start]-len) { + if (only_spaces) + only_spaces = 0; + continue; + } + return count; + } + + return only_spaces ? count : 0; +} + +static int has_blank_line_before(struct strbuf **lines, int start) +{ + for (;start = 0; start--) { + if (lines[start]-buf[0] == comment_line_char) + continue; + return contains_only_spaces(lines[start]-buf); + } + return 0; +} + +static void print_lines(struct strbuf **lines, int start, int end) +{ + int i; + for (i = start; lines[i] i end; i++) + printf(%s, lines[i]-buf); +} + +static int process_input_file(struct strbuf **lines, + struct trailer_item **in_tok_first, + struct trailer_item **in_tok_last) +{ + int count = 0; + int patch_start, trailer_start, i; + + /* Get the line count */ + while (lines[count]) + count++; + + patch_start = find_patch_start(lines, count); + trailer_start = find_trailer_start(lines, patch_start); + + /* Print lines before the trailers as is */ + print_lines(lines, 0, trailer_start); + + if (!has_blank_line_before(lines, trailer_start - 1)) + printf(\n); + + /* Parse trailer lines */ + for (i = trailer_start; i patch_start; i++) { + struct trailer_item *new = create_trailer_item(lines[i]-buf); + add_trailer_item(in_tok_first, in_tok_last, new); + } + + return patch_start; +} -- 2.0.1.674.ga7f57b7 -- 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 v13 00/11] 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 keep builtin/commit.c uncontaminated by any more hard-wired logic, like what we have for the signed-off-by line. Any new things can and should be doable in hooks, and this filter would help writing these hooks. And that is why the design goal of the filter is to make it at least as powerful as the built-in logic we have for signed-off-by lines; that would allow us to later eject the hard-wired logic for signed-off-by line from the main codepath, if/when we wanted to. Alternatively, we could build a library-ish API around this filter code and replace the hard-wired logic for signed-off-by line with a call into that API, if/when we wanted to, but that requires (in addition to the at least as powerful as the built-in logic) that the implementation of this stand-alone filter can be cleanly made into a reusable library, so that is a bit higher bar to cross than everything can be doable with hooks alternative. 2) Current state: Currently the usage string of this command is: git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...] The following features are implemented: - the result is printed on stdout - the --trailer arguments are interpreted - messages read from file... or stdin are interpreted - the trailer.separators option in the config is interpreted (new) - the trailer.where option is interpreted (new) - the trailer.ifexists option is interpreted (new) - the trailer.ifmissing option is interpreted (new) - the trailer.token.key options are interpreted - the trailer.token.where options are interpreted - the trailer.token.ifexist options are interpreted - the trailer.token.ifmissing options are interpreted - the trailer.token.command config works - $ARG can be used in commands - messages can contain a patch - lines in messages starting with a comment char are ignored - there are 49 tests - there is some documentation - there are examples in the documentation 3) Changes since version 12, thanks to Jakub, Michael, Johan and Junio: * end and start values for trailer.token.where have been implemented * end has been made the default value for where, but this default value can be changed using the new trailer.where config variable * addIfDifferentNeighbor is now the default value for ifexists, but this default can be changed using the new trailer.ifexists config variable * the new trailer.ifmissing can be used to change the default value for ifmissing (which is add) * by default the only separator is ':', this can be changed by using the new trailer.separators config variable * only the configured separators (or just ':' by default) are used for both input parsing and output printing; the only exception is that '=' is always accepted as separator when parsing --trailer 'tokensepvalue' command line arguments, for compatibility with other git commands * 14 tests have been added Only patches 7/11, 9/11 and 10/11 have not been changed since v12. Christian Couder (11): trailer: add data structures and basic functions trailer: process trailers from input message and arguments trailer: read and process config information trailer: process command line trailer arguments trailer: parse trailers from file or stdin trailer: put all the processing together and print trailer: add interpret-trailers command trailer: add tests for git interpret-trailers trailer: execute command from 'trailer.name.command' trailer: add tests for commands in config file Documentation: add documentation for 'git interpret-trailers' .gitignore | 1 + Documentation/git-interpret-trailers.txt | 308 +++ Makefile | 2 + builtin.h| 1 + builtin/interpret-trailers.c | 44 ++ command-list.txt | 1 + git.c| 1 + t/t7513-interpret-trailers.sh| 850 +++ trailer.c| 830 ++ trailer.h
[PATCH v13 04/11] trailer: process command line trailer arguments
Parse the trailer command line arguments and put the result into an arg_tok doubly linked list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 125 ++ 1 file changed, 125 insertions(+) diff --git a/trailer.c b/trailer.c index 2d391f3..b9d3ed4 100644 --- a/trailer.c +++ b/trailer.c @@ -1,4 +1,5 @@ #include cache.h +#include string-list.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -457,3 +458,127 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb) } return 0; } + +static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *trailer) +{ + size_t len; + struct strbuf seps = STRBUF_INIT; + strbuf_addstr(seps, separators); + strbuf_addch(seps, '='); + len = strcspn(trailer, seps.buf); + strbuf_release(seps); + if (len == 0) + return error(_(empty trailer token in trailer '%s'), trailer); + if (len strlen(trailer)) { + strbuf_add(tok, trailer, len); + strbuf_trim(tok); + strbuf_addstr(val, trailer + len + 1); + strbuf_trim(val); + } else { + strbuf_addstr(tok, trailer); + strbuf_trim(tok); + } + return 0; +} + + +static void duplicate_conf(struct conf_info *dst, struct conf_info *src) +{ + *dst = *src; + if (src-name) + dst-name = xstrdup(src-name); + if (src-key) + dst-key = xstrdup(src-key); + if (src-command) + dst-command = xstrdup(src-command); +} + +static const char *token_from_item(struct trailer_item *item) +{ + if (item-conf.key) + return item-conf.key; + + return item-conf.name; +} + +static struct trailer_item *new_trailer_item(struct trailer_item *conf_item, +char *tok, char *val) +{ + struct trailer_item *new = xcalloc(sizeof(*new), 1); + new-value = val; + + if (conf_item) { + duplicate_conf(new-conf, conf_item-conf); + new-token = xstrdup(token_from_item(conf_item)); + free(tok); + } else { + duplicate_conf(new-conf, default_conf_info); + new-token = tok; + } + + return new; +} + +static int token_matches_item(const char *tok, struct trailer_item *item, int alnum_len) +{ + if (!strncasecmp(tok, item-conf.name, alnum_len)) + return 1; + return item-conf.key ? !strncasecmp(tok, item-conf.key, alnum_len) : 0; +} + +static struct trailer_item *create_trailer_item(const char *string) +{ + struct strbuf tok = STRBUF_INIT; + struct strbuf val = STRBUF_INIT; + struct trailer_item *item; + int tok_alnum_len; + + if (parse_trailer(tok, val, string)) + return NULL; + + tok_alnum_len = alnum_len(tok.buf, tok.len); + + /* Lookup if the token matches something in the config */ + for (item = first_conf_item; item; item = item-next) { + if (token_matches_item(tok.buf, item, tok_alnum_len)) { + strbuf_release(tok); + return new_trailer_item(item, + NULL, + strbuf_detach(val, NULL)); + } + } + + return new_trailer_item(NULL, + strbuf_detach(tok, NULL), + strbuf_detach(val, NULL)); +} + +static void add_trailer_item(struct trailer_item **first, +struct trailer_item **last, +struct trailer_item *new) +{ + if (!new) + return; + if (!*last) { + *first = new; + *last = new; + } else { + (*last)-next = new; + new-previous = *last; + *last = new; + } +} + +static struct trailer_item *process_command_line_args(struct string_list *trailers) +{ + struct trailer_item *arg_tok_first = NULL; + struct trailer_item *arg_tok_last = NULL; + struct string_list_item *tr; + + for_each_string_list_item(tr, trailers) { + struct trailer_item *new = create_trailer_item(tr-string); + add_trailer_item(arg_tok_first, arg_tok_last, new); + } + + return arg_tok_first; +} -- 2.0.1.674.ga7f57b7 -- 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 v13 01/11] trailer: add data structures and basic functions
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 Signed-off-by: Junio C Hamano gits...@pobox.com --- Makefile | 1 + trailer.c | 64 +++ 2 files changed, 65 insertions(+) create mode 100644 trailer.c diff --git a/Makefile b/Makefile index 63a210d..ef82972 100644 --- a/Makefile +++ b/Makefile @@ -888,6 +888,7 @@ LIB_OBJS += submodule-config.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..2adc1b7 --- /dev/null +++ b/trailer.c @@ -0,0 +1,64 @@ +#include cache.h +/* + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + */ + +enum action_where { WHERE_END, WHERE_AFTER, WHERE_BEFORE, WHERE_START }; +enum action_if_exists { EXISTS_ADD_IF_DIFFERENT_NEIGHBOR, EXISTS_ADD_IF_DIFFERENT, + EXISTS_ADD, EXISTS_REPLACE, EXISTS_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_exists if_exists; + enum action_if_missing if_missing; +}; + +static struct conf_info default_conf_info; + +struct trailer_item { + struct trailer_item *previous; + struct trailer_item *next; + const char *token; + const char *value; + struct conf_info conf; +}; + +static struct trailer_item *first_conf_item; + +static char *separators = :; + +static int after_or_end(enum action_where where) +{ + return (where == WHERE_AFTER) || (where == WHERE_END); +} + +/* 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 - 1])) + len--; + return len; +} + +static int same_token(struct trailer_item *a, struct trailer_item *b) +{ + size_t a_alnum_len = alnum_len(a-token, strlen(a-token)); + size_t b_alnum_len = alnum_len(b-token, strlen(b-token)); + size_t min_alnum_len = (a_alnum_len b_alnum_len) ? b_alnum_len : a_alnum_len; + + return !strncasecmp(a-token, b-token, min_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) +{ + return same_token(a, b) same_value(a, b); +} -- 2.0.1.674.ga7f57b7 -- 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 v13 02/11] trailer: process trailers from input message and arguments
Implement the logic to process trailers from the input message and from arguments. At the beginning trailers from the input message are in their own in_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 in_tok list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 210 ++ 1 file changed, 210 insertions(+) diff --git a/trailer.c b/trailer.c index 2adc1b7..4940e06 100644 --- a/trailer.c +++ b/trailer.c @@ -62,3 +62,213 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b) { return same_token(a, b) same_value(a, b); } + +static void free_trailer_item(struct trailer_item *item) +{ + free(item-conf.name); + free(item-conf.key); + free(item-conf.command); + free((char *)item-token); + free((char *)item-value); + free(item); +} + +static void update_last(struct trailer_item **last) +{ + if (*last) + while ((*last)-next != NULL) + *last = (*last)-next; +} + +static void update_first(struct trailer_item **first) +{ + if (*first) + while ((*first)-previous != NULL) + *first = (*first)-previous; +} + +static void add_arg_to_input_list(struct trailer_item *on_tok, + struct trailer_item *arg_tok, + struct trailer_item **first, + struct trailer_item **last) +{ + if (after_or_end(arg_tok-conf.where)) { + arg_tok-next = on_tok-next; + on_tok-next = arg_tok; + arg_tok-previous = on_tok; + if (arg_tok-next) + arg_tok-next-previous = arg_tok; + update_last(last); + } else { + arg_tok-previous = on_tok-previous; + on_tok-previous = arg_tok; + arg_tok-next = on_tok; + if (arg_tok-previous) + arg_tok-previous-next = arg_tok; + update_first(first); + } +} + +static int check_if_different(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + int check_all) +{ + enum action_where where = arg_tok-conf.where; + do { + if (!in_tok) + return 1; + if (same_trailer(in_tok, arg_tok)) + return 0; + /* +* if we want to add a trailer after another one, +* we have to check those before this one +*/ + in_tok = after_or_end(where) ? in_tok-previous : in_tok-next; + } while (check_all); + return 1; +} + +static void remove_from_list(struct trailer_item *item, +struct trailer_item **first, +struct trailer_item **last) +{ + struct trailer_item *next = item-next; + struct trailer_item *previous = item-previous; + + if (next) { + item-next-previous = previous; + item-next = NULL; + } else if (last) + *last = previous; + + if (previous) { + item-previous-next = next; + item-previous = NULL; + } else if (first) + *first = 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 apply_arg_if_exists(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + struct trailer_item *on_tok, + struct trailer_item **in_tok_first, + struct trailer_item **in_tok_last) +{ + switch (arg_tok-conf.if_exists) { + case EXISTS_DO_NOTHING: + free_trailer_item(arg_tok); + break; + case EXISTS_REPLACE: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + remove_from_list(in_tok, in_tok_first, in_tok_last); + free_trailer_item(in_tok); + break; + case EXISTS_ADD: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + break; + case EXISTS_ADD_IF_DIFFERENT: + if (check_if_different(in_tok, arg_tok, 1)) + add_arg_to_input_list(on_tok, arg_tok
[PATCH v13 06/11] 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 81 +-- trailer.h | 6 + 2 files changed, 75 insertions(+), 12 deletions(-) create mode 100644 trailer.h diff --git a/trailer.c b/trailer.c index 46e2fcb..8e41b8f 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,6 @@ #include cache.h #include string-list.h +#include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -82,6 +83,35 @@ static void free_trailer_item(struct trailer_item *item) free(item); } +static char last_non_space_char(const char *s) +{ + int i; + for (i = strlen(s) - 1; i = 0; i--) + if (!isspace(s[i])) + return s[i]; + return '\0'; +} + +static void print_tok_val(const char *tok, const char *val) +{ + char c = last_non_space_char(tok); + if (!c) + return; + if (strchr(separators, c)) + printf(%s%s\n, tok, val); + else + printf(%s%c %s\n, tok, separators[0], 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 update_last(struct trailer_item **last) { if (*last) @@ -489,18 +519,6 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra return 0; } - -static void duplicate_conf(struct conf_info *dst, struct conf_info *src) -{ - *dst = *src; - if (src-name) - dst-name = xstrdup(src-name); - if (src-key) - dst-key = xstrdup(src-key); - if (src-command) - dst-command = xstrdup(src-command); -} - static const char *token_from_item(struct trailer_item *item) { if (item-conf.key) @@ -705,3 +723,42 @@ static int process_input_file(struct strbuf **lines, return patch_start; } + +static void free_all(struct trailer_item **first) +{ + while (*first) { + struct trailer_item *item = remove_first(first); + free_trailer_item(item); + } +} + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers) +{ + struct trailer_item *in_tok_first = NULL; + struct trailer_item *in_tok_last = NULL; + struct trailer_item *arg_tok_first; + struct strbuf **lines; + int patch_start; + + /* Default config must be setup first */ + git_config(git_trailer_default_config, NULL); + git_config(git_trailer_config, NULL); + + lines = read_input_file(file); + + /* Print the lines before the trailers */ + patch_start = process_input_file(lines, in_tok_first, in_tok_last); + + arg_tok_first = process_command_line_args(trailers); + + process_trailers_lists(in_tok_first, in_tok_last, arg_tok_first); + + print_all(in_tok_first, trim_empty); + + free_all(in_tok_first); + + /* Print the lines after the trailers as is */ + print_lines(lines, patch_start, INT_MAX); + + strbuf_list_free(lines); +} diff --git a/trailer.h b/trailer.h new file mode 100644 index 000..8eb25d5 --- /dev/null +++ b/trailer.h @@ -0,0 +1,6 @@ +#ifndef TRAILER_H +#define TRAILER_H + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers); + +#endif /* TRAILER_H */ -- 2.0.1.674.ga7f57b7 -- 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 v13 08/11] trailer: add tests for git interpret-trailers
Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 726 ++ 1 file changed, 726 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..fa37565 --- /dev/null +++ b/t/t7513-interpret-trailers.sh @@ -0,0 +1,726 @@ +#!/bin/sh +# +# Copyright (c) 2013, 2014 Christian Couder +# + +test_description='git interpret-trailers' + +. ./test-lib.sh + +# When 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. + +test_expect_success 'setup' ' + : empty + 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 + sed -e s/ Z\$/ / complex_message_trailers -\EOF + Fixes: Z + Acked-by: Z + Reviewed-by: Z + Signed-off-by: Z + EOF + cat basic_patch -\EOF + --- +foo.txt | 2 +- +1 file changed, 1 insertion(+), 1 deletion(-) + + diff --git a/foo.txt b/foo.txt + index 0353767..1d91aa1 100644 + --- a/foo.txt + +++ b/foo.txt + @@ -1,3 +1,3 @@ + + -bar + +baz + + -- + 1.9.rc0.11.ga562ddc + + EOF +' + +test_expect_success 'without config' ' + sed -e s/ Z\$/ / expected -\EOF + + ack: Peff + Reviewed-by: Z + Acked-by: Johan + EOF + git interpret-trailers --trailer ack = Peff --trailer Reviewed-by \ + --trailer Acked-by: Johan empty actual + test_cmp expected actual +' + +test_expect_success 'without config in another order' ' + sed -e s/ Z\$/ / expected -\EOF + + Acked-by: Johan + Reviewed-by: Z + ack: Peff + EOF + git interpret-trailers --trailer Acked-by: Johan --trailer Reviewed-by \ + --trailer ack = Peff empty actual + test_cmp expected actual +' + +test_expect_success '--trim-empty without config' ' + cat expected -\EOF + + ack: Peff + Acked-by: Johan + EOF + git interpret-trailers --trim-empty --trailer ack=Peff \ + --trailer Reviewed-by --trailer Acked-by: Johan \ + --trailer sob: empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup' ' + git config trailer.ack.key Acked-by: + cat expected -\EOF + + Acked-by: Peff + EOF + git interpret-trailers --trim-empty --trailer ack = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by :Peff empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup and := as separators' ' + git config trailer.separators := + git config trailer.ack.key Acked-by= + cat expected -\EOF + + Acked-by= Peff + EOF + git interpret-trailers --trim-empty --trailer ack = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by= Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by : Peff empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup and % as separators' ' + git config trailer.separators % + cat expected -\EOF + + bug% 42 + count% 10 + bug% 422 + EOF + git interpret-trailers --trim-empty --trailer bug = 42 \ + --trailer count%10 --trailer test: stuff \ + --trailer bug % 422 empty actual + test_cmp expected actual +' + +test_expect_success 'with % as separators and a message with trailers' ' + cat special_message -\EOF + Special Message + + bug% 42 + count% 10 + bug% 422 + EOF + cat expected -\EOF + Special Message + + bug% 42 + count% 10 + bug% 422 + count% 100 + EOF + git interpret-trailers --trailer count%100 \ + special_message actual + test_cmp expected actual +' + +test_expect_success 'with config setup and :=# as separators' ' + git config trailer.separators :=# + git
[PATCH v13 10/11] trailer: add tests for commands in config file
And add a few other tests for some special cases. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 124 ++ 1 file changed, 124 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index fa37565..562c00a 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -723,4 +723,128 @@ test_expect_success 'default where is now after' ' 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \A U Thor aut...@example.com\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=22 \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using commiter information' ' + git config trailer.sign.ifExists addIfDifferent + git config trailer.sign.command echo \\$GIT_COMMITTER_NAME \$GIT_COMMITTER_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: C O Mitter commit...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \\$GIT_AUTHOR_NAME \$GIT_AUTHOR_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists 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 + sed -e s/ Z\$/ / expected -EOF + Fixes: $FIXED + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with failing command using $ARG' ' + git config trailer.fix.ifExists overwrite + git config trailer.fix.command false \$ARG + cat complex_message_body expected + sed -e s/ Z\$/ / expected -EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with empty tokens' ' + cat expected -EOF + + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer : --trailer :test actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with command but no key' ' + git config --unset trailer.sign.key + cat expected -EOF + + sign: A U Thor aut...@example.com + EOF + git interpret-trailers actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with no command and no key' ' + git config --unset trailer.review.key + cat expected -EOF + + review: Junio + sign: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review:Junio actual -EOF + EOF + test_cmp expected actual +' + test_done -- 2.0.1.674
[PATCH v13 11/11] Documentation: add documentation for 'git interpret-trailers'
While at it add git-interpret-trailers to command-list.txt. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-interpret-trailers.txt | 308 +++ command-list.txt | 1 + 2 files changed, 309 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt new file mode 100644 index 000..cf5b194 --- /dev/null +++ b/Documentation/git-interpret-trailers.txt @@ -0,0 +1,308 @@ +git-interpret-trailers(1) += + +NAME + +git-interpret-trailers - help add stuctured information into commit messages + +SYNOPSIS + +[verse] +'git interpret-trailers' [--trim-empty] [(--trailer token[(=|:)value])...] [file...] + +DESCRIPTION +--- +Help adding 'trailers' lines, that look similar to RFC 822 e-mail +headers, at the end of the otherwise free-form part of a commit +message. + +This command reads some patches or commit messages from either the +file arguments or the standard input if no file is specified. Then +this command applies the arguments passed using the `--trailer` +option, if any, to the commit message part of each input file. The +result is emitted on the standard output. + +Some configuration variables control the way the `--trailer` arguments +are applied to each commit message and the way any existing trailer in +the commit message is changed. They also make it possible to +automatically add some trailers. + +By default, a 'token=value' or 'token:value' argument given +using `--trailer` will be appended after the existing trailers only if +the last trailer has a different (token, value) pair (or if there +is no existing trailer). The token and value parts will be trimmed +to remove starting and trailing whitespace, and the resulting trimmed +token and value will appear in the message like this: + + +token: value + + +This means that the trimmed token and value will be separated by +`': '` (one colon followed by one space). + +By default the new trailer will appear at the end of all the existing +trailers. If there is no existing trailer, the new trailer will appear +after the commit message part of the ouput, and, if there is no line +with only spaces at the end of the commit message part, one blank line +will be added before the new trailer. + +The trailers are recognized in the input message using the following +rules: + +* by default only lines that contains a ':' (colon) are considered + trailers, + +* the trailer lines must all be next to each other, + +* after them it's only possible to have some lines that contain only + spaces, and then a patch; the patch part is recognized using the + fact that its first line starts with '---' (three minus signs), + +* before them there must be at least one line with only spaces. + +Note that 'trailers' do not follow and are not intended to follow many +rules for RFC 822 headers. For example they do not follow the line +folding rules, the encoding rules and probably many other rules. + +OPTIONS +--- +--trim-empty:: + If the value part of any trailer contains only whitespace, + the whole trailer will be removed from the resulting message. + This apply to existing trailers as well as new trailers. + +--trailer token[(=|:)value]:: + Specify a (token, value) pair that should be applied as a + trailer to the input messages. See the description of this + command. + +CONFIGURATION VARIABLES +--- + +trailer.separators:: + This option tells which characters are recognized as trailer + separators. By default only ':' is recognized as a trailer + separator, except that '=' is always accepted on the command + line for compatibility with other git commands. ++ +The first character given by this option will be the default character +used when another separator is not specified in the config for this +trailer. ++ +For example, if the value for this option is %=$, then only lines +using the format 'tokensepvalue' with sep containing '%', '=' +or '$' and then spaces will be considered trailers. And '%' will be +the default separator used, so by default trailers will appear like: +'token% value' (one percent sign and one space will appear between +the token and the value). + +trailer.where:: + This option tells where a new trailer will be added. ++ +This can be `end`, which is the default, `start`, `after` or `before`. ++ +If it is `end`, then each new trailer will appear at the end of the +existing trailers. ++ +If it is `start`, then each new trailer will appear at the start, +instead of the end, of the existing trailers. ++ +If it is `after`, then each new trailer will appear just after the +last trailer with the same
[PATCH v13 07/11] 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- .gitignore | 1 + Makefile | 1 + builtin.h| 1 + builtin/interpret-trailers.c | 44 git.c| 1 + 5 files changed, 48 insertions(+) create mode 100644 builtin/interpret-trailers.c diff --git a/.gitignore b/.gitignore index cf0d191..85593de 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 ef82972..0b06ae0 100644 --- a/Makefile +++ b/Makefile @@ -953,6 +953,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 5d91f31..b87df70 100644 --- a/builtin.h +++ b/builtin.h @@ -73,6 +73,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..46838d2 --- /dev/null +++ b/builtin/interpret-trailers.c @@ -0,0 +1,44 @@ +/* + * Builtin git interpret-trailers + * + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + * + */ + +#include cache.h +#include builtin.h +#include parse-options.h +#include string-list.h +#include trailer.h + +static const char * const git_interpret_trailers_usage[] = { + N_(git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...]), + NULL +}; + +int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) +{ + int trim_empty = 0; + struct string_list trailers = STRING_LIST_INIT_DUP; + + struct option options[] = { + OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty trailers)), + OPT_STRING_LIST(0, trailer, trailers, N_(trailer), + N_(trailer(s) to add)), + OPT_END() + }; + + argc = parse_options(argc, argv, prefix, options, +git_interpret_trailers_usage, 0); + + if (argc) { + int i; + for (i = 0; i argc; i++) + process_trailers(argv[i], trim_empty, trailers); + } else + process_trailers(NULL, trim_empty, trailers); + + string_list_clear(trailers, 0); + + return 0; +} diff --git a/git.c b/git.c index 5ebb32f..e327a90 100644 --- a/git.c +++ b/git.c @@ -417,6 +417,7 @@ static struct cmd_struct commands[] = { { index-pack, cmd_index_pack, RUN_SETUP_GENTLY }, { init, cmd_init_db, NO_SETUP }, { init-db, cmd_init_db, NO_SETUP }, + { 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 }, -- 2.0.1.674.ga7f57b7 -- 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 v13 09/11] trailer: execute command from 'trailer.name.command'
Let the user specify a command that will give on its standard output the value to use for the specified trailer. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 66 +++ 1 file changed, 66 insertions(+) diff --git a/trailer.c b/trailer.c index 8e41b8f..baef034 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,7 @@ #include cache.h #include string-list.h +#include run-command.h +#include string-list.h #include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org @@ -14,6 +16,7 @@ struct conf_info { char *name; char *key; char *command; + unsigned command_uses_arg : 1; enum action_where where; enum action_if_exists if_exists; enum action_if_missing if_missing; @@ -33,6 +36,8 @@ static struct trailer_item *first_conf_item; static char *separators = :; +#define TRAILER_ARG_STRING $ARG + static int after_or_end(enum action_where where) { return (where == WHERE_AFTER) || (where == WHERE_END); @@ -73,6 +78,13 @@ static inline int contains_only_spaces(const char *str) return !*s; } +static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b) +{ + const char *ptr = strstr(sb-buf, a); + if (ptr) + strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b)); +} + static void free_trailer_item(struct trailer_item *item) { free(item-conf.name); @@ -478,6 +490,7 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb) if (conf-command) warning(_(more than one %s), conf_key); conf-command = xstrdup(value); + conf-command_uses_arg = !!strstr(conf-command, TRAILER_ARG_STRING); break; case TRAILER_WHERE: if (set_where(conf, value)) @@ -519,6 +532,46 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra return 0; } +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); + result = xstrdup(); + } else + result = strbuf_detach(buf, NULL); + + strbuf_release(cmd); + return result; +} + static const char *token_from_item(struct trailer_item *item) { if (item-conf.key) @@ -537,6 +590,10 @@ static struct trailer_item *new_trailer_item(struct trailer_item *conf_item, duplicate_conf(new-conf, conf_item-conf); new-token = xstrdup(token_from_item(conf_item)); free(tok); + if (conf_item-conf.command_uses_arg || !val) { + new-value = apply_command(conf_item-conf.command, val); + free(val); + } } else { duplicate_conf(new-conf, default_conf_info); new-token = tok; @@ -600,12 +657,21 @@ static struct trailer_item *process_command_line_args(struct string_list *traile struct trailer_item *arg_tok_first = NULL; struct trailer_item *arg_tok_last = NULL; struct string_list_item *tr; + struct trailer_item *item; for_each_string_list_item(tr, trailers) { struct trailer_item *new = create_trailer_item(tr-string); 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; } -- 2.0.1.674.ga7f57b7 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo
Re: [PATCH v13 00/11] Add interpret-trailers builtin
On Sat, Aug 16, 2014 at 6:06 PM, Christian Couder chrisc...@tuxfamily.org wrote: 3) Changes since version 12, thanks to Jakub, Michael, Johan and Junio: * end and start values for trailer.token.where have been implemented * end has been made the default value for where, but this default value can be changed using the new trailer.where config variable * addIfDifferentNeighbor is now the default value for ifexists, but this default can be changed using the new trailer.ifexists config variable * the new trailer.ifmissing can be used to change the default value for ifmissing (which is add) * by default the only separator is ':', this can be changed by using the new trailer.separators config variable * only the configured separators (or just ':' by default) are used for both input parsing and output printing; the only exception is that '=' is always accepted as separator when parsing --trailer 'tokensepvalue' command line arguments, for compatibility with other git commands * 14 tests have been added I forgot the following change: * overwrite value for ifexists option has been replaced with replace value; when used with where = after or where = before, replace does the same thing as overwrite did; but when used with where = end or where = start, the new trailer appears at the end or the start of the existing trailers, instead of where the replaced trailer was -- 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 v13 11/11] Documentation: add documentation for 'git interpret-trailers'
On Thu, Aug 21, 2014 at 12:05 AM, Marc Branchaud marcn...@xiplink.com wrote: On 14-08-16 12:06 PM, Christian Couder wrote: +The trailers are recognized in the input message using the following +rules: + +* by default only lines that contains a ':' (colon) are considered s/contains/contain/ Ok. + trailers, + +* the trailer lines must all be next to each other, + +* after them it's only possible to have some lines that contain only + spaces, and then a patch; the patch part is recognized using the + fact that its first line starts with '---' (three minus signs), Is that starts with or consists solely of? It is starts with. (The starts_with() function is used.) + +* before them there must be at least one line with only spaces. I had little bit of trouble parsing those three points, and it seems like a lot of text to describe something simple. How about a single paragraph: Existing trailers are extracted from the input message by looking for a group of one or more lines that contain a colon (by default), where the group is preceded by one or more empty (or whitespace-only) lines. The group must either be at the end of the message or be the last non-whitespace lines before a line that starts with '---' (three minus signs). Ok, I will use something like that, thanks. Also, will a trailer be recognized if there is whitespace before and/or after the separator? Yes. Can there be whitespace before the token? Yes. In the token? Yes. (I don't feel strongly about the answers to these questions, they just came to mind as I read the documentation.) Ok, I will add something. +Note that 'trailers' do not follow and are not intended to follow many +rules for RFC 822 headers. For example they do not follow the line +folding rules, the encoding rules and probably many other rules. + +OPTIONS +--- +--trim-empty:: + If the value part of any trailer contains only whitespace, + the whole trailer will be removed from the resulting message. + This apply to existing trailers as well as new trailers. + +--trailer token[(=|:)value]:: + Specify a (token, value) pair that should be applied as a + trailer to the input messages. See the description of this + command. + +CONFIGURATION VARIABLES +--- + +trailer.separators:: + This option tells which characters are recognized as trailer + separators. By default only ':' is recognized as a trailer + separator, except that '=' is always accepted on the command + line for compatibility with other git commands. ++ +The first character given by this option will be the default character +used when another separator is not specified in the config for this +trailer. ++ +For example, if the value for this option is %=$, then only lines +using the format 'tokensepvalue' with sep containing '%', '=' +or '$' and then spaces will be considered trailers. And '%' will be +the default separator used, so by default trailers will appear like: +'token% value' (one percent sign and one space will appear between +the token and the value). + +trailer.where:: + This option tells where a new trailer will be added. ++ +This can be `end`, which is the default, `start`, `after` or `before`. ++ +If it is `end`, then each new trailer will appear at the end of the +existing trailers. ++ +If it is `start`, then each new trailer will appear at the start, +instead of the end, of the existing trailers. ++ +If it is `after`, then each new trailer will appear just after the +last trailer with the same token. ++ +If it is `before`, then each new trailer will appear just before the +last trailer with the same token. It seems to me that it would be more sensible to make the new trailer appear before the *first* trailer with the same token. Yeah, it is a copy-paste mistake that I will fix. Thanks for the careful review, 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] bisect: save heap memory. allocate only the required amount
On Mon, Aug 25, 2014 at 3:35 PM, Jeff King p...@peff.net wrote: On Sun, Aug 24, 2014 at 07:47:24PM +0530, Arjun Sreedharan wrote: diff --git a/bisect.c b/bisect.c index d6e851d..c96aab0 100644 --- a/bisect.c +++ b/bisect.c @@ -215,10 +215,13 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n } qsort(array, cnt, sizeof(*array), compare_commit_dist); for (p = list, i = 0; i cnt; i++) { - struct name_decoration *r = xmalloc(sizeof(*r) + 100); + char name[100]; + sprintf(name, dist=%d, array[i].distance); + int name_len = strlen(name); + struct name_decoration *r = xmalloc(sizeof(*r) + name_len); This allocation should be name_len + 1 for the NUL-terminator, no? I wondered about that too, but as struct name_decoration is defined like this: struct name_decoration { struct name_decoration *next; int type; char name[1]; }; the .name field of this struct already has one char, so the allocation above should be ok. It looks like add_name_decoration in log-tree already handles half of what you are adding here. Can we just make that available globally (it is manipulating the already-global struct decoration name_decoration)? Yeah, it looks like it should be better. Note that add_name_decoration() does: int nlen = strlen(name); struct name_decoration *res = xmalloc(sizeof(struct name_decoration) + nlen); so it also relies on the fact that .name contains one char. I also notice that we do not set r-type at all, meaning the decoration lookup code in log-tree will access uninitialized memory (worse, it will use it as a pointer offset into the color list; I got a segfault when I tried to run git rev-list --bisect-all v1.8.0..v1.9.0). I think we need this: diff --git a/bisect.c b/bisect.c index d6e851d..e2a7682 100644 --- a/bisect.c +++ b/bisect.c @@ -219,6 +219,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n struct object *obj = (array[i].commit-object); sprintf(r-name, dist=%d, array[i].distance); + r-type = 0; r-next = add_decoration(name_decoration, obj, r); p-item = array[i].commit; p = p-next; at a minimum. Yeah if we don't use add_name_decoration() we would need that. Thanks for noticing. It looks like this was a regression caused by eb3005e (commit.h: add 'type' to struct name_decoration, 2010-06-19). Which makes me wonder if anybody actually _uses_ --bisect-all (which AFAICT is the only way to trigger the problem), but since it's public, I guess we should keep it. Yeah, we should probably keep it. I think the sane thing here is to stop advertising name_decoration as a global, and make all callers use add_name_decoration. That makes it easier for callers like this one, and would have caught the regression caused be eb3005e (the compiler would have noticed that we were not passing a type parameter to the function). I agree. Thanks, 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 v2 0/6] clean up author parsing
On Wed, Aug 27, 2014 at 9:55 AM, Jeff King p...@peff.net wrote: [2/6]: record_author_info: fix memory leak on malformed commit [3/6]: record_author_info: use find_commit_header s/record_author_info/record_author_date/ Thanks, 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: Git Bug Report: bisect run failed to locate the right commit
Hi, On Wed, Aug 27, 2014 at 3:42 PM, 李祐棠 r01942...@ntu.edu.tw wrote: Dear Git developers: I just found a suspicious bug that might cause by git-bisect run. Version: The git version is 1.9.2, running on Archlinux 3.14.2 The step to produce the error: Here is the repository I participate: https://github.com/gawel/pyquery It got a issue #74, that gives a wrong result in version 1.2.8 and this is fixed in 1.2.9 Here is the manual test script I use is manualscript.py: I use git bisect manually, search from 1.2.9(bad) to 1.2.8(good), I locate the commit that fixes this issue. The running log is attached in this file(manual). However if I use the automatic script git bisect run with the script auto script: It will give a wrong answer, the log file is also attached(auto) Your manual log contains: Bisecting: 0 revisions left to test after this (roughly 1 step) [300cd0822505a4bd308acd1520ff3ef0f20f8635] fixed issue #19 $ ./manualscript.py False False While your auto log contains: Bisecting: 0 revisions left to test after this (roughly 1 step) [300cd0822505a4bd308acd1520ff3ef0f20f8635] fixed issue #19 running ./autoscript.py False True So for the commit 300cd0822505a4bd308acd1520ff3ef0f20f8635 it looks like your manualscript.py and your autoscript.py don't return the same result. Could you check that by doing something like: $ git checkout -b testscripts 300cd0822505a4bd308acd1520ff3ef0f20f8635 $ ./manualscript.py $ echo manualscript.py exit code: $? $ ./autoscript.py $ echo autoscript.py exit code: $? ? By the way you can get a better bisect log by running git bisect log after your bisection. 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: Git Bug Report: bisect run failed to locate the right commit, detail testing
Hi, On Thu, Aug 28, 2014 at 3:49 AM, 李祐棠 r01942...@ntu.edu.tw wrote: Dear git developers: Allow me to describe the testing situation more detail: First the testing repository is in https://github.com/gawel/pyquery my git version is 1.9.2 running on Archlinux 3.14.2 I try to track issue #74(which is closed now) It give result False/True in version 1.2.8(good), and result False/False in version 1.2.9(bad). The testing script are manualscript.py and autoscript.py Both of them implement the test case describe in issue #74. Th only difference is that autoscript.py call sys.exit() to return the testing value. First we test with git-bisect manually with manualscript.py: The testing result is shown in 'bisectmanual_manualscript' Then we test with git-bisect manually with autoscript.py This time we echo $? every time we execute autoscript.py, and the testing result is shown in 'bisectmanual_autoscript' In both situation the script give the same result, and the return value of autoscript.py is correct, too. However, if we use git-bisect-run with autoscript.py, it will show a different result. The testing result is shown in 'bisectrun'. The log shows that autoscript.py output False/False all the way. As Mr. Couder said, there is some checkout commit that autoscript.py and manualscript.py give different result, for example commit d22159bb32510e9eacf6c5c2408a79792e99fe76. If I checkout this commit outside bisect state and run manualscript.py and autoscript.py, they both give False/True result. So, I guess there is some problem in the checkout procedure in bisect-run, so the commit didn't successfully checkout. I don't think so. I tried to run the following command many times in a row: for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done And here is the log: $ for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done Note: checking out 'bc1b16509cec70de7a32354026443fca777f4d7d'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_name HEAD is now at bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) False False Previous HEAD position was bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) HEAD is now at b81a9e8... fixed issue #9 False False Previous HEAD position was b81a9e8... fixed issue #9 HEAD is now at d22159b... test pseudo classes; documentation effort False True $ for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done Previous HEAD position was d22159b... test pseudo classes; documentation effort HEAD is now at bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) False False Previous HEAD position was bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) HEAD is now at b81a9e8... fixed issue #9 False False Previous HEAD position was b81a9e8... fixed issue #9 HEAD is now at d22159b... test pseudo classes; documentation effort False True $ for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done Previous HEAD position was d22159b... test pseudo classes; documentation effort HEAD is now at bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) False False Previous HEAD position was bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) HEAD is now at b81a9e8... fixed issue #9 False False Previous HEAD position was b81a9e8... fixed issue #9 HEAD is now at d22159b... test pseudo classes; documentation effort False False $ for rev in bc1b16509cec70de7a32354026443fca777f4d7d b81a9e8a2b0d48ec0c64d6de14293dd4a680a20b d22159bb32510e9eacf6c5c2408a79792e99fe76; do git checkout $rev; python -m pyquery.autoscript; done Previous HEAD position was d22159b... test pseudo classes; documentation effort HEAD is now at bc1b165... created a .gitignore file(which is almost a copy of .hgignore with some minor changes and comments) False False Previous HEAD position was
Re: Next Git conference or meeting
Hi Shawn and Peff, On Wed, Sep 3, 2014 at 10:59 PM, Jeff King p...@peff.net wrote: On Wed, Sep 03, 2014 at 10:15:15AM -0700, Shawn Pearce wrote: I hadn't realized Git is turning 10 next year. Just been too busy using Git to pay attention to its upcoming anniversary. Let me talk to some folks at Google and see if we can organize something here in Mountain View, or help the LinuxFoundation sponsor something. Did you talk to some folks? Christian mentioned that he talked to some GitHub folks at LinuxCon. Those folks have also started thinking about things. :) Things are still very tentative at this point, but I think they are considering something like the Git Merge conference we did earlier, and doing it in June in Europe (maybe Paris). I know they were going to reach out to Linux Foundation folks to try to jointly plan something, but I don't know if that has happened yet. Could you ask if they talked to Linux Foundation folks? It would be very easy for me to come if it was in Paris, but I'd rather make it easy for Junio, Linus, you guys, and other contributors, unless everyone wants to take advantage of the conference to visit Paris :-) So it seems there are a lot of different people who are all potentially interested in planning or taking part, and they should all be talking to each other. :) Maybe we should have a mailing list for that. Thanks, 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: Next Git conference or meeting
On Wed, Sep 10, 2014 at 10:14 PM, Jeff King p...@peff.net wrote: On Tue, Sep 09, 2014 at 05:49:06PM +0200, Christian Couder wrote: Could you ask if they talked to Linux Foundation folks? I've just asked; I'll let you know if I hear. Thanks. I've seen LF folks mentioned a few times in this thread. Are there specific people (with email addresses) that I should be pointing them at? I just sent Ted, Shawn and you the LF guy's contact information. Feel free to contact him. Thanks, 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
[PATCH v14 08/11] trailer: add tests for git interpret-trailers
Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 726 ++ 1 file changed, 726 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..fa37565 --- /dev/null +++ b/t/t7513-interpret-trailers.sh @@ -0,0 +1,726 @@ +#!/bin/sh +# +# Copyright (c) 2013, 2014 Christian Couder +# + +test_description='git interpret-trailers' + +. ./test-lib.sh + +# When 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. + +test_expect_success 'setup' ' + : empty + 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 + sed -e s/ Z\$/ / complex_message_trailers -\EOF + Fixes: Z + Acked-by: Z + Reviewed-by: Z + Signed-off-by: Z + EOF + cat basic_patch -\EOF + --- +foo.txt | 2 +- +1 file changed, 1 insertion(+), 1 deletion(-) + + diff --git a/foo.txt b/foo.txt + index 0353767..1d91aa1 100644 + --- a/foo.txt + +++ b/foo.txt + @@ -1,3 +1,3 @@ + + -bar + +baz + + -- + 1.9.rc0.11.ga562ddc + + EOF +' + +test_expect_success 'without config' ' + sed -e s/ Z\$/ / expected -\EOF + + ack: Peff + Reviewed-by: Z + Acked-by: Johan + EOF + git interpret-trailers --trailer ack = Peff --trailer Reviewed-by \ + --trailer Acked-by: Johan empty actual + test_cmp expected actual +' + +test_expect_success 'without config in another order' ' + sed -e s/ Z\$/ / expected -\EOF + + Acked-by: Johan + Reviewed-by: Z + ack: Peff + EOF + git interpret-trailers --trailer Acked-by: Johan --trailer Reviewed-by \ + --trailer ack = Peff empty actual + test_cmp expected actual +' + +test_expect_success '--trim-empty without config' ' + cat expected -\EOF + + ack: Peff + Acked-by: Johan + EOF + git interpret-trailers --trim-empty --trailer ack=Peff \ + --trailer Reviewed-by --trailer Acked-by: Johan \ + --trailer sob: empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup' ' + git config trailer.ack.key Acked-by: + cat expected -\EOF + + Acked-by: Peff + EOF + git interpret-trailers --trim-empty --trailer ack = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by :Peff empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup and := as separators' ' + git config trailer.separators := + git config trailer.ack.key Acked-by= + cat expected -\EOF + + Acked-by= Peff + EOF + git interpret-trailers --trim-empty --trailer ack = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by= Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by : Peff empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup and % as separators' ' + git config trailer.separators % + cat expected -\EOF + + bug% 42 + count% 10 + bug% 422 + EOF + git interpret-trailers --trim-empty --trailer bug = 42 \ + --trailer count%10 --trailer test: stuff \ + --trailer bug % 422 empty actual + test_cmp expected actual +' + +test_expect_success 'with % as separators and a message with trailers' ' + cat special_message -\EOF + Special Message + + bug% 42 + count% 10 + bug% 422 + EOF + cat expected -\EOF + Special Message + + bug% 42 + count% 10 + bug% 422 + count% 100 + EOF + git interpret-trailers --trailer count%100 \ + special_message actual + test_cmp expected actual +' + +test_expect_success 'with config setup and :=# as separators' ' + git config trailer.separators :=# + git
[PATCH v14 10/11] trailer: add tests for commands in config file
And add a few other tests for some special cases. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 125 ++ 1 file changed, 125 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index fa37565..c5b86ff 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -723,4 +723,129 @@ test_expect_success 'default where is now after' ' 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \A U Thor aut...@example.com\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=22 \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using commiter information' ' + git config trailer.sign.ifExists addIfDifferent + git config trailer.sign.command echo \\$GIT_COMMITTER_NAME \$GIT_COMMITTER_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: C O Mitter commit...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \\$GIT_AUTHOR_NAME \$GIT_AUTHOR_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists replace + 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 + sed -e s/ Z\$/ / expected -EOF + Fixes: $FIXED + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with failing command using $ARG' ' + git config trailer.fix.ifExists replace + git config trailer.fix.command false \$ARG + cat complex_message_body expected + sed -e s/ Z\$/ / expected -EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with empty tokens' ' + git config --unset trailer.fix.command + cat expected -EOF + + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer : --trailer :test actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with command but no key' ' + git config --unset trailer.sign.key + cat expected -EOF + + sign: A U Thor aut...@example.com + EOF + git interpret-trailers actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with no command and no key' ' + git config --unset trailer.review.key + cat expected -EOF + + review: Junio + sign: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review:Junio actual -EOF + EOF + test_cmp expected
[PATCH v14 07/11] 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- .gitignore | 1 + Makefile | 1 + builtin.h| 1 + builtin/interpret-trailers.c | 44 git.c| 1 + 5 files changed, 48 insertions(+) create mode 100644 builtin/interpret-trailers.c diff --git a/.gitignore b/.gitignore index cf0d191..85593de 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 ef82972..0b06ae0 100644 --- a/Makefile +++ b/Makefile @@ -953,6 +953,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 5d91f31..b87df70 100644 --- a/builtin.h +++ b/builtin.h @@ -73,6 +73,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..46838d2 --- /dev/null +++ b/builtin/interpret-trailers.c @@ -0,0 +1,44 @@ +/* + * Builtin git interpret-trailers + * + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + * + */ + +#include cache.h +#include builtin.h +#include parse-options.h +#include string-list.h +#include trailer.h + +static const char * const git_interpret_trailers_usage[] = { + N_(git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...]), + NULL +}; + +int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) +{ + int trim_empty = 0; + struct string_list trailers = STRING_LIST_INIT_DUP; + + struct option options[] = { + OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty trailers)), + OPT_STRING_LIST(0, trailer, trailers, N_(trailer), + N_(trailer(s) to add)), + OPT_END() + }; + + argc = parse_options(argc, argv, prefix, options, +git_interpret_trailers_usage, 0); + + if (argc) { + int i; + for (i = 0; i argc; i++) + process_trailers(argv[i], trim_empty, trailers); + } else + process_trailers(NULL, trim_empty, trailers); + + string_list_clear(trailers, 0); + + return 0; +} diff --git a/git.c b/git.c index 5ebb32f..e327a90 100644 --- a/git.c +++ b/git.c @@ -417,6 +417,7 @@ static struct cmd_struct commands[] = { { index-pack, cmd_index_pack, RUN_SETUP_GENTLY }, { init, cmd_init_db, NO_SETUP }, { init-db, cmd_init_db, NO_SETUP }, + { 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 }, -- 2.0.3.960.g41c6e4c -- 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 v14 00/11] 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 keep builtin/commit.c uncontaminated by any more hard-wired logic, like what we have for the signed-off-by line. Any new things can and should be doable in hooks, and this filter would help writing these hooks. And that is why the design goal of the filter is to make it at least as powerful as the built-in logic we have for signed-off-by lines; that would allow us to later eject the hard-wired logic for signed-off-by line from the main codepath, if/when we wanted to. Alternatively, we could build a library-ish API around this filter code and replace the hard-wired logic for signed-off-by line with a call into that API, if/when we wanted to, but that requires (in addition to the at least as powerful as the built-in logic) that the implementation of this stand-alone filter can be cleanly made into a reusable library, so that is a bit higher bar to cross than everything can be doable with hooks alternative. 2) Current state Currently the usage string of this command is: git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...] The following features are implemented: - the result is printed on stdout - the --trailer arguments are interpreted - messages read from file... or stdin are interpreted - the trailer.separators option in the config is interpreted - the trailer.where option is interpreted - the trailer.ifexists option is interpreted - the trailer.ifmissing option is interpreted - the trailer.token.key options are interpreted - the trailer.token.where options are interpreted - the trailer.token.ifexist options are interpreted - the trailer.token.ifmissing options are interpreted - the trailer.token.command config works - $ARG can be used in commands - messages can contain a patch - lines in messages starting with a comment char are ignored - there are 49 tests - there is some documentation - there are examples in the documentation 3) Changes since version 13, thanks to Marc and Junio * improved documentation (patch 11/11) * rework the way trailer.token.command are handled, so that it is simpler to understand and closer to what is described in the documentation (patches 9/11, 10/11 and 11/11) Christian Couder (11): trailer: add data structures and basic functions trailer: process trailers from input message and arguments trailer: read and process config information trailer: process command line trailer arguments trailer: parse trailers from file or stdin trailer: put all the processing together and print trailer: add interpret-trailers command trailer: add tests for git interpret-trailers trailer: execute command from 'trailer.name.command' trailer: add tests for commands in config file Documentation: add documentation for 'git interpret-trailers' .gitignore | 1 + Documentation/git-interpret-trailers.txt | 313 Makefile | 2 + builtin.h| 1 + builtin/interpret-trailers.c | 44 ++ command-list.txt | 1 + git.c| 1 + t/t7513-interpret-trailers.sh| 851 +++ trailer.c| 847 ++ trailer.h| 6 + 10 files changed, 2067 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt 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 -- 2.0.3.960.g41c6e4c -- 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 v14 05/11] trailer: parse trailers from file or stdin
Read trailers from a file or from stdin, parse the trailers and then put the result into a doubly linked list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 123 ++ 1 file changed, 123 insertions(+) diff --git a/trailer.c b/trailer.c index b9d3ed4..46e2fcb 100644 --- a/trailer.c +++ b/trailer.c @@ -64,6 +64,14 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b) return same_token(a, b) same_value(a, b); } +static inline int contains_only_spaces(const char *str) +{ + const char *s = str; + while (*s isspace(*s)) + s++; + return !*s; +} + static void free_trailer_item(struct trailer_item *item) { free(item-conf.name); @@ -582,3 +590,118 @@ static struct trailer_item *process_command_line_args(struct string_list *traile return arg_tok_first; } + +static struct strbuf **read_input_file(const char *file) +{ + struct strbuf **lines; + struct strbuf sb = STRBUF_INIT; + + if (file) { + if (strbuf_read_file(sb, file, 0) 0) + die_errno(_(could not read input file '%s'), file); + } else { + if (strbuf_read(sb, fileno(stdin), 0) 0) + die_errno(_(could not read from stdin)); + } + + lines = strbuf_split(sb, '\n'); + + strbuf_release(sb); + + return lines; +} + +/* + * Return the (0 based) index of the start of the patch or the line + * count if there is no patch in the message. + */ +static int find_patch_start(struct strbuf **lines, int count) +{ + int i; + + /* Get the start of the patch part if any */ + for (i = 0; i count; i++) { + if (starts_with(lines[i]-buf, ---)) + return i; + } + + return count; +} + +/* + * Return the (0 based) index of the first trailer line or count if + * there are no trailers. Trailers are searched only in the lines from + * index (count - 1) down to index 0. + */ +static int find_trailer_start(struct strbuf **lines, int count) +{ + int start, only_spaces = 1; + + /* +* Get the start of the trailers by looking starting from the end +* for a line with only spaces before lines with one separator. +*/ + for (start = count - 1; start = 0; start--) { + if (lines[start]-buf[0] == comment_line_char) + continue; + if (contains_only_spaces(lines[start]-buf)) { + if (only_spaces) + continue; + return start + 1; + } + if (strcspn(lines[start]-buf, separators) lines[start]-len) { + if (only_spaces) + only_spaces = 0; + continue; + } + return count; + } + + return only_spaces ? count : 0; +} + +static int has_blank_line_before(struct strbuf **lines, int start) +{ + for (;start = 0; start--) { + if (lines[start]-buf[0] == comment_line_char) + continue; + return contains_only_spaces(lines[start]-buf); + } + return 0; +} + +static void print_lines(struct strbuf **lines, int start, int end) +{ + int i; + for (i = start; lines[i] i end; i++) + printf(%s, lines[i]-buf); +} + +static int process_input_file(struct strbuf **lines, + struct trailer_item **in_tok_first, + struct trailer_item **in_tok_last) +{ + int count = 0; + int patch_start, trailer_start, i; + + /* Get the line count */ + while (lines[count]) + count++; + + patch_start = find_patch_start(lines, count); + trailer_start = find_trailer_start(lines, patch_start); + + /* Print lines before the trailers as is */ + print_lines(lines, 0, trailer_start); + + if (!has_blank_line_before(lines, trailer_start - 1)) + printf(\n); + + /* Parse trailer lines */ + for (i = trailer_start; i patch_start; i++) { + struct trailer_item *new = create_trailer_item(lines[i]-buf); + add_trailer_item(in_tok_first, in_tok_last, new); + } + + return patch_start; +} -- 2.0.3.960.g41c6e4c -- 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 v14 01/11] trailer: add data structures and basic functions
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 Signed-off-by: Junio C Hamano gits...@pobox.com --- Makefile | 1 + trailer.c | 64 +++ 2 files changed, 65 insertions(+) create mode 100644 trailer.c diff --git a/Makefile b/Makefile index 63a210d..ef82972 100644 --- a/Makefile +++ b/Makefile @@ -888,6 +888,7 @@ LIB_OBJS += submodule-config.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..2adc1b7 --- /dev/null +++ b/trailer.c @@ -0,0 +1,64 @@ +#include cache.h +/* + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + */ + +enum action_where { WHERE_END, WHERE_AFTER, WHERE_BEFORE, WHERE_START }; +enum action_if_exists { EXISTS_ADD_IF_DIFFERENT_NEIGHBOR, EXISTS_ADD_IF_DIFFERENT, + EXISTS_ADD, EXISTS_REPLACE, EXISTS_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_exists if_exists; + enum action_if_missing if_missing; +}; + +static struct conf_info default_conf_info; + +struct trailer_item { + struct trailer_item *previous; + struct trailer_item *next; + const char *token; + const char *value; + struct conf_info conf; +}; + +static struct trailer_item *first_conf_item; + +static char *separators = :; + +static int after_or_end(enum action_where where) +{ + return (where == WHERE_AFTER) || (where == WHERE_END); +} + +/* 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 - 1])) + len--; + return len; +} + +static int same_token(struct trailer_item *a, struct trailer_item *b) +{ + size_t a_alnum_len = alnum_len(a-token, strlen(a-token)); + size_t b_alnum_len = alnum_len(b-token, strlen(b-token)); + size_t min_alnum_len = (a_alnum_len b_alnum_len) ? b_alnum_len : a_alnum_len; + + return !strncasecmp(a-token, b-token, min_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) +{ + return same_token(a, b) same_value(a, b); +} -- 2.0.3.960.g41c6e4c -- 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 v14 04/11] trailer: process command line trailer arguments
Parse the trailer command line arguments and put the result into an arg_tok doubly linked list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 125 ++ 1 file changed, 125 insertions(+) diff --git a/trailer.c b/trailer.c index 2d391f3..b9d3ed4 100644 --- a/trailer.c +++ b/trailer.c @@ -1,4 +1,5 @@ #include cache.h +#include string-list.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -457,3 +458,127 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb) } return 0; } + +static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *trailer) +{ + size_t len; + struct strbuf seps = STRBUF_INIT; + strbuf_addstr(seps, separators); + strbuf_addch(seps, '='); + len = strcspn(trailer, seps.buf); + strbuf_release(seps); + if (len == 0) + return error(_(empty trailer token in trailer '%s'), trailer); + if (len strlen(trailer)) { + strbuf_add(tok, trailer, len); + strbuf_trim(tok); + strbuf_addstr(val, trailer + len + 1); + strbuf_trim(val); + } else { + strbuf_addstr(tok, trailer); + strbuf_trim(tok); + } + return 0; +} + + +static void duplicate_conf(struct conf_info *dst, struct conf_info *src) +{ + *dst = *src; + if (src-name) + dst-name = xstrdup(src-name); + if (src-key) + dst-key = xstrdup(src-key); + if (src-command) + dst-command = xstrdup(src-command); +} + +static const char *token_from_item(struct trailer_item *item) +{ + if (item-conf.key) + return item-conf.key; + + return item-conf.name; +} + +static struct trailer_item *new_trailer_item(struct trailer_item *conf_item, +char *tok, char *val) +{ + struct trailer_item *new = xcalloc(sizeof(*new), 1); + new-value = val; + + if (conf_item) { + duplicate_conf(new-conf, conf_item-conf); + new-token = xstrdup(token_from_item(conf_item)); + free(tok); + } else { + duplicate_conf(new-conf, default_conf_info); + new-token = tok; + } + + return new; +} + +static int token_matches_item(const char *tok, struct trailer_item *item, int alnum_len) +{ + if (!strncasecmp(tok, item-conf.name, alnum_len)) + return 1; + return item-conf.key ? !strncasecmp(tok, item-conf.key, alnum_len) : 0; +} + +static struct trailer_item *create_trailer_item(const char *string) +{ + struct strbuf tok = STRBUF_INIT; + struct strbuf val = STRBUF_INIT; + struct trailer_item *item; + int tok_alnum_len; + + if (parse_trailer(tok, val, string)) + return NULL; + + tok_alnum_len = alnum_len(tok.buf, tok.len); + + /* Lookup if the token matches something in the config */ + for (item = first_conf_item; item; item = item-next) { + if (token_matches_item(tok.buf, item, tok_alnum_len)) { + strbuf_release(tok); + return new_trailer_item(item, + NULL, + strbuf_detach(val, NULL)); + } + } + + return new_trailer_item(NULL, + strbuf_detach(tok, NULL), + strbuf_detach(val, NULL)); +} + +static void add_trailer_item(struct trailer_item **first, +struct trailer_item **last, +struct trailer_item *new) +{ + if (!new) + return; + if (!*last) { + *first = new; + *last = new; + } else { + (*last)-next = new; + new-previous = *last; + *last = new; + } +} + +static struct trailer_item *process_command_line_args(struct string_list *trailers) +{ + struct trailer_item *arg_tok_first = NULL; + struct trailer_item *arg_tok_last = NULL; + struct string_list_item *tr; + + for_each_string_list_item(tr, trailers) { + struct trailer_item *new = create_trailer_item(tr-string); + add_trailer_item(arg_tok_first, arg_tok_last, new); + } + + return arg_tok_first; +} -- 2.0.3.960.g41c6e4c -- 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 v14 11/11] Documentation: add documentation for 'git interpret-trailers'
While at it add git-interpret-trailers to command-list.txt. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-interpret-trailers.txt | 313 +++ command-list.txt | 1 + 2 files changed, 314 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt new file mode 100644 index 000..db1e9da --- /dev/null +++ b/Documentation/git-interpret-trailers.txt @@ -0,0 +1,313 @@ +git-interpret-trailers(1) += + +NAME + +git-interpret-trailers - help add stuctured information into commit messages + +SYNOPSIS + +[verse] +'git interpret-trailers' [--trim-empty] [(--trailer token[(=|:)value])...] [file...] + +DESCRIPTION +--- +Help adding 'trailers' lines, that look similar to RFC 822 e-mail +headers, at the end of the otherwise free-form part of a commit +message. + +This command reads some patches or commit messages from either the +file arguments or the standard input if no file is specified. Then +this command applies the arguments passed using the `--trailer` +option, if any, to the commit message part of each input file. The +result is emitted on the standard output. + +Some configuration variables control the way the `--trailer` arguments +are applied to each commit message and the way any existing trailer in +the commit message is changed. They also make it possible to +automatically add some trailers. + +By default, a 'token=value' or 'token:value' argument given +using `--trailer` will be appended after the existing trailers only if +the last trailer has a different (token, value) pair (or if there +is no existing trailer). The token and value parts will be trimmed +to remove starting and trailing whitespace, and the resulting trimmed +token and value will appear in the message like this: + + +token: value + + +This means that the trimmed token and value will be separated by +`': '` (one colon followed by one space). + +By default the new trailer will appear at the end of all the existing +trailers. If there is no existing trailer, the new trailer will appear +after the commit message part of the ouput, and, if there is no line +with only spaces at the end of the commit message part, one blank line +will be added before the new trailer. + +Existing trailers are extracted from the input message by looking for +a group of one or more lines that contain a colon (by default), where +the group is preceded by one or more empty (or whitespace-only) lines. +The group must either be at the end of the message or be the last +non-whitespace lines before a line that starts with '---'. Such three +minus signs start the patch part of the message. + +When reading trailers, there can be whitespaces before and after the +token, the separator and the value. There can also be whitespaces +indide the token and the value. + +Note that 'trailers' do not follow and are not intended to follow many +rules for RFC 822 headers. For example they do not follow the line +folding rules, the encoding rules and probably many other rules. + +OPTIONS +--- +--trim-empty:: + If the value part of any trailer contains only whitespace, + the whole trailer will be removed from the resulting message. + This apply to existing trailers as well as new trailers. + +--trailer token[(=|:)value]:: + Specify a (token, value) pair that should be applied as a + trailer to the input messages. See the description of this + command. + +CONFIGURATION VARIABLES +--- + +trailer.separators:: + This option tells which characters are recognized as trailer + separators. By default only ':' is recognized as a trailer + separator, except that '=' is always accepted on the command + line for compatibility with other git commands. ++ +The first character given by this option will be the default character +used when another separator is not specified in the config for this +trailer. ++ +For example, if the value for this option is %=$, then only lines +using the format 'tokensepvalue' with sep containing '%', '=' +or '$' and then spaces will be considered trailers. And '%' will be +the default separator used, so by default trailers will appear like: +'token% value' (one percent sign and one space will appear between +the token and the value). + +trailer.where:: + This option tells where a new trailer will be added. ++ +This can be `end`, which is the default, `start`, `after` or `before`. ++ +If it is `end`, then each new trailer will appear at the end of the +existing trailers. ++ +If it is `start`, then each new trailer will appear at the start, +instead of the end, of the existing trailers
[PATCH v14 06/11] 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 81 +-- trailer.h | 6 + 2 files changed, 75 insertions(+), 12 deletions(-) create mode 100644 trailer.h diff --git a/trailer.c b/trailer.c index 46e2fcb..8e41b8f 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,6 @@ #include cache.h #include string-list.h +#include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -82,6 +83,35 @@ static void free_trailer_item(struct trailer_item *item) free(item); } +static char last_non_space_char(const char *s) +{ + int i; + for (i = strlen(s) - 1; i = 0; i--) + if (!isspace(s[i])) + return s[i]; + return '\0'; +} + +static void print_tok_val(const char *tok, const char *val) +{ + char c = last_non_space_char(tok); + if (!c) + return; + if (strchr(separators, c)) + printf(%s%s\n, tok, val); + else + printf(%s%c %s\n, tok, separators[0], 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 update_last(struct trailer_item **last) { if (*last) @@ -489,18 +519,6 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra return 0; } - -static void duplicate_conf(struct conf_info *dst, struct conf_info *src) -{ - *dst = *src; - if (src-name) - dst-name = xstrdup(src-name); - if (src-key) - dst-key = xstrdup(src-key); - if (src-command) - dst-command = xstrdup(src-command); -} - static const char *token_from_item(struct trailer_item *item) { if (item-conf.key) @@ -705,3 +723,42 @@ static int process_input_file(struct strbuf **lines, return patch_start; } + +static void free_all(struct trailer_item **first) +{ + while (*first) { + struct trailer_item *item = remove_first(first); + free_trailer_item(item); + } +} + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers) +{ + struct trailer_item *in_tok_first = NULL; + struct trailer_item *in_tok_last = NULL; + struct trailer_item *arg_tok_first; + struct strbuf **lines; + int patch_start; + + /* Default config must be setup first */ + git_config(git_trailer_default_config, NULL); + git_config(git_trailer_config, NULL); + + lines = read_input_file(file); + + /* Print the lines before the trailers */ + patch_start = process_input_file(lines, in_tok_first, in_tok_last); + + arg_tok_first = process_command_line_args(trailers); + + process_trailers_lists(in_tok_first, in_tok_last, arg_tok_first); + + print_all(in_tok_first, trim_empty); + + free_all(in_tok_first); + + /* Print the lines after the trailers as is */ + print_lines(lines, patch_start, INT_MAX); + + strbuf_list_free(lines); +} diff --git a/trailer.h b/trailer.h new file mode 100644 index 000..8eb25d5 --- /dev/null +++ b/trailer.h @@ -0,0 +1,6 @@ +#ifndef TRAILER_H +#define TRAILER_H + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers); + +#endif /* TRAILER_H */ -- 2.0.3.960.g41c6e4c -- 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 v14 02/11] trailer: process trailers from input message and arguments
Implement the logic to process trailers from the input message and from arguments. At the beginning trailers from the input message are in their own in_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 in_tok list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 210 ++ 1 file changed, 210 insertions(+) diff --git a/trailer.c b/trailer.c index 2adc1b7..4940e06 100644 --- a/trailer.c +++ b/trailer.c @@ -62,3 +62,213 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b) { return same_token(a, b) same_value(a, b); } + +static void free_trailer_item(struct trailer_item *item) +{ + free(item-conf.name); + free(item-conf.key); + free(item-conf.command); + free((char *)item-token); + free((char *)item-value); + free(item); +} + +static void update_last(struct trailer_item **last) +{ + if (*last) + while ((*last)-next != NULL) + *last = (*last)-next; +} + +static void update_first(struct trailer_item **first) +{ + if (*first) + while ((*first)-previous != NULL) + *first = (*first)-previous; +} + +static void add_arg_to_input_list(struct trailer_item *on_tok, + struct trailer_item *arg_tok, + struct trailer_item **first, + struct trailer_item **last) +{ + if (after_or_end(arg_tok-conf.where)) { + arg_tok-next = on_tok-next; + on_tok-next = arg_tok; + arg_tok-previous = on_tok; + if (arg_tok-next) + arg_tok-next-previous = arg_tok; + update_last(last); + } else { + arg_tok-previous = on_tok-previous; + on_tok-previous = arg_tok; + arg_tok-next = on_tok; + if (arg_tok-previous) + arg_tok-previous-next = arg_tok; + update_first(first); + } +} + +static int check_if_different(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + int check_all) +{ + enum action_where where = arg_tok-conf.where; + do { + if (!in_tok) + return 1; + if (same_trailer(in_tok, arg_tok)) + return 0; + /* +* if we want to add a trailer after another one, +* we have to check those before this one +*/ + in_tok = after_or_end(where) ? in_tok-previous : in_tok-next; + } while (check_all); + return 1; +} + +static void remove_from_list(struct trailer_item *item, +struct trailer_item **first, +struct trailer_item **last) +{ + struct trailer_item *next = item-next; + struct trailer_item *previous = item-previous; + + if (next) { + item-next-previous = previous; + item-next = NULL; + } else if (last) + *last = previous; + + if (previous) { + item-previous-next = next; + item-previous = NULL; + } else if (first) + *first = 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 apply_arg_if_exists(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + struct trailer_item *on_tok, + struct trailer_item **in_tok_first, + struct trailer_item **in_tok_last) +{ + switch (arg_tok-conf.if_exists) { + case EXISTS_DO_NOTHING: + free_trailer_item(arg_tok); + break; + case EXISTS_REPLACE: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + remove_from_list(in_tok, in_tok_first, in_tok_last); + free_trailer_item(in_tok); + break; + case EXISTS_ADD: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + break; + case EXISTS_ADD_IF_DIFFERENT: + if (check_if_different(in_tok, arg_tok, 1)) + add_arg_to_input_list(on_tok, arg_tok
[PATCH v14 09/11] trailer: execute command from 'trailer.name.command'
Let the user specify a command that will give on its standard output the value to use for the specified trailer. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 85 ++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/trailer.c b/trailer.c index 8e41b8f..0e76310 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,7 @@ #include cache.h #include string-list.h +#include run-command.h +#include string-list.h #include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org @@ -33,6 +35,8 @@ static struct trailer_item *first_conf_item; static char *separators = :; +#define TRAILER_ARG_STRING $ARG + static int after_or_end(enum action_where where) { return (where == WHERE_AFTER) || (where == WHERE_END); @@ -73,6 +77,13 @@ static inline int contains_only_spaces(const char *str) return !*s; } +static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b) +{ + const char *ptr = strstr(sb-buf, a); + if (ptr) + strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b)); +} + static void free_trailer_item(struct trailer_item *item) { free(item-conf.name); @@ -198,6 +209,63 @@ static struct trailer_item *remove_first(struct trailer_item **first) return item; } +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); + result = xstrdup(); + } else + result = strbuf_detach(buf, NULL); + + strbuf_release(cmd); + return result; +} + +static void apply_item_command(struct trailer_item *in_tok, struct trailer_item *arg_tok) +{ + if (arg_tok-conf.command) { + const char *arg; + if (arg_tok-value arg_tok-value[0]) { + arg = arg_tok-value; + } else { + if (in_tok in_tok-value) + arg = xstrdup(in_tok-value); + else + arg = xstrdup(); + } + arg_tok-value = apply_command(arg_tok-conf.command, arg); + free((char *)arg); + } +} + static void apply_arg_if_exists(struct trailer_item *in_tok, struct trailer_item *arg_tok, struct trailer_item *on_tok, @@ -209,16 +277,19 @@ static void apply_arg_if_exists(struct trailer_item *in_tok, free_trailer_item(arg_tok); break; case EXISTS_REPLACE: + apply_item_command(in_tok, arg_tok); add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); remove_from_list(in_tok, in_tok_first, in_tok_last); free_trailer_item(in_tok); break; case EXISTS_ADD: + apply_item_command(in_tok, arg_tok); add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); break; case EXISTS_ADD_IF_DIFFERENT: + apply_item_command(in_tok, arg_tok); if (check_if_different(in_tok, arg_tok, 1)) add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); @@ -226,6 +297,7 @@ static void apply_arg_if_exists(struct trailer_item *in_tok, free_trailer_item(arg_tok); break; case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR: + apply_item_command(in_tok, arg_tok); if (check_if_different(on_tok, arg_tok, 0)) add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); @@ -249,6 +321,7 @@ static void apply_arg_if_missing(struct trailer_item **in_tok_first, case MISSING_ADD
[PATCH v14 03/11] trailer: read and process config information
Read the configuration to get trailer information, and then process it and store 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 185 ++ 1 file changed, 185 insertions(+) diff --git a/trailer.c b/trailer.c index 4940e06..2d391f3 100644 --- a/trailer.c +++ b/trailer.c @@ -272,3 +272,188 @@ static void process_trailers_lists(struct trailer_item **in_tok_first, 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 if (!strcasecmp(end, value)) + item-where = WHERE_END; + else if (!strcasecmp(start, value)) + item-where = WHERE_START; + else + return -1; + return 0; +} + +static int set_if_exists(struct conf_info *item, const char *value) +{ + if (!strcasecmp(addIfDifferent, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT; + else if (!strcasecmp(addIfDifferentNeighbor, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR; + else if (!strcasecmp(add, value)) + item-if_exists = EXISTS_ADD; + else if (!strcasecmp(replace, value)) + item-if_exists = EXISTS_REPLACE; + else if (!strcasecmp(doNothing, value)) + item-if_exists = EXISTS_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; +} + +static void duplicate_conf(struct conf_info *dst, struct conf_info *src) +{ + *dst = *src; + if (src-name) + dst-name = xstrdup(src-name); + if (src-key) + dst-key = xstrdup(src-key); + if (src-command) + dst-command = xstrdup(src-command); +} + +static struct trailer_item *get_conf_item(const 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); + duplicate_conf(item-conf, default_conf_info); + item-conf.name = xstrdup(name); + + if (!previous) + first_conf_item = item; + else { + previous-next = item; + item-previous = previous; + } + + return item; +} + +enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE, +TRAILER_IF_EXISTS, TRAILER_IF_MISSING }; + +static struct { + const char *name; + enum trailer_info_type type; +} trailer_config_items[] = { + { key, TRAILER_KEY }, + { command, TRAILER_COMMAND }, + { where, TRAILER_WHERE }, + { ifexists, TRAILER_IF_EXISTS }, + { ifmissing, TRAILER_IF_MISSING } +}; + +static int git_trailer_default_config(const char *conf_key, const char *value, void *cb) +{ + const char *trailer_item, *variable_name; + + if (!skip_prefix(conf_key, trailer., trailer_item)) + return 0; + + variable_name = strrchr(trailer_item, '.'); + if (!variable_name) { + if (!strcmp(trailer_item, where)) { + if (set_where(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifexists)) { + if (set_if_exists(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifmissing)) { + if (set_if_missing(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, separators)) { + separators = xstrdup(value); + } + } + return 0
Re: [PATCH v13 00/11] Add interpret-trailers builtin
From: Junio C Hamano gits...@pobox.com Christian Couder chrisc...@tuxfamily.org writes: '=' is always accepted as separator when parsing --trailer 'tokensepvalue' command line arguments, for compatibility with other git commands Hmph. Which of other commands take --option foo=bar? Puzzled... Most commands accept --option=value. I know that it is different from --option foo=bar, but it could reduce user mistakes if we also accept --trailer foo=bar. And if we accept it, then whatever the user has configured in the trailer.separators option, --trailer foo=bar will always work and add the default separator. So it is a generic way to add fooseparator bar to the trailers whatever the user want as a separator. But maybe I should reword the documentation to add this reason. (I did not do that in the v14 series I just sent.) Thanks, 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 v14 01/11] trailer: add data structures and basic functions
On Mon, Sep 15, 2014 at 10:39 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder chrisc...@tuxfamily.org writes: +/* Get the length of buf from its beginning until its last alphanumeric character */ That makes it sound as if feeding abc%de#f@ to the function returns 3 for abc, but For me the last alphanumeric character in abc%de#f@ is f, so it is the length from the beginning to f so it should return 8. +static size_t alnum_len(const char *buf, size_t len) +{ + while (len 0 !isalnum(buf[len - 1])) + len--; + return len; +} doesn't it look at '@', be unhappy and decrement, look at 'f' and break out to return the length of abc%de#f? Yeah, that's the expected behavior. Perhaps that behaviour _is_ what you want, but then the comment is lying, no? I don't think so, but maybe you are parsing the comment in a different way than I am. What would you suggest instead? Thanks, 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 v14 01/11] trailer: add data structures and basic functions
On Wed, Sep 17, 2014 at 9:58 AM, Jeff King p...@peff.net wrote: On Tue, Sep 16, 2014 at 10:01:11AM +0200, Christian Couder wrote: On Mon, Sep 15, 2014 at 10:39 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder chrisc...@tuxfamily.org writes: +/* Get the length of buf from its beginning until its last alphanumeric character */ That makes it sound as if feeding abc%de#f@ to the function returns 3 for abc, but For me the last alphanumeric character in abc%de#f@ is f, so it is the length from the beginning to f so it should return 8. FWIW, I parsed the comment as you intended, but I do think it is a bit unclear (especially given the name, as it is skipping over more than just alnums). From reading the calling code, it looks like the intent is to take a token string like Signed-off-by: and find that the : is part of the ending punctuation, but that the - are retained as internal punctuation. Would it make sense as: /* * Return the length of the string not including any final * punctuation. E.g., the input Signed-off-by: would return * 13, stripping the trailing punctuation but retaining * internal punctuation. */ int token_len_without_separator(const char *token) ... The name is a bit clunky, but hopefully it is more clear what the point is. Ok, I will use that in the next version. Thanks, 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
[PATCH v15 07/11] 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- .gitignore | 1 + Makefile | 1 + builtin.h| 1 + builtin/interpret-trailers.c | 44 git.c| 1 + 5 files changed, 48 insertions(+) create mode 100644 builtin/interpret-trailers.c diff --git a/.gitignore b/.gitignore index cf0d191..85593de 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 ef82972..0b06ae0 100644 --- a/Makefile +++ b/Makefile @@ -953,6 +953,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 5d91f31..b87df70 100644 --- a/builtin.h +++ b/builtin.h @@ -73,6 +73,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..46838d2 --- /dev/null +++ b/builtin/interpret-trailers.c @@ -0,0 +1,44 @@ +/* + * Builtin git interpret-trailers + * + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + * + */ + +#include cache.h +#include builtin.h +#include parse-options.h +#include string-list.h +#include trailer.h + +static const char * const git_interpret_trailers_usage[] = { + N_(git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...]), + NULL +}; + +int cmd_interpret_trailers(int argc, const char **argv, const char *prefix) +{ + int trim_empty = 0; + struct string_list trailers = STRING_LIST_INIT_DUP; + + struct option options[] = { + OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty trailers)), + OPT_STRING_LIST(0, trailer, trailers, N_(trailer), + N_(trailer(s) to add)), + OPT_END() + }; + + argc = parse_options(argc, argv, prefix, options, +git_interpret_trailers_usage, 0); + + if (argc) { + int i; + for (i = 0; i argc; i++) + process_trailers(argv[i], trim_empty, trailers); + } else + process_trailers(NULL, trim_empty, trailers); + + string_list_clear(trailers, 0); + + return 0; +} diff --git a/git.c b/git.c index 5ebb32f..e327a90 100644 --- a/git.c +++ b/git.c @@ -417,6 +417,7 @@ static struct cmd_struct commands[] = { { index-pack, cmd_index_pack, RUN_SETUP_GENTLY }, { init, cmd_init_db, NO_SETUP }, { init-db, cmd_init_db, NO_SETUP }, + { 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 }, -- 2.0.3.960.g41c6e4c -- 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 v15 01/11] trailer: add data structures and basic functions
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 Signed-off-by: Junio C Hamano gits...@pobox.com --- Makefile | 1 + trailer.c | 69 +++ 2 files changed, 70 insertions(+) create mode 100644 trailer.c diff --git a/Makefile b/Makefile index 63a210d..ef82972 100644 --- a/Makefile +++ b/Makefile @@ -888,6 +888,7 @@ LIB_OBJS += submodule-config.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..ac323b1 --- /dev/null +++ b/trailer.c @@ -0,0 +1,69 @@ +#include cache.h +/* + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + */ + +enum action_where { WHERE_END, WHERE_AFTER, WHERE_BEFORE, WHERE_START }; +enum action_if_exists { EXISTS_ADD_IF_DIFFERENT_NEIGHBOR, EXISTS_ADD_IF_DIFFERENT, + EXISTS_ADD, EXISTS_REPLACE, EXISTS_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_exists if_exists; + enum action_if_missing if_missing; +}; + +static struct conf_info default_conf_info; + +struct trailer_item { + struct trailer_item *previous; + struct trailer_item *next; + const char *token; + const char *value; + struct conf_info conf; +}; + +static struct trailer_item *first_conf_item; + +static char *separators = :; + +static int after_or_end(enum action_where where) +{ + return (where == WHERE_AFTER) || (where == WHERE_END); +} + +/* + * Return the length of the string not including any final + * punctuation. E.g., the input Signed-off-by: would return + * 13, stripping the trailing punctuation but retaining + * internal punctuation. + */ +static size_t token_len_without_separator(const char *token, size_t len) +{ + while (len 0 !isalnum(token[len - 1])) + len--; + return len; +} + +static int same_token(struct trailer_item *a, struct trailer_item *b) +{ + size_t a_len = token_len_without_separator(a-token, strlen(a-token)); + size_t b_len = token_len_without_separator(b-token, strlen(b-token)); + size_t min_len = (a_len b_len) ? b_len : a_len; + + return !strncasecmp(a-token, b-token, min_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) +{ + return same_token(a, b) same_value(a, b); +} -- 2.0.3.960.g41c6e4c -- 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 v15 00/11] 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 keep builtin/commit.c uncontaminated by any more hard-wired logic, like what we have for the signed-off-by line. Any new things can and should be doable in hooks, and this filter would help writing these hooks. And that is why the design goal of the filter is to make it at least as powerful as the built-in logic we have for signed-off-by lines; that would allow us to later eject the hard-wired logic for signed-off-by line from the main codepath, if/when we wanted to. Alternatively, we could build a library-ish API around this filter code and replace the hard-wired logic for signed-off-by line with a call into that API, if/when we wanted to, but that requires (in addition to the at least as powerful as the built-in logic) that the implementation of this stand-alone filter can be cleanly made into a reusable library, so that is a bit higher bar to cross than everything can be doable with hooks alternative. 2) Current state Currently the usage string of this command is: git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...] The following features are implemented: - the result is printed on stdout - the --trailer arguments are interpreted - messages read from file... or stdin are interpreted - the trailer.separators option in the config is interpreted - the trailer.where option is interpreted - the trailer.ifexists option is interpreted - the trailer.ifmissing option is interpreted - the trailer.token.key options are interpreted - the trailer.token.where options are interpreted - the trailer.token.ifexist options are interpreted - the trailer.token.ifmissing options are interpreted - the trailer.token.command config works - $ARG can be used in commands - messages can contain a patch - lines in messages starting with a comment char are ignored - there are 49 tests - there is some documentation - there are examples in the documentation 3) Changes since version 14, thanks to Jeff and Junio * renamed alnum_len() to token_len_without_separator() and improve comment before this function (patch 1/11 and 4/11) Christian Couder (11): trailer: add data structures and basic functions trailer: process trailers from input message and arguments trailer: read and process config information trailer: process command line trailer arguments trailer: parse trailers from file or stdin trailer: put all the processing together and print trailer: add interpret-trailers command trailer: add tests for git interpret-trailers trailer: execute command from 'trailer.name.command' trailer: add tests for commands in config file Documentation: add documentation for 'git interpret-trailers' .gitignore | 1 + Documentation/git-interpret-trailers.txt | 313 Makefile | 2 + builtin.h| 1 + builtin/interpret-trailers.c | 44 ++ command-list.txt | 1 + git.c| 1 + t/t7513-interpret-trailers.sh| 851 ++ trailer.c| 852 +++ trailer.h| 6 + 10 files changed, 2072 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt 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 -- 2.0.3.960.g41c6e4c -- 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 v15 04/11] trailer: process command line trailer arguments
Parse the trailer command line arguments and put the result into an arg_tok doubly linked list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 125 ++ 1 file changed, 125 insertions(+) diff --git a/trailer.c b/trailer.c index 668dc33..bf58980 100644 --- a/trailer.c +++ b/trailer.c @@ -1,4 +1,5 @@ #include cache.h +#include string-list.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -462,3 +463,127 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb) } return 0; } + +static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *trailer) +{ + size_t len; + struct strbuf seps = STRBUF_INIT; + strbuf_addstr(seps, separators); + strbuf_addch(seps, '='); + len = strcspn(trailer, seps.buf); + strbuf_release(seps); + if (len == 0) + return error(_(empty trailer token in trailer '%s'), trailer); + if (len strlen(trailer)) { + strbuf_add(tok, trailer, len); + strbuf_trim(tok); + strbuf_addstr(val, trailer + len + 1); + strbuf_trim(val); + } else { + strbuf_addstr(tok, trailer); + strbuf_trim(tok); + } + return 0; +} + + +static void duplicate_conf(struct conf_info *dst, struct conf_info *src) +{ + *dst = *src; + if (src-name) + dst-name = xstrdup(src-name); + if (src-key) + dst-key = xstrdup(src-key); + if (src-command) + dst-command = xstrdup(src-command); +} + +static const char *token_from_item(struct trailer_item *item) +{ + if (item-conf.key) + return item-conf.key; + + return item-conf.name; +} + +static struct trailer_item *new_trailer_item(struct trailer_item *conf_item, +char *tok, char *val) +{ + struct trailer_item *new = xcalloc(sizeof(*new), 1); + new-value = val; + + if (conf_item) { + duplicate_conf(new-conf, conf_item-conf); + new-token = xstrdup(token_from_item(conf_item)); + free(tok); + } else { + duplicate_conf(new-conf, default_conf_info); + new-token = tok; + } + + return new; +} + +static int token_matches_item(const char *tok, struct trailer_item *item, int tok_len) +{ + if (!strncasecmp(tok, item-conf.name, tok_len)) + return 1; + return item-conf.key ? !strncasecmp(tok, item-conf.key, tok_len) : 0; +} + +static struct trailer_item *create_trailer_item(const char *string) +{ + struct strbuf tok = STRBUF_INIT; + struct strbuf val = STRBUF_INIT; + struct trailer_item *item; + int tok_len; + + if (parse_trailer(tok, val, string)) + return NULL; + + tok_len = token_len_without_separator(tok.buf, tok.len); + + /* Lookup if the token matches something in the config */ + for (item = first_conf_item; item; item = item-next) { + if (token_matches_item(tok.buf, item, tok_len)) { + strbuf_release(tok); + return new_trailer_item(item, + NULL, + strbuf_detach(val, NULL)); + } + } + + return new_trailer_item(NULL, + strbuf_detach(tok, NULL), + strbuf_detach(val, NULL)); +} + +static void add_trailer_item(struct trailer_item **first, +struct trailer_item **last, +struct trailer_item *new) +{ + if (!new) + return; + if (!*last) { + *first = new; + *last = new; + } else { + (*last)-next = new; + new-previous = *last; + *last = new; + } +} + +static struct trailer_item *process_command_line_args(struct string_list *trailers) +{ + struct trailer_item *arg_tok_first = NULL; + struct trailer_item *arg_tok_last = NULL; + struct string_list_item *tr; + + for_each_string_list_item(tr, trailers) { + struct trailer_item *new = create_trailer_item(tr-string); + add_trailer_item(arg_tok_first, arg_tok_last, new); + } + + return arg_tok_first; +} -- 2.0.3.960.g41c6e4c -- 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 v15 06/11] 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 81 +-- trailer.h | 6 + 2 files changed, 75 insertions(+), 12 deletions(-) create mode 100644 trailer.h diff --git a/trailer.c b/trailer.c index c3e096f..094c6e8 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,6 @@ #include cache.h #include string-list.h +#include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -87,6 +88,35 @@ static void free_trailer_item(struct trailer_item *item) free(item); } +static char last_non_space_char(const char *s) +{ + int i; + for (i = strlen(s) - 1; i = 0; i--) + if (!isspace(s[i])) + return s[i]; + return '\0'; +} + +static void print_tok_val(const char *tok, const char *val) +{ + char c = last_non_space_char(tok); + if (!c) + return; + if (strchr(separators, c)) + printf(%s%s\n, tok, val); + else + printf(%s%c %s\n, tok, separators[0], 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 update_last(struct trailer_item **last) { if (*last) @@ -494,18 +524,6 @@ static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *tra return 0; } - -static void duplicate_conf(struct conf_info *dst, struct conf_info *src) -{ - *dst = *src; - if (src-name) - dst-name = xstrdup(src-name); - if (src-key) - dst-key = xstrdup(src-key); - if (src-command) - dst-command = xstrdup(src-command); -} - static const char *token_from_item(struct trailer_item *item) { if (item-conf.key) @@ -710,3 +728,42 @@ static int process_input_file(struct strbuf **lines, return patch_start; } + +static void free_all(struct trailer_item **first) +{ + while (*first) { + struct trailer_item *item = remove_first(first); + free_trailer_item(item); + } +} + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers) +{ + struct trailer_item *in_tok_first = NULL; + struct trailer_item *in_tok_last = NULL; + struct trailer_item *arg_tok_first; + struct strbuf **lines; + int patch_start; + + /* Default config must be setup first */ + git_config(git_trailer_default_config, NULL); + git_config(git_trailer_config, NULL); + + lines = read_input_file(file); + + /* Print the lines before the trailers */ + patch_start = process_input_file(lines, in_tok_first, in_tok_last); + + arg_tok_first = process_command_line_args(trailers); + + process_trailers_lists(in_tok_first, in_tok_last, arg_tok_first); + + print_all(in_tok_first, trim_empty); + + free_all(in_tok_first); + + /* Print the lines after the trailers as is */ + print_lines(lines, patch_start, INT_MAX); + + strbuf_list_free(lines); +} diff --git a/trailer.h b/trailer.h new file mode 100644 index 000..8eb25d5 --- /dev/null +++ b/trailer.h @@ -0,0 +1,6 @@ +#ifndef TRAILER_H +#define TRAILER_H + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers); + +#endif /* TRAILER_H */ -- 2.0.3.960.g41c6e4c -- 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 v15 11/11] Documentation: add documentation for 'git interpret-trailers'
While at it add git-interpret-trailers to command-list.txt. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-interpret-trailers.txt | 313 +++ command-list.txt | 1 + 2 files changed, 314 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt new file mode 100644 index 000..db1e9da --- /dev/null +++ b/Documentation/git-interpret-trailers.txt @@ -0,0 +1,313 @@ +git-interpret-trailers(1) += + +NAME + +git-interpret-trailers - help add stuctured information into commit messages + +SYNOPSIS + +[verse] +'git interpret-trailers' [--trim-empty] [(--trailer token[(=|:)value])...] [file...] + +DESCRIPTION +--- +Help adding 'trailers' lines, that look similar to RFC 822 e-mail +headers, at the end of the otherwise free-form part of a commit +message. + +This command reads some patches or commit messages from either the +file arguments or the standard input if no file is specified. Then +this command applies the arguments passed using the `--trailer` +option, if any, to the commit message part of each input file. The +result is emitted on the standard output. + +Some configuration variables control the way the `--trailer` arguments +are applied to each commit message and the way any existing trailer in +the commit message is changed. They also make it possible to +automatically add some trailers. + +By default, a 'token=value' or 'token:value' argument given +using `--trailer` will be appended after the existing trailers only if +the last trailer has a different (token, value) pair (or if there +is no existing trailer). The token and value parts will be trimmed +to remove starting and trailing whitespace, and the resulting trimmed +token and value will appear in the message like this: + + +token: value + + +This means that the trimmed token and value will be separated by +`': '` (one colon followed by one space). + +By default the new trailer will appear at the end of all the existing +trailers. If there is no existing trailer, the new trailer will appear +after the commit message part of the ouput, and, if there is no line +with only spaces at the end of the commit message part, one blank line +will be added before the new trailer. + +Existing trailers are extracted from the input message by looking for +a group of one or more lines that contain a colon (by default), where +the group is preceded by one or more empty (or whitespace-only) lines. +The group must either be at the end of the message or be the last +non-whitespace lines before a line that starts with '---'. Such three +minus signs start the patch part of the message. + +When reading trailers, there can be whitespaces before and after the +token, the separator and the value. There can also be whitespaces +indide the token and the value. + +Note that 'trailers' do not follow and are not intended to follow many +rules for RFC 822 headers. For example they do not follow the line +folding rules, the encoding rules and probably many other rules. + +OPTIONS +--- +--trim-empty:: + If the value part of any trailer contains only whitespace, + the whole trailer will be removed from the resulting message. + This apply to existing trailers as well as new trailers. + +--trailer token[(=|:)value]:: + Specify a (token, value) pair that should be applied as a + trailer to the input messages. See the description of this + command. + +CONFIGURATION VARIABLES +--- + +trailer.separators:: + This option tells which characters are recognized as trailer + separators. By default only ':' is recognized as a trailer + separator, except that '=' is always accepted on the command + line for compatibility with other git commands. ++ +The first character given by this option will be the default character +used when another separator is not specified in the config for this +trailer. ++ +For example, if the value for this option is %=$, then only lines +using the format 'tokensepvalue' with sep containing '%', '=' +or '$' and then spaces will be considered trailers. And '%' will be +the default separator used, so by default trailers will appear like: +'token% value' (one percent sign and one space will appear between +the token and the value). + +trailer.where:: + This option tells where a new trailer will be added. ++ +This can be `end`, which is the default, `start`, `after` or `before`. ++ +If it is `end`, then each new trailer will appear at the end of the +existing trailers. ++ +If it is `start`, then each new trailer will appear at the start, +instead of the end, of the existing trailers
[PATCH v15 10/11] trailer: add tests for commands in config file
And add a few other tests for some special cases. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 125 ++ 1 file changed, 125 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index fa37565..c5b86ff 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -723,4 +723,129 @@ test_expect_success 'default where is now after' ' 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \A U Thor aut...@example.com\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=22 \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using commiter information' ' + git config trailer.sign.ifExists addIfDifferent + git config trailer.sign.command echo \\$GIT_COMMITTER_NAME \$GIT_COMMITTER_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: C O Mitter commit...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \\$GIT_AUTHOR_NAME \$GIT_AUTHOR_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists replace + 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 + sed -e s/ Z\$/ / expected -EOF + Fixes: $FIXED + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with failing command using $ARG' ' + git config trailer.fix.ifExists replace + git config trailer.fix.command false \$ARG + cat complex_message_body expected + sed -e s/ Z\$/ / expected -EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with empty tokens' ' + git config --unset trailer.fix.command + cat expected -EOF + + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer : --trailer :test actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with command but no key' ' + git config --unset trailer.sign.key + cat expected -EOF + + sign: A U Thor aut...@example.com + EOF + git interpret-trailers actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with no command and no key' ' + git config --unset trailer.review.key + cat expected -EOF + + review: Junio + sign: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review:Junio actual -EOF + EOF + test_cmp expected
[PATCH v15 03/11] trailer: read and process config information
Read the configuration to get trailer information, and then process it and store 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 185 ++ 1 file changed, 185 insertions(+) diff --git a/trailer.c b/trailer.c index be0ad65..668dc33 100644 --- a/trailer.c +++ b/trailer.c @@ -277,3 +277,188 @@ static void process_trailers_lists(struct trailer_item **in_tok_first, 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 if (!strcasecmp(end, value)) + item-where = WHERE_END; + else if (!strcasecmp(start, value)) + item-where = WHERE_START; + else + return -1; + return 0; +} + +static int set_if_exists(struct conf_info *item, const char *value) +{ + if (!strcasecmp(addIfDifferent, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT; + else if (!strcasecmp(addIfDifferentNeighbor, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR; + else if (!strcasecmp(add, value)) + item-if_exists = EXISTS_ADD; + else if (!strcasecmp(replace, value)) + item-if_exists = EXISTS_REPLACE; + else if (!strcasecmp(doNothing, value)) + item-if_exists = EXISTS_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; +} + +static void duplicate_conf(struct conf_info *dst, struct conf_info *src) +{ + *dst = *src; + if (src-name) + dst-name = xstrdup(src-name); + if (src-key) + dst-key = xstrdup(src-key); + if (src-command) + dst-command = xstrdup(src-command); +} + +static struct trailer_item *get_conf_item(const 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); + duplicate_conf(item-conf, default_conf_info); + item-conf.name = xstrdup(name); + + if (!previous) + first_conf_item = item; + else { + previous-next = item; + item-previous = previous; + } + + return item; +} + +enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE, +TRAILER_IF_EXISTS, TRAILER_IF_MISSING }; + +static struct { + const char *name; + enum trailer_info_type type; +} trailer_config_items[] = { + { key, TRAILER_KEY }, + { command, TRAILER_COMMAND }, + { where, TRAILER_WHERE }, + { ifexists, TRAILER_IF_EXISTS }, + { ifmissing, TRAILER_IF_MISSING } +}; + +static int git_trailer_default_config(const char *conf_key, const char *value, void *cb) +{ + const char *trailer_item, *variable_name; + + if (!skip_prefix(conf_key, trailer., trailer_item)) + return 0; + + variable_name = strrchr(trailer_item, '.'); + if (!variable_name) { + if (!strcmp(trailer_item, where)) { + if (set_where(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifexists)) { + if (set_if_exists(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifmissing)) { + if (set_if_missing(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, separators)) { + separators = xstrdup(value); + } + } + return 0
[PATCH v15 05/11] trailer: parse trailers from file or stdin
Read trailers from a file or from stdin, parse the trailers and then put the result into a doubly linked list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 123 ++ 1 file changed, 123 insertions(+) diff --git a/trailer.c b/trailer.c index bf58980..c3e096f 100644 --- a/trailer.c +++ b/trailer.c @@ -69,6 +69,14 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b) return same_token(a, b) same_value(a, b); } +static inline int contains_only_spaces(const char *str) +{ + const char *s = str; + while (*s isspace(*s)) + s++; + return !*s; +} + static void free_trailer_item(struct trailer_item *item) { free(item-conf.name); @@ -587,3 +595,118 @@ static struct trailer_item *process_command_line_args(struct string_list *traile return arg_tok_first; } + +static struct strbuf **read_input_file(const char *file) +{ + struct strbuf **lines; + struct strbuf sb = STRBUF_INIT; + + if (file) { + if (strbuf_read_file(sb, file, 0) 0) + die_errno(_(could not read input file '%s'), file); + } else { + if (strbuf_read(sb, fileno(stdin), 0) 0) + die_errno(_(could not read from stdin)); + } + + lines = strbuf_split(sb, '\n'); + + strbuf_release(sb); + + return lines; +} + +/* + * Return the (0 based) index of the start of the patch or the line + * count if there is no patch in the message. + */ +static int find_patch_start(struct strbuf **lines, int count) +{ + int i; + + /* Get the start of the patch part if any */ + for (i = 0; i count; i++) { + if (starts_with(lines[i]-buf, ---)) + return i; + } + + return count; +} + +/* + * Return the (0 based) index of the first trailer line or count if + * there are no trailers. Trailers are searched only in the lines from + * index (count - 1) down to index 0. + */ +static int find_trailer_start(struct strbuf **lines, int count) +{ + int start, only_spaces = 1; + + /* +* Get the start of the trailers by looking starting from the end +* for a line with only spaces before lines with one separator. +*/ + for (start = count - 1; start = 0; start--) { + if (lines[start]-buf[0] == comment_line_char) + continue; + if (contains_only_spaces(lines[start]-buf)) { + if (only_spaces) + continue; + return start + 1; + } + if (strcspn(lines[start]-buf, separators) lines[start]-len) { + if (only_spaces) + only_spaces = 0; + continue; + } + return count; + } + + return only_spaces ? count : 0; +} + +static int has_blank_line_before(struct strbuf **lines, int start) +{ + for (;start = 0; start--) { + if (lines[start]-buf[0] == comment_line_char) + continue; + return contains_only_spaces(lines[start]-buf); + } + return 0; +} + +static void print_lines(struct strbuf **lines, int start, int end) +{ + int i; + for (i = start; lines[i] i end; i++) + printf(%s, lines[i]-buf); +} + +static int process_input_file(struct strbuf **lines, + struct trailer_item **in_tok_first, + struct trailer_item **in_tok_last) +{ + int count = 0; + int patch_start, trailer_start, i; + + /* Get the line count */ + while (lines[count]) + count++; + + patch_start = find_patch_start(lines, count); + trailer_start = find_trailer_start(lines, patch_start); + + /* Print lines before the trailers as is */ + print_lines(lines, 0, trailer_start); + + if (!has_blank_line_before(lines, trailer_start - 1)) + printf(\n); + + /* Parse trailer lines */ + for (i = trailer_start; i patch_start; i++) { + struct trailer_item *new = create_trailer_item(lines[i]-buf); + add_trailer_item(in_tok_first, in_tok_last, new); + } + + return patch_start; +} -- 2.0.3.960.g41c6e4c -- 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 v15 02/11] trailer: process trailers from input message and arguments
Implement the logic to process trailers from the input message and from arguments. At the beginning trailers from the input message are in their own in_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 in_tok list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 210 ++ 1 file changed, 210 insertions(+) diff --git a/trailer.c b/trailer.c index ac323b1..be0ad65 100644 --- a/trailer.c +++ b/trailer.c @@ -67,3 +67,213 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b) { return same_token(a, b) same_value(a, b); } + +static void free_trailer_item(struct trailer_item *item) +{ + free(item-conf.name); + free(item-conf.key); + free(item-conf.command); + free((char *)item-token); + free((char *)item-value); + free(item); +} + +static void update_last(struct trailer_item **last) +{ + if (*last) + while ((*last)-next != NULL) + *last = (*last)-next; +} + +static void update_first(struct trailer_item **first) +{ + if (*first) + while ((*first)-previous != NULL) + *first = (*first)-previous; +} + +static void add_arg_to_input_list(struct trailer_item *on_tok, + struct trailer_item *arg_tok, + struct trailer_item **first, + struct trailer_item **last) +{ + if (after_or_end(arg_tok-conf.where)) { + arg_tok-next = on_tok-next; + on_tok-next = arg_tok; + arg_tok-previous = on_tok; + if (arg_tok-next) + arg_tok-next-previous = arg_tok; + update_last(last); + } else { + arg_tok-previous = on_tok-previous; + on_tok-previous = arg_tok; + arg_tok-next = on_tok; + if (arg_tok-previous) + arg_tok-previous-next = arg_tok; + update_first(first); + } +} + +static int check_if_different(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + int check_all) +{ + enum action_where where = arg_tok-conf.where; + do { + if (!in_tok) + return 1; + if (same_trailer(in_tok, arg_tok)) + return 0; + /* +* if we want to add a trailer after another one, +* we have to check those before this one +*/ + in_tok = after_or_end(where) ? in_tok-previous : in_tok-next; + } while (check_all); + return 1; +} + +static void remove_from_list(struct trailer_item *item, +struct trailer_item **first, +struct trailer_item **last) +{ + struct trailer_item *next = item-next; + struct trailer_item *previous = item-previous; + + if (next) { + item-next-previous = previous; + item-next = NULL; + } else if (last) + *last = previous; + + if (previous) { + item-previous-next = next; + item-previous = NULL; + } else if (first) + *first = 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 apply_arg_if_exists(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + struct trailer_item *on_tok, + struct trailer_item **in_tok_first, + struct trailer_item **in_tok_last) +{ + switch (arg_tok-conf.if_exists) { + case EXISTS_DO_NOTHING: + free_trailer_item(arg_tok); + break; + case EXISTS_REPLACE: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + remove_from_list(in_tok, in_tok_first, in_tok_last); + free_trailer_item(in_tok); + break; + case EXISTS_ADD: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + break; + case EXISTS_ADD_IF_DIFFERENT: + if (check_if_different(in_tok, arg_tok, 1)) + add_arg_to_input_list(on_tok, arg_tok
[PATCH v15 08/11] trailer: add tests for git interpret-trailers
Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 726 ++ 1 file changed, 726 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..fa37565 --- /dev/null +++ b/t/t7513-interpret-trailers.sh @@ -0,0 +1,726 @@ +#!/bin/sh +# +# Copyright (c) 2013, 2014 Christian Couder +# + +test_description='git interpret-trailers' + +. ./test-lib.sh + +# When 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. + +test_expect_success 'setup' ' + : empty + 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 + sed -e s/ Z\$/ / complex_message_trailers -\EOF + Fixes: Z + Acked-by: Z + Reviewed-by: Z + Signed-off-by: Z + EOF + cat basic_patch -\EOF + --- +foo.txt | 2 +- +1 file changed, 1 insertion(+), 1 deletion(-) + + diff --git a/foo.txt b/foo.txt + index 0353767..1d91aa1 100644 + --- a/foo.txt + +++ b/foo.txt + @@ -1,3 +1,3 @@ + + -bar + +baz + + -- + 1.9.rc0.11.ga562ddc + + EOF +' + +test_expect_success 'without config' ' + sed -e s/ Z\$/ / expected -\EOF + + ack: Peff + Reviewed-by: Z + Acked-by: Johan + EOF + git interpret-trailers --trailer ack = Peff --trailer Reviewed-by \ + --trailer Acked-by: Johan empty actual + test_cmp expected actual +' + +test_expect_success 'without config in another order' ' + sed -e s/ Z\$/ / expected -\EOF + + Acked-by: Johan + Reviewed-by: Z + ack: Peff + EOF + git interpret-trailers --trailer Acked-by: Johan --trailer Reviewed-by \ + --trailer ack = Peff empty actual + test_cmp expected actual +' + +test_expect_success '--trim-empty without config' ' + cat expected -\EOF + + ack: Peff + Acked-by: Johan + EOF + git interpret-trailers --trim-empty --trailer ack=Peff \ + --trailer Reviewed-by --trailer Acked-by: Johan \ + --trailer sob: empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup' ' + git config trailer.ack.key Acked-by: + cat expected -\EOF + + Acked-by: Peff + EOF + git interpret-trailers --trim-empty --trailer ack = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by :Peff empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup and := as separators' ' + git config trailer.separators := + git config trailer.ack.key Acked-by= + cat expected -\EOF + + Acked-by= Peff + EOF + git interpret-trailers --trim-empty --trailer ack = Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by= Peff empty actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by : Peff empty actual + test_cmp expected actual +' + +test_expect_success 'with config setup and % as separators' ' + git config trailer.separators % + cat expected -\EOF + + bug% 42 + count% 10 + bug% 422 + EOF + git interpret-trailers --trim-empty --trailer bug = 42 \ + --trailer count%10 --trailer test: stuff \ + --trailer bug % 422 empty actual + test_cmp expected actual +' + +test_expect_success 'with % as separators and a message with trailers' ' + cat special_message -\EOF + Special Message + + bug% 42 + count% 10 + bug% 422 + EOF + cat expected -\EOF + Special Message + + bug% 42 + count% 10 + bug% 422 + count% 100 + EOF + git interpret-trailers --trailer count%100 \ + special_message actual + test_cmp expected actual +' + +test_expect_success 'with config setup and :=# as separators' ' + git config trailer.separators :=# + git
[PATCH v15 09/11] trailer: execute command from 'trailer.name.command'
Let the user specify a command that will give on its standard output the value to use for the specified trailer. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 85 ++- 1 file changed, 84 insertions(+), 1 deletion(-) diff --git a/trailer.c b/trailer.c index 094c6e8..50f9547 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,7 @@ #include cache.h #include string-list.h +#include run-command.h +#include string-list.h #include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org @@ -33,6 +35,8 @@ static struct trailer_item *first_conf_item; static char *separators = :; +#define TRAILER_ARG_STRING $ARG + static int after_or_end(enum action_where where) { return (where == WHERE_AFTER) || (where == WHERE_END); @@ -78,6 +82,13 @@ static inline int contains_only_spaces(const char *str) return !*s; } +static inline void strbuf_replace(struct strbuf *sb, const char *a, const char *b) +{ + const char *ptr = strstr(sb-buf, a); + if (ptr) + strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b)); +} + static void free_trailer_item(struct trailer_item *item) { free(item-conf.name); @@ -203,6 +214,63 @@ static struct trailer_item *remove_first(struct trailer_item **first) return item; } +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); + result = xstrdup(); + } else + result = strbuf_detach(buf, NULL); + + strbuf_release(cmd); + return result; +} + +static void apply_item_command(struct trailer_item *in_tok, struct trailer_item *arg_tok) +{ + if (arg_tok-conf.command) { + const char *arg; + if (arg_tok-value arg_tok-value[0]) { + arg = arg_tok-value; + } else { + if (in_tok in_tok-value) + arg = xstrdup(in_tok-value); + else + arg = xstrdup(); + } + arg_tok-value = apply_command(arg_tok-conf.command, arg); + free((char *)arg); + } +} + static void apply_arg_if_exists(struct trailer_item *in_tok, struct trailer_item *arg_tok, struct trailer_item *on_tok, @@ -214,16 +282,19 @@ static void apply_arg_if_exists(struct trailer_item *in_tok, free_trailer_item(arg_tok); break; case EXISTS_REPLACE: + apply_item_command(in_tok, arg_tok); add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); remove_from_list(in_tok, in_tok_first, in_tok_last); free_trailer_item(in_tok); break; case EXISTS_ADD: + apply_item_command(in_tok, arg_tok); add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); break; case EXISTS_ADD_IF_DIFFERENT: + apply_item_command(in_tok, arg_tok); if (check_if_different(in_tok, arg_tok, 1)) add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); @@ -231,6 +302,7 @@ static void apply_arg_if_exists(struct trailer_item *in_tok, free_trailer_item(arg_tok); break; case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR: + apply_item_command(in_tok, arg_tok); if (check_if_different(on_tok, arg_tok, 0)) add_arg_to_input_list(on_tok, arg_tok, in_tok_first, in_tok_last); @@ -254,6 +326,7 @@ static void apply_arg_if_missing(struct trailer_item **in_tok_first, case MISSING_ADD
Re: [PATCH RFC] git-am: support any number of signatures
Hi Michael, On Mon, Sep 22, 2014 at 4:01 PM, Michael S. Tsirkin m...@redhat.com wrote: Hi Junio, Christian, it's been a while. I see that the work on trailers is going on. I tried going over the documentation but I could not figure out how would one implement multiple signatures using the trailers mechanism. As a reminder, this old patchset (that I replied to) enhanced git am -s with an option to add different signatures depending on the option passed to the -s flag. E.g. I have [am a] signoff = Acked-by: Michael S. Tsirkin m...@redhat.com [am r] signoff = Reviewed-by: Michael S. Tsirkin m...@redhat.com [am t] signoff = Tested-by: Michael S. Tsirkin m...@redhat.com and now: git am -s art adds all 3 signatures when applying the patch. This is probably not as simple as you would like but it works with something like: $ git interpret-trailers --trailer Acked-by: Michael S. Tsirkin m...@redhat.com --trailer Reviewed-by: Michael S. Tsirkin m...@redhat.com --trailer Tested-by: Michael S. Tsirkin m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch and then: $ git am to_apply/*.patch Also by using something like: $ git config trailer.a.key Acked-by $ git config trailer.r.key Reviewed-by $ git config trailer.t.key Tested-by the first command could be simplified to: $ git interpret-trailers --trailer a: Michael S. Tsirkin m...@redhat.com --trailer r: Michael S. Tsirkin m...@redhat.com --trailer t: Michael S. Tsirkin m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch And if you use an env variable: $ ME=Michael S. Tsirkin m...@redhat.com $ git interpret-trailers --trailer a: $ME --trailer r: $ME --trailer t: $ME 0001-foo.patch to_apply/0001-foo.patch Maybe later we will integrate git interpret-trailers with git commit, git am and other commands, so that you can do directly: git am --trailer a: $ME --trailer r: $ME --trailer t: $ME 0001-foo.patch Maybe we wil also assign a one letter shortcut to --trailer, for example z, so that could be: git am -z a: $ME -z r: $ME -z t: $ME 0001-foo.patch We could also allow many separators in the same -z argument as long as they are separated by say ~, so you could have: git am -z a: $ME~r: $ME~t: $ME 0001-foo.patch And then we could also allow people to define default values for trailers with something like: $ git config trailer.a.defaultvalue Michael S. Tsirkin m...@redhat.com $ git config trailer.r.defaultvalue Michael S. Tsirkin m...@redhat.com $ git config trailer.t.defaultvalue Michael S. Tsirkin m...@redhat.com So that in the end you could have: git am -z a~r~t 0001-foo.patch which is very close to git am -s art. 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 RFC] git-am: support any number of signatures
On Tue, Sep 23, 2014 at 10:07 AM, Michael S. Tsirkin m...@redhat.com wrote: On Tue, Sep 23, 2014 at 09:45:50AM +0200, Christian Couder wrote: This is probably not as simple as you would like but it works with something like: $ git interpret-trailers --trailer Acked-by: Michael S. Tsirkin m...@redhat.com --trailer Reviewed-by: Michael S. Tsirkin m...@redhat.com --trailer Tested-by: Michael S. Tsirkin m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch and then: $ git am to_apply/*.patch Also by using something like: $ git config trailer.a.key Acked-by $ git config trailer.r.key Reviewed-by $ git config trailer.t.key Tested-by I would like multiple keys to match a specific letter, e.g. as a maintainer I need both reviewed by and signed off by when I apply a patch, I like applying them with a single -s m. That's different from what you implemented in your patch. And franckly I think that for this kind of specific use cases, you could create your own aliases, either Git aliases or just shell aliases. For example if we implement default values and make git am call git interpret-trailers, a shell alias could simply be: alias gamsm='git am --trailer r --trailer s' I use git log --oneline --decorate --graph very often, so I made my own alias for it, and I suppose a lot of other people have done so. The number of people who will use trailers will probably be much smaller than the number of people using git log, so if we don't make shortcuts for git log --oneline --decorate --graph, I see no ground to ask for a specific shortcut that adds both a reviewed by and a signed off by. the first command could be simplified to: $ git interpret-trailers --trailer a: Michael S. Tsirkin m...@redhat.com --trailer r: Michael S. Tsirkin m...@redhat.com --trailer t: Michael S. Tsirkin m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch And if you use an env variable: $ ME=Michael S. Tsirkin m...@redhat.com $ git interpret-trailers --trailer a: $ME --trailer r: $ME --trailer t: $ME 0001-foo.patch to_apply/0001-foo.patch Maybe later we will integrate git interpret-trailers with git commit, git am and other commands, so that you can do directly: git am --trailer a: $ME --trailer r: $ME --trailer t: $ME 0001-foo.patch Maybe we wil also assign a one letter shortcut to --trailer, for example z, so that could be: git am -z a: $ME -z r: $ME -z t: $ME 0001-foo.patch -s could apply here, right? I don't know what we will do with -s. Maybe if we use -z, we don't need -s. It doesn't have a parameter at the moment. We will have to discuss that kind of thing when we make it possible for git commit, git am and maybe other commands to accept trailers arguments and pass them to git interpret-trailers. In his email Junio seems to say that we don't need a shortcut like -z, we could only have --trailer. And I think that it is indeed sound to at least wait a little before using up one shortcut like -z in many commands. We could also allow many separators in the same -z argument as long as they are separated by say ~, I think -z a -z r -z t is enough. Great! I think you will likely have at least --trailer a --trailer r --trailer t, but I don't think it is too bad as you can use aliases to make it shorter to type. so you could have: git am -z a: $ME~r: $ME~t: $ME 0001-foo.patch And then we could also allow people to define default values for trailers with something like: $ git config trailer.a.defaultvalue Michael S. Tsirkin m...@redhat.com $ git config trailer.r.defaultvalue Michael S. Tsirkin m...@redhat.com $ git config trailer.t.defaultvalue Michael S. Tsirkin m...@redhat.com I'm kind of confused by the key/value concept. A defaultvalue would be the value used when no value is passed. The key is just what we will use in the first part of the trailer (the part before the separator). For example with the above defaultvalue and key, --trailer a: Junio gits...@pobox.com would add: Acked-by: Junio gits...@pobox.com while --trailer a would add: Acked-by: Michael S. Tsirkin m...@redhat.com Can't I define the whole 'Acked-by: Michael S. Tsirkin m...@redhat.com' string as the key? The whole 'Acked-by: Michael S. Tsirkin m...@redhat.com' is a full trailer, not a key. And it is not possible right now to define a full trailer. Maybe we could find a way to make it possible, but a default value and a way to have a small nickname for the token (like a for Acked-by) should get people quite far. And then for very specific use cases, it may be better to use aliases anyway. So that in the end you could have: git am -z a~r~t 0001-foo.patch which is very close to git am -s art. If I figure out the defaultvalue thing, I might find the time to work on git am integration. That would be great! Thanks, 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
Re: [PATCH RFC] git-am: support any number of signatures
On Tue, Sep 23, 2014 at 7:15 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder christian.cou...@gmail.com writes: This is probably not as simple as you would like but it works with something like: $ git interpret-trailers --trailer Acked-by: Michael S. Tsirkin m...@redhat.com --trailer Reviewed-by: Michael S. Tsirkin m...@redhat.com --trailer Tested-by: Michael S. Tsirkin m...@redhat.com 0001-foo.patch to_apply/0001-foo.patch and then: $ git am to_apply/*.patch If I understand it correctly, Michael is envisioning to implement his git am -s art (I would recommend against reusing -s for this, though. git am --trailer art is fine) and doing so by using interpret-trailers as an internal implementation detail, so I would say the above is a perfectly fine way to do so. An equivalent of that command line is synthesized and run internally in his version of git am when his git am sees --trailer art option using those am.{a,r,t}.trailer configuration variables. Yeah, that's the idea, except that I think --trailer art should mean a trailer like: art: default value (if there is no trailer.art.key config variable defined). Having am.{a,r,t}.trailer configuration variables to define full trailers seems too specific and quite confusing regarding how git interpret-trailers work without those variables. Also by using something like: $ git config trailer.a.key Acked-by $ git config trailer.r.key Reviewed-by $ git config trailer.t.key Tested-by the first command could be simplified to: So I think this mechanism buys Michael's use case very little, if any. It might be useful in other contexts, though. What would be more interesting is if the primitives you have, e.g. replace, append, etc. are sufficient to express his use case and similar ones. For example, when working on multiple trailers (e.g. am --trailer art would muck with three kinds), how should do this if exists at the end and do that otherwise work? The way the trailer.foo.ifexists and trailer.foo.where work is quite orthogonal to the way we decide what the content of the trailer is. If we make --trailer art mean --trailer Acked-by: Michael --trailer Reviewed-by: Michael --trailer Tested-by: Michael, then it should work as if we had passed the latter to the command line. To an existing message ends with Michael's Signed-off-by:, if his git am --trailer arts is called to add these three and then a Signed-off-by: from him, should it add an extra S-o-b (because his existing S-o-b will no longer be the last one after adding Acked and others), or should it refrain from doing so? Can you express both preferences? The default for trailer.where is end, and for trailer.ifexists it is addIfDifferentNeighbor. That means that by default it will add the four new trailers at the end. If either trailer.ifexists or trailer.S-o-b.ifexists is set to addIfDifferent, then only the first 3 new trailers will be added at the end. So yes you can express both preferences. Another thing that got me wondered this morning while I was thinking about this topic was if replace is flexible enough. We may want to have if an entry exists (not necessarily at the end), remove it and then append a new one with this value at the end to implement Last-tested-by: me@my.domain, for example. That's what replace does already. That's why I changed the previous name for this option from overwrite to replace. You have an overwrite behavior with where = after and ifexist = replace, and you have a remove old one and append new one behavior with where = end and ifexist = replace. Thanks, 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 RFC] git-am: support any number of signatures
From: Junio C Hamano gits...@pobox.com On Thu, Sep 25, 2014 at 3:04 AM, Christian Couder christian.cou...@gmail.com wrote: To an existing message ends with Michael's Signed-off-by:, if his git am --trailer arts is called to add these three and then a Signed-off-by: from him, should it add an extra S-o-b (because his existing S-o-b will no longer be the last one after adding Acked and others), or should it refrain from doing so? Can you express both preferences? The default for trailer.where is end, and for trailer.ifexists it is addIfDifferentNeighbor. That means that by default it will add the four new trailers at the end. If either trailer.ifexists or trailer.S-o-b.ifexists is set to addIfDifferent, then only the first 3 new trailers will be added at the end. So yes you can express both preferences. Suppose that the original ends with Sob, and Michael's am invokes interpret-trailers with Acked/Reviewed/Tested/Sob in this order. The first three are set to addifDifferent and Sob is set to addIfDifferentNeighbor, which would be the traditional and sensible default. Now, how is addIfDifferentNeighbor interpreted? Adding the new Sob with this single request to add these four is done on a message that has the same Sob at the end (i.e. Neighbor). Does _you_ adding of Acked/Reviewed/Tested invalidate the Neighbor-ness and you add the Sob anew? The trailers are processed in the order they appear on the command line. And when a trailer is processed, the input message it sees is the result from the processing of all the previous trailers on the command line. It is not any more the original input message. So after Acked/Reviewed/Tested have been added, when the S-o-b at the end of the command line is processed, it sees an input message that has the Tested trailer at the end. That's why with addIfDifferentNeighbor the S-o-b will still be added at the end. If that is what happens, it is not a workable workaround to set Sob to addIfDifferent only for this invocation. Setting S-o-b to addIfDifferent for this invocation would not add the S-o-b at the end, because another S-o-b still exists in the input message seen when the last S-o-b is processed. Alternatively, if Neighbor-ness is evaluated first before you add A/R/T in response to this request, then you'd refrain from adding a duplicate Sob. It wasn't quite clear from your description what your design was, and your explanation above is not still clear, at least to me. I hope it is clearer now. Maybe I should add something in the doc to better explain how it works. Thanks, 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 RFC] git-am: support any number of signatures
On Sun, Sep 28, 2014 at 9:32 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder chrisc...@tuxfamily.org writes: From: Junio C Hamano gits...@pobox.com If that is what happens, it is not a workable workaround to set Sob to addIfDifferent only for this invocation. Setting S-o-b to addIfDifferent for this invocation would not add the S-o-b at the end, because another S-o-b still exists in the input message seen when the last S-o-b is processed. So there is no workaround whatsoever, which is worse X-. I think there might be a misunderstanding. With v16 you can easily choose if you want to have the S-o-b in the output or not, when there is already one, see: $ cat test.txt Signed-off-by: Michael $ cat test.txt | git interpret-trailers --trailer 'Acked-by: Michael' --trailer 'Reviewed-by: Michael' --trailer 'Tested-by: Michael' --trailer 'Signed-off-by: Michael' Signed-off-by: Michael Acked-by: Michael Reviewed-by: Michael Tested-by: Michael Signed-off-by: Michael $ cat test.txt | git -c 'trailer.ifexists=addIfDifferent' interpret-trailers --trailer 'Acked-by: Michael' --trailer 'Reviewed-by: Michael' --trailer 'Tested-by: Michael' --trailer 'Signed-off-by: Michael' Signed-off-by: Michael Acked-by: Michael Reviewed-by: Michael Tested-by: Michael Or: $ cat test.txt | git -c 'trailer.Signed-off-by.ifexists=addIfDifferent' interpret-trailers --trailer 'Acked-by: Michael' --trailer 'Reviewed-by: Michael' --trailer 'Tested-by: Michael' --trailer 'Signed-off-by: Michael' Signed-off-by: Michael Acked-by: Michael Reviewed-by: Michael Tested-by: Michael (There was a small bug in v15 with the last command above.) Alternatively, if Neighbor-ness is evaluated first before you add A/R/T in response to this request, then you'd refrain from adding a duplicate Sob. It wasn't quite clear from your description what your design was, and your explanation above is not still clear, at least to me. I hope it is clearer now. Maybe I should add something in the doc to better explain how it works. I doubt that it would help the users materially to document that we chose to implement a less useful way when there are multiple ways in which a feature can work, though. Unless I am mis-reading you and you are actually saying that the users can emulate the atomic variant without much hassle by doing X and Y, that is. If so, it would help readers to document them. If you would like me to document the 3 above commands in an example, I am ok to do that. Thanks, 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 RFC] git-am: support any number of signatures
On Sun, Oct 12, 2014 at 11:36 AM, Christian Couder christian.cou...@gmail.com wrote: With v16 you can easily choose if you want to have the S-o-b in the output or not, when there is already one, ... By the way, I sent v16 just before the above email, but the series still hasn't hit the mailing list. Did some of you guys in cc receive something? -- 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 v16 03/11] trailer: read and process config information
Read the configuration to get trailer information, and then process it and store 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 185 ++ 1 file changed, 185 insertions(+) diff --git a/trailer.c b/trailer.c index be0ad65..668dc33 100644 --- a/trailer.c +++ b/trailer.c @@ -277,3 +277,188 @@ static void process_trailers_lists(struct trailer_item **in_tok_first, 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 if (!strcasecmp(end, value)) + item-where = WHERE_END; + else if (!strcasecmp(start, value)) + item-where = WHERE_START; + else + return -1; + return 0; +} + +static int set_if_exists(struct conf_info *item, const char *value) +{ + if (!strcasecmp(addIfDifferent, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT; + else if (!strcasecmp(addIfDifferentNeighbor, value)) + item-if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR; + else if (!strcasecmp(add, value)) + item-if_exists = EXISTS_ADD; + else if (!strcasecmp(replace, value)) + item-if_exists = EXISTS_REPLACE; + else if (!strcasecmp(doNothing, value)) + item-if_exists = EXISTS_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; +} + +static void duplicate_conf(struct conf_info *dst, struct conf_info *src) +{ + *dst = *src; + if (src-name) + dst-name = xstrdup(src-name); + if (src-key) + dst-key = xstrdup(src-key); + if (src-command) + dst-command = xstrdup(src-command); +} + +static struct trailer_item *get_conf_item(const 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); + duplicate_conf(item-conf, default_conf_info); + item-conf.name = xstrdup(name); + + if (!previous) + first_conf_item = item; + else { + previous-next = item; + item-previous = previous; + } + + return item; +} + +enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE, +TRAILER_IF_EXISTS, TRAILER_IF_MISSING }; + +static struct { + const char *name; + enum trailer_info_type type; +} trailer_config_items[] = { + { key, TRAILER_KEY }, + { command, TRAILER_COMMAND }, + { where, TRAILER_WHERE }, + { ifexists, TRAILER_IF_EXISTS }, + { ifmissing, TRAILER_IF_MISSING } +}; + +static int git_trailer_default_config(const char *conf_key, const char *value, void *cb) +{ + const char *trailer_item, *variable_name; + + if (!skip_prefix(conf_key, trailer., trailer_item)) + return 0; + + variable_name = strrchr(trailer_item, '.'); + if (!variable_name) { + if (!strcmp(trailer_item, where)) { + if (set_where(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifexists)) { + if (set_if_exists(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, ifmissing)) { + if (set_if_missing(default_conf_info, value) 0) + warning(_(unknown value '%s' for key '%s'), + value, conf_key); + } else if (!strcmp(trailer_item, separators)) { + separators = xstrdup(value); + } + } + return 0
[PATCH v16 02/11] trailer: process trailers from input message and arguments
Implement the logic to process trailers from the input message and from arguments. At the beginning trailers from the input message are in their own in_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 in_tok list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 210 ++ 1 file changed, 210 insertions(+) diff --git a/trailer.c b/trailer.c index ac323b1..be0ad65 100644 --- a/trailer.c +++ b/trailer.c @@ -67,3 +67,213 @@ static int same_trailer(struct trailer_item *a, struct trailer_item *b) { return same_token(a, b) same_value(a, b); } + +static void free_trailer_item(struct trailer_item *item) +{ + free(item-conf.name); + free(item-conf.key); + free(item-conf.command); + free((char *)item-token); + free((char *)item-value); + free(item); +} + +static void update_last(struct trailer_item **last) +{ + if (*last) + while ((*last)-next != NULL) + *last = (*last)-next; +} + +static void update_first(struct trailer_item **first) +{ + if (*first) + while ((*first)-previous != NULL) + *first = (*first)-previous; +} + +static void add_arg_to_input_list(struct trailer_item *on_tok, + struct trailer_item *arg_tok, + struct trailer_item **first, + struct trailer_item **last) +{ + if (after_or_end(arg_tok-conf.where)) { + arg_tok-next = on_tok-next; + on_tok-next = arg_tok; + arg_tok-previous = on_tok; + if (arg_tok-next) + arg_tok-next-previous = arg_tok; + update_last(last); + } else { + arg_tok-previous = on_tok-previous; + on_tok-previous = arg_tok; + arg_tok-next = on_tok; + if (arg_tok-previous) + arg_tok-previous-next = arg_tok; + update_first(first); + } +} + +static int check_if_different(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + int check_all) +{ + enum action_where where = arg_tok-conf.where; + do { + if (!in_tok) + return 1; + if (same_trailer(in_tok, arg_tok)) + return 0; + /* +* if we want to add a trailer after another one, +* we have to check those before this one +*/ + in_tok = after_or_end(where) ? in_tok-previous : in_tok-next; + } while (check_all); + return 1; +} + +static void remove_from_list(struct trailer_item *item, +struct trailer_item **first, +struct trailer_item **last) +{ + struct trailer_item *next = item-next; + struct trailer_item *previous = item-previous; + + if (next) { + item-next-previous = previous; + item-next = NULL; + } else if (last) + *last = previous; + + if (previous) { + item-previous-next = next; + item-previous = NULL; + } else if (first) + *first = 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 apply_arg_if_exists(struct trailer_item *in_tok, + struct trailer_item *arg_tok, + struct trailer_item *on_tok, + struct trailer_item **in_tok_first, + struct trailer_item **in_tok_last) +{ + switch (arg_tok-conf.if_exists) { + case EXISTS_DO_NOTHING: + free_trailer_item(arg_tok); + break; + case EXISTS_REPLACE: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + remove_from_list(in_tok, in_tok_first, in_tok_last); + free_trailer_item(in_tok); + break; + case EXISTS_ADD: + add_arg_to_input_list(on_tok, arg_tok, + in_tok_first, in_tok_last); + break; + case EXISTS_ADD_IF_DIFFERENT: + if (check_if_different(in_tok, arg_tok, 1)) + add_arg_to_input_list(on_tok, arg_tok
[PATCH v16 04/11] trailer: process command line trailer arguments
Parse the trailer command line arguments and put the result into an arg_tok doubly linked list. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 112 ++ 1 file changed, 112 insertions(+) diff --git a/trailer.c b/trailer.c index 668dc33..b5666b3 100644 --- a/trailer.c +++ b/trailer.c @@ -1,4 +1,5 @@ #include cache.h +#include string-list.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -462,3 +463,114 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb) } return 0; } + +static int parse_trailer(struct strbuf *tok, struct strbuf *val, const char *trailer) +{ + size_t len; + struct strbuf seps = STRBUF_INIT; + strbuf_addstr(seps, separators); + strbuf_addch(seps, '='); + len = strcspn(trailer, seps.buf); + strbuf_release(seps); + if (len == 0) + return error(_(empty trailer token in trailer '%s'), trailer); + if (len strlen(trailer)) { + strbuf_add(tok, trailer, len); + strbuf_trim(tok); + strbuf_addstr(val, trailer + len + 1); + strbuf_trim(val); + } else { + strbuf_addstr(tok, trailer); + strbuf_trim(tok); + } + return 0; +} + +static const char *token_from_item(struct trailer_item *item, char *tok) +{ + if (item-conf.key) + return item-conf.key; + if (tok) + return tok; + return item-conf.name; +} + +static struct trailer_item *new_trailer_item(struct trailer_item *conf_item, +char *tok, char *val) +{ + struct trailer_item *new = xcalloc(sizeof(*new), 1); + new-value = val; + + if (conf_item) { + duplicate_conf(new-conf, conf_item-conf); + new-token = xstrdup(token_from_item(conf_item, tok)); + free(tok); + } else { + duplicate_conf(new-conf, default_conf_info); + new-token = tok; + } + + return new; +} + +static int token_matches_item(const char *tok, struct trailer_item *item, int tok_len) +{ + if (!strncasecmp(tok, item-conf.name, tok_len)) + return 1; + return item-conf.key ? !strncasecmp(tok, item-conf.key, tok_len) : 0; +} + +static struct trailer_item *create_trailer_item(const char *string) +{ + struct strbuf tok = STRBUF_INIT; + struct strbuf val = STRBUF_INIT; + struct trailer_item *item; + int tok_len; + + if (parse_trailer(tok, val, string)) + return NULL; + + tok_len = token_len_without_separator(tok.buf, tok.len); + + /* Lookup if the token matches something in the config */ + for (item = first_conf_item; item; item = item-next) { + if (token_matches_item(tok.buf, item, tok_len)) + return new_trailer_item(item, + strbuf_detach(tok, NULL), + strbuf_detach(val, NULL)); + } + + return new_trailer_item(NULL, + strbuf_detach(tok, NULL), + strbuf_detach(val, NULL)); +} + +static void add_trailer_item(struct trailer_item **first, +struct trailer_item **last, +struct trailer_item *new) +{ + if (!new) + return; + if (!*last) { + *first = new; + *last = new; + } else { + (*last)-next = new; + new-previous = *last; + *last = new; + } +} + +static struct trailer_item *process_command_line_args(struct string_list *trailers) +{ + struct trailer_item *arg_tok_first = NULL; + struct trailer_item *arg_tok_last = NULL; + struct string_list_item *tr; + + for_each_string_list_item(tr, trailers) { + struct trailer_item *new = create_trailer_item(tr-string); + add_trailer_item(arg_tok_first, arg_tok_last, new); + } + + return arg_tok_first; +} -- 2.1.0.rc0.248.gb91fdbc -- 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 v16 01/11] trailer: add data structures and basic functions
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 Signed-off-by: Junio C Hamano gits...@pobox.com --- Makefile | 1 + trailer.c | 69 +++ 2 files changed, 70 insertions(+) create mode 100644 trailer.c diff --git a/Makefile b/Makefile index 63a210d..ef82972 100644 --- a/Makefile +++ b/Makefile @@ -888,6 +888,7 @@ LIB_OBJS += submodule-config.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..ac323b1 --- /dev/null +++ b/trailer.c @@ -0,0 +1,69 @@ +#include cache.h +/* + * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org + */ + +enum action_where { WHERE_END, WHERE_AFTER, WHERE_BEFORE, WHERE_START }; +enum action_if_exists { EXISTS_ADD_IF_DIFFERENT_NEIGHBOR, EXISTS_ADD_IF_DIFFERENT, + EXISTS_ADD, EXISTS_REPLACE, EXISTS_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_exists if_exists; + enum action_if_missing if_missing; +}; + +static struct conf_info default_conf_info; + +struct trailer_item { + struct trailer_item *previous; + struct trailer_item *next; + const char *token; + const char *value; + struct conf_info conf; +}; + +static struct trailer_item *first_conf_item; + +static char *separators = :; + +static int after_or_end(enum action_where where) +{ + return (where == WHERE_AFTER) || (where == WHERE_END); +} + +/* + * Return the length of the string not including any final + * punctuation. E.g., the input Signed-off-by: would return + * 13, stripping the trailing punctuation but retaining + * internal punctuation. + */ +static size_t token_len_without_separator(const char *token, size_t len) +{ + while (len 0 !isalnum(token[len - 1])) + len--; + return len; +} + +static int same_token(struct trailer_item *a, struct trailer_item *b) +{ + size_t a_len = token_len_without_separator(a-token, strlen(a-token)); + size_t b_len = token_len_without_separator(b-token, strlen(b-token)); + size_t min_len = (a_len b_len) ? b_len : a_len; + + return !strncasecmp(a-token, b-token, min_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) +{ + return same_token(a, b) same_value(a, b); +} -- 2.1.0.rc0.248.gb91fdbc -- 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 v16 10/11] trailer: add tests for commands in config file
And add a few other tests for some special cases. Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t7513-interpret-trailers.sh | 125 ++ 1 file changed, 125 insertions(+) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index ad36cf8..fee41e8 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -735,4 +735,129 @@ test_expect_success 'default where is now after' ' 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \A U Thor aut...@example.com\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=22 \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with command using commiter information' ' + git config trailer.sign.ifExists addIfDifferent + git config trailer.sign.command echo \\$GIT_COMMITTER_NAME \$GIT_COMMITTER_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: C O Mitter commit...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists addIfDifferentNeighbor + git config trailer.sign.command echo \\$GIT_AUTHOR_NAME \$GIT_AUTHOR_EMAIL\ + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer 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.ifExists replace + 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 + sed -e s/ Z\$/ / expected -EOF + Fixes: $FIXED + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with failing command using $ARG' ' + git config trailer.fix.ifExists replace + git config trailer.fix.command false \$ARG + cat complex_message_body expected + sed -e s/ Z\$/ / expected -EOF + Fixes: Z + Acked-by= Z + Reviewed-by: + Signed-off-by: Z + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review: --trailer fix=HEAD \ + complex_message actual + test_cmp expected actual +' + +test_expect_success 'with empty tokens' ' + git config --unset trailer.fix.command + cat expected -EOF + + Signed-off-by: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer : --trailer :test actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with command but no key' ' + git config --unset trailer.sign.key + cat expected -EOF + + sign: A U Thor aut...@example.com + EOF + git interpret-trailers actual -EOF + EOF + test_cmp expected actual +' + +test_expect_success 'with no command and no key' ' + git config --unset trailer.review.key + cat expected -EOF + + review: Junio + sign: A U Thor aut...@example.com + EOF + git interpret-trailers --trailer review:Junio actual -EOF + EOF + test_cmp expected
[PATCH v16 00/11] Add interpret-trailers builtin
[Sorry to resend this v16, but the series didn't make it to list the first time...] 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 keep builtin/commit.c uncontaminated by any more hard-wired logic, like what we have for the signed-off-by line. Any new things can and should be doable in hooks, and this filter would help writing these hooks. And that is why the design goal of the filter is to make it at least as powerful as the built-in logic we have for signed-off-by lines; that would allow us to later eject the hard-wired logic for signed-off-by line from the main codepath, if/when we wanted to. Alternatively, we could build a library-ish API around this filter code and replace the hard-wired logic for signed-off-by line with a call into that API, if/when we wanted to, but that requires (in addition to the at least as powerful as the built-in logic) that the implementation of this stand-alone filter can be cleanly made into a reusable library, so that is a bit higher bar to cross than everything can be doable with hooks alternative. 2) Current state Currently the usage string of this command is: git interpret-trailers [--trim-empty] [(--trailer token[(=|:)value])...] [file...] The following features are implemented: - the result is printed on stdout - the --trailer arguments are interpreted - messages read from file... or stdin are interpreted - the trailer.separators option in the config is interpreted - the trailer.where option is interpreted - the trailer.ifexists option is interpreted - the trailer.ifmissing option is interpreted - the trailer.token.key options are interpreted - the trailer.token.where options are interpreted - the trailer.token.ifexist options are interpreted - the trailer.token.ifmissing options are interpreted - the trailer.token.command config works - $ARG can be used in commands - messages can contain a patch - lines in messages starting with a comment char are ignored - there are 50 tests - there is some documentation - there are examples in the documentation 3) Changes since version 15, thanks to Michael S. T. and Junio * avoid trailing whitespaces in the documentation by using sed (patch 11/11) * fix a bug when a config option is passed on the command line (patch 4/11 and 8/11) Christian Couder (11): trailer: add data structures and basic functions trailer: process trailers from input message and arguments trailer: read and process config information trailer: process command line trailer arguments trailer: parse trailers from file or stdin trailer: put all the processing together and print trailer: add interpret-trailers command trailer: add tests for git interpret-trailers trailer: execute command from 'trailer.name.command' trailer: add tests for commands in config file Documentation: add documentation for 'git interpret-trailers' .gitignore | 1 + Documentation/git-interpret-trailers.txt | 314 +++ Makefile | 2 + builtin.h| 1 + builtin/interpret-trailers.c | 44 ++ command-list.txt | 1 + git.c| 1 + t/t7513-interpret-trailers.sh| 863 +++ trailer.c| 851 ++ trailer.h| 6 + 10 files changed, 2084 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt 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 -- 2.1.0.rc0.248.gb91fdbc -- 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 v16 06/11] 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 Signed-off-by: Junio C Hamano gits...@pobox.com --- trailer.c | 69 +++ trailer.h | 6 ++ 2 files changed, 75 insertions(+) create mode 100644 trailer.h diff --git a/trailer.c b/trailer.c index 4f0de3b..b0be0d7 100644 --- a/trailer.c +++ b/trailer.c @@ -1,5 +1,6 @@ #include cache.h #include string-list.h +#include trailer.h /* * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org */ @@ -87,6 +88,35 @@ static void free_trailer_item(struct trailer_item *item) free(item); } +static char last_non_space_char(const char *s) +{ + int i; + for (i = strlen(s) - 1; i = 0; i--) + if (!isspace(s[i])) + return s[i]; + return '\0'; +} + +static void print_tok_val(const char *tok, const char *val) +{ + char c = last_non_space_char(tok); + if (!c) + return; + if (strchr(separators, c)) + printf(%s%s\n, tok, val); + else + printf(%s%c %s\n, tok, separators[0], 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 update_last(struct trailer_item **last) { if (*last) @@ -697,3 +727,42 @@ static int process_input_file(struct strbuf **lines, return patch_start; } + +static void free_all(struct trailer_item **first) +{ + while (*first) { + struct trailer_item *item = remove_first(first); + free_trailer_item(item); + } +} + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers) +{ + struct trailer_item *in_tok_first = NULL; + struct trailer_item *in_tok_last = NULL; + struct trailer_item *arg_tok_first; + struct strbuf **lines; + int patch_start; + + /* Default config must be setup first */ + git_config(git_trailer_default_config, NULL); + git_config(git_trailer_config, NULL); + + lines = read_input_file(file); + + /* Print the lines before the trailers */ + patch_start = process_input_file(lines, in_tok_first, in_tok_last); + + arg_tok_first = process_command_line_args(trailers); + + process_trailers_lists(in_tok_first, in_tok_last, arg_tok_first); + + print_all(in_tok_first, trim_empty); + + free_all(in_tok_first); + + /* Print the lines after the trailers as is */ + print_lines(lines, patch_start, INT_MAX); + + strbuf_list_free(lines); +} diff --git a/trailer.h b/trailer.h new file mode 100644 index 000..8eb25d5 --- /dev/null +++ b/trailer.h @@ -0,0 +1,6 @@ +#ifndef TRAILER_H +#define TRAILER_H + +void process_trailers(const char *file, int trim_empty, struct string_list *trailers); + +#endif /* TRAILER_H */ -- 2.1.0.rc0.248.gb91fdbc -- 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