[PATCH v12 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.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 v12 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 | 444 ++ 1 file changed, 444 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..9911c7b --- /dev/null +++ b/t/t7513-interpret-trailers.sh @@ -0,0 +1,444 @@ +#!/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' ' + 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 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: 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 actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by :Peff actual + test_cmp expected actual +' + +test_expect_success 'with config setup and = sign' ' + git config trailer.ack.key Acked-by= + cat expected -\EOF + + Acked-by= Peff + EOF + git interpret-trailers --trim-empty --trailer ack = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by= Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty --trailer Acked-by : Peff actual + test_cmp expected actual +' + +test_expect_success 'with config setup and # sign' ' + git config trailer.bug.key Bug # + cat expected -\EOF + + Bug #42 + EOF + git interpret-trailers --trim-empty --trailer bug = 42 actual + test_cmp expected actual +' + +test_expect_success 'with commit basic message' ' + cat basic_message expected + echo expected + git interpret-trailers basic_message actual + test_cmp expected actual +' + +test_expect_success 'with basic patch' ' + cat basic_message input + cat basic_patch input + cat basic_message expected + echo expected + cat basic_patch expected + git interpret-trailers input actual + test_cmp expected actual +' + +test_expect_success 'with commit complex message as argument' ' + cat complex_message_body complex_message_trailers complex_message + cat complex_message_body expected + sed -e s/ Z\$/ / expected -\EOF + Fixes: Z + Acked-by= Z + Reviewed-by: Z + Signed-off-by: Z + EOF + git interpret-trailers complex_message actual + test_cmp expected actual +' + +test_expect_success 'with 2 files arguments' ' + cat basic_message expected + echo expected + cat basic_patch expected + git interpret-trailers complex_message input actual + test_cmp expected actual +' + +test_expect_success 'with message that has comments' ' + cat basic_message message_with_comments + sed -e s/ Z
[PATCH v12 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 | 198 ++
1 file changed, 198 insertions(+)
diff --git a/trailer.c b/trailer.c
index db93a63..52108c2 100644
--- a/trailer.c
+++ b/trailer.c
@@ -47,3 +47,201 @@ static size_t alnum_len(const char *buf, size_t len)
len--;
return len;
}
+
+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 add_arg_to_input_list(struct trailer_item *in_tok,
+ struct trailer_item *arg_tok)
+{
+ if (arg_tok-conf.where == WHERE_AFTER) {
+ arg_tok-next = in_tok-next;
+ in_tok-next = arg_tok;
+ arg_tok-previous = in_tok;
+ if (arg_tok-next)
+ arg_tok-next-previous = arg_tok;
+ } else {
+ arg_tok-previous = in_tok-previous;
+ in_tok-previous = arg_tok;
+ arg_tok-next = in_tok;
+ if (arg_tok-previous)
+ arg_tok-previous-next = arg_tok;
+ }
+}
+
+static int check_if_different(struct trailer_item *in_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len, int check_all)
+{
+ enum action_where where = arg_tok-conf.where;
+ do {
+ if (!in_tok)
+ return 1;
+ if (same_trailer(in_tok, arg_tok, alnum_len))
+ return 0;
+ /*
+* if we want to add a trailer after another one,
+* we have to check those before this one
+*/
+ in_tok = (where == WHERE_AFTER) ? in_tok-previous :
in_tok-next;
+ } while (check_all);
+ return 1;
+}
+
+static void apply_arg_if_exists(struct trailer_item *in_tok,
+ struct trailer_item *arg_tok,
+ int alnum_len)
+{
+ switch (arg_tok-conf.if_exists) {
+ case EXISTS_DO_NOTHING:
+ free_trailer_item(arg_tok);
+ break;
+ case EXISTS_OVERWRITE:
+ free((char *)in_tok-value);
+ in_tok-value = xstrdup(arg_tok-value);
+ free_trailer_item(arg_tok);
+ break;
+ case EXISTS_ADD:
+ add_arg_to_input_list(in_tok, arg_tok);
+ break;
+ case EXISTS_ADD_IF_DIFFERENT:
+ if (check_if_different(in_tok, arg_tok, alnum_len, 1))
+ add_arg_to_input_list(in_tok, arg_tok);
+ else
+ free_trailer_item(arg_tok);
+ break;
+ case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:
+ if (check_if_different(in_tok, arg_tok, alnum_len, 0))
+ add_arg_to_input_list(in_tok, arg_tok);
+ else
+ free_trailer_item(arg_tok);
+ break;
+ }
+}
+
+static void remove_from_list(struct trailer_item *item,
+struct trailer_item **first)
+{
+ if (item-next)
+ item-next-previous = item-previous;
+ if (item-previous)
+ item-previous-next = item-next;
+ else
+ *first = item-next;
+}
+
+static struct trailer_item *remove_first(struct trailer_item **first)
+{
+ struct trailer_item *item = *first;
+ *first = item-next;
+ if (item-next) {
+ item-next-previous = NULL;
+ item-next = NULL;
+ }
+ return item;
+}
+
+static void process_input_token(struct trailer_item *in_tok,
+ struct trailer_item **arg_tok_first,
+ enum action_where where)
+{
+ struct trailer_item *arg_tok;
+ struct trailer_item *next_arg;
+
+ int after = where == WHERE_AFTER;
+ int tok_alnum_len = alnum_len(in_tok-token, strlen(in_tok-token));
+
+ for (arg_tok = *arg_tok_first; arg_tok; arg_tok = next_arg) {
+ next_arg = arg_tok-next;
+ if (!same_token(in_tok, arg_tok, tok_alnum_len))
+ continue;
+ if (arg_tok-conf.where != where)
+ continue;
+ remove_from_list(arg_tok, arg_tok_first);
+ apply_arg_if_exists(in_tok
[PATCH v12 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.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 - messages can contain a patch - lines in messages starting with a comment char are ignored (new) - there are 35 tests - there is some documentation - there are examples in the documentation (new) Possible improvements: - integration with git commit - support GIT_COMMIT_PROTO env variable in commands 3) Changes since version 11, thanks to Michael, Jonathan and Junio: * the patch to ignore comments in messages has been squashed into the original series (5/11 and 8/11) * the comment char config variable is used to ignore comments in messages (as suggested by Michael) (5/11) * the patch that add examples in the documentation has been squashed into the original (11/11) * one of the examples has been split into 2 examples (as suggested by Junio) (11/11) * there is no special case for # when printing the trailers, (as suggested by Junio); so we always use ': ' as separator, except when trailer.token.key ends with a non alphanumeric character; in that case we add no separator as we suppose that the key already contains a separator (6/11, 8/11 and 10/11) * documentation has been improved to be clearer, and more explicit, especially about how the key config variable is used (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 | 264 +++ Makefile | 2 + builtin.h| 1 + builtin/interpret-trailers.c | 44 ++ command-list.txt | 1 + git.c| 1 + t/t7513-interpret-trailers.sh| 568 +++ trailer.c| 755 +++ trailer.h| 6 + 10 files changed, 1643 insertions(+) create mode 100644 Documentation/git-interpret-trailers.txt create mode 100644 builtin/interpret-trailers.c create mode 100755 t/t7513
[PATCH v12 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 | 264 +++ command-list.txt | 1 + 2 files changed, 265 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..45f1ed4 --- /dev/null +++ b/Documentation/git-interpret-trailers.txt @@ -0,0 +1,264 @@ +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 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 + + +This means that the trimmed token and value will be separated by +`': '` (one colon followed by one space). + +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 commit message +part of the ouput. + +The trailers are recognized in the input message using the following +rules: + +* 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.token.key:: + This `key` will be used instead of token in the + trailer. After the last alphanumeric character, this variable + can contain some non alphanumeric characters, like for example + `'%'` (one percent sign), `' = '` (one space followed by one + equal sign and then one space), `' #'` (one space followed by + one number sign) or even just `' '` (one space), that will be + used to separate the token from the value in the + trailer. The default separator, `': '` (one colon followed by + one space), which is the usual standard, is added only if the + last character in `key` is alphanumeric, so watch out for + unwanted trailing spaces in this variable. + +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
[PATCH v12 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 | 146 ++
1 file changed, 146 insertions(+)
diff --git a/trailer.c b/trailer.c
index 52108c2..f376be5 100644
--- a/trailer.c
+++ b/trailer.c
@@ -25,6 +25,8 @@ struct trailer_item {
struct conf_info conf;
};
+static struct trailer_item *first_conf_item;
+
static int same_token(struct trailer_item *a, struct trailer_item *b, int
alnum_len)
{
return !strncasecmp(a-token, b-token, alnum_len);
@@ -245,3 +247,147 @@ static void process_trailers_lists(struct trailer_item
**in_tok_first,
apply_arg_if_missing(in_tok_first, in_tok_last, arg_tok);
}
}
+
+static int set_where(struct conf_info *item, const char *value)
+{
+ if (!strcasecmp(after, value))
+ item-where = WHERE_AFTER;
+ else if (!strcasecmp(before, value))
+ item-where = WHERE_BEFORE;
+ else
+ return -1;
+ return 0;
+}
+
+static int set_if_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(overwrite, value))
+ item-if_exists = EXISTS_OVERWRITE;
+ 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 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);
+ 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_config(const char *conf_key, const char *value, void
*cb)
+{
+ const char *trailer_item, *variable_name;
+ struct trailer_item *item;
+ struct conf_info *conf;
+ char *name = NULL;
+ enum trailer_info_type type;
+ int i;
+
+ trailer_item = skip_prefix(conf_key, trailer.);
+ if (!trailer_item)
+ return 0;
+
+ variable_name = strrchr(trailer_item, '.');
+ if (!variable_name) {
+ warning(_(two level trailer config variable %s), conf_key);
+ return 0;
+ }
+
+ variable_name++;
+ for (i = 0; i ARRAY_SIZE(trailer_config_items); i++) {
+ if (strcmp(trailer_config_items[i].name, variable_name))
+ continue;
+ name = xstrndup(trailer_item, variable_name - trailer_item -
1);
+ type = trailer_config_items[i].type;
+ break;
+ }
+
+ if (!name)
+ return 0;
+
+ item = get_conf_item(name);
+ conf = item-conf;
+ free(name);
+
+ switch (type) {
+ case TRAILER_KEY:
+ if (conf-key)
+ warning(_(more than one %s), conf_key);
+ conf-key = xstrdup(value);
+ break;
+ case TRAILER_COMMAND:
+ if (conf-command)
+ warning(_(more than one %s), conf_key);
+ conf-command = xstrdup(value);
+ break;
+ case
[PATCH v12 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.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 v12 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 f79a369..40ad1a1 100644
--- a/trailer.c
+++ b/trailer.c
@@ -51,6 +51,14 @@ static size_t alnum_len(const char *buf, size_t len)
return len;
}
+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);
@@ -509,3 +517,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 ':'.
+*/
+ 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 (strchr(lines[start]-buf, ':')) {
+ 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;
+}
--
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 v12 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 d648939..eaf692b 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);
@@ -401,6 +413,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))
@@ -437,6 +450,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)
{
@@ -467,6 +519,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;
@@ -528,12 +584,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.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 v12 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 | 118 ++
1 file changed, 118 insertions(+)
diff --git a/trailer.c b/trailer.c
index f376be5..f79a369 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
*/
@@ -391,3 +392,120 @@ 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 = strcspn(trailer, =:);
+ 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
+ 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;
+}
--
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 v12 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 | 56
trailer.h | 6 ++
2 files changed, 62 insertions(+)
create mode 100644 trailer.h
diff --git a/trailer.c b/trailer.c
index 40ad1a1..d648939 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,24 @@ 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
+ 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)
{
@@ -632,3 +651,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.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: [PATCH v10 11/12] Documentation: add documentation for 'git interpret-trailers'
From: Michael Haggerty mhag...@alum.mit.edu On 04/25/2014 11:07 PM, Christian Couder wrote: I don't think there is a lot of complexity. But maybe I need to explain how it works better. Feel free to suggest me sentences I could add. I am really excited about having better support for trailers in Git, and I want to thank you for your work. For me the promise of trailers is * A way for users to add information to commits for whatever purpose they want, without having to convince upstream to built support in. Yeah, I agree this is the main purpose of trailers. * A standard format for that information, so that all tools can agree how to read/write trailers without being confused by or breaking trailers that they didn't know about in advance. Yeah, but don't you think this goal can sometimes go against the previous goal? I mean, if some users for their project think that it's better, for example, if they use trailers like Fix #42 instead of Fix: 42, because their bug tracking system supports Fix #42 better, we should let them do what suits them better, even if Git supports them not as well as if they used Fix: 42. * A format that is straightforward enough that it can be machine- readable with minimum ambiguity. Yeah, but again this could go against the main purpose of trailers above. * Some command-line tools to make it easy for scripts to work with trailers, and that serve as a reference implementation that other Git implementations can imitate. Yeah, ok, as long as we keep in mind the main purpose. For example, I totally expect that we will soon want a command-line tool for inquiring about the presence and contents of trailers, for use in scripting. Eventually we will want to be able to do stuff like git trailers --get-all s-o-b origin/master..origin/next git rev-list --trailer=s-o-b:gits...@pobox.com master git trailers --pipe --draft \ --add-first fixes \ --append '# You can delete the following line:' \ --append s-o-b:$GIT_AUTHOR_NAME $GIT_AUTHOR_EMAIL \ --unset private git trailers --pipe --verify --tidy-up Yeah, feel free to help make this kind of things possible :-) I think it is really important to nail down the format of trailers tightly enough that everybody who reads/writes a commit message agrees about exactly what trailers are there. I think we should have a default format for trailers that is clear, but we should not force users to use this format. Because forcing it would go against the main goal of trailers that you listed first above. For example the specification might look something like: A commit message can optionally end with a block of trailers. The trailers, if present, must be separated from the rest of the commit message by one or more blank lines (lines that contain only whitespace). There must be no blank lines within the list of trailers. It is allowed to have blank lines after the trailers. Each trailer line must match the following Perl regular expression: ^([A-Za-z0-9_-]+)\s*:\s*(.*[^\s])\s*$ The string matching the first group is called the key and the string matching the second is called the value. Keys are considered to be case-insensitive [or should they be case-sensitive?]. The interpretation of values is left entirely up to the application. Values must not be empty. I tried to be clearer in the v12 I just posted, and I think it should be enough to be very clear. We might want to tweak a little the specifications later, so being too strict might be counter productive. And as other tools might already use trailers in a case-sensitive way and yet other tools in a case-insensitive way, I am not sure we would gain anything by specifying if keys or values should be interpreted in a case-sensitive or case-insensitive way. On the contrary we might upset people already using some of these tools for no good reason. However, in --draft and --cleanup modes, empty values *are* allowed, as are comments (lines starting with `core.commentchar`) within the trailer block. In --draft mode such lines are passed through unchanged, and in --cleanup mode such lines are removed. I am not sure we should use modes. I think options like --trim-empty, --allow-comments, --allow-empty might be clearer. I'm not saying this is the exact definition that we want; I'm just providing an example of the level of precision that I think is needed. Yeah, but I think too much precision can be counter productive. With regard to the separator character, my concern is not about how to document the rules for this one tool. It's more about having really well-defined rules that are consistent between reading and writing. For me it seems silly to let git interpret-trailers output trailers that it doesn't know how to read back in, and pretty much be a show-stopper
Re: [PATCH v1 0/3] Add --graft option to git replace
From: Junio C Hamano gits...@pobox.com Subject: Re: [PATCH v1 0/3] Add --graft option to git replace Date: Fri, 23 May 2014 09:59:05 -0700 Christian Couder chrisc...@tuxfamily.org writes: Here is a small patch series to implement: git replace [-f] --graft commit [parent...] The changes since the RFC/PATCH are the following: - in patch 1/3, parse_commit_buffer() is now used to make sure commit is not corrupt - patch 2/3 add some tests - patch 3/3 add some documentation About the documentation, maybe we should add that --graft can now be used instead of grafts in .git/info/grafts, and maybe we could add an example that shows how it can be done. Or a procedure that reads .git/info/grafts, converts it to a set of replacements and drops .git/info/grafts. A sample script could be thrown in to contrib/ somewhere as convert-graft-to-replace.sh. Ok, I just sent a patch that adds such a sample script. 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 v2 2/4] replace: add test for --graft
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..ca45a84 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' ' test -z $(git replace) ' +test_expect_success '--graft with and without already replaced object' ' + test $(git log --oneline | wc -l) = 7 + git replace --graft $HASH5 + test $(git log --oneline | wc -l) = 3 + git cat-file -p $HASH5 | test_must_fail grep parent + test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 + git replace --force -g $HASH5 $HASH4 $HASH3 + git cat-file -p $HASH5 | grep parent $HASH4 + git cat-file -p $HASH5 | grep parent $HASH3 + git replace -d $HASH5 +' + test_done -- 2.0.0.rc0.40.gd30ccc4 -- 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 0/4] Add --graft option to git replace
Here is a small patch series to implement: git replace [-f] --graft commit [parent...] This patch series goes on top of the patch series that implements --edit. The changes since v1 are the following thanks to Eric, Junio and Peff: - change commit message of patch 1/4 - added patch 4/4 that was previously sent separately - in patch 4/4 rename the existing grafts file into grafts.bak Christian Couder (4): replace: add --graft option replace: add test for --graft Documentation: replace: add --graft option contrib: add convert-grafts-to-replace-refs.sh Documentation/git-replace.txt | 10 builtin/replace.c | 84 ++- contrib/convert-grafts-to-replace-refs.sh | 29 +++ t/t6050-replace.sh| 12 + 4 files changed, 134 insertions(+), 1 deletion(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh -- 2.0.0.rc0.40.gd30ccc4 -- 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 1/4] 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.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 84 ++-
1 file changed, 83 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..9d1e82f 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,76 @@ 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 commit *commit;
+ 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);
+ commit = lookup_commit_or_die(old, old_ref);
+ if (read_sha1_commit(old, buf))
+ die(_(Invalid commit: '%s'), old_ref);
+
+ /* make sure the commit is not corrupt */
+ if (parse_commit_buffer(commit, buf.buf, buf.len))
+ die(_(Could not parse 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 +374,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 +398,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
[PATCH v2 3/4] Documentation: replace: add --graft option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 61461b9..491875e 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement 'git replace' [-f] --edit object +'git replace' [-f] --graft commit [parent...] 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -73,6 +74,13 @@ OPTIONS newly created object. See linkgit:git-var[1] for details about how the editor will be chosen. +--graft commit [parent...]:: + Create a graft commit. A new commit is created with the same + content as commit except that its parents will be + [parent...] instead of commit's parents. A replacement ref + is then created to replace commit with the newly created + commit. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or -- 2.0.0.rc0.40.gd30ccc4 -- 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 4/4] contrib: add convert-grafts-to-replace-refs.sh
This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of git replace.
While at it let's mention this new script in the
git replace documentation for the --graft option.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Documentation/git-replace.txt | 4 +++-
contrib/convert-grafts-to-replace-refs.sh | 29 +
2 files changed, 32 insertions(+), 1 deletion(-)
create mode 100755 contrib/convert-grafts-to-replace-refs.sh
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as commit except that its parents will be
[parent...] instead of commit's parents. A replacement ref
is then created to replace commit with the newly created
- commit.
+ commit. See contrib/convert-grafts-to-replace-refs.sh for an
+ example script based on this option that can convert grafts to
+ replace refs.
-l pattern::
--list pattern::
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..7718a53
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+die () {
+ echo 2 $@
+ exit 1
+}
+
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE | while read definition
+do
+ test -n $definition {
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Convertion failed for: $definition
+ }
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+ die Could not mv '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
+
+echo Success!
+echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs!
+echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'
--
2.0.0.rc0.40.gd30ccc4
--
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 v1 1/3] replace: add --graft option
From: Jeff King p...@peff.net On Thu, May 22, 2014 at 11:33:04PM +0200, Christian Couder wrote: 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 I think this script at the end should end up in the documentation or a contrib script (probably the former, as it is short enough that somebody might just cut-and-paste). The graft file ignores comments and blank lines, so maybe grep '^[^#]' would be in order. And maybe it's just me, but I think spacing it like: grep '^[^#]' .git/info/grafts | while read line; do git replace --graft $line done is more readable (I think some would even argue for putting the do on a separate line). Thanks, I used something like that in the contrib script. +/* make sure the commit is not corrupt */ +if (parse_commit_buffer(commit, buf.buf, buf.len)) +die(_(Could not parse commit: '%s'), old_ref); I guess the checks here are sufficient to make... +/* 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 */ ...this number-based parsing safe, though it would miss removing a stray parent line elsewhere in the commit. Yeah, but I don't think that it is a problem. Those parent lines are not standard in the first place, because they are not parsed by parse_commit_buffer(). And I don't think this option should mess with non standard stuff. It still feels rather magical to me, as we are depending on unspoken format guarantees defined inside parse_commit_buffer. My opinion is that we are depending on the standard way to parse headers, and that's good. I think it would be black magic to mess with non standard stuff. I'd prefer something like the line-based parser I showed in the other thread, but I suppose it may just be a matter of preference. Yeah probably. 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 4/4] contrib: add convert-grafts-to-replace-refs.sh
From: Eric Sunshine sunsh...@sunshineco.com
On Sun, Jun 1, 2014 at 11:10 AM, Christian Couder
chrisc...@tuxfamily.org wrote:
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE | while read definition
+do
+ test -n $definition {
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Convertion failed for: $definition
s/Convertion/Conversion/ [1]
[1]:
http://git.661346.n2.nabble.com/Re-PATCH-contrib-add-convert-grafts-to-replace-refs-sh-tp7611822.html
Ooops, sorry I forgot this.
+ }
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+ die Could not mv '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
Could not rename... might be a bit more friendly to non-Unixy folk.
Ok, I will use rename.
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 v3 2/4] replace: add test for --graft
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..ca45a84 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' ' test -z $(git replace) ' +test_expect_success '--graft with and without already replaced object' ' + test $(git log --oneline | wc -l) = 7 + git replace --graft $HASH5 + test $(git log --oneline | wc -l) = 3 + git cat-file -p $HASH5 | test_must_fail grep parent + test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 + git replace --force -g $HASH5 $HASH4 $HASH3 + git cat-file -p $HASH5 | grep parent $HASH4 + git cat-file -p $HASH5 | grep parent $HASH3 + git replace -d $HASH5 +' + test_done -- 2.0.0.rc0.40.gd30ccc4 -- 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 3/4] Documentation: replace: add --graft option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 61461b9..491875e 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement 'git replace' [-f] --edit object +'git replace' [-f] --graft commit [parent...] 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -73,6 +74,13 @@ OPTIONS newly created object. See linkgit:git-var[1] for details about how the editor will be chosen. +--graft commit [parent...]:: + Create a graft commit. A new commit is created with the same + content as commit except that its parents will be + [parent...] instead of commit's parents. A replacement ref + is then created to replace commit with the newly created + commit. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or -- 2.0.0.rc0.40.gd30ccc4 -- 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 1/4] 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.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 84 ++-
1 file changed, 83 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..9d1e82f 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,76 @@ 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 commit *commit;
+ 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);
+ commit = lookup_commit_or_die(old, old_ref);
+ if (read_sha1_commit(old, buf))
+ die(_(Invalid commit: '%s'), old_ref);
+
+ /* make sure the commit is not corrupt */
+ if (parse_commit_buffer(commit, buf.buf, buf.len))
+ die(_(Could not parse 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 +374,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 +398,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
[PATCH v3 4/4] contrib: add convert-grafts-to-replace-refs.sh
This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of git replace.
While at it let's mention this new script in the
git replace documentation for the --graft option.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Documentation/git-replace.txt | 4 +++-
contrib/convert-grafts-to-replace-refs.sh | 29 +
2 files changed, 32 insertions(+), 1 deletion(-)
create mode 100755 contrib/convert-grafts-to-replace-refs.sh
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as commit except that its parents will be
[parent...] instead of commit's parents. A replacement ref
is then created to replace commit with the newly created
- commit.
+ commit. See contrib/convert-grafts-to-replace-refs.sh for an
+ example script based on this option that can convert grafts to
+ replace refs.
-l pattern::
--list pattern::
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..8472879
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+die () {
+ echo 2 $@
+ exit 1
+}
+
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE | while read definition
+do
+ test -n $definition {
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Conversion failed for: $definition
+ }
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+ die Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
+
+echo Success!
+echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs!
+echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'
--
2.0.0.rc0.40.gd30ccc4
--
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 0/4] Add --graft option to git replace
Here is a small patch series to implement: git replace [-f] --graft commit [parent...] This patch series goes on top of the patch series that implements --edit. There is only one change since v2 thanks to Eric: - improve error messages in convert-grafts-to-replace-refs.sh (patch 4/4) Christian Couder (4): replace: add --graft option replace: add test for --graft Documentation: replace: add --graft option contrib: add convert-grafts-to-replace-refs.sh Documentation/git-replace.txt | 10 builtin/replace.c | 84 ++- contrib/convert-grafts-to-replace-refs.sh | 29 +++ t/t6050-replace.sh| 12 + 4 files changed, 134 insertions(+), 1 deletion(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh -- 2.0.0.rc0.40.gd30ccc4 -- 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: Best practices/conventions for tags and references in commit message
On Thu, Jun 5, 2014 at 4:10 PM, Thomas Koch tho...@koch.ro wrote: On Tuesday, May 27, 2014 03:49:24 PM Johan Herland wrote: Search the mailing list archives for git-interpret-trailers. It's coming. Nice! I started a table to collect how different projects or tools use trailers: https://git.wiki.kernel.org/index.php/CommitMessageConventions#Trailers Great! It would be even better if you could add links to one or more actual commits that contain the specified trailer. For example for openstack, it would be nice to have real examples of DocImpact, SecurityImpact and UpgradeImpact. It would be nice to see more examples and in the long run to have some best practices recommended by gits documentation and supported across different bug trackers, changelog generators, statistic generators, repository viewers, etc.. Yeah, what is also interesting is that some people/projects/tools use things that are in some ways trailer like. For example GitHub parses commit messages for things like fix #1234 and there are also people adding refs at the end of commit messages like: [1] http://www.example.com/example_ref.html 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 v3 1/4] replace: add --graft option
On Thu, Jun 5, 2014 at 11:49 PM, Junio C Hamano gits...@pobox.com wrote:
Christian Couder chrisc...@tuxfamily.org writes:
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ 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);
+ commit = lookup_commit_or_die(old, old_ref);
Not a problem with this patch, but the above sequence somehow makes
me wonder if lookup-commit-or-die is a good API for this sort of
thing. Wouldn't it be more natural if it took not unsigned char
old[20] but anything that would be understood by get_sha1()?
It could be that this particular caller is peculiar and all the
existing callers are happy, though. I didn't git grep to spot
patterns in existing callers.
lookup_commit_or_die() looked like a good API to me because I saw that
it checked a lot of things and die in case of problems, which could
make the patch shorter.
+ if (read_sha1_commit(old, buf))
+ die(_(Invalid commit: '%s'), old_ref);
+ /* make sure the commit is not corrupt */
+ if (parse_commit_buffer(commit, buf.buf, buf.len))
+ die(_(Could not parse commit: '%s'), old_ref);
It is unclear to me what you are trying to achieve with these two.
If the earlier lookup-commit has returned a commit object that has
already been parsed, parse_commit_buffer() would not check anything,
would it?
Yeah, you are right. I missed the fact that lookup_commit_or_die()
calls parse_object() which itself calls read_sha1_file() and then
parse_object_buffer() which calls parse_commit_buffer().
Here is a backtrace that shows this:
#0 parse_commit_buffer (item=0x8597b0, buffer=0x851730, size=228) at
commit.c:251
#1 0x004fa215 in parse_object_buffer (sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234,
type=OBJ_COMMIT, size=228,
buffer=0x851730, eaten_p=0x7fffdacc) at object.c:198
#2 0x004fa50a in parse_object (sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234) at
object.c:264
#3 0x004a89ef in lookup_commit_reference_gently
(sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234, quiet=0) at
commit.c:38
#4 0x004a8a48 in lookup_commit_reference (sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234) at
commit.c:47
#5 0x004a8a67 in lookup_commit_or_die (sha1=0x7fffdbf0
\tA\247\235J\213\376u\212\226\311^[\371\343^\330\234,
ref_name=0x7fffe465
093e41a79d4a8bfe3c758a96c95e5bf9e35ed89c) at commit.c:52
#6 0x0047f89a in create_graft (argc=1, argv=0x7fffe130,
refdir=0x0, force=0) at builtin/replace.c:353
#7 0x0047ff71 in cmd_replace (argc=1, argv=0x7fffe130,
prefix=0x0) at builtin/replace.c:461
#8 0x00405441 in run_builtin (p=0x7eee90, argc=3,
argv=0x7fffe130) at git.c:314
#9 0x0040563a in handle_builtin (argc=3, argv=0x7fffe130)
at git.c:487
#10 0x00405754 in run_argv (argcp=0x7fffe01c,
argv=0x7fffe020) at git.c:533
#11 0x004058f9 in main (argc=3, av=0x7fffe128) at git.c:616
A typical sequence would look more like this:
commit = lookup_commit(...);
if (parse_commit(commit))
oops there is an error;
/* otherwise */
use(commit-buffer);
without reading a commit object using low-level read-sha1-file
interface yourself, no?
Yeah, or I could just rely on the fact that lookup_commit_or_die()
already parses the commit, with something like this:
if (get_sha1(old_ref, old) 0)
die(_(Not a valid object name: '%s'), old_ref);
/* parse the commit buffer to make sure the commit is not corrupt */
commit = lookup_commit_or_die(old, old_ref);
/* find existing parents */
parent_start = buf.buf;
parent_start += 46; /* tree + hex sha1 + \n */
parent_end = parent_start;
...
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 v3 1/4] replace: add --graft option
On Fri, Jun 6, 2014 at 5:29 PM, Christian Couder christian.cou...@gmail.com wrote: Yeah, or I could just rely on the fact that lookup_commit_or_die() already parses the commit, with something like this: if (get_sha1(old_ref, old) 0) die(_(Not a valid object name: '%s'), old_ref); /* parse the commit buffer to make sure the commit is not corrupt */ commit = lookup_commit_or_die(old, old_ref); /* find existing parents */ parent_start = buf.buf; parent_start += 46; /* tree + hex sha1 + \n */ parent_end = parent_start; This last part should be: /* find existing parents */ strbuf_addstr(buf, commit-buffer); parent_start = buf.buf; parent_start += 46; /* tree + hex sha1 + \n */ parent_end = parent_start; ... I will send an updated patch series soon. -- 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 v3 4/4] contrib: add convert-grafts-to-replace-refs.sh
On Thu, Jun 5, 2014 at 11:55 PM, Junio C Hamano gits...@pobox.com wrote:
Christian Couder chrisc...@tuxfamily.org writes:
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..8472879
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+die () {
+ echo 2 $@
+ exit 1
+}
Don't we install git-sh-setup in GIT_EXEC_PATH, in order to allow
these third-party scripts to begin with:
. $(git --exec-path)/git-sh-setup
just like our own scripted Porcelains?
Ok, I will use that.
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE | while read definition
+do
Format the above like so:
grep '^[^# ]' $GRAFTS_FILE |
while read definition
do
which is easier to see what that do is doing.
Ok.
+ test -n $definition {
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Conversion failed for: $definition
+ }
Hmph, why not if/then/fi?
Ok, I will use if/then/fi.
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 v4 2/4] replace: add test for --graft
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..ca45a84 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' ' test -z $(git replace) ' +test_expect_success '--graft with and without already replaced object' ' + test $(git log --oneline | wc -l) = 7 + git replace --graft $HASH5 + test $(git log --oneline | wc -l) = 3 + git cat-file -p $HASH5 | test_must_fail grep parent + test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 + git replace --force -g $HASH5 $HASH4 $HASH3 + git cat-file -p $HASH5 | grep parent $HASH4 + git cat-file -p $HASH5 | grep parent $HASH3 + git replace -d $HASH5 +' + test_done -- 2.0.0.rc0.40.gd30ccc4 -- 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 v4 0/4] Add --graft option to git replace
Here is a small patch series to implement: git replace [-f] --graft commit [parent...] This patch series goes on top of the patch series that implements --edit. The changes since v3, thanks to Junio, are: - remove function read_sha1_commit() and its call as well as a call to parse_commit_buffer(), as lookup_commit_or_die() already reads the commit buffer and parses it (patch 1/4) - source git-sh-setup and other small cosmetic changes in convert-grafts-to-replace-refs.sh (patch 4/4) Christian Couder (4): replace: add --graft option replace: add test for --graft Documentation: replace: add --graft option contrib: add convert-grafts-to-replace-refs.sh Documentation/git-replace.txt | 10 + builtin/replace.c | 62 ++- contrib/convert-grafts-to-replace-refs.sh | 28 ++ t/t6050-replace.sh| 12 ++ 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh -- 2.0.0.rc0.40.gd30ccc4 -- 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 v4 4/4] contrib: add convert-grafts-to-replace-refs.sh
This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of git replace.
While at it let's mention this new script in the
git replace documentation for the --graft option.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Documentation/git-replace.txt | 4 +++-
contrib/convert-grafts-to-replace-refs.sh | 28
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100755 contrib/convert-grafts-to-replace-refs.sh
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as commit except that its parents will be
[parent...] instead of commit's parents. A replacement ref
is then created to replace commit with the newly created
- commit.
+ commit. See contrib/convert-grafts-to-replace-refs.sh for an
+ example script based on this option that can convert grafts to
+ replace refs.
-l pattern::
--list pattern::
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..0cbc917
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+. $(git --exec-path)/git-sh-setup
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE |
+while read definition
+do
+ if test -n $definition
+ then
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Conversion failed for: $definition
+ fi
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+ die Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
+
+echo Success!
+echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs!
+echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'
--
2.0.0.rc0.40.gd30ccc4
--
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 v4 3/4] Documentation: replace: add --graft option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 61461b9..491875e 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement 'git replace' [-f] --edit object +'git replace' [-f] --graft commit [parent...] 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -73,6 +74,13 @@ OPTIONS newly created object. See linkgit:git-var[1] for details about how the editor will be chosen. +--graft commit [parent...]:: + Create a graft commit. A new commit is created with the same + content as commit except that its parents will be + [parent...] instead of commit's parents. A replacement ref + is then created to replace commit with the newly created + commit. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or -- 2.0.0.rc0.40.gd30ccc4 -- 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 v4 1/4] 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.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 62 ++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..91eda61 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,54 @@ static int edit_and_replace(const char *object_ref, int
force)
return replace_object_sha1(object_ref, old, replacement, new, force);
}
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ 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);
+ commit = lookup_commit_or_die(old, old_ref);
+
+ /* find existing parents */
+ strbuf_addstr(buf, commit-buffer);
+ 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 +352,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 +376,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 a replacement,
git_replace_usage, options);
@@ -348,6 +402,12 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
git_replace_usage, options);
return edit_and_replace(argv[0], force);
+ case MODE_GRAFT:
+ if (argc 1)
+ usage_msg_opt(-g needs at least one argument,
+ git_replace_usage, options);
+ return create_graft(argc, argv, force);
+
case MODE_LIST:
if (argc 1)
usage_msg_opt
Re: [PATCH v3 1/4] replace: add --graft option
On Fri, Jun 6, 2014 at 5:44 PM, Christian Couder christian.cou...@gmail.com wrote: /* find existing parents */ strbuf_addstr(buf, commit-buffer); Unfortunately, it looks like the above will not work if the commit-buffer contains an embedded NUL. I wonder if it is a real problem or not. -- 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 15/15] commit: record buffer length in cache
From: Jeff King p...@peff.net --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2313,7 +2313,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt, ident, ident, path, (!contents_from ? path : (!strcmp(contents_from, -) ? standard input : contents_from))); - set_commit_buffer(commit, strbuf_detach(msg, NULL)); + set_commit_buffer(commit, msg.buf, msg.len); I find the above strange. I would have done something like: - set_commit_buffer(commit, strbuf_detach(msg, NULL)); + size_t size; + char *buf = strbuf_detach(msg, size); + set_commit_buffer(commit, buf, size); 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 15/15] commit: record buffer length in cache
From: Jeff King p...@peff.net
Subject: Re: [PATCH 15/15] commit: record buffer length in cache
Date: Tue, 10 Jun 2014 16:33:49 -0400
On Tue, Jun 10, 2014 at 01:27:13AM -0400, Jeff King wrote:
I find the above strange. I would have done something like:
- set_commit_buffer(commit, strbuf_detach(msg, NULL));
+ size_t size;
+ char *buf = strbuf_detach(msg, size);
+ set_commit_buffer(commit, buf, size);
It is a little strange. You can't do:
set_commit_buffer(commit, strbuf_detach(msg, NULL), msg.len);
because the second argument resets msg.len as a side effect. Doing it
your way is longer, but perhaps is a bit more canonical. In general, one
should call strbuf_detach to ensure that the buffer is allocated (and
does not point to slopbuf). That's guaranteed here, because we just put
contents into the buffer, but it's probably more hygienic to use the
more verbose form.
I was trying to avoid cluttering up the function with the extra variable
definitions. I ended up with the change below, which I think is clear,
correct, and pushes the complexity outside of the function.
Yeah, great!
diff --git a/builtin/blame.c b/builtin/blame.c
index cde19eb..53f43ab 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2266,6 +2266,18 @@ static void append_merge_parents(struct commit_list
**tail)
}
/*
+ * This isn't as simple as passing sb-buf and sb-len, because we
+ * want to transfer ownership of the buffer to the commit (so we
+ * must use detach).
+ */
+static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf
*sb)
+{
+ size_t len;
+ void *buf = strbuf_detach(sb, len);
+ set_commit_buffer(c, buf, len);
+}
+
+/*
* Prepare a dummy commit that represents the work tree (or staged) item.
* Note that annotating work tree item never works in the reverse.
*/
@@ -2313,7 +2325,7 @@ static struct commit *fake_working_tree_commit(struct
diff_options *opt,
ident, ident, path,
(!contents_from ? path :
(!strcmp(contents_from, -) ? standard input :
contents_from)));
- set_commit_buffer(commit, strbuf_detach(msg, NULL));
+ set_commit_buffer_from_strbuf(commit, msg);
if (!contents_from || strcmp(-, contents_from)) {
struct stat st;
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: What's cooking in git.git (Jun 2014, #03; Tue, 10)
On Wed, Jun 11, 2014 at 12:19 AM, Junio C Hamano gits...@pobox.com wrote: * cc/interpret-trailers (2014-05-28) 11 commits - Documentation: add documentation for 'git interpret-trailers' - trailer: add tests for commands in config file - trailer: execute command from 'trailer.name.command' - trailer: add tests for git interpret-trailers - trailer: add interpret-trailers command - trailer: put all the processing together and print - trailer: parse trailers from file or stdin - trailer: process command line trailer arguments - trailer: read and process config information - trailer: process trailers from input message and arguments - trailer: add data structures and basic functions A new filter to programatically edit the tail end of the commit log messages. What is the status of this one? I think I saw reviews by Michael but after that I do not recall seeing any updates. Yeah, I am very busy with other things these days, but I should have more time after next week. 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 v5 1/7] 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.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/replace.c | 66 ++-
1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..3515979 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,58 @@ static int edit_and_replace(const char *object_ref, int
force)
return replace_object_sha1(object_ref, old, replacement, new, force);
}
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ struct strbuf new_parents = STRBUF_INIT;
+ const char *parent_start, *parent_end;
+ const char *buffer;
+ unsigned long size;
+ int i;
+
+ if (get_sha1(old_ref, old) 0)
+ die(_(Not a valid object name: '%s'), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
+
+ /* find existing parents */
+ buffer = get_commit_buffer(commit, size);
+ strbuf_add(buf, buffer, size);
+ unuse_commit_buffer(commit, buffer);
+ 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 +356,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 +380,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 a replacement,
git_replace_usage, options);
@@ -348,6 +406,12 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
git_replace_usage, options);
return edit_and_replace(argv[0], force);
+ case MODE_GRAFT:
+ if (argc 1)
+ usage_msg_opt(-g needs at least one argument
[PATCH v5 2/7] replace: add test for --graft
Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t6050-replace.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..ca45a84 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' ' test -z $(git replace) ' +test_expect_success '--graft with and without already replaced object' ' + test $(git log --oneline | wc -l) = 7 + git replace --graft $HASH5 + test $(git log --oneline | wc -l) = 3 + git cat-file -p $HASH5 | test_must_fail grep parent + test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 + git replace --force -g $HASH5 $HASH4 $HASH3 + git cat-file -p $HASH5 | grep parent $HASH4 + git cat-file -p $HASH5 | grep parent $HASH3 + git replace -d $HASH5 +' + test_done -- 2.0.0.421.g786a89d.dirty -- 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 v5 0/7] Add --graft option to git replace
Here is a small patch series to implement: git replace [-f] --graft commit [parent...] This patch series goes on top of the patch series that implements --edit. The changes since v4, thanks to Junio and Peff, are: - The series has been rebased on top of Peff's series to store the commit buffer length (jk/commit-buffer-length). This changes patch 1/7 and fixes the problem that we lost everything after a NUL byte in the commit buffer. - Patches 5/7, 6/7 and 7/7 have been added to lose a gpg signature in the original commit buffer. Notes: - Maybe patches could be reordered or squashed, but I prefered not to do it in this round so that changes compared to previous version are easier to spot. - Junio suggested to drop the merge-tag of a parent we are not going to keep, but to keep the merge-tag of a parent we are keeping. This has not yet been done. It is unfortunate that merge-tags don't use a format like the trailer format, because we could have factorised droping a merge-tag in the git interpret-trailers command. Christian Couder (7): replace: add --graft option replace: add test for --graft Documentation: replace: add --graft option contrib: add convert-grafts-to-replace-refs.sh replace: refactor replacing parents replace: remove signature when using --graft replace: add test for --graft with signed commit Documentation/git-replace.txt | 10 builtin/replace.c | 79 ++- commit.c | 34 + commit.h | 2 + contrib/convert-grafts-to-replace-refs.sh | 28 +++ t/t6050-replace.sh| 34 + 6 files changed, 186 insertions(+), 1 deletion(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh -- 2.0.0.421.g786a89d.dirty -- 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 v5 6/7] replace: remove signature when using --graft
It could be misleading to keep a signature in a
replacement commit, so let's remove it.
Note that there should probably be a way to sign
the replacement commit created when using --graft,
but this can be dealt with in another commit or
patch series.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 5 +
commit.c | 34 ++
commit.h | 2 ++
3 files changed, 41 insertions(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index ad47237..000db65 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -344,6 +344,11 @@ static int create_graft(int argc, const char **argv, int
force)
replace_parents(buf, argc, argv);
+ if (remove_signature(buf))
+ warning(_(the original commit '%s' has a gpg signature.\n
+ It will be removed in the replacement commit!),
+ old_ref);
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_(could not write replacement commit for: '%s'), old_ref);
diff --git a/commit.c b/commit.c
index fb7897c..54e157d 100644
--- a/commit.c
+++ b/commit.c
@@ -1177,6 +1177,40 @@ int parse_signed_commit(const struct commit *commit,
return saw_signature;
}
+int remove_signature(struct strbuf *buf)
+{
+ const char *line = buf-buf;
+ const char *tail = buf-buf + buf-len;
+ int in_signature = 0;
+ const char *sig_start = NULL;
+ const char *sig_end = NULL;
+
+ while (line tail) {
+ const char *next = memchr(line, '\n', tail - line);
+ next = next ? next + 1 : tail;
+
+ if (in_signature line[0] == ' ')
+ sig_end = next;
+ else if (starts_with(line, gpg_sig_header)
+line[gpg_sig_header_len] == ' ') {
+ sig_start = line;
+ sig_end = next;
+ in_signature = 1;
+ } else {
+ if (*line == '\n')
+ /* dump the whole remainder of the buffer */
+ next = tail;
+ in_signature = 0;
+ }
+ line = next;
+ }
+
+ if (sig_start)
+ strbuf_remove(buf, sig_start - buf-buf, sig_end - sig_start);
+
+ return sig_start != NULL;
+}
+
static void handle_signed_tag(struct commit *parent, struct
commit_extra_header ***tail)
{
struct merge_remote_desc *desc;
diff --git a/commit.h b/commit.h
index 2e1492a..4234dae 100644
--- a/commit.h
+++ b/commit.h
@@ -327,6 +327,8 @@ struct commit *get_merge_parent(const char *name);
extern int parse_signed_commit(const struct commit *commit,
struct strbuf *message, struct strbuf
*signature);
+extern int remove_signature(struct strbuf *buf);
+
extern void print_commit_list(struct commit_list *list,
const char *format_cur,
const char *format_last);
--
2.0.0.421.g786a89d.dirty
--
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 v5 3/7] Documentation: replace: add --graft option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-replace.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 61461b9..491875e 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement 'git replace' [-f] --edit object +'git replace' [-f] --graft commit [parent...] 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -73,6 +74,13 @@ OPTIONS newly created object. See linkgit:git-var[1] for details about how the editor will be chosen. +--graft commit [parent...]:: + Create a graft commit. A new commit is created with the same + content as commit except that its parents will be + [parent...] instead of commit's parents. A replacement ref + is then created to replace commit with the newly created + commit. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or -- 2.0.0.421.g786a89d.dirty -- 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 v5 7/7] replace: add test for --graft with signed commit
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 22 ++
1 file changed, 22 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index ca45a84..80b85e3 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -7,6 +7,7 @@ test_description='Tests replace refs functionality'
exec /dev/null
. ./test-lib.sh
+. $TEST_DIRECTORY/lib-gpg.sh
add_and_commit_file()
{
@@ -363,4 +364,25 @@ test_expect_success '--graft with and without already
replaced object' '
git replace -d $HASH5
'
+test_expect_success GPG 'set up a signed commit' '
+ echo line 17 hello
+ echo line 18 hello
+ git add hello
+ test_tick
+ git commit --quiet -S -m hello: 2 more lines in a signed commit
+ HASH8=$(git rev-parse --verify HEAD)
+ git verify-commit $HASH8
+'
+
+test_expect_success GPG '--graft with a signed commit' '
+ git cat-file commit $HASH8 orig
+ git replace --graft $HASH8
+ git cat-file commit $HASH8 repl
+ test_must_fail grep gpgsig repl
+ diff -u orig repl | grep ^-parent $HASH7
+ diff -u orig repl | grep ^-gpgsig -BEGIN PGP SIGNATURE-
+ test_must_fail git verify-commit $HASH8
+ git replace -d $HASH8
+'
+
test_done
--
2.0.0.421.g786a89d.dirty
--
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 v5 4/7] contrib: add convert-grafts-to-replace-refs.sh
This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of git replace.
While at it let's mention this new script in the
git replace documentation for the --graft option.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-replace.txt | 4 +++-
contrib/convert-grafts-to-replace-refs.sh | 28
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100755 contrib/convert-grafts-to-replace-refs.sh
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as commit except that its parents will be
[parent...] instead of commit's parents. A replacement ref
is then created to replace commit with the newly created
- commit.
+ commit. See contrib/convert-grafts-to-replace-refs.sh for an
+ example script based on this option that can convert grafts to
+ replace refs.
-l pattern::
--list pattern::
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..0cbc917
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+. $(git --exec-path)/git-sh-setup
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE |
+while read definition
+do
+ if test -n $definition
+ then
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Conversion failed for: $definition
+ fi
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+ die Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
+
+echo Success!
+echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs!
+echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'
--
2.0.0.421.g786a89d.dirty
--
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 v5 5/7] replace: refactor replacing parents
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 42 +-
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 3515979..ad47237 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -295,27 +295,14 @@ static int edit_and_replace(const char *object_ref, int
force)
return replace_object_sha1(object_ref, old, replacement, new, force);
}
-static int create_graft(int argc, const char **argv, int force)
+static void replace_parents(struct strbuf *buf, int argc, const char **argv)
{
- unsigned char old[20], new[20];
- const char *old_ref = argv[0];
- struct commit *commit;
- struct strbuf buf = STRBUF_INIT;
struct strbuf new_parents = STRBUF_INIT;
const char *parent_start, *parent_end;
- const char *buffer;
- unsigned long size;
int i;
- if (get_sha1(old_ref, old) 0)
- die(_(Not a valid object name: '%s'), old_ref);
- commit = lookup_commit_or_die(old, old_ref);
-
/* find existing parents */
- buffer = get_commit_buffer(commit, size);
- strbuf_add(buf, buffer, size);
- unuse_commit_buffer(commit, buffer);
- parent_start = buf.buf;
+ parent_start = buf-buf;
parent_start += 46; /* tree + hex sha1 + \n */
parent_end = parent_start;
@@ -332,13 +319,34 @@ static int create_graft(int argc, const char **argv, int
force)
}
/* replace existing parents with new ones */
- strbuf_splice(buf, parent_start - buf.buf, parent_end - parent_start,
+ strbuf_splice(buf, parent_start - buf-buf, parent_end - parent_start,
new_parents.buf, new_parents.len);
+ strbuf_release(new_parents);
+}
+
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ const char *buffer;
+ unsigned long size;
+
+ if (get_sha1(old_ref, old) 0)
+ die(_(Not a valid object name: '%s'), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
+
+ buffer = get_commit_buffer(commit, size);
+ strbuf_add(buf, buffer, size);
+ unuse_commit_buffer(commit, buffer);
+
+ replace_parents(buf, argc, argv);
+
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))
--
2.0.0.421.g786a89d.dirty
--
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 v3 1/4] replace: add --graft option
On Sun, Jun 8, 2014 at 10:18 AM, Junio C Hamano gits...@pobox.com wrote: On Sat, Jun 7, 2014 at 11:49 PM, Christian Couder christian.cou...@gmail.com wrote: On Fri, Jun 6, 2014 at 5:44 PM, Christian Couder christian.cou...@gmail.com wrote: /* find existing parents */ strbuf_addstr(buf, commit-buffer); Unfortunately, it looks like the above will not work if the commit-buffer contains an embedded NUL. I wonder if it is a real problem or not. Yes, it is a real problem (there was another thread on this regarding the code path that verifies GPG signature on the commit itself), which incidentally reminds us to another thing to think about in your patch as well. I *think* you shoud drop the GPG signature on the commit itself, and you also should drop the merge-tag of a parent you are not going to keep, but should keep the merge-tag of a parent you are keeping. In the v5 of the patch series, I now drop the GPG signature on the commit itself. Now, after having read the recent thread about git verify-commit, I understand that you also want me to drop the signature of a tag that was merged, because such signatures are added to the commit message. But I wonder how far should we go in this path. For example merge commits have a title like Merge branch 'dev' or Merge tag 'stuff', but this does not make sense any more if the replacement commit drops the parent corresponding to 'dev' or 'stuff'. And I don't think we should change the commit title. 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 v12 11/11] Documentation: add documentation for 'git interpret-trailers'
From: Junio C Hamano gits...@pobox.com Christian Couder chrisc...@tuxfamily.org writes: +The trailers are recognized in the input message using the following +rules: + +* 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. While I agree with Michael on the other thread that we should limit the syntax and start with ':' only, if you really want to allow random syntax like Bug #12345 and Acked-by= Peff, for which you have demonstrations in the tests in the other patch, the above rule should be updated to also allow prefix matches to possible set of keys defined by the user, so that an existing line that is produced by your tool, e.g. Acked-by= Peff, can be picked up as matching with some token having a key Acked-by= . Otherwise, the input side of your tool is inconsistent with the output side of your own tool, and that will make the flexiblity of the output side useless. I don't think that the flexibility of the output side would be useless. We already emit stuff like: (cherry picked from commit f72baf07969242882128aff4c95ec8059e7fd054) and we don't care about any input side when we do that. So being able to emit lot of different stuff is valuable even if we are not yet able to parse it. For example what if people wanted cherry-pick messages written like: Cherry picked from f72baf0796 (do this and that, 2014-01-01) we just cannot let them have the above if we decide that ':' has to always be used as the separator. We also emit stuff like Merge commit '71260bf' or Merge tag 'mystuff' or Merge branch 'dev' and we don't let people customize this and we don't care about any input side. And when there is a merge conflict we emit: Conflicts: file1 file2 ... instead of: Conflicts: file1 Conflicts: file2 ... Of course at least for cherry-pick it would have been nice if since the beginning we would have written something in a canonical trailer way like: Cherry-picked-from: f72baf0796 This way we could now use git interpret-trailers to both emit the above and to read it. But it is still be better to just be able to emit it than to not be able to do anything about it. Because if we are able to emit it with git interpret-trailers, then we can let people customize how it is emitted, and this might be enough for many people. Also now those who are ok to output it using the canonical way, could now configure that. And this is not just for us, something like: fixes #42 (do this and that, 2014-01-01) for example could be nice for both Github and human beings. 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 v3 1/4] replace: add --graft option
On Mon, Jun 30, 2014 at 8:37 AM, Junio C Hamano gits...@pobox.com wrote: Christian Couder christian.cou...@gmail.com writes: Now, after having read the recent thread about git verify-commit, I understand that you also want me to drop the signature of a tag that was merged, because such signatures are added to the commit message. Huh? I am not sure if I follow. Perhaps we are talking about different things? I think we are talking about the same thing but I might not have been clear. When you are preparing a replacement for an existing commit that merges a signed tag, there are two cases: - The replacement commit still merges the same signed tag; or - The replacement commit does not merge that signed tag (it may become a single-parent commit, or it may stay to be a merge but merges a different commit on the side branch). In the former, it would be sensible to keep the mergetag and propagate it to the replacement; it is a signature over the tip of the side branch being merged by the original (and the replacement) merge, and the replacement will not affect the validity of the signature at all. Ok, this is what is done right now by the patch series. In the latter, we do want to drop the mergetag for the parent you are losing in the replacement, because by definition it will be irrelevant. Yeah, it might be a good idea to drop the mergetag, but note that anyway such a commit probably has a title like Merge tag 'tag' and we won't automatically change this title and this title will be wrong (because we are not merging anymore this tag). So anyway in this case, --graft will do something that is not good. So it might be better in this case to just error out and say that it would be better to use --edit instead of --graft. -- 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 v12 11/11] Documentation: add documentation for 'git interpret-trailers'
On Mon, Jun 30, 2014 at 1:57 PM, Jakub Narębski jna...@gmail.com wrote: Christian Couder wrote: + + +* 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 How to configure git-interpret-trailers command so that it follow current rules for DCO: * Signed-off-by: is always at bottom; we can have signoff+signoff+ack+signoff * Signed-off-by: can repeat itself with the same author; this denotes steps in coming up with current version of the patch. * but we shouldn't repeat the same signoff one after another Right now something like: signoff+signoff+ack+signoff is not supported. It could be, if someone implements more options for the trailer.token.where config variable. Right now the only options are after and before, but it could be possible to have end and start too. And maybe end should be the default instead of after. When I first worked on this series I was under the impression that people wanted to group all the trailers with the same token together. So we want to allow this: Signed-off-by: A U Thor aut...@example.com Signed-off-by: Joe R. Hacker j...@hacker.com Acked-by: D E Veloper develo...@example.com Signed-off-by: C O Mitter commit...@example.com but prevent this Signed-off-by: C O Mitter commit...@example.com Signed-off-by: C O Mitter commit...@example.com This can be prevented by using addIfDifferentNeighbor, for example like this: $ git config trailer.sign.ifexists addIfDifferentNeighbor So I think the full config for what you want would be something like: $ git config trailer.sign.key Signed-off-by: $ git config trailer.sign.ifmissing add $ git config trailer.sign.ifexists addIfDifferentNeighbor $ git config trailer.sign.where end $ git config trailer.sign.command 'echo $(git config user.name) $(git config user.email)' $ git config trailer.ack.key Acked-by: $ git config trailer.ack.ifmissing add $ git config trailer.ack.ifexists addIfDifferentNeighbor $ git config trailer.ack.where end 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 v5 2/7] replace: add test for --graft
On Wed, Jul 2, 2014 at 10:49 PM, Junio C Hamano gits...@pobox.com wrote:
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t6050-replace.sh | 12
1 file changed, 12 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 68b3cb2..ca45a84 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' '
test -z $(git replace)
'
+test_expect_success '--graft with and without already replaced object' '
+ test $(git log --oneline | wc -l) = 7
+ git replace --graft $HASH5
+ test $(git log --oneline | wc -l) = 3
+ git cat-file -p $HASH5 | test_must_fail grep parent
Please do not run non-git command under test_must_fail.
Ok, I think I will just use the following then:
test_must_fail git rev-parse $HASH5^1
+ test_must_fail git replace --graft $HASH5 $HASH4 $HASH3
+ git replace --force -g $HASH5 $HASH4 $HASH3
+ git cat-file -p $HASH5 | grep parent $HASH4
+ git cat-file -p $HASH5 | grep parent $HASH3
+ git replace -d $HASH5
+'
+
test_done
For all these git cat-file -p $commit | grep ..., I would think
you should add commit_has_parents helper function to allow a bit
more careful testing. As written, the test will mistake a commit
with string parent $HASHx anywhere, not in the header part, and
the test does not ensure that $HASH{3,4} is the {first,second}
parent.
commit_has_parents $HASH5 $HASH4 $HASH3
would then may be implemented like:
commit_has_parents () {
git cat-file commit $1 payload
sed -n -e '/^$/q' -e 's/^parent //p' payload actual
shift
printf 'parent %s\n' $@ expect
test_cmp expect actual
}
I think I'll rather use something like:
test $(git rev-parse $HASH5^1) = $HASH4
test $(git rev-parse $HASH5^2) = $HASH3
...
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 v5 5/7] replace: refactor replacing parents
On Wed, Jul 2, 2014 at 11:05 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder chrisc...@tuxfamily.org writes: Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 42 +- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c index 3515979..ad47237 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -295,27 +295,14 @@ static int edit_and_replace(const char *object_ref, int force) return replace_object_sha1(object_ref, old, replacement, new, force); } -static int create_graft(int argc, const char **argv, int force) +static void replace_parents(struct strbuf *buf, int argc, const char **argv) It is somewhat strange to see that a new function introduced earlier in the series is rewritten with a refactoring. Shouldn't the new function have been done right from the beginning instead? Yeah, I will do it right from the beginning in the next patch series. -- 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 v5 6/7] replace: remove signature when using --graft
On Wed, Jul 2, 2014 at 11:19 PM, Junio C Hamano gits...@pobox.com wrote:
Christian Couder chrisc...@tuxfamily.org writes:
It could be misleading to keep a signature in a
replacement commit, so let's remove it.
Note that there should probably be a way to sign
the replacement commit created when using --graft,
but this can be dealt with in another commit or
patch series.
Both paragraphs read very sensibly.
Thanks.
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -344,6 +344,11 @@ static int create_graft(int argc, const char **argv,
int force)
replace_parents(buf, argc, argv);
+ if (remove_signature(buf))
+ warning(_(the original commit '%s' has a gpg signature.\n
+ It will be removed in the replacement commit!),
Hmmm... does the second line of this message start with the usual
warning: prefix?
Ok, I will use following:
if (remove_signature(buf)) {
warning(_(the original commit '%s' has a gpg signature.), old_ref);
warning(_(the signature will be removed in the replacement commit!));
}
diff --git a/commit.c b/commit.c
index fb7897c..54e157d 100644
--- a/commit.c
+++ b/commit.c
@@ -1177,6 +1177,40 @@ int parse_signed_commit(const struct commit *commit,
return saw_signature;
}
+int remove_signature(struct strbuf *buf)
+{
+ const char *line = buf-buf;
+ const char *tail = buf-buf + buf-len;
+ int in_signature = 0;
+ const char *sig_start = NULL;
+ const char *sig_end = NULL;
+
+ while (line tail) {
+ const char *next = memchr(line, '\n', tail - line);
+ next = next ? next + 1 : tail;
This almost makes me wonder if we want something similar to
strchrnul() we use for NUL-terminated strings, and I suspect that
you would find more instances by running git grep -A2 memchr.
I don't know what such a helper function should be named, though.
Certainly not memchrnul().
I can add this to a GSoC microproject page for next year.
+ if (in_signature line[0] == ' ')
+ sig_end = next;
+ else if (starts_with(line, gpg_sig_header)
+ line[gpg_sig_header_len] == ' ') {
+ sig_start = line;
+ sig_end = next;
+ in_signature = 1;
+ } else {
+ if (*line == '\n')
+ /* dump the whole remainder of the buffer */
+ next = tail;
+ in_signature = 0;
+ }
+ line = next;
+ }
+
+ if (sig_start)
+ strbuf_remove(buf, sig_start - buf-buf, sig_end - sig_start);
If there are two instances of gpg_sig, this will remove only the
last one, but there is no chance both signatures of such a commit
can validate OK, and we won't be losing something in between anyway,
so it should be fine.
Ok.
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 v5 7/7] replace: add test for --graft with signed commit
On Wed, Jul 2, 2014 at 11:22 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder chrisc...@tuxfamily.org writes: +test_expect_success GPG 'set up a signed commit' ' + echo line 17 hello + echo line 18 hello Style? Yeah, I will change it to: echo line 17 hello echo line 18 hello + git add hello + test_tick + git commit --quiet -S -m hello: 2 more lines in a signed commit + HASH8=$(git rev-parse --verify HEAD) + git verify-commit $HASH8 +' + +test_expect_success GPG '--graft with a signed commit' ' + git cat-file commit $HASH8 orig + git replace --graft $HASH8 + git cat-file commit $HASH8 repl + test_must_fail grep gpgsig repl + diff -u orig repl | grep ^-parent $HASH7 + diff -u orig repl | grep ^-gpgsig -BEGIN PGP SIGNATURE- Almost the same comment as the one for 2/7 applies here. Ok, I will find a way to make it better. 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 v6 00/10] Add --graft option to git replace
Here is a small series to implement: git replace [-f] --graft commit [parent...] This patch series goes on top of the patch series that implements --edit. The changes since v5, thanks to Junio, are: - new patch 1/10 to clean up redirection style in t6050 - new patches 8/10, 9/10 and 10/10 to check mergetags - add functions to test parents in patch 3/10 and 7/10 - improve testing signed commits in patch 7/10 - improve warning when removing commit signature in patch 6/10 Christian Couder (10): replace: cleanup redirection style in tests replace: add --graft option replace: add test for --graft Documentation: replace: add --graft option contrib: add convert-grafts-to-replace-refs.sh replace: remove signature when using --graft replace: add test for --graft with signed commit commit: add for_each_mergetag() replace: check mergetags when using --graft replace: add test for --graft with a mergetag Documentation/git-replace.txt | 10 +++ builtin/replace.c | 126 +++- commit.c | 47 +++ commit.h | 7 ++ contrib/convert-grafts-to-replace-refs.sh | 28 +++ log-tree.c| 15 +--- t/t6050-replace.sh| 135 -- 7 files changed, 332 insertions(+), 36 deletions(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh -- 2.0.0.421.g786a89d.dirty -- 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 v6 01/10] replace: cleanup redirection style in tests
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 48 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..fb07ad2 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -27,36 +27,36 @@ HASH6= HASH7= test_expect_success 'set up buggy branch' ' - echo line 1 hello - echo line 2 hello - echo line 3 hello - echo line 4 hello + echo line 1 hello + echo line 2 hello + echo line 3 hello + echo line 4 hello add_and_commit_file hello 4 lines HASH1=$(git rev-parse --verify HEAD) - echo line BUG hello - echo line 6 hello - echo line 7 hello - echo line 8 hello + echo line BUG hello + echo line 6 hello + echo line 7 hello + echo line 8 hello add_and_commit_file hello 4 more lines with a BUG HASH2=$(git rev-parse --verify HEAD) - echo line 9 hello - echo line 10 hello + echo line 9 hello + echo line 10 hello add_and_commit_file hello 2 more lines HASH3=$(git rev-parse --verify HEAD) - echo line 11 hello + echo line 11 hello add_and_commit_file hello 1 more line HASH4=$(git rev-parse --verify HEAD) - sed -e s/BUG/5/ hello hello.new + sed -e s/BUG/5/ hello hello.new mv hello.new hello add_and_commit_file hello BUG fixed HASH5=$(git rev-parse --verify HEAD) - echo line 12 hello - echo line 13 hello + echo line 12 hello + echo line 13 hello add_and_commit_file hello 2 more lines HASH6=$(git rev-parse --verify HEAD) - echo line 14 hello - echo line 15 hello - echo line 16 hello + echo line 14 hello + echo line 15 hello + echo line 16 hello add_and_commit_file hello again 3 more lines HASH7=$(git rev-parse --verify HEAD) ' @@ -95,7 +95,7 @@ test_expect_success 'tag replaced commit' ' ' test_expect_success 'git fsck works' ' - git fsck master fsck_master.out + git fsck master fsck_master.out grep dangling commit $R fsck_master.out grep dangling tag $(cat .git/refs/tags/mytag) fsck_master.out test -z $(git fsck) @@ -217,14 +217,14 @@ test_expect_success 'fetch branch with replacement' ' ( cd clone_dir git fetch origin refs/heads/tofetch:refs/heads/parallel3 - git log --pretty=oneline parallel3 output.txt + git log --pretty=oneline parallel3 output.txt ! grep $PARA3 output.txt - git show $PARA3 para3.txt + git show $PARA3 para3.txt grep A U Thor para3.txt git fetch origin refs/replace/*:refs/replace/* - git log --pretty=oneline parallel3 output.txt + git log --pretty=oneline parallel3 output.txt grep $PARA3 output.txt - git show $PARA3 para3.txt + git show $PARA3 para3.txt grep O Thor para3.txt ) ' @@ -302,7 +302,7 @@ test_expect_success 'test --format medium' ' echo $PARA3 - $S echo $MYTAG - $HASH1 } | sort expected - git replace -l --format medium | sort actual + git replace -l --format medium | sort actual test_cmp expected actual ' @@ -314,7 +314,7 @@ test_expect_success 'test --format long' ' echo $PARA3 (commit) - $S (commit) echo $MYTAG (tag) - $HASH1 (commit) } | sort expected - git replace --format=long | sort actual + git replace --format=long | sort actual test_cmp expected actual ' -- 2.0.0.421.g786a89d.dirty -- 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 v6 02/10] 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.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/replace.c | 74 ++-
1 file changed, 73 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..ad47237 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,66 @@ static int edit_and_replace(const char *object_ref, int
force)
return replace_object_sha1(object_ref, old, replacement, new, force);
}
+static void replace_parents(struct strbuf *buf, int argc, const char **argv)
+{
+ struct strbuf new_parents = STRBUF_INIT;
+ const char *parent_start, *parent_end;
+ int i;
+
+ /* 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);
+
+ strbuf_release(new_parents);
+}
+
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ const char *buffer;
+ unsigned long size;
+
+ if (get_sha1(old_ref, old) 0)
+ die(_(Not a valid object name: '%s'), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
+
+ buffer = get_commit_buffer(commit, size);
+ strbuf_add(buf, buffer, size);
+ unuse_commit_buffer(commit, buffer);
+
+ replace_parents(buf, argc, argv);
+
+ if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+ die(_(could not write replacement commit for: '%s'), old_ref);
+
+ 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 +364,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 +388,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 a replacement,
git_replace_usage, options);
@@ -348,6 +414,12 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
git_replace_usage, options);
return edit_and_replace(argv[0], force);
+ case
[PATCH v6 06/10] replace: remove signature when using --graft
It could be misleading to keep a signature in a
replacement commit, so let's remove it.
Note that there should probably be a way to sign
the replacement commit created when using --graft,
but this can be dealt with in another commit or
patch series.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 5 +
commit.c | 34 ++
commit.h | 2 ++
3 files changed, 41 insertions(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index ad47237..cc29ef2 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -344,6 +344,11 @@ static int create_graft(int argc, const char **argv, int
force)
replace_parents(buf, argc, argv);
+ if (remove_signature(buf)) {
+ warning(_(the original commit '%s' has a gpg signature.),
old_ref);
+ warning(_(the signature will be removed in the replacement
commit!));
+ }
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_(could not write replacement commit for: '%s'), old_ref);
diff --git a/commit.c b/commit.c
index fb7897c..54e157d 100644
--- a/commit.c
+++ b/commit.c
@@ -1177,6 +1177,40 @@ int parse_signed_commit(const struct commit *commit,
return saw_signature;
}
+int remove_signature(struct strbuf *buf)
+{
+ const char *line = buf-buf;
+ const char *tail = buf-buf + buf-len;
+ int in_signature = 0;
+ const char *sig_start = NULL;
+ const char *sig_end = NULL;
+
+ while (line tail) {
+ const char *next = memchr(line, '\n', tail - line);
+ next = next ? next + 1 : tail;
+
+ if (in_signature line[0] == ' ')
+ sig_end = next;
+ else if (starts_with(line, gpg_sig_header)
+line[gpg_sig_header_len] == ' ') {
+ sig_start = line;
+ sig_end = next;
+ in_signature = 1;
+ } else {
+ if (*line == '\n')
+ /* dump the whole remainder of the buffer */
+ next = tail;
+ in_signature = 0;
+ }
+ line = next;
+ }
+
+ if (sig_start)
+ strbuf_remove(buf, sig_start - buf-buf, sig_end - sig_start);
+
+ return sig_start != NULL;
+}
+
static void handle_signed_tag(struct commit *parent, struct
commit_extra_header ***tail)
{
struct merge_remote_desc *desc;
diff --git a/commit.h b/commit.h
index 2e1492a..4234dae 100644
--- a/commit.h
+++ b/commit.h
@@ -327,6 +327,8 @@ struct commit *get_merge_parent(const char *name);
extern int parse_signed_commit(const struct commit *commit,
struct strbuf *message, struct strbuf
*signature);
+extern int remove_signature(struct strbuf *buf);
+
extern void print_commit_list(struct commit_list *list,
const char *format_cur,
const char *format_last);
--
2.0.0.421.g786a89d.dirty
--
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 v6 05/10] contrib: add convert-grafts-to-replace-refs.sh
This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of git replace.
While at it let's mention this new script in the
git replace documentation for the --graft option.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-replace.txt | 4 +++-
contrib/convert-grafts-to-replace-refs.sh | 28
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100755 contrib/convert-grafts-to-replace-refs.sh
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as commit except that its parents will be
[parent...] instead of commit's parents. A replacement ref
is then created to replace commit with the newly created
- commit.
+ commit. See contrib/convert-grafts-to-replace-refs.sh for an
+ example script based on this option that can convert grafts to
+ replace refs.
-l pattern::
--list pattern::
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..0cbc917
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+. $(git --exec-path)/git-sh-setup
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE |
+while read definition
+do
+ if test -n $definition
+ then
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Conversion failed for: $definition
+ fi
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+ die Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
+
+echo Success!
+echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs!
+echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'
--
2.0.0.421.g786a89d.dirty
--
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 v6 10/10] replace: add test for --graft with a mergetag
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 22 ++ 1 file changed, 22 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 15fd541..3bb8d06 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -416,4 +416,26 @@ test_expect_success GPG '--graft with a signed commit' ' git replace -d $HASH8 ' +test_expect_success GPG 'set up a merge commit with a mergetag' ' + git reset --hard HEAD + git checkout -b test_branch HEAD~2 + echo line 1 from test branch hello + echo line 2 from test branch hello + git add hello + test_tick + git commit -m hello: 2 more lines from a test branch + HASH9=$(git rev-parse --verify HEAD) + git tag -s -m tag for testing with a mergetag test_tag HEAD + git checkout master + git merge -s ours test_tag + HASH10=$(git rev-parse --verify HEAD) + git cat-file commit $HASH10 | grep ^mergetag object +' + +test_expect_success GPG '--graft on a commit with a mergetag' ' + test_must_fail git replace --graft $HASH10 $HASH8^1 + git replace --graft $HASH10 $HASH8^1 $HASH9 + git replace -d $HASH10 +' + test_done -- 2.0.0.421.g786a89d.dirty -- 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 v6 04/10] Documentation: replace: add --graft option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-replace.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 61461b9..491875e 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement 'git replace' [-f] --edit object +'git replace' [-f] --graft commit [parent...] 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -73,6 +74,13 @@ OPTIONS newly created object. See linkgit:git-var[1] for details about how the editor will be chosen. +--graft commit [parent...]:: + Create a graft commit. A new commit is created with the same + content as commit except that its parents will be + [parent...] instead of commit's parents. A replacement ref + is then created to replace commit with the newly created + commit. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or -- 2.0.0.421.g786a89d.dirty -- 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 v6 07/10] replace: add test for --graft with signed commit
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 25 +
1 file changed, 25 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d80a89e..15fd541 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -7,6 +7,7 @@ test_description='Tests replace refs functionality'
exec /dev/null
. ./test-lib.sh
+. $TEST_DIRECTORY/lib-gpg.sh
add_and_commit_file()
{
@@ -391,4 +392,28 @@ test_expect_success '--graft with and without already
replaced object' '
git replace -d $HASH5
'
+test_expect_success GPG 'set up a signed commit' '
+ echo line 17 hello
+ echo line 18 hello
+ git add hello
+ test_tick
+ git commit --quiet -S -m hello: 2 more lines in a signed commit
+ HASH8=$(git rev-parse --verify HEAD)
+ git verify-commit $HASH8
+'
+
+test_expect_success GPG '--graft with a signed commit' '
+ git cat-file commit $HASH8 orig
+ git replace --graft $HASH8
+ git cat-file commit $HASH8 repl
+ commit_buffer_contains_parents $HASH8
+ commit_has_parents $HASH8
+ test_must_fail git verify-commit $HASH8
+ sed -n -e /^tree /p -e /^author /p -e /^committer /p orig
expected
+ echo expected
+ sed -e /^$/q repl actual
+ test_cmp expected actual
+ git replace -d $HASH8
+'
+
test_done
--
2.0.0.421.g786a89d.dirty
--
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 v6 03/10] replace: add test for --graft
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t6050-replace.sh | 40
1 file changed, 40 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index fb07ad2..d80a89e 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -18,6 +18,33 @@ add_and_commit_file()
git commit --quiet -m $_file: $_msg
}
+commit_buffer_contains_parents()
+{
+git cat-file commit $1 payload
+sed -n -e '/^$/q' -e '/^parent /p' payload actual
+shift
+: expected
+for _parent
+do
+ echo parent $_parent expected || return 1
+done
+test_cmp actual expected
+}
+
+commit_has_parents()
+{
+_parent_number=1
+_commit=$1
+shift
+for _parent
+do
+ _found=$(git rev-parse --verify $_commit^$_parent_number) || return 1
+ test $_found = $_parent || return 1
+ _parent_number=$(( $_parent_number + 1 ))
+done
+test_must_fail git rev-parse --verify $_commit^$_parent_number
+}
+
HASH1=
HASH2=
HASH3=
@@ -351,4 +378,17 @@ test_expect_success 'replace ref cleanup' '
test -z $(git replace)
'
+test_expect_success '--graft with and without already replaced object' '
+ test $(git log --oneline | wc -l) = 7
+ git replace --graft $HASH5
+ test $(git log --oneline | wc -l) = 3
+ commit_buffer_contains_parents $HASH5
+ commit_has_parents $HASH5
+ test_must_fail git replace --graft $HASH5 $HASH4 $HASH3
+ git replace --force -g $HASH5 $HASH4 $HASH3
+ commit_buffer_contains_parents $HASH5 $HASH4 $HASH3
+ commit_has_parents $HASH5 $HASH4 $HASH3
+ git replace -d $HASH5
+'
+
test_done
--
2.0.0.421.g786a89d.dirty
--
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 v6 09/10] replace: check mergetags when using --graft
When using --graft, with a mergetag in the original
commit, we should check that the commit pointed to by
the mergetag is still a parent of then new commit we
create, otherwise the mergetag could be misleading.
If the commit pointed to by the mergetag is no more
a parent of the new commit, we could remove the
mergetag, but in this case there is a good chance
that the title or other elements of the commit might
also be misleading. So let's just error out and
suggest to use --edit instead on the commit.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index cc29ef2..2290529 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -13,6 +13,7 @@
#include refs.h
#include parse-options.h
#include run-command.h
+#include tag.h
static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
@@ -325,6 +326,50 @@ static void replace_parents(struct strbuf *buf, int argc,
const char **argv)
strbuf_release(new_parents);
}
+struct check_mergetag_data {
+ int argc;
+ const char **argv;
+};
+
+static void check_one_mergetag(struct commit *commit,
+ struct commit_extra_header *extra,
+ void *data)
+{
+ struct check_mergetag_data *mergetag_data = (struct check_mergetag_data
*)data;
+ const char *ref = mergetag_data-argv[0];
+ unsigned char tag_sha1[20];
+ struct tag *tag;
+ int i;
+
+ hash_sha1_file(extra-value, extra-len, typename(OBJ_TAG), tag_sha1);
+ tag = lookup_tag(tag_sha1);
+ if (!tag)
+ die(_(bad mergetag in commit '%s'), ref);
+ if (parse_tag_buffer(tag, extra-value, extra-len))
+ die(_(malformed mergetag in commit '%s'), ref);
+
+ /* iterate over new parents */
+ for (i = 1; i mergetag_data-argc; i++) {
+ unsigned char sha1[20];
+ if (get_sha1(mergetag_data-argv[i], sha1) 0)
+ die(_(Not a valid object name: '%s'),
mergetag_data-argv[i]);
+ if (!hashcmp(tag-tagged-sha1, sha1))
+ return; /* found */
+ }
+
+ die(_(original commit '%s' contains mergetag '%s' that is discarded;
+ use --edit instead of --graft), ref, sha1_to_hex(tag_sha1));
+}
+
+static void check_mergetags(struct commit *commit, int argc, const char **argv)
+{
+ struct check_mergetag_data mergetag_data;
+
+ mergetag_data.argc = argc;
+ mergetag_data.argv = argv;
+ for_each_mergetag(check_one_mergetag, commit, mergetag_data);
+}
+
static int create_graft(int argc, const char **argv, int force)
{
unsigned char old[20], new[20];
@@ -349,6 +394,8 @@ static int create_graft(int argc, const char **argv, int
force)
warning(_(the signature will be removed in the replacement
commit!));
}
+ check_mergetags(commit, argc, argv);
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_(could not write replacement commit for: '%s'), old_ref);
--
2.0.0.421.g786a89d.dirty
--
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 v6 08/10] commit: add for_each_mergetag()
In the same way as there is for_each_ref() to
iterate on refs, it might be useful to have
for_each_mergetag() to iterate on the mergetags
of a given commit.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
commit.c | 13 +
commit.h | 5 +
log-tree.c | 15 ---
3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/commit.c b/commit.c
index 54e157d..0f83ff7 100644
--- a/commit.c
+++ b/commit.c
@@ -1348,6 +1348,19 @@ struct commit_extra_header
*read_commit_extra_headers(struct commit *commit,
return extra;
}
+void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
+{
+ struct commit_extra_header *extra, *to_free;
+
+ to_free = read_commit_extra_headers(commit, NULL);
+ for (extra = to_free; extra; extra = extra-next) {
+ if (strcmp(extra-key, mergetag))
+ continue; /* not a merge tag */
+ fn(commit, extra, data);
+ }
+ free_commit_extra_headers(to_free);
+}
+
static inline int standard_header_field(const char *field, size_t len)
{
return ((len == 4 !memcmp(field, tree , 5)) ||
diff --git a/commit.h b/commit.h
index 4234dae..c81ba85 100644
--- a/commit.h
+++ b/commit.h
@@ -312,6 +312,11 @@ extern struct commit_extra_header
*read_commit_extra_headers(struct commit *, co
extern void free_commit_extra_headers(struct commit_extra_header *extra);
+typedef void (*each_mergetag_fn)(struct commit *commit, struct
commit_extra_header *extra,
+void *cb_data);
+
+extern void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void
*data);
+
struct merge_remote_desc {
struct object *obj; /* the named object, could be a tag */
const char *name;
diff --git a/log-tree.c b/log-tree.c
index 10e6844..706ed4c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -413,10 +413,11 @@ static int is_common_merge(const struct commit *commit)
!commit-parents-next-next);
}
-static void show_one_mergetag(struct rev_info *opt,
+static void show_one_mergetag(struct commit *commit,
struct commit_extra_header *extra,
- struct commit *commit)
+ void *data)
{
+ struct rev_info *opt = (struct rev_info *)data;
unsigned char sha1[20];
struct tag *tag;
struct strbuf verify_message;
@@ -463,15 +464,7 @@ static void show_one_mergetag(struct rev_info *opt,
static void show_mergetag(struct rev_info *opt, struct commit *commit)
{
- struct commit_extra_header *extra, *to_free;
-
- to_free = read_commit_extra_headers(commit, NULL);
- for (extra = to_free; extra; extra = extra-next) {
- if (strcmp(extra-key, mergetag))
- continue; /* not a merge tag */
- show_one_mergetag(opt, extra, commit);
- }
- free_commit_extra_headers(to_free);
+ for_each_mergetag(show_one_mergetag, commit, opt);
}
void show_log(struct rev_info *opt)
--
2.0.0.421.g786a89d.dirty
--
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 v6 09/10] replace: check mergetags when using --graft
From: Junio C Hamano gits...@pobox.com Christian Couder chrisc...@tuxfamily.org writes: When using --graft, with a mergetag in the original commit, we should check that the commit pointed to by the mergetag is still a parent of then new commit we create, otherwise the mergetag could be misleading. If the commit pointed to by the mergetag is no more a parent of the new commit, we could remove the mergetag, but in this case there is a good chance that the title or other elements of the commit might also be misleading. So let's just error out and suggest to use --edit instead on the commit. I do not quite understand the reasoning. If you have a merge you earlier made with a signed tag, and then want to redo the merge with an updated tip of that branch (perhaps the side branch was earlier based on maint but now was rebased on master), it will perfectly be normal to expect that the title or other elements of the resulting merge to stay the same. Yeah, but then you might also want to have a mergetag for the updated tip of the branch and --graft will not put it in the new commit, so it would be better to use --edit in this case. Why is it a good idea to error it out? Because sometimes, in complex cases, it is misleading to do as if you can do the right thing, when there is a good chance you cannot. If the argument were 'replace --graft that changes the parents is likely to affect merge summary message, so error out and suggest to use --edit instead', regardless of the 'mergetag', I'd understand it, but that would essentially mean that 'replace --graft' should never be used, so... I think that when replace --graft is used on a regular commit there is much better chance that the resulting replacement commit will be as the user expect it to be. 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 v6 03/10] replace: add test for --graft
From: Junio C Hamano gits...@pobox.com
Christian Couder chrisc...@tuxfamily.org writes:
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t6050-replace.sh | 40
1 file changed, 40 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index fb07ad2..d80a89e 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -18,6 +18,33 @@ add_and_commit_file()
git commit --quiet -m $_file: $_msg
}
+commit_buffer_contains_parents()
SP before (), perhaps?
Ok.
+{
+git cat-file commit $1 payload
+sed -n -e '/^$/q' -e '/^parent /p' payload actual
+shift
+: expected
+for _parent
+do
+echo parent $_parent expected || return 1
+done
You can redirect the entire loop to simplify the above 5 lines,
which would read better, no?
for _parent
do
echo parent $_parent
done expect
Ok, thanks.
+test_cmp actual expected
As test_cmp normally runs diff, it is better to compare expect
with actual, not the other way around; running the test verbosely,
i.e. t6050-replace.sh -v, shows how the actual output differs from
what is expected when diagnosing breakage and would be more useful
that way.
Ok.
If your goal is to test both the object contents with this code
*and* the overall system behaviour with the $child^$parent below,
perhaps they should be in the same commit_has_parents function,
without forcing the caller to choose between the two or call both,
no?
Yeah, will do.
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 v6 08/10] commit: add for_each_mergetag()
From: Junio C Hamano gits...@pobox.com Christian Couder chrisc...@tuxfamily.org writes: In the same way as there is for_each_ref() to iterate on refs, it might be useful to have for_each_mergetag() to iterate on the mergetags of a given commit. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Heh, might be useful is an understatement ;-) We won't apply a change that might be useful very lightly, but this refactoring already uses the helper in existing code, showing that it *is* useful, no? Let's have this early in the series, or perhaps even independent of the replace series. Ok, 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 v6 02/10] replace: add --graft option
On Wed, Jul 9, 2014 at 4:59 PM, Junio C Hamano gits...@pobox.com wrote:
Christian Couder chrisc...@tuxfamily.org writes:
+static void replace_parents(struct strbuf *buf, int argc, const char **argv)
+{
+ struct strbuf new_parents = STRBUF_INIT;
+ const char *parent_start, *parent_end;
+ int i;
+
+ /* 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++) {
It looks somewhat strange that both replace_parents() and
create_graft() take familiar-looking argc, argv pair, but one
ignores argv[0] and uses the remainder and the other uses argv[0].
Shouldn't this function consume argv[] starting from [0] for
consistency? You'd obviously need to update the caller to adjust
the arguments it gives to this function.
Ok, will do.
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+...
+
+ replace_parents(buf, argc, argv);
+
+ if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+ die(_(could not write replacement commit for: '%s'),
old_ref);
+
+ strbuf_release(buf);
+
+ if (!hashcmp(old, new))
+ return error(new commit is the same as the old one: '%s',
sha1_to_hex(old));
Is this really an error? It may be a warning-worthy situation for a
user or a script to end up doing a no-op graft, e.g.
git replace --graft HEAD HEAD^
but I wonder if it is more convenient to signal an error (like this
patch does) or just ignore the request and return without adding the
replace ref.
As the user might expect that a new replace ref was created on success
(0 exit code), and as we should at least warn if we would create a
commit that is the same as an existing one, I think it is just simpler
to error out in this case.
Though maybe we could use a special exit code (for example 2) in this
case, so that the user might more easily ignore this error in a
script.
Other than these two points, looks good to me.
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 v6 02/10] replace: add --graft option
On Thu, Jul 10, 2014 at 7:36 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: As the user might expect that a new replace ref was created on success (0 exit code), and as we should at least warn if we would create a commit that is the same as an existing one,... Why is it an event that needs a warning? I do not buy that as we should at least at all. Here you ask why this event needs a warning... Ehh, it came a bit differently from what I meant. Perhaps s/do not buy/do not understand/ is closer to what I think---that is, it is not like I with a strong conviction think you are wrong. It is more like I do not understand why you think it needs a warning, meaing you would need to explain it better. If you say make sure A's parent is B and then you asked the same thing again when there already is a replacement in place, that should be a no-op. (When there is already a replacement in place we error out in replace_object_sha1() unless --force is used. What we are talking about here is what happens if the replacement commit is the same as the original commit.) Making sure A's parent is B would be an idempotent operation, no? Why not just make sure A's parent is already B and report Your wish has been granted to the user? ... and here you say we should report your wish has been granted... Why would it be simpler for the user to get an error, inspect the situation and realize that his wish has been granted after all? ... but for me reporting to the user your wish has been granted and warning (or errorring out) saying the new commit would be the same as the old one are nearly the same thing. So I wonder what exactly you are not happy with. Is it the fact that I use the error() function, because it would prefix the message with fatal: and that would be too scary? Is it because with error() the exit code would not be 0? Is it because the message new commit is the same as the old one: '%s' is too scary or unnecessarily technical by itself? Is it ok if I just print the message on stderr without warning: or fatal: in front of it? By the way, when I say As ... and ..., I think it is just simpler to error out in this case., I mean that it is simpler from the developer's point of view, not necessarily for the user. Of course I am ok with improving things for the user even if it requires more code/work, but it is difficult to properly do that if I don't see how I could do it. 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 v6 02/10] replace: add --graft option
On Fri, Jul 11, 2014 at 4:22 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder christian.cou...@gmail.com writes: On Thu, Jul 10, 2014 at 7:36 PM, Junio C Hamano gits...@pobox.com wrote: Making sure A's parent is B would be an idempotent operation, no? Why not just make sure A's parent is already B and report Your wish has been granted to the user? ... and here you say we should report your wish has been granted... Normal way for git replace to report that is to exit with status 0 and without any noise, I would think. In a similar case git replace --edit we error out instead of just exiting (with status 0), see: f22166b5fee7dc (replace: make sure --edit results in a different object) -- 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 v7 7/9] replace: add test for --graft with signed commit
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 24
1 file changed, 24 insertions(+)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index f854dae..cebab63 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -7,6 +7,7 @@ test_description='Tests replace refs functionality'
exec /dev/null
. ./test-lib.sh
+. $TEST_DIRECTORY/lib-gpg.sh
add_and_commit_file ()
{
@@ -394,4 +395,27 @@ test_expect_success '--graft with and without already
replaced object' '
git replace -d $HASH5
'
+test_expect_success GPG 'set up a signed commit' '
+ echo line 17 hello
+ echo line 18 hello
+ git add hello
+ test_tick
+ git commit --quiet -S -m hello: 2 more lines in a signed commit
+ HASH8=$(git rev-parse --verify HEAD)
+ git verify-commit $HASH8
+'
+
+test_expect_success GPG '--graft with a signed commit' '
+ git cat-file commit $HASH8 orig
+ git replace --graft $HASH8
+ git cat-file commit $HASH8 repl
+ commit_has_parents $HASH8
+ test_must_fail git verify-commit $HASH8
+ sed -n -e /^tree /p -e /^author /p -e /^committer /p orig
expected
+ echo expected
+ sed -e /^$/q repl actual
+ test_cmp expected actual
+ git replace -d $HASH8
+'
+
test_done
--
2.0.0.421.g786a89d.dirty
--
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 v7 4/9] Documentation: replace: add --graft option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-replace.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 61461b9..491875e 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement 'git replace' [-f] --edit object +'git replace' [-f] --graft commit [parent...] 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -73,6 +74,13 @@ OPTIONS newly created object. See linkgit:git-var[1] for details about how the editor will be chosen. +--graft commit [parent...]:: + Create a graft commit. A new commit is created with the same + content as commit except that its parents will be + [parent...] instead of commit's parents. A replacement ref + is then created to replace commit with the newly created + commit. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or -- 2.0.0.421.g786a89d.dirty -- 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 v7 3/9] replace: add test for --graft
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
t/t6050-replace.sh | 45 -
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index fb07ad2..f854dae 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -8,7 +8,7 @@ exec /dev/null
. ./test-lib.sh
-add_and_commit_file()
+add_and_commit_file ()
{
_file=$1
_msg=$2
@@ -18,6 +18,38 @@ add_and_commit_file()
git commit --quiet -m $_file: $_msg
}
+commit_buffer_contains_parents ()
+{
+git cat-file commit $1 payload
+sed -n -e '/^$/q' -e '/^parent /p' payload actual
+shift
+for _parent
+do
+ echo parent $_parent
+done expected
+test_cmp expected actual
+}
+
+commit_peeling_shows_parents ()
+{
+_parent_number=1
+_commit=$1
+shift
+for _parent
+do
+ _found=$(git rev-parse --verify $_commit^$_parent_number) || return 1
+ test $_found = $_parent || return 1
+ _parent_number=$(( $_parent_number + 1 ))
+done
+test_must_fail git rev-parse --verify $_commit^$_parent_number
+}
+
+commit_has_parents ()
+{
+commit_buffer_contains_parents $@
+commit_peeling_shows_parents $@
+}
+
HASH1=
HASH2=
HASH3=
@@ -351,4 +383,15 @@ test_expect_success 'replace ref cleanup' '
test -z $(git replace)
'
+test_expect_success '--graft with and without already replaced object' '
+ test $(git log --oneline | wc -l) = 7
+ git replace --graft $HASH5
+ test $(git log --oneline | wc -l) = 3
+ commit_has_parents $HASH5
+ test_must_fail git replace --graft $HASH5 $HASH4 $HASH3
+ git replace --force -g $HASH5 $HASH4 $HASH3
+ commit_has_parents $HASH5 $HASH4 $HASH3
+ git replace -d $HASH5
+'
+
test_done
--
2.0.0.421.g786a89d.dirty
--
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 v7 5/9] contrib: add convert-grafts-to-replace-refs.sh
This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of git replace.
While at it let's mention this new script in the
git replace documentation for the --graft option.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/git-replace.txt | 4 +++-
contrib/convert-grafts-to-replace-refs.sh | 28
2 files changed, 31 insertions(+), 1 deletion(-)
create mode 100755 contrib/convert-grafts-to-replace-refs.sh
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as commit except that its parents will be
[parent...] instead of commit's parents. A replacement ref
is then created to replace commit with the newly created
- commit.
+ commit. See contrib/convert-grafts-to-replace-refs.sh for an
+ example script based on this option that can convert grafts to
+ replace refs.
-l pattern::
--list pattern::
diff --git a/contrib/convert-grafts-to-replace-refs.sh
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..0cbc917
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts
+
+. $(git --exec-path)/git-sh-setup
+
+test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE'
+
+grep '^[^# ]' $GRAFTS_FILE |
+while read definition
+do
+ if test -n $definition
+ then
+ echo Converting: $definition
+ git replace --graft $definition ||
+ die Conversion failed for: $definition
+ fi
+done
+
+mv $GRAFTS_FILE $GRAFTS_FILE.bak ||
+ die Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'
+
+echo Success!
+echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs!
+echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'
--
2.0.0.421.g786a89d.dirty
--
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 v7 9/9] replace: add test for --graft with a mergetag
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 22 ++ 1 file changed, 22 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index cebab63..4d5a25e 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -418,4 +418,26 @@ test_expect_success GPG '--graft with a signed commit' ' git replace -d $HASH8 ' +test_expect_success GPG 'set up a merge commit with a mergetag' ' + git reset --hard HEAD + git checkout -b test_branch HEAD~2 + echo line 1 from test branch hello + echo line 2 from test branch hello + git add hello + test_tick + git commit -m hello: 2 more lines from a test branch + HASH9=$(git rev-parse --verify HEAD) + git tag -s -m tag for testing with a mergetag test_tag HEAD + git checkout master + git merge -s ours test_tag + HASH10=$(git rev-parse --verify HEAD) + git cat-file commit $HASH10 | grep ^mergetag object +' + +test_expect_success GPG '--graft on a commit with a mergetag' ' + test_must_fail git replace --graft $HASH10 $HASH8^1 + git replace --graft $HASH10 $HASH8^1 $HASH9 + git replace -d $HASH10 +' + test_done -- 2.0.0.421.g786a89d.dirty -- 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 v7 8/9] replace: check mergetags when using --graft
When using --graft, with a mergetag in the original
commit, we should check that the commit pointed to by
the mergetag is still a parent of then new commit we
create, otherwise the mergetag could be misleading.
If the commit pointed to by the mergetag is no more
a parent of the new commit, we could remove the
mergetag, but in this case there is a good chance
that the title or other elements of the commit might
also be misleading. So let's just error out and
suggest to use --edit instead on the commit.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 47 +++
1 file changed, 47 insertions(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index 52f73ce..d29026f 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -13,6 +13,7 @@
#include refs.h
#include parse-options.h
#include run-command.h
+#include tag.h
static const char * const git_replace_usage[] = {
N_(git replace [-f] object replacement),
@@ -325,6 +326,50 @@ static void replace_parents(struct strbuf *buf, int argc,
const char **argv)
strbuf_release(new_parents);
}
+struct check_mergetag_data {
+ int argc;
+ const char **argv;
+};
+
+static void check_one_mergetag(struct commit *commit,
+ struct commit_extra_header *extra,
+ void *data)
+{
+ struct check_mergetag_data *mergetag_data = (struct check_mergetag_data
*)data;
+ const char *ref = mergetag_data-argv[0];
+ unsigned char tag_sha1[20];
+ struct tag *tag;
+ int i;
+
+ hash_sha1_file(extra-value, extra-len, typename(OBJ_TAG), tag_sha1);
+ tag = lookup_tag(tag_sha1);
+ if (!tag)
+ die(_(bad mergetag in commit '%s'), ref);
+ if (parse_tag_buffer(tag, extra-value, extra-len))
+ die(_(malformed mergetag in commit '%s'), ref);
+
+ /* iterate over new parents */
+ for (i = 1; i mergetag_data-argc; i++) {
+ unsigned char sha1[20];
+ if (get_sha1(mergetag_data-argv[i], sha1) 0)
+ die(_(Not a valid object name: '%s'),
mergetag_data-argv[i]);
+ if (!hashcmp(tag-tagged-sha1, sha1))
+ return; /* found */
+ }
+
+ die(_(original commit '%s' contains mergetag '%s' that is discarded;
+ use --edit instead of --graft), ref, sha1_to_hex(tag_sha1));
+}
+
+static void check_mergetags(struct commit *commit, int argc, const char **argv)
+{
+ struct check_mergetag_data mergetag_data;
+
+ mergetag_data.argc = argc;
+ mergetag_data.argv = argv;
+ for_each_mergetag(check_one_mergetag, commit, mergetag_data);
+}
+
static int create_graft(int argc, const char **argv, int force)
{
unsigned char old[20], new[20];
@@ -349,6 +394,8 @@ static int create_graft(int argc, const char **argv, int
force)
warning(_(the signature will be removed in the replacement
commit!));
}
+ check_mergetags(commit, argc, argv);
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_(could not write replacement commit for: '%s'), old_ref);
--
2.0.0.421.g786a89d.dirty
--
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 v7 0/9] Add --graft option to git replace
Here is a small series to implement: git replace [-f] --graft commit [parent...] This patch series goes on top of the patch series that implements --edit. The changes since v6, thanks to Junio, are: - the patch that adds for_each_mergetag() is not part of the series any more; it is managed by Junio in another dedicated series - replace_parents() iterates over all the argv it is passed, not just from argv[1] to argv[argc - 1], in patch 2/9 - in t6050-replace.sh, commit_buffer_contains_parents() has been simplified, in patch 3/9 - in t6050-replace.sh, there is now a space between function names and (), in patch 3/9 - in t6050-replace.sh, there has been a refactoring regarding how commit parenthood is tested, in patchs 3/9 and 7/9; we only use commit_has_parents() that now tests everything - in t6050-replace.sh, commit_buffer_contains_parents() compares the 'expected' and 'actual' files in the right order, in patch 3/9 We still error if the replacement commit that would be created would be the same as the existing one, as we already do when using --edit. If people care, I suggest they send a patch to change both --graft and --edit at the same time. Christian Couder (9): replace: cleanup redirection style in tests replace: add --graft option replace: add test for --graft Documentation: replace: add --graft option contrib: add convert-grafts-to-replace-refs.sh replace: remove signature when using --graft replace: add test for --graft with signed commit replace: check mergetags when using --graft replace: add test for --graft with a mergetag Documentation/git-replace.txt | 10 +++ builtin/replace.c | 126 ++- commit.c | 34 commit.h | 2 + contrib/convert-grafts-to-replace-refs.sh | 28 ++ t/t6050-replace.sh| 139 -- 6 files changed, 313 insertions(+), 26 deletions(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh -- 2.0.0.421.g786a89d.dirty -- 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 v7 2/9] 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.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/replace.c | 74 ++-
1 file changed, 73 insertions(+), 1 deletion(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..7459359 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,66 @@ static int edit_and_replace(const char *object_ref, int
force)
return replace_object_sha1(object_ref, old, replacement, new, force);
}
+static void replace_parents(struct strbuf *buf, int argc, const char **argv)
+{
+ struct strbuf new_parents = STRBUF_INIT;
+ const char *parent_start, *parent_end;
+ int i;
+
+ /* 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 = 0; 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);
+
+ strbuf_release(new_parents);
+}
+
+static int create_graft(int argc, const char **argv, int force)
+{
+ unsigned char old[20], new[20];
+ const char *old_ref = argv[0];
+ struct commit *commit;
+ struct strbuf buf = STRBUF_INIT;
+ const char *buffer;
+ unsigned long size;
+
+ if (get_sha1(old_ref, old) 0)
+ die(_(Not a valid object name: '%s'), old_ref);
+ commit = lookup_commit_or_die(old, old_ref);
+
+ buffer = get_commit_buffer(commit, size);
+ strbuf_add(buf, buffer, size);
+ unuse_commit_buffer(commit, buffer);
+
+ replace_parents(buf, argc - 1, argv[1]);
+
+ if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+ die(_(could not write replacement commit for: '%s'), old_ref);
+
+ 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 +364,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 +388,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 a replacement,
git_replace_usage, options);
@@ -348,6 +414,12 @@ int cmd_replace(int argc, const char **argv, const char
*prefix)
git_replace_usage, options);
return edit_and_replace(argv[0], force
[PATCH v7 1/9] replace: cleanup redirection style in tests
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 48 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..fb07ad2 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -27,36 +27,36 @@ HASH6= HASH7= test_expect_success 'set up buggy branch' ' - echo line 1 hello - echo line 2 hello - echo line 3 hello - echo line 4 hello + echo line 1 hello + echo line 2 hello + echo line 3 hello + echo line 4 hello add_and_commit_file hello 4 lines HASH1=$(git rev-parse --verify HEAD) - echo line BUG hello - echo line 6 hello - echo line 7 hello - echo line 8 hello + echo line BUG hello + echo line 6 hello + echo line 7 hello + echo line 8 hello add_and_commit_file hello 4 more lines with a BUG HASH2=$(git rev-parse --verify HEAD) - echo line 9 hello - echo line 10 hello + echo line 9 hello + echo line 10 hello add_and_commit_file hello 2 more lines HASH3=$(git rev-parse --verify HEAD) - echo line 11 hello + echo line 11 hello add_and_commit_file hello 1 more line HASH4=$(git rev-parse --verify HEAD) - sed -e s/BUG/5/ hello hello.new + sed -e s/BUG/5/ hello hello.new mv hello.new hello add_and_commit_file hello BUG fixed HASH5=$(git rev-parse --verify HEAD) - echo line 12 hello - echo line 13 hello + echo line 12 hello + echo line 13 hello add_and_commit_file hello 2 more lines HASH6=$(git rev-parse --verify HEAD) - echo line 14 hello - echo line 15 hello - echo line 16 hello + echo line 14 hello + echo line 15 hello + echo line 16 hello add_and_commit_file hello again 3 more lines HASH7=$(git rev-parse --verify HEAD) ' @@ -95,7 +95,7 @@ test_expect_success 'tag replaced commit' ' ' test_expect_success 'git fsck works' ' - git fsck master fsck_master.out + git fsck master fsck_master.out grep dangling commit $R fsck_master.out grep dangling tag $(cat .git/refs/tags/mytag) fsck_master.out test -z $(git fsck) @@ -217,14 +217,14 @@ test_expect_success 'fetch branch with replacement' ' ( cd clone_dir git fetch origin refs/heads/tofetch:refs/heads/parallel3 - git log --pretty=oneline parallel3 output.txt + git log --pretty=oneline parallel3 output.txt ! grep $PARA3 output.txt - git show $PARA3 para3.txt + git show $PARA3 para3.txt grep A U Thor para3.txt git fetch origin refs/replace/*:refs/replace/* - git log --pretty=oneline parallel3 output.txt + git log --pretty=oneline parallel3 output.txt grep $PARA3 output.txt - git show $PARA3 para3.txt + git show $PARA3 para3.txt grep O Thor para3.txt ) ' @@ -302,7 +302,7 @@ test_expect_success 'test --format medium' ' echo $PARA3 - $S echo $MYTAG - $HASH1 } | sort expected - git replace -l --format medium | sort actual + git replace -l --format medium | sort actual test_cmp expected actual ' @@ -314,7 +314,7 @@ test_expect_success 'test --format long' ' echo $PARA3 (commit) - $S (commit) echo $MYTAG (tag) - $HASH1 (commit) } | sort expected - git replace --format=long | sort actual + git replace --format=long | sort actual test_cmp expected actual ' -- 2.0.0.421.g786a89d.dirty -- 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 v7 6/9] replace: remove signature when using --graft
It could be misleading to keep a signature in a
replacement commit, so let's remove it.
Note that there should probably be a way to sign
the replacement commit created when using --graft,
but this can be dealt with in another commit or
patch series.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 5 +
commit.c | 34 ++
commit.h | 2 ++
3 files changed, 41 insertions(+)
diff --git a/builtin/replace.c b/builtin/replace.c
index 7459359..52f73ce 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -344,6 +344,11 @@ static int create_graft(int argc, const char **argv, int
force)
replace_parents(buf, argc - 1, argv[1]);
+ if (remove_signature(buf)) {
+ warning(_(the original commit '%s' has a gpg signature.),
old_ref);
+ warning(_(the signature will be removed in the replacement
commit!));
+ }
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_(could not write replacement commit for: '%s'), old_ref);
diff --git a/commit.c b/commit.c
index 0fd04ae..27c3c05 100644
--- a/commit.c
+++ b/commit.c
@@ -1177,6 +1177,40 @@ int parse_signed_commit(const struct commit *commit,
return saw_signature;
}
+int remove_signature(struct strbuf *buf)
+{
+ const char *line = buf-buf;
+ const char *tail = buf-buf + buf-len;
+ int in_signature = 0;
+ const char *sig_start = NULL;
+ const char *sig_end = NULL;
+
+ while (line tail) {
+ const char *next = memchr(line, '\n', tail - line);
+ next = next ? next + 1 : tail;
+
+ if (in_signature line[0] == ' ')
+ sig_end = next;
+ else if (starts_with(line, gpg_sig_header)
+line[gpg_sig_header_len] == ' ') {
+ sig_start = line;
+ sig_end = next;
+ in_signature = 1;
+ } else {
+ if (*line == '\n')
+ /* dump the whole remainder of the buffer */
+ next = tail;
+ in_signature = 0;
+ }
+ line = next;
+ }
+
+ if (sig_start)
+ strbuf_remove(buf, sig_start - buf-buf, sig_end - sig_start);
+
+ return sig_start != NULL;
+}
+
static void handle_signed_tag(struct commit *parent, struct
commit_extra_header ***tail)
{
struct merge_remote_desc *desc;
diff --git a/commit.h b/commit.h
index b695aa4..c81ba85 100644
--- a/commit.h
+++ b/commit.h
@@ -332,6 +332,8 @@ struct commit *get_merge_parent(const char *name);
extern int parse_signed_commit(const struct commit *commit,
struct strbuf *message, struct strbuf
*signature);
+extern int remove_signature(struct strbuf *buf);
+
extern void print_commit_list(struct commit_list *list,
const char *format_cur,
const char *format_last);
--
2.0.0.421.g786a89d.dirty
--
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] replace: fix replacing object with itself
[Sorry to resend this. I am really bad at making gmail on my Android smartphone send plain text emails.] On Fri, Nov 14, 2014 at 11:45 PM, Junio C Hamano gits...@pobox.com wrote: Manzur Mukhitdinov manzu...@gmail.com writes: When object is replaced with itself git shows unhelpful messages like(git log): fatal: replace depth too high for object SHA1 Prevents user from replacing object with itself(with test for checking this case). Signed-off-by: Manzur Mukhitdinov manzu...@gmail.com --- The patch is not wrong per-se, but I wonder how useful this do not replace itself but all other forms of loops are not checked at all would be in practice. If your user did this: git replace A B ;# pretend as if what is in B is in A git replace B C ;# pretend as if what is in C is in B git replace C A ;# pretend as if we have loop git log C she would not be helped with this patch at all, no? Yeah, but such loops are much less likely mistakes and are more difficult to detect, so I think this patch is at least a good first step. We have the replace depth thing, which is a poor-man's substitute for loop detection, primarily because we do not want to incur high cost of loop detection at runtime. Shouldn't we be doing at least the same amount of loop-avoidance check, if we really want to avoid triggering the replace depth check at runtime? We could check at replace ref creation time, but what if the user already has a ref that replaces A using B, and then a fetch adds a ref that replaces B using A? Maybe we should accept that we have to rely on the runtime replace depth anyway, and improve its output first. 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] Introduce a hook to run after formatting patches
On Mon, Nov 17, 2014 at 8:20 PM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: (I am not saying that there should be an easy way to drop cruft left by third-party systems such as Change-id: line) ... Heh, that was should not be, but I guess it was probably obvious. Sorry for the noise. I am not sure it is very easy yet but as Change-id: ... line are trailers, you can do that with git interpret-trailers. For example: $ echo -e \nChange-id: stuff\nOther: thing | git -c trailer.Change-id.ifexists=replace interpret-trailers --trim-empty --trailer Change-id= Other: thing The idea is that the above command replaces an existing Change-id: stuff trailer with an empty Change-id: trailer and then removes all the empty trailers. 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] Introduce a hook to run after formatting patches
On Fri, Nov 21, 2014 at 12:33 AM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: So I have read the man page on the trailers and it seems like the solution to my problem in removing parts from the commit message. However I did not find out, if it can be run automatically, whenever calling format-patch Maybe all that is missing here is an option git config format.enable_trailers ? Yeah, we could use config variables or command options or both to make it easier to enable it and pass it arguments. For example we could add: 1) an --interpret-trailers option to git commit, git format-patch, git am and maybe others too, so that git interpret-trailers is called (without arguments), 2) an --trailer trailer option (that can be repeated) to the same commands, so that git interpret-trailers is called with the --trailer trailer arguments, 3) a trailer.enable_commands = 'format-patch, commit' config variable, so that git interpret-trailers is called (without arguments) every time one of the specified command is used, 4) your suggested format.enable_trailers and maybe commit.enable_trailers and others like that, 5) a trailer command for rebase -i todo lists. My preference would be for 1), 2), 3) and 5). The idea has been to first give a standalone text transmonger as a filter for let people to try out, so that we know what kind of changes are useful (e.g. insert s-o-b at the very end) and make sure the configuration language to specify the changes is easy and expressive enough, which is more or less what we have in 'master'. Once we gain experience (and that may result in updates to what is in 'master'), in the second phase, we would figure out what code paths can make use of this text transmonger (e.g. your configuration variable format.trailers to affect the format-patch code path) and integrate it more tightly to the codebase. We are not there yet. Yeah, interpret-trailers will be in Git 2.2 without the above suggested features, but maybe, if we can get some feedback from users about which features they would use, we can aim to have some in Git 2.3. 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 0/2] git-am: add --message-id/--no-message-id options
On Tue, Nov 25, 2014 at 3:00 PM, Paolo Bonzini bonz...@gnu.org wrote: From: Paolo Bonzini pbonz...@redhat.com This series adds a --message-id option to git-mailinfo and git-am. git-am also gets an am.messageid configuration key to set the default, and a --no-message-id option to override the configuration key. (I'm not sure of the usefulness of a mailinfo.messageid option, so I left it out; this follows the example of -k instead of --scissors). This option can be useful in order to associate commit messages with mailing list discussions. If both --message-id and -s are specified, the Signed-off-by goes last. This is coming out more or less naturally out of the git-am implementation, but is also tested in t4150-am.sh. Did you have a look at git interpret-trailers currently in 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
Re: [PATCH 0/2] git-am: add --message-id/--no-message-id options
On Tue, Nov 25, 2014 at 6:01 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 25/11/2014 17:27, Christian Couder wrote: From: Paolo Bonzini pbonz...@redhat.com This series adds a --message-id option to git-mailinfo and git-am. git-am also gets an am.messageid configuration key to set the default, and a --no-message-id option to override the configuration key. (I'm not sure of the usefulness of a mailinfo.messageid option, so I left it out; this follows the example of -k instead of --scissors). This option can be useful in order to associate commit messages with mailing list discussions. If both --message-id and -s are specified, the Signed-off-by goes last. This is coming out more or less naturally out of the git-am implementation, but is also tested in t4150-am.sh. Did you have a look at git interpret-trailers currently in master? Hmm, now I have. As far as I understand, all the git-am hooks are called on the commit rather than the incoming email: all headers are lost by the time git-mailinfo exits, including the Message-Id. And you cannot call any hook before git-mailinfo because git-mailinfo is where the Content-Transfer-Encoding is processed. How would you integrate git-interpret-trailers in git-mailinfo? I don't know exactly, but people may want to add trailers when they run git-am, see: http://thread.gmane.org/gmane.comp.version-control.git/251412/ and we decided that it was better to let something like git interpret-trailers decide how they should be handled. Maybe if git-interpret-trailers could be called from git-mailinfo with some arguments coming from git-am, it could be configured with something like: git config trailer.Message-Id.command 'perl -ne '\''print $1 if m/^Message-Id: (.*)$/'\'' $ARG' So git am --trailer 'Message-Id: msg-file' msg-file would call git mailinfo ... that would call git interpret-trailers --trailer 'Message-Id: msg-file' that would call perl -ne 'print $1 if m/^Message-Id: (.*)$/' msg-file and the output of this command, let's call it $id, would be put into a Message-Id: $id trailer in the commit message. This way there is nothing specific to Message-Id in the code and people can decide using other trailer.Message-Id.* config variables exactly where the Message-Id trailer would be in the commit message. 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 0/2] git-am: add --message-id/--no-message-id options
On Wed, Nov 26, 2014 at 10:07 AM, Paolo Bonzini bonz...@gnu.org wrote: On 25/11/2014 22:21, Christian Couder wrote: On Tue, Nov 25, 2014 at 6:01 PM, Paolo Bonzini pbonz...@redhat.com wrote: As far as I understand, all the git-am hooks are called on the commit rather than the incoming email: all headers are lost by the time git-mailinfo exits, including the Message-Id. And you cannot call any hook before git-mailinfo because git-mailinfo is where the Content-Transfer-Encoding is processed. How would you integrate git-interpret-trailers in git-mailinfo? I don't know exactly, but people may want to add trailers when they run git-am, see: http://thread.gmane.org/gmane.comp.version-control.git/251412/ and we decided that it was better to let something like git interpret-trailers decide how they should be handled. Maybe if git-interpret-trailers could be called from git-mailinfo with some arguments coming from git-am, it could be configured with something like: git config trailer.Message-Id.command 'perl -ne '\''print $1 if m/^Message-Id: (.*)$/'\'' $ARG' So git am --trailer 'Message-Id: msg-file' msg-file would call git mailinfo ... that would call git interpret-trailers --trailer 'Message-Id: msg-file' that would call perl -ne 'print $1 if m/^Message-Id: (.*)$/' msg-file and the output of this command, let's call it $id, would be put into a Message-Id: $id trailer in the commit message. I think overloading trailer.Message-Id.command is not a good idea, because it would prevent using git interpret-trailers to add a message id manually (git interpret-trailers --trailer message-id='foo@bar'). Well, it is possible to configure a trailer.Message-Id.command that can detect if it is passed an existing file or not. If it is passed an existing file, it could lookup the message id in the file and print it, otherwise it would just print what it is passed. Another possibility could be to add a third output file to git-mailinfo, including all the headers. Then a hook could be called with the headers and commit message. Yeah, but this hook could not do everything, because some people might want to add trailers from the command line anyway. So git interpret-trailers could be called once with the command line arguments and once inside the hook. If the user wants to have some processing done by some commands for different trailers, it makes sense to have all the processing done by commands specified in the trailer.token.command config variables, instead of having some of it done by such config variables and other done in some hooks. The question is: what would it be used for? There aren't that many mail headers, and most of them (From, Subject, Date) are recorded in the commit anyway. One idea could be to record who was a recipient of the original message, even if no Cc line was added explicitly. In most projects, Cc is often added randomly, but I guess that's a valid usecase. I can certainly code the above hook instead of this approach if Junio thinks it's better. Yes, recording Cc'ed people in Cc: trailers is a very valid use case. I don't think the trailer.token.command mechanism supports that well if people want only one person per Cc: trailer. That's something that could be improved in git-interpret-trailers. In the meanwhile, I have thought of a couple additions to git interpret-trailers and I can submit patches for them. You are welcome to suggest and even more to submit patches for additions to it. If you want, you can also have a look at some of these threads for some things that have been suggested already: http://thread.gmane.org/gmane.comp.version-control.git/259614/ thread.gmane.org/gmane.comp.version-control.git/259275/ 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] introduce git root
On Mon, Dec 1, 2014 at 4:04 AM, Junio C Hamano gits...@pobox.com wrote: If I were redoing this today, I would probably nominate the git potty as such a kitchen synk command. We have --man-path that shows the location of the manual pages, --exec-path[=path] that either shows or allows us to override the path to the subcommands, and --show-prefix, --show-toplevel, and friends may feel quite at home there. I wonder if we could reuse git config which is already a kitchen synk command to get/set a lot of parameters. Maybe we could dedicate a git or virtual or proc or sys (like /proc or /sys in Linux) namespace for these special config parameters that would not necessarily reflect something in the config file. git config git.man-path would be the same as git --man-path. git config git.root would be the same as git rev-parse --show-toplevel. git config git.exec-path mypath would allow us to override the path to the subcommands, probably by saving something in the config file. If we wanted for example to try just once a special exec-path we could use: git -c git.exec-path=/path/to/git-foo foo 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] introduce git root
On Tue, Dec 2, 2014 at 8:04 AM, Jeff King p...@peff.net wrote: On Mon, Dec 01, 2014 at 05:17:22AM +0100, Christian Couder wrote: On Mon, Dec 1, 2014 at 4:04 AM, Junio C Hamano gits...@pobox.com wrote: If I were redoing this today, I would probably nominate the git potty as such a kitchen synk command. We have --man-path that shows the location of the manual pages, --exec-path[=path] that either shows or allows us to override the path to the subcommands, and --show-prefix, --show-toplevel, and friends may feel quite at home there. I wonder if we could reuse git config which is already a kitchen synk command to get/set a lot of parameters. Maybe we could dedicate a git or virtual or proc or sys (like /proc or /sys in Linux) namespace for these special config parameters that would not necessarily reflect something in the config file. git config git.man-path would be the same as git --man-path. git config git.root would be the same as git rev-parse --show-toplevel. git config git.exec-path mypath would allow us to override the path to the subcommands, probably by saving something in the config file. What would: git config git.root foo That would output an error message saying that we cannot change the git.root value. git config git.root That would output the same as git rev-parse --show-toplevel. output? No matter what the answer is, I do not relish the thought of trying to explain it in the documentation. :) Yeah, maybe it is better if we don't reuse git config. There is also git var, which is a catch-all for printing some deduced environmental defaults. I'd be just as happy to see it go away, though. Yeah, maybe we could use git var for more variables. Having: git --exec-path git --toplevel git --author-ident all work would make sense to me (I often get confused between git --foo and git rev-parse --foo when trying to get the exec-path and git-dir). And I don't think it's too late to move in this direction. We'd have to keep the old interfaces around, of course, but it would immediately improve discoverability and consistency. I don't like reusing the git command for that puropose, because it clutters it and makes it difficult to improve. For example let's suppose that we decide to have a git info command. It could work like this: $ git info sequence.editor vim $ git info core.editor emacs $ git info --verbose sequence.editor sequence.editor = vim GIT_EDITOR = emacs core.editor = nano $ git info --verbose core.editor GIT_EDITOR = emacs core.editor = nano So the --verbose option could explain why the sequence.editor is vim by displaying the all the relevant options with their values from the most important to the least important. 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] introduce git root
Jeff King p...@peff.net wrote: Some of the discussion has involved mixing config options into this kitchen sink, but that does not make any sense to me (and is why I find git var -l so odd). Config options are fundamentally different, in that they are set and retrieved, not computed (from other config variables, or from hard-coded values). And we already have a nice tool for working with them (well...nice-ish, let's say). Yeah, but git config cannot say which config option applies in some context and why. For example, to chose the editor all the following could apply: GIT_SEQUENCE_EDITOR env variable sequence.editor config variable GIT_EDITOR env variable core.editor config variable VISUAL env variable EDITOR env variable editor configured at compile time and the user or our own scripts right now cannot easily know which editor should be used when editing the sequence list. The best they can do is: - first check if GIT_SEQUENCE_EDITOR is set, and if yes, use it - then check if sequence.editor config variable is set, and if yes, use it - then use git var GIT_EDITOR that will check the other options I don't think it is very nice. Jeff King p...@peff.net also wrote: My issue is only that git --foo has other options besides computables. So you need to name each option in a way that makes it clear it is reporting a computable and not doing something else. Take git --pager for instance. That would be a natural choice to replace git var GIT_PAGER. But shouldn't --pager be the opposite of the existing --no-pager? So instead we probably need some namespace to indicate that it is a showing option. Like --show-pager. And then for consistency, we would probably want to move --exec-path to --show-exec-path, creating a new --show- namespace. Or we could call that namespace git var. :) I agree with that, but I think it could be better if there was also a notion of context, I do not think git var --exec-path is a good idea, nor GIT_EXEC_PATH for the environment-variable confusion you mentioned. I was thinking of just creating a new namespace, like: git var exec-path git var author-ident I agree that this is nice, but I wonder what we would do for the sequence editor and the default editor. Maybe: git var sequence-editor git var editor That would already be nicer than what we have now, but maybe we should consider the following instead: git var sequence.editor git var core.editor (and maybe also some aliases to core.editor, like: git var default.editor git var editor) I think sequence.editor and core.editor are better because: - they use the same syntax as the config variables, so they are easier to remember and to discover, and - they provide a notion of context. The notion of context is interesting because suppose that we later introduce the commit.editor config variable. If we decide now that foo.editor means just core.editor if we don't know about any editor variable related to the foo context, then the scripts that might later be written using git var commit.editor will not have to worry about the fact that previous versions of Git didn't know about commit.editor. People could even start using git var commit.editor now, because it would work even if commit.editor is unused by git commit. Of course when the user asks for git var foo.editor and we don't know about any editor variable related to the foo context, we first should check if foo.editor exists in the config file and we should use that if it exists, before we default to git var core.editor. 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] introduce git root
On Fri, Dec 5, 2014 at 10:27 AM, Jeff King p...@peff.net wrote: On Fri, Dec 05, 2014 at 03:27:17AM +0100, Christian Couder wrote: I do not think git var --exec-path is a good idea, nor GIT_EXEC_PATH for the environment-variable confusion you mentioned. I was thinking of just creating a new namespace, like: git var exec-path git var author-ident I agree that this is nice, but I wonder what we would do for the sequence editor and the default editor. Maybe: git var sequence-editor git var editor Again, I think we're mostly agreeing. Context and hierarchy and falling back are good things. Whatever we call the variables, editor and sequence-editor and foo-editor should have a predictable and consistent form. I like the idea of foo-editor automatically falling back to editor even if we don't know what foo is. Yeah but that means that we have to use something other than - to separate the context from the name, because we already have names like exec-path, html-path and man-path. But the one place I do not agree is: I think sequence.editor and core.editor are better because: - they use the same syntax as the config variables, so they are easier to remember and to discover, and I really don't like using core.editor here, because it has the same name as a config variable, but it is _not_ the config variable. It happens to use the config variable as one of the inputs to its computation, but in many cases: git config core.editor and git var core.editor will produce different values. Yeah, but I don't think it is a problem. They are different commands, so it can be expected that they do different things. For example, if you use git log origin/master you get a different ouput than if you use git show origin/master, though you still use the same origin/master notation. When you use git show you consider only the commit pointed to by origin/master and when you use git log you consider the same commit but also all its ancestors. In the same way, when you use git config core.editor you consider only the value of the core.editor logical variable in the config files, while when you would use git var core.editor you would consider the value of the core.editor logical variable in both the config files and the environment variables. They are entirely different namespaces. Using the same syntax and name seems unnecessarily confusing to me. Even still using dotted hierarchies, but giving them different names (e.g., editor, editor.sequence, editor.foo) would make it more obvious that they are not the same thing. Using yet another namespace or syntax when we could reuse an existing one is what would seem unnecessarily confusing to me. The value of the editor logical variable in the sequence context is related to the sequence.editor logical value in the config file, because the later can directly influence the former. So there is a reason to use the same notation. 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: Accept-language test fails on Mac OS
On Sun, Dec 7, 2014 at 8:18 AM, Jeff King p...@peff.net wrote: On Sat, Dec 06, 2014 at 10:04:06PM +0100, Torsten Bögershausen wrote: I get this: expecting success: check_language ko-KR, *;q=0.1 ko_KR.UTF-8 de_DE.UTF-8 ja_JP.UTF-8 en_US.UTF-8 check_language de-DE, *;q=0.1 de_DE.UTF-8 ja_JP.UTF-8 en_US.UTF-8 check_language ja-JP, *;q=0.1 ja_JP.UTF-8 en_US.UTF-8 check_language en-US, *;q=0.1 en_US.UTF-8 --- expect 2014-12-06 21:00:59.0 + +++ actual 2014-12-06 21:00:59.0 + @@ -1 +0,0 @@ -Accept-Language: de-DE, *;q=0.1 not ok 25 - git client sends Accept-Language based on LANGUAGE, LC_ALL, LC_MESSAGES and LANG I can reproduce the same problem here (Debian unstable). I actually ran into three issues (aside from needing to use Junio's SQUASH commit, to avoid the \r bash-ism): 1. I couldn't build without including locale.h, for the definition of setlocale() and the LC_MESSAGES constant (both used in get_preferred_languages). I'm not sure what portability issues there are with including it unconditionally. Should this possibly be tied into gettext.c, which already uses setlocale? Yeah, pu build is broken on Ubuntu 14.04 too, because of 7567fad2431eb38291fd74a70f603e5746c6f728 (http: send Accept-Language header if possible). 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: [RFC/PATCH 0/5] git-glossary
On Mon, Dec 8, 2014 at 4:38 PM, Michael J Gruber g...@drmicha.warpmail.net wrote: More and more people use Git in localised setups, which usually means mixed localisation setups - not only, but also because of our English man pages. Here's an attempt at leveraging our current infrastructure for helping those poor mixed localisation folks. The idea is to keep the most important iterms in the glossary and translate at least these. If the problem is related to all the man pages, shouldn't the solution apply to all the man pages? 1/5: generate glossary term list automatically from gitglossary.txt 2/5: introduce git-glossary command which helps with lookups Couldn't you improve git-help ? 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] introduce git root
On Tue, Dec 9, 2014 at 7:17 PM, Junio C Hamano gits...@pobox.com wrote:
Jeff King p...@peff.net writes:
But the one place I do not agree is:
I think sequence.editor and core.editor are better because:
- they use the same syntax as the config variables, so they are easier
to remember and to discover, and
I really don't like using core.editor here, because it has the same
name as a config variable, but it is _not_ the config variable. It
happens to use the config variable as one of the inputs to its
computation, but in many cases:
git config core.editor
and
git var core.editor
will produce different values. They are entirely different namespaces.
Using the same syntax and name seems unnecessarily confusing to me.
I think this is a valid concern.
It is a tangent, the current definition of git_editor helper reads
like this:
git_editor() {
if test -z ${GIT_EDITOR:+set}
then
GIT_EDITOR=$(git var GIT_EDITOR) || return $?
fi
eval $GIT_EDITOR '$@'
}
If git var editor were to compute a reasonable value from the
various user settings, and because GIT_EDITOR is among the sources
of user settings, I wonder if the surrounding if test -z then...fi
should be there.
The pager side seems to do (halfway) the right thing:
git_pager() {
if test -t 1
then
GIT_PAGER=$(git var GIT_PAGER)
else
GIT_PAGER=cat
fi
: ${LESS=-FRX}
: ${LV=-c}
export LESS LV
eval $GIT_PAGER '$@'
}
The initial test -t 1 is we do not page to non-terminal, but we
ask git var to take care of PAGER/GIT_PAGER fallback/precedence.
It is tempting to argue that we do not page to non-terminal should
also be part of various user settings for git var to take into
account and fold this if test -t 1...then...else...fi also into
git var, but because a typical command line git var will be used
on would look like this:
GIT_PAGER=$(git var pager)
eval $GIT_PAGER ...
with the standard output stream of git var not connected to
terminal, that would not fly very well, and I am not sure what
should be done about it.
This is another tangent that comes back to the original how to name
them? question, but I wonder if a convention like this would work.
* When asking for a feature (e.g. what editor to use), if there
is a git-specific environment variable that can be set to
override all other settings (e.g. $GIT_EDITOR trumps $EDITOR
environment or core.editor configuration), git var can be
queried with the name of that strong environment variable.
* All other features without such a strong environment variables
should not be spelled as if there is such an environment variable
the user can set in order to avoid confusion.
Does that mean that we would also have the following:
- git var GIT_DIR
- git var GIT_EXEC_PATH
- git var GIT_HTML_PATH
- git var GIT_WORK_TREE
- git var GIT_PREFIX
...
but not:
- git var GIT_SHARED_INDEX_PATH
- git var GIT_TOP_LEVEL
- git var GIT_CDUP
?
For the above 3 variables we would have:
- git var shared-index-path
- git var top-level
- git var cdup
And then what if we add the GIT_SHARED_INDEX_PATH env variable for example?
Would we need to deprecate git var shared-index-path?
We also would have:
- git var GIT_EDITOR
but:
- git var web-browser (or git var web.browser like the config option?)
And when we add a GIT_WEB_BROWSER or GIT_BROWSER env variable, we
deprecate git var web-browser (or git var web.browser?)
I doesn't look like user and future friendly to me, but maybe I
misunderstood something.
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: confusion with some `git reset` commands
On Tue, Dec 16, 2014 at 7:39 PM, Arup Rakshit arupraks...@rocketmail.com wrote: From the command help I see - [arup@to_do_app]$ git reset -h You can also use git help reset to have the full man page. It has a lot more information. But I am looking for any differences - Do you have some use cases in mind? 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: Misbehaving git bisect bad HEAD
On Mon, Dec 22, 2014 at 3:04 PM, Andreas Schwab sch...@linux-m68k.org wrote:
Running git bisect bad should be the same as git bisect bad HEAD,
shouldn't it?
Yeah, it should.
When replaying this bisect log on the Linux kernel tree:
git bisect start
# bad: [97bf6af1f928216fd6c5a66e8a57bfa95a659672] Linux 3.19-rc1
git bisect bad 97bf6af1f928216fd6c5a66e8a57bfa95a659672
# good: [b2776bf7149bddd1f4161f14f79520f17fc1d71d] Linux 3.18
git bisect good b2776bf7149bddd1f4161f14f79520f17fc1d71d
# good: [70e71ca0af244f48a5dcf56dc435243792e3a495] Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next
git bisect good 70e71ca0af244f48a5dcf56dc435243792e3a495
# good: [988adfdffdd43cfd841df734664727993076d7cb] Merge branch 'drm-next' of
git://people.freedesktop.org/~airlied/linux
git bisect good 988adfdffdd43cfd841df734664727993076d7cb
# good: [b024793188002b9eed452b5f6a04d45003ed5772] staging: rtl8723au:
phy_SsPwrSwitch92CU() was never called with bRegSSPwrLvl != 1
git bisect good b024793188002b9eed452b5f6a04d45003ed5772
# good: [66dcff86ba40eebb5133cccf450878f2bba102ef] Merge tag 'for-linus' of
git://git.kernel.org/pub/scm/virt/kvm/kvm
git bisect good 66dcff86ba40eebb5133cccf450878f2bba102ef
# bad: [88a57667f2990f00b019d46c8426441c9e516d51] Merge branch
'perf-urgent-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 88a57667f2990f00b019d46c8426441c9e516d51
# good: [0ec28c37c21a2b4393692e832e11a7573ac545e2] Merge tag 'media/v3.19-2'
of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
git bisect good 0ec28c37c21a2b4393692e832e11a7573ac545e2
# good: [c0f486fde3f353232c1cc2fd4d62783ac782a467] Merge tag
'pm+acpi-3.19-rc1-2' of
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
git bisect good c0f486fde3f353232c1cc2fd4d62783ac782a467
# bad: [34b85e3574424beb30e4cd163e6da2e2282d2683] Merge tag 'powerpc-3.19-2'
of git://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux
git bisect bad 34b85e3574424beb30e4cd163e6da2e2282d2683
# good: [64ec45bff6b3dade2643ed4c0f688a15ecf46ea2] Merge tag 'for_linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
git bisect good 64ec45bff6b3dade2643ed4c0f688a15ecf46ea2
Running git bisect bad gives this:
$ git bisect bad
Bisecting: 6 revisions left to test after this (roughly 3 steps)
[ec2aef5a8d3c14272f7a2d29b34f1f8e71f2be5b] power/perf/hv-24x7: Use
kmem_cache_free() instead of kfree
Running git bisect bad HEAD instead gives this:
$ git bisect bad HEAD
Bisecting: a merge base must be tested
[56548fc0e86cb9156af7a7e1f15ba78f251dafaf] powerpc/powernv: Return to cpu
offline loop when finished in KVM guest
This is git 2.2.1.
I think it is a very old bug.
The following patch should fix it:
diff --git a/git-bisect.sh b/git-bisect.sh
index 6cda2b5..26a336a 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -200,7 +200,8 @@ is_expected_rev() {
check_expected_revs() {
for _rev in $@; do
- if ! is_expected_rev $_rev
+ _parsed=$(git rev-parse --verify $_rev)
+ if ! is_expected_rev $_parsed
then
rm -f $GIT_DIR/BISECT_ANCESTORS_OK
rm -f $GIT_DIR/BISECT_EXPECTED_REV
I will send a proper patch later.
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 2/2] bisect: add test to check that revs are properly parsed
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6030-bisect-porcelain.sh | 9 + 1 file changed, 9 insertions(+) diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 064f5ce..e6abe65 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -779,4 +779,13 @@ test_expect_success 'bisect log: only skip commits left' ' git bisect reset ' +test_expect_success 'git bisect bad HEAD behaves as git bisect bad' ' + git checkout parallel + git bisect start HEAD $HASH1 + git bisect good HEAD + git bisect bad HEAD + test $HASH6 = $(git rev-parse --verify HEAD) + git bisect reset +' + test_done -- 2.1.2.555.gfbecd99 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] bisect: parse revs before passing them to check_expected_revs()
When running for example git bisect bad HEAD or
git bisect good master, the parameter passed to
git bisect (bad|good) has to be parsed into a
commit hash before checking if it is the expected
commit or not.
We could do that in is_expected_rev() or in
check_expected_revs(), but it is already done in
bisect_state(). Let's just store the hash values
that result from this parsing, and then reuse
them after all the parsing is done.
This way we can also use a for loop over these
values to call bisect_write() on them, instead of
using eval.
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
I think that it is a better patch than the one I sent
previously to the list as with this one we parse revs
only once.
Merry Christmas!
git-bisect.sh | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index 6cda2b5..2fc07ac 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -237,15 +237,18 @@ bisect_state() {
check_expected_revs $rev ;;
2,bad|*,good|*,skip)
shift
- eval=''
+ hash_list=''
for rev in $@
do
sha=$(git rev-parse --verify $rev^{commit}) ||
die $(eval_gettext Bad rev input: \$rev)
- eval=$eval bisect_write '$state' '$sha';
+ hash_list=$hash_list $sha
done
- eval $eval
- check_expected_revs $@ ;;
+ for rev in $hash_list
+ do
+ bisect_write $state $rev
+ done
+ check_expected_revs $hash_list ;;
*,bad)
die $(gettext 'git bisect bad' can take only one argument.)
;;
*)
--
2.1.2.555.gfbecd99
--
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 1/2] bisect: parse revs before passing them to check_expected_revs()
From: Junio C Hamano gits...@pobox.com
Christian Couder chrisc...@tuxfamily.org writes:
When running for example git bisect bad HEAD or
git bisect good master, the parameter passed to
git bisect (bad|good) has to be parsed into a
commit hash before checking if it is the expected
commit or not.
Hmm, is that because you wrote commit object name in 40-hex in the
EXPECTED_REV and you need to compare with what the user gave you
which could be symbolic?
Yes, that's the reason.
The conversion makes sense, but why is it a bad thing to say
git bisect bad maint
when 'maint' is not what you checked out in the current bisect run
in the first place (perhaps you checked if it is good or bad manually
before you started bisecting)?
It is not a bad thing to test another commit than the one that has
been checked out and then to say if it is good or bad. But if you
do that then it is safer to check if a merge base should be tested.
I can discuss this point further and there are indeed some
optimisations we could implement in this area, but I think it is
better to try to just fix the bug first.
diff --git a/git-bisect.sh b/git-bisect.sh
index 6cda2b5..2fc07ac 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -237,15 +237,18 @@ bisect_state() {
check_expected_revs $rev ;;
2,bad|*,good|*,skip)
This part accepts arbitrary number of revs when running good and
skip, e.g.
git bisect good maint master next
and it loops
Yeah.
shift
-eval=''
+hash_list=''
for rev in $@
...
+for rev in $hash_list
+do
+bisect_write $state $rev
+done
+check_expected_revs $hash_list ;;
But check_expected_revs loops and leaves the loop early when it
finds anything that is not expected.
... goes and looks ...
Hmph, I think the logic in check_expected_revs is not wrong, but
this helper function is grossly misnamed. It is not checking and
rejecting the user input---it is checking to see if it can bypass
check_good_are_ancestors_of_bad() which is expensive, so when it
sees any one of the input is not what it checked out, it just
disables the optimization.
Yeah, that's the idea. If you have a better name for
check_expected_revs(), I can change it in another patch.
And yeah, check_good_are_ancestors_of_bad() is expensive to compute
and also expensive because it might mean that more tests have to be
performed by the user to be safe.
OK, will queue.
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 3/3] Documentation: add trailer.trimEmpty config variable
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-interpret-trailers.txt | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index d6d9231..816dd65 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -64,10 +64,12 @@ folding rules, the encoding rules and probably many other rules. OPTIONS --- ---trim-empty:: +--[no-]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. + This applies to existing trailers as well as new + trailers. This overrides a 'trailer.trimempty' configuration + variable. --trailer token[(=|:)value]:: Specify a (token, value) pair that should be applied as a @@ -77,6 +79,11 @@ OPTIONS CONFIGURATION VARIABLES --- +trailer.trimempty:: + This option tells if by default trailers with an empty value + part should be trimmed or not. See also the '--[no-]trim-empty' + option above. + trailer.separators:: This option tells which characters are recognized as trailer separators. By default only ':' is recognized as a trailer -- 2.2.1.313.gcc831f2 -- 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
