From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
This patch series implements a new command:
git interpret-trailers
and an infrastructure to process trailers that can be reused,
for example in "commit.c".
1) Rationale:
This command should help with RFC 822 style headers, called
"trailers", that are found at the end of commit messages.
(Note that these headers do not follow and are not intended to
follow many rules that are in RFC 822. For example they do not
follow the line breaking rules, the encoding rules and probably
many other rules.)
For a long time, these trailers have become a de facto standard
way to add helpful information into commit messages.
Until now git commit has only supported the well known
"Signed-off-by: " trailer, that is used by many projects like
the Linux kernel and Git.
It is better to implement features for these trailers first in a
new command rather than in builtin/commit.c, because this way the
prepare-commit-msg and commit-msg hooks can reuse this command.
2) Current state:
Currently the usage string of this command is:
git interpret-trailers [--trim-empty] [--infile=<file>] [(<token>[(=|:)<value>])...]
The following features are implemented:
- the result is printed on stdout
- the [<token>[=<value>]>] arguments are interpreted
- a commit message passed using the "--infile=file" option is interpreted
- if "--infile" is not used, a commit message is read from stdin
- the "trailer.<token>.key" options in the config are interpreted
- the "trailer.<token>.where" options are interpreted
- the "trailer.<token>.ifExist" options are interpreted
- the "trailer.<token>.ifMissing" options are interpreted
- the "trailer.<token>.command" config works
- $ARG can be used in commands
- ditto for GIT_{AUTHOR,COMMITTER}_{NAME,EMAIL} env variables
- there are some tests
- there is some documentation
The following features are planned but not yet implemented:
- add more tests related to commands
- add examples in documentation
- integration with "git commit"
Possible improvements:
- support GIT_COMMIT_PROTO env variable in commands
3) Changes since version 3, thanks to Eric and Junio:
* the usage string/synopsis of the command was improved
* some spelling/wording mistakes in the doc were fixed
* some style issues were fixed
Christian Couder (17):
Add data structures and basic functions for commit trailers
trailer: process trailers from file and arguments
trailer: read and process config information
trailer: process command line trailer arguments
strbuf: add strbuf_isspace()
trailer: parse trailers from input file
trailer: put all the processing together and print
trailer: add interpret-trailers command
trailer: add tests for "git interpret-trailers"
trailer: if no input file is passed, read from stdin
trailer: add new_trailer_item() function
strbuf: add strbuf_replace()
trailer: execute command from 'trailer.<name>.command'
trailer: add tests for trailer command
trailer: set author and committer env variables
trailer: add tests for commands using env variables
Documentation: add documentation for 'git interpret-trailers'
.gitignore | 1 +
Documentation/git-interpret-trailers.txt | 132 +++++++
Makefile | 2 +
builtin.h | 1 +
builtin/interpret-trailers.c | 36 ++
git.c | 1 +
strbuf.c | 14 +
strbuf.h | 4 +
t/t7513-interpret-trailers.sh | 262 +++++++++++++
trailer.c | 637 +++++++++++++++++++++++++++++++
trailer.h | 6 +
11 files changed, 1096 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
--
1.8.5.2.201.gacc5987
From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
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 <redacted>
---
Makefile | 1 +
trailer.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
create mode 100644 trailer.c
@@ -0,0 +1,48 @@+#include"cache.h"+/*+*Copyright(c)2013ChristianCouder<chriscool@tuxfamily.org>+*/++enumaction_where{WHERE_AFTER,WHERE_BEFORE};+enumaction_if_exist{EXIST_ADD_IF_DIFFERENT,EXIST_ADD_IF_DIFFERENT_NEIGHBOR,+EXIST_ADD,EXIST_OVERWRITE,EXIST_DO_NOTHING};+enumaction_if_missing{MISSING_ADD,MISSING_DO_NOTHING};++structconf_info{+char*name;+char*key;+char*command;+enumaction_wherewhere;+enumaction_if_existif_exist;+enumaction_if_missingif_missing;+};++structtrailer_item{+structtrailer_item*previous;+structtrailer_item*next;+constchar*token;+constchar*value;+structconf_info*conf;+};++staticintsame_token(structtrailer_item*a,structtrailer_item*b,intalnum_len)+{+return!strncasecmp(a->token,b->token,alnum_len);+}++staticintsame_value(structtrailer_item*a,structtrailer_item*b)+{+return!strcasecmp(a->value,b->value);+}++staticintsame_trailer(structtrailer_item*a,structtrailer_item*b,intalnum_len)+{+returnsame_token(a,b,alnum_len)&&same_value(a,b);+}++/* Get the length of buf from its beginning until its last alphanumeric character */+staticsize_talnum_len(constchar*buf,size_tlen)+{+while(--len>=0&&!isalnum(buf[len]));+returnlen+1;+}
From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
This patch implements the logic that process trailers
from file and arguments.
At the beginning trailers from file are in their own
infile_tok doubly linked list, and trailers from
arguments are in their own arg_tok doubly linked list.
The lists are traversed and when an arg_tok should be
"applied", it is removed from its list and inserted
into the infile_tok list.
Signed-off-by: Christian Couder <redacted>
---
trailer.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 189 insertions(+)
@@ -46,3 +46,192 @@ static size_t alnum_len(const char *buf, size_t len)while(--len>=0&&!isalnum(buf[len]));returnlen+1;}++staticvoidadd_arg_to_infile(structtrailer_item*infile_tok,+structtrailer_item*arg_tok)+{+if(arg_tok->conf->where==WHERE_AFTER){+arg_tok->next=infile_tok->next;+infile_tok->next=arg_tok;+arg_tok->previous=infile_tok;+if(arg_tok->next)+arg_tok->next->previous=arg_tok;+}else{+arg_tok->previous=infile_tok->previous;+infile_tok->previous=arg_tok;+arg_tok->next=infile_tok;+if(arg_tok->previous)+arg_tok->previous->next=arg_tok;+}+}++staticintcheck_if_different(structtrailer_item*infile_tok,+structtrailer_item*arg_tok,+intalnum_len,intcheck_all)+{+enumaction_wherewhere=arg_tok->conf->where;+do{+if(!infile_tok)+return1;+if(same_trailer(infile_tok,arg_tok,alnum_len))+return0;+/*+*ifwewanttoaddatrailerafteranotherone,+*wehavetocheckthosebeforethisone+*/+infile_tok=(where==WHERE_AFTER)?infile_tok->previous:infile_tok->next;+}while(check_all);+return1;+}++staticvoidapply_arg_if_exist(structtrailer_item*infile_tok,+structtrailer_item*arg_tok,+intalnum_len)+{+switch(arg_tok->conf->if_exist){+caseEXIST_DO_NOTHING:+free(arg_tok);+break;+caseEXIST_OVERWRITE:+free((char*)infile_tok->value);+infile_tok->value=xstrdup(arg_tok->value);+free(arg_tok);+break;+caseEXIST_ADD:+add_arg_to_infile(infile_tok,arg_tok);+break;+caseEXIST_ADD_IF_DIFFERENT:+if(check_if_different(infile_tok,arg_tok,alnum_len,1))+add_arg_to_infile(infile_tok,arg_tok);+else+free(arg_tok);+break;+caseEXIST_ADD_IF_DIFFERENT_NEIGHBOR:+if(check_if_different(infile_tok,arg_tok,alnum_len,0))+add_arg_to_infile(infile_tok,arg_tok);+else+free(arg_tok);+break;+}+}++staticvoidremove_from_list(structtrailer_item*item,+structtrailer_item**first)+{+if(item->next)+item->next->previous=item->previous;+if(item->previous)+item->previous->next=item->next;+else+*first=item->next;+}++staticstructtrailer_item*remove_first(structtrailer_item**first)+{+structtrailer_item*item=*first;+*first=item->next;+if(item->next){+item->next->previous=NULL;+item->next=NULL;+}+returnitem;+}++staticvoidprocess_infile_tok(structtrailer_item*infile_tok,+structtrailer_item**arg_tok_first,+enumaction_wherewhere)+{+structtrailer_item*arg_tok;+structtrailer_item*next_arg;++inttok_alnum_len=alnum_len(infile_tok->token,strlen(infile_tok->token));+for(arg_tok=*arg_tok_first;arg_tok;arg_tok=next_arg){+next_arg=arg_tok->next;+if(same_token(infile_tok,arg_tok,tok_alnum_len)&&+arg_tok->conf->where==where){+/* Remove arg_tok from list */+remove_from_list(arg_tok,arg_tok_first);+/* Apply arg */+apply_arg_if_exist(infile_tok,arg_tok,tok_alnum_len);+/*+*Ifarghasbeenaddedtoinfile,+*thenweneedtoprocessittoonow.+*/+if((where==WHERE_AFTER?infile_tok->next:infile_tok->previous)==arg_tok)+infile_tok=arg_tok;+}+}+}++staticvoidupdate_last(structtrailer_item**last)+{+if(*last)+while((*last)->next!=NULL)+*last=(*last)->next;+}++staticvoidupdate_first(structtrailer_item**first)+{+if(*first)+while((*first)->previous!=NULL)+*first=(*first)->previous;+}++staticvoidapply_arg_if_missing(structtrailer_item**infile_tok_first,+structtrailer_item**infile_tok_last,+structtrailer_item*arg_tok)+{+structtrailer_item**infile_tok;+enumaction_wherewhere;++switch(arg_tok->conf->if_missing){+caseMISSING_DO_NOTHING:+free(arg_tok);+break;+caseMISSING_ADD:+where=arg_tok->conf->where;+infile_tok=(where==WHERE_AFTER)?infile_tok_last:infile_tok_first;+if(*infile_tok){+add_arg_to_infile(*infile_tok,arg_tok);+*infile_tok=arg_tok;+}else{+*infile_tok_first=arg_tok;+*infile_tok_last=arg_tok;+}+break;+}+}++staticvoidprocess_trailers_lists(structtrailer_item**infile_tok_first,+structtrailer_item**infile_tok_last,+structtrailer_item**arg_tok_first)+{+structtrailer_item*infile_tok;+structtrailer_item*arg_tok;++if(!*arg_tok_first)+return;++/* Process infile from end to start */+for(infile_tok=*infile_tok_last;infile_tok;infile_tok=infile_tok->previous){+process_infile_tok(infile_tok,arg_tok_first,WHERE_AFTER);+}++update_last(infile_tok_last);++if(!*arg_tok_first)+return;++/* Process infile from start to end */+for(infile_tok=*infile_tok_first;infile_tok;infile_tok=infile_tok->next){+process_infile_tok(infile_tok,arg_tok_first,WHERE_BEFORE);+}++update_first(infile_tok_first);++/* Process args left */+while(*arg_tok_first){+arg_tok=remove_first(arg_tok_first);+apply_arg_if_missing(infile_tok_first,infile_tok_last,arg_tok);+}+}
@@ -212,4 +212,31 @@ test_expect_success 'with input from stdin' 'test_cmpexpectedactual'+test_expect_success'with simple command''+gitconfigtrailer.sign.key"Signed-off-by: "&&+gitconfigtrailer.sign.where"after"&&+gitconfigtrailer.sign.ifExist"addIfDifferentNeighbor"&&+gitconfigtrailer.sign.command"echo \"A U Thor <author@example.com>\""&&+catcomplex_message_body>expected&&+printf"Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor <author@example.com>\n">>expected&&+gitinterpret-trailers"review:""fix=22"<complex_message>actual&&+test_cmpexpectedactual+'++test_expect_success'setup a commit''+echo"Content of the first commit.">a.txt&&+gitadda.txt&&+gitcommit-m"Add file a.txt"+'++test_expect_success'with command using $ARG''+gitconfigtrailer.fix.ifExist"overwrite"&&+gitconfigtrailer.fix.command"git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 \$ARG"&&+FIXED=$(gitlog-1--oneline--format="%h (%s)"--abbrev-commit--abbrev=14HEAD)&&+catcomplex_message_body>expected&&+printf"Fixes: $FIXED\nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor <author@example.com>\n">>expected&&+gitinterpret-trailers"review:""fix=HEAD"<complex_message>actual&&+test_cmpexpectedactual+'+ test_done
From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
This patch implements reading the configuration
to get trailer information, and then processing
it and storing it in a doubly linked list.
The config information is stored in the list
whose first item is pointed to by:
static struct trailer_item *first_conf_item;
Signed-off-by: Christian Couder <redacted>
---
trailer.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 127 insertions(+)
@@ -235,3 +237,128 @@ static void process_trailers_lists(struct trailer_item **infile_tok_first,apply_arg_if_missing(infile_tok_first,infile_tok_last,arg_tok);}}++staticintset_where(structconf_info*item,constchar*value)+{+if(!strcasecmp("after",value))+item->where=WHERE_AFTER;+elseif(!strcasecmp("before",value))+item->where=WHERE_BEFORE;+else+return1;+return0;+}++staticintset_if_exist(structconf_info*item,constchar*value)+{+if(!strcasecmp("addIfDifferent",value))+item->if_exist=EXIST_ADD_IF_DIFFERENT;+elseif(!strcasecmp("addIfDifferentNeighbor",value))+item->if_exist=EXIST_ADD_IF_DIFFERENT_NEIGHBOR;+elseif(!strcasecmp("add",value))+item->if_exist=EXIST_ADD;+elseif(!strcasecmp("overwrite",value))+item->if_exist=EXIST_OVERWRITE;+elseif(!strcasecmp("doNothing",value))+item->if_exist=EXIST_DO_NOTHING;+else+return1;+return0;+}++staticintset_if_missing(structconf_info*item,constchar*value)+{+if(!strcasecmp("doNothing",value))+item->if_missing=MISSING_DO_NOTHING;+elseif(!strcasecmp("add",value))+item->if_missing=MISSING_ADD;+else+return1;+return0;+}++enumtrailer_info_type{TRAILER_VALUE,TRAILER_COMMAND,TRAILER_WHERE,+TRAILER_IF_EXIST,TRAILER_IF_MISSING};++staticintset_name_and_type(constchar*conf_key,constchar*suffix,+enumtrailer_info_typetype,+char**pname,enumtrailer_info_type*ptype)+{+intret=ends_with(conf_key,suffix);+if(ret){+*pname=xstrndup(conf_key,strlen(conf_key)-strlen(suffix));+*ptype=type;+}+returnret;+}++staticstructtrailer_item*get_conf_item(char*name)+{+structtrailer_item*item;+structtrailer_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))+returnitem;+}++/* Item does not already exists, create it */+item=xcalloc(sizeof(structtrailer_item),1);+item->conf=xcalloc(sizeof(structconf_info),1);+item->conf->name=xstrdup(name);++if(!previous)+first_conf_item=item;+else{+previous->next=item;+item->previous=previous;+}++returnitem;+}++staticintgit_trailer_config(constchar*conf_key,constchar*value,void*cb)+{+if(starts_with(conf_key,"trailer.")){+constchar*orig_conf_key=conf_key;+structtrailer_item*item;+structconf_info*conf;+char*name;+enumtrailer_info_typetype;++conf_key+=8;+if(!set_name_and_type(conf_key,".key",TRAILER_VALUE,&name,&type)&&+!set_name_and_type(conf_key,".command",TRAILER_COMMAND,&name,&type)&&+!set_name_and_type(conf_key,".where",TRAILER_WHERE,&name,&type)&&+!set_name_and_type(conf_key,".ifexist",TRAILER_IF_EXIST,&name,&type)&&+!set_name_and_type(conf_key,".ifmissing",TRAILER_IF_MISSING,&name,&type))+return0;++item=get_conf_item(name);+conf=item->conf;++if(type==TRAILER_VALUE){+if(conf->key)+warning(_("more than one %s"),orig_conf_key);+conf->key=xstrdup(value);+}elseif(type==TRAILER_COMMAND){+if(conf->command)+warning(_("more than one %s"),orig_conf_key);+conf->command=xstrdup(value);+}elseif(type==TRAILER_WHERE){+if(set_where(conf,value))+warning(_("unknown value '%s' for key '%s'"),value,orig_conf_key);+}elseif(type==TRAILER_IF_EXIST){+if(set_if_exist(conf,value))+warning(_("unknown value '%s' for key '%s'"),value,orig_conf_key);+}elseif(type==TRAILER_IF_MISSING){+if(set_if_missing(conf,value))+warning(_("unknown value '%s' for key '%s'"),value,orig_conf_key);+}else+die("internal bug in trailer.c");+}+return0;+}
@@ -223,6 +223,26 @@ test_expect_success 'with simple command' 'test_cmpexpectedactual'+test_expect_success'with command using commiter information''+gitconfigtrailer.sign.ifExist"addIfDifferent"&&+gitconfigtrailer.sign.command"echo \"\$GIT_COMMITTER_NAME <\$GIT_COMMITTER_EMAIL>\""&&+catcomplex_message_body>expected&&+printf"Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: C O Mitter <committer@example.com>\n">>expected&&+gitinterpret-trailers"review:""fix=22"<complex_message>actual&&+test_cmpexpectedactual+'++test_expect_success'with command using author information''+gitconfigtrailer.sign.key"Signed-off-by: "&&+gitconfigtrailer.sign.where"after"&&+gitconfigtrailer.sign.ifExist"addIfDifferentNeighbor"&&+gitconfigtrailer.sign.command"echo \"\$GIT_AUTHOR_NAME <\$GIT_AUTHOR_EMAIL>\""&&+catcomplex_message_body>expected&&+printf"Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \nSigned-off-by: A U Thor <author@example.com>\n">>expected&&+gitinterpret-trailers"review:""fix=22"<complex_message>actual&&+test_cmpexpectedactual+'+ test_expect_success'setup a commit''echo"Content of the first commit.">a.txt&&gitadda.txt&&
From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
It is simpler and more natural if the "git interpret-trailers"
is made a filter as its output already goes to sdtout.
Signed-off-by: Christian Couder <redacted>
---
builtin/interpret-trailers.c | 2 +-
t/t7513-interpret-trailers.sh | 7 +++++++
trailer.c | 15 +++++++++------
3 files changed, 17 insertions(+), 7 deletions(-)
@@ -464,8 +464,13 @@ static struct strbuf **read_input_file(const char *infile){structstrbufsb=STRBUF_INIT;-if(strbuf_read_file(&sb,infile,0)<0)-die_errno(_("could not read input file '%s'"),infile);+if(infile){+if(strbuf_read_file(&sb,infile,0)<0)+die_errno(_("could not read input file '%s'"),infile);+}else{+if(strbuf_read(&sb,fileno(stdin),0)<0)+die_errno(_("could not read from stdin"));+}returnstrbuf_split(&sb,'\n');}
@@ -530,10 +535,8 @@ void process_trailers(const char *infile, int trim_empty, int argc, const char *git_config(git_trailer_config,NULL);-/* Print the non trailer part of infile */-if(infile){-process_input_file(infile,&infile_tok_first,&infile_tok_last);-}+/* Print the non trailer part of infile (or stdin if infile is NULL) */+process_input_file(infile,&infile_tok_first,&infile_tok_last);arg_tok_first=process_command_line_args(argc,argv);
@@ -111,6 +111,9 @@ extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);externvoidstrbuf_splice(structstrbuf*,size_tpos,size_tlen,constvoid*,size_t);+/* first occurence of a replaced with b */+externvoidstrbuf_replace(structstrbuf*,constchar*a,constchar*b);+externvoidstrbuf_add_commented_lines(structstrbuf*out,constchar*buf,size_tsize);externvoidstrbuf_add(structstrbuf*,constvoid*,size_t);
@@ -368,6 +372,7 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)if(conf->command)warning(_("more than one %s"),orig_conf_key);conf->command=xstrdup(value);+conf->command_uses_arg=!!strstr(conf->command,TRAILER_ARG_STRING);}elseif(type==TRAILER_WHERE){if(set_where(conf,value))warning(_("unknown value '%s' for key '%s'"),value,orig_conf_key);
@@ -0,0 +1,132 @@+git-interpret-trailers(1)+=========================++NAME+----+git-interpret-trailers - help add stuctured information into commit messages++SYNOPSIS+--------+[verse]+'git interpret-trailers' [--trim-empty] [--infile=<file>] [(<token>[(=|:)<value>])...]++DESCRIPTION+-----------+Help add RFC 822-like headers, called 'trailers', at the end of the+otherwise free-form part of a commit message.++Unless `--infile=<file>` is used, this command is a filter. It reads+the standard input for a commit message and applies the `token`+arguments, if any, to this message. The resulting message is emited on+the standard output.++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 whitespaces, and the resulting trimmed 'token'+and 'value' will appear in the message like this:++------------------------------------------------+token: value+------------------------------------------------++By default, if there are already trailers with the same 'token' the+new trailer will appear just after the last trailer with the same+'token'. Otherwise it will appear at the end of the message.++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.++OPTIONS+-------+--trim-empty::+ If the 'value' part of any trailer contains only whitespaces,+ the whole trailer will be removed from the resulting message.++----infile=file::+ Read the commit message from `file` instead of the standard+ input.++CONFIGURATION VARIABLES+-----------------------++trailer.<token>.key::+ This 'key' will be used instead of 'token' in the+ 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.++trailer.<token>.where::+ This can be either `after`, which is the default, or+ `before`. If it is `before`, then a trailer with the specified+ token, will appear before, instead of after, other trailers+ with the same token, or otherwise at the beginning, instead of+ at the end, of all the trailers.++trailer.<token>.ifexist::+ This option makes it possible to choose what action will be+ performed when there is already at least one trailer with the+ same token in the message.+++The valid values for this option are: `addIfDifferent` (this is the+default), `addIfDifferentNeighbor`, `add`, `overwrite` or `doNothing`.+++With `addIfDifferent`, a new trailer will be added only if no trailer+with the same (token, value) pair is already in the message.+++With `addIfDifferentNeighbor`, a new trailer will be added only if no+trailer with the same (token, value) pair is above or below the line+where the new trailer will be added.+++With `add`, a new trailer will be added, even if some trailers with+the same (token, value) pair are already in the message.+++With `overwrite`, the new trailer will overwrite an existing trailer+with the same token.+++With `doNothing`, nothing will be done, that is no new trailer will be+added if there is already one with the same token in the message.++trailer.<token>.ifmissing::+ This option makes it possible to choose what action will be+ performed when there is not yet any trailer with the same+ token in the message.+++The valid values for this option are: `add` (this is the default) and+`doNothing`.+++With `add`, a new trailer will be added.+++With `doNothing`, nothing will be done.++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.+++The following environment variables are set when the command is run:+GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME,+GIT_COMMITTER_EMAIL.++SEE ALSO+--------+linkgit:git-commit[1]++GIT+---+Part of the linkgit:git[1] suite
From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
This is a small refactoring to prepare for the next steps.
Signed-off-by: Christian Couder <redacted>
---
trailer.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
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 <redacted>
---
trailer.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
@@ -501,3 +521,23 @@ static void process_input_file(const char *infile,add_trailer_item(infile_tok_first,infile_tok_last,new);}}++voidprocess_trailers(constchar*infile,inttrim_empty,intargc,constchar**argv)+{+structtrailer_item*infile_tok_first=NULL;+structtrailer_item*infile_tok_last=NULL;+structtrailer_item*arg_tok_first;++git_config(git_trailer_config,NULL);++/* Print the non trailer part of infile */+if(infile){+process_input_file(infile,&infile_tok_first,&infile_tok_last);+}++arg_tok_first=process_command_line_args(argc,argv);++process_trailers_lists(&infile_tok_first,&infile_tok_last,&arg_tok_first);++print_all(infile_tok_first,trim_empty);+}
From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
This patch parses the trailer command line arguments
and put the result into an arg_tok doubly linked
list.
Signed-off-by: Christian Couder <redacted>
---
trailer.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
@@ -362,3 +362,80 @@ static int git_trailer_config(const char *conf_key, const char *value, void *cb)}return0;}++staticvoidparse_trailer(structstrbuf*tok,structstrbuf*val,constchar*trailer)+{+char*end=strchr(trailer,'=');+if(!end)+end=strchr(trailer,':');+if(end){+strbuf_add(tok,trailer,end-trailer);+strbuf_trim(tok);+strbuf_addstr(val,end+1);+strbuf_trim(val);+}else{+strbuf_addstr(tok,trailer);+strbuf_trim(tok);+}+}++staticstructtrailer_item*create_trailer_item(constchar*string)+{+structstrbuftok=STRBUF_INIT;+structstrbufval=STRBUF_INIT;+structtrailer_item*new;+structtrailer_item*item;+inttok_alnum_len;++parse_trailer(&tok,&val,string);++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(!strncasecmp(tok.buf,item->conf->key,tok_alnum_len)||+!strncasecmp(tok.buf,item->conf->name,tok_alnum_len)){+new=xcalloc(sizeof(structtrailer_item),1);+new->conf=item->conf;+new->token=xstrdup(item->conf->key);+new->value=strbuf_detach(&val,NULL);+strbuf_release(&tok);+returnnew;+}+}++new=xcalloc(sizeof(structtrailer_item),1);+new->conf=xcalloc(sizeof(structconf_info),1);+new->token=strbuf_detach(&tok,NULL);+new->value=strbuf_detach(&val,NULL);++returnnew;+}++staticvoidadd_trailer_item(structtrailer_item**first,+structtrailer_item**last,+structtrailer_item*new)+{+if(!*last){+*first=new;+*last=new;+}else{+(*last)->next=new;+new->previous=*last;+*last=new;+}+}++staticstructtrailer_item*process_command_line_args(intargc,constchar**argv)+{+inti;+structtrailer_item*arg_tok_first=NULL;+structtrailer_item*arg_tok_last=NULL;++for(i=0;i<argc;i++){+structtrailer_item*new=create_trailer_item(argv[i]);+add_trailer_item(&arg_tok_first,&arg_tok_last,new);+}++returnarg_tok_first;+}
From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
This patch reads trailers from an input file, parses
them and puts the result into a doubly linked list.
Signed-off-by: Christian Couder <redacted>
---
trailer.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
@@ -439,3 +439,65 @@ static struct trailer_item *process_command_line_args(int argc, const char **argreturnarg_tok_first;}++staticstructstrbuf**read_input_file(constchar*infile)+{+structstrbufsb=STRBUF_INIT;++if(strbuf_read_file(&sb,infile,0)<0)+die_errno(_("could not read input file '%s'"),infile);++returnstrbuf_split(&sb,'\n');+}++/*+*Returnthethe(0based)indexofthefirsttrailerline+*orthelinecountiftherearenotrailers.+*/+staticintfind_trailer_start(structstrbuf**lines)+{+intcount,start,empty=1;++/* Get the line count */+for(count=0;lines[count];count++);++/*+*Getthestartofthetrailersbylookingstartingfromtheend+*foralinewithonlyspacesbeforelineswithone':'.+*/+for(start=count-1;start>=0;start--){+if(strbuf_isspace(lines[start])){+if(empty)+continue;+returnstart+1;+}+if(strchr(lines[start]->buf,':')){+if(empty)+empty=0;+continue;+}+returncount;+}++returnempty?count:start+1;+}++staticvoidprocess_input_file(constchar*infile,+structtrailer_item**infile_tok_first,+structtrailer_item**infile_tok_last)+{+structstrbuf**lines=read_input_file(infile);+intstart=find_trailer_start(lines);+inti;++/* Print non trailer lines as is */+for(i=0;lines[i]&&i<start;i++){+printf("%s",lines[i]->buf);+}++/* Parse trailer lines */+for(i=start;lines[i];i++){+structtrailer_item*new=create_trailer_item(lines[i]->buf);+add_trailer_item(infile_tok_first,infile_tok_last,new);+}+}
From: Christian Couder <hidden> Date: 2016-06-15 22:59:46
This helper function checks if a strbuf
contains only space chars or not.
Signed-off-by: Christian Couder <redacted>
---
strbuf.c | 7 +++++++
strbuf.h | 1 +
2 files changed, 8 insertions(+)
From: Eric Sunshine <hidden> Date: 2016-06-15 22:59:46
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
[off-list ref] wrote:
quoted hunk
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 <redacted>
---
Maybe these functions defined in the header should all be 'static
inline' rather than just 'static'? Making them inline would be
consistent with functions defined in other git headers.
+
+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]));
'len' has type size_t, which is unsigned, so the conditional '--len >=
0' will always be true (which will result in a crash if 'buf' contains
no alphanumerics).
Quoting from the strbuf documentation:
... strbufs may have embedded NULs. An strbuf is NUL
terminated for convenience, but no function in the
strbuf API actually relies on the string being free of
NULs.
So, the termination condition (*b) of this loop is questionable.
Looping from 0 to < sb->len makes more sense.
+ return !*b;
Ditto for the return. This will incorrectly return 'true' if an
embedded NUL is encountered.
+}
+
struct strbuf **strbuf_split_buf(const char *str, size_t slen,
int terminator, int max)
{
@@ -111,6 +111,9 @@ extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);externvoidstrbuf_splice(structstrbuf*,size_tpos,size_tlen,constvoid*,size_t);+/* first occurence of a replaced with b */+externvoidstrbuf_replace(structstrbuf*,constchar*a,constchar*b);
Updating Documentation/technical/api-strbuf.txt to mention this new
function would be appropriate.
From: Eric Sunshine <hidden> Date: 2016-06-15 22:59:47
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
[off-list ref] wrote:
quoted hunk
This patch implements the logic that process trailers
from file and arguments.
At the beginning trailers from file are in their own
infile_tok doubly linked list, and trailers from
arguments are in their own arg_tok doubly linked list.
The lists are traversed and when an arg_tok should be
"applied", it is removed from its list and inserted
into the infile_tok list.
Signed-off-by: Christian Couder <redacted>
---
This is freeing arg_tok, but isn't it leaking arg_tok->conf, and
conf->name, conf->key, conf->command? Ditto for all the other
free(arg_tok) invocations elsewhere in the file.
> + break;
Redundant comments (saying the same thing as the code) can make the
code slightly more difficult to read.
+ /*
+ * If arg has been added to infile,
+ * then we need to process it too now.
+ */
+ if ((where == WHERE_AFTER ? infile_tok->next : infile_tok->previous) == arg_tok)
+ infile_tok = arg_tok;
+ }
+ }
+}
From: Eric Sunshine <hidden> Date: 2016-06-15 22:59:47
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
[off-list ref] wrote:
quoted hunk
This patch implements reading the configuration
to get trailer information, and then processing
it and storing it in a doubly linked list.
The config information is stored in the list
whose first item is pointed to by:
static struct trailer_item *first_conf_item;
Signed-off-by: Christian Couder <redacted>
---
From: Eric Sunshine <hidden> Date: 2016-06-15 22:59:47
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
[off-list ref] wrote:
quoted hunk
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 <redacted>
---
One might reasonably expect trailer.h and the process_trailers()
declaration to be introduced by patch 7/17 ("trailer: put all the
processing together and print") in which process_trailers() is defined
in trailer.c.
From: Eric Sunshine <hidden> Date: 2016-06-15 22:59:47
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
[off-list ref] wrote:
quoted hunk
This patch parses the trailer command line arguments
and put the result into an arg_tok doubly linked
list.
Signed-off-by: Christian Couder <redacted>
---
From: Eric Sunshine <hidden> Date: 2016-06-15 22:59:47
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
[off-list ref] wrote:
quoted hunk
This patch reads trailers from an input file, parses
them and puts the result into a doubly linked list.
Signed-off-by: Christian Couder <redacted>
---
trailer.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
From: Eric Sunshine <hidden> Date: 2016-06-15 22:59:47
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
[off-list ref] wrote:
quoted hunk
It is simpler and more natural if the "git interpret-trailers"
is made a filter as its output already goes to sdtout.
Signed-off-by: Christian Couder <redacted>
---
@@ -464,8 +464,13 @@ static struct strbuf **read_input_file(const char *infile){structstrbufsb=STRBUF_INIT;-if(strbuf_read_file(&sb,infile,0)<0)-die_errno(_("could not read input file '%s'"),infile);+if(infile){+if(strbuf_read_file(&sb,infile,0)<0)+die_errno(_("could not read input file '%s'"),infile);+}else{+if(strbuf_read(&sb,fileno(stdin),0)<0)
strbuf_fread(), perhaps?
quoted hunk
+ die_errno(_("could not read from stdin"));
+ }
return strbuf_split(&sb, '\n');
}
@@ -530,10 +535,8 @@ void process_trailers(const char *infile, int trim_empty, int argc, const char * git_config(git_trailer_config, NULL);- /* Print the non trailer part of infile */- if (infile) {- process_input_file(infile, &infile_tok_first, &infile_tok_last);- }+ /* Print the non trailer part of infile (or stdin if infile is NULL) */+ process_input_file(infile, &infile_tok_first, &infile_tok_last); arg_tok_first = process_command_line_args(argc, argv);--
From: Eric Sunshine <hidden> Date: 2016-06-15 22:59:47
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
[off-list ref] wrote:
This is a small refactoring to prepare for the next steps.
Since this is all brand new code, wouldn't it make more sense to
structure it in this fashion in the first place when introduced in
patch 4/17? It's not clear why it should be introduced with poorer
structure and then later cleaned up.
One might reasonably expect trailer.h and the process_trailers()
declaration to be introduced by patch 7/17 ("trailer: put all the
processing together and print") in which process_trailers() is defined
in trailer.c.
On the other hand, I think that it is not so nice to add a header file
like trailer.h unless it is included at least once.
Maybe I can squash this patch with the previous one, but then the
series might be a little bit more difficult to understand.
Thanks,
Christian.
From: Christian Couder <hidden> Date: 2016-06-15 22:59:51
From: Eric Sunshine <redacted>
On Thu, Jan 30, 2014 at 1:49 AM, Christian Couder
[off-list ref] wrote:
quoted
It is simpler and more natural if the "git interpret-trailers"
is made a filter as its output already goes to sdtout.
Signed-off-by: Christian Couder <redacted>
---
@@ -464,8 +464,13 @@ static struct strbuf **read_input_file(const char *infile){structstrbufsb=STRBUF_INIT;-if(strbuf_read_file(&sb,infile,0)<0)-die_errno(_("could not read input file '%s'"),infile);+if(infile){+if(strbuf_read_file(&sb,infile,0)<0)+die_errno(_("could not read input file '%s'"),infile);+}else{+if(strbuf_read(&sb,fileno(stdin),0)<0)
strbuf_fread(), perhaps?
I chose strbuf_read() because it can be passed 0 as a size hint, while
strbuf_fread() must be passed an exact size.
(As we might read from stdin, we might not be able to know the exact
size before we start reading.)
Thanks,
Christian.