[PATCH v16 05/11] trailer: parse trailers from file or stdin

[Christian Couder]

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 b5666b3..4f0de3b 100644
--- a/trailer.c
+++ b/trailer.c
@@ -69,6 +69,14 @@ static int same_trailer(struct trailer_item *a, struct 
trailer_item *b)
return same_token(a, b)  same_value(a, b);
 }
 
+static inline int contains_only_spaces(const char *str)
+{
+   const char *s = str;
+   while (*s  isspace(*s))
+   s++;
+   return !*s;
+}
+
 static void free_trailer_item(struct trailer_item *item)
 {
free(item-conf.name);
@@ -574,3 +582,118 @@ static struct trailer_item 
*process_command_line_args(struct string_list *traile
 
return arg_tok_first;
 }
+
+static struct strbuf **read_input_file(const char *file)
+{
+   struct strbuf **lines;
+   struct strbuf sb = STRBUF_INIT;
+
+   if (file) {
+   if (strbuf_read_file(sb, file, 0)  0)
+   die_errno(_(could not read input file '%s'), file);
+   } else {
+   if (strbuf_read(sb, fileno(stdin), 0)  0)
+   die_errno(_(could not read from stdin));
+   }
+
+   lines = strbuf_split(sb, '\n');
+
+   strbuf_release(sb);
+
+   return lines;
+}
+
+/*
+ * Return the (0 based) index of the start of the patch or the line
+ * count if there is no patch in the message.
+ */
+static int find_patch_start(struct strbuf **lines, int count)
+{
+   int i;
+
+   /* Get the start of the patch part if any */
+   for (i = 0; i  count; i++) {
+   if (starts_with(lines[i]-buf, ---))
+   return i;
+   }
+
+   return count;
+}
+
+/*
+ * Return the (0 based) index of the first trailer line or count if
+ * there are no trailers. Trailers are searched only in the lines from
+ * index (count - 1) down to index 0.
+ */
+static int find_trailer_start(struct strbuf **lines, int count)
+{
+   int start, only_spaces = 1;
+
+   /*
+* Get the start of the trailers by looking starting from the end
+* for a line with only spaces before lines with one separator.
+*/
+   for (start = count - 1; start = 0; start--) {
+   if (lines[start]-buf[0] == comment_line_char)
+   continue;
+   if (contains_only_spaces(lines[start]-buf)) {
+   if (only_spaces)
+   continue;
+   return start + 1;
+   }
+   if (strcspn(lines[start]-buf, separators)  lines[start]-len) 
{
+   if (only_spaces)
+   only_spaces = 0;
+   continue;
+   }
+   return count;
+   }
+
+   return only_spaces ? count : 0;
+}
+
+static int has_blank_line_before(struct strbuf **lines, int start)
+{
+   for (;start = 0; start--) {
+   if (lines[start]-buf[0] == comment_line_char)
+   continue;
+   return contains_only_spaces(lines[start]-buf);
+   }
+   return 0;
+}
+
+static void print_lines(struct strbuf **lines, int start, int end)
+{
+   int i;
+   for (i = start; lines[i]  i  end; i++)
+   printf(%s, lines[i]-buf);
+}
+
+static int process_input_file(struct strbuf **lines,
+ struct trailer_item **in_tok_first,
+ struct trailer_item **in_tok_last)
+{
+   int count = 0;
+   int patch_start, trailer_start, i;
+
+   /* Get the line count */
+   while (lines[count])
+   count++;
+
+   patch_start = find_patch_start(lines, count);
+   trailer_start = find_trailer_start(lines, patch_start);
+
+   /* Print lines before the trailers as is */
+   print_lines(lines, 0, trailer_start);
+
+   if (!has_blank_line_before(lines, trailer_start - 1))
+   printf(\n);
+
+   /* Parse trailer lines */
+   for (i = trailer_start; i  patch_start; i++) {
+   struct trailer_item *new = create_trailer_item(lines[i]-buf);
+   add_trailer_item(in_tok_first, in_tok_last, new);
+   }
+
+   return patch_start;
+}
-- 
2.1.0.rc0.248.gb91fdbc


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v16 08/11] trailer: add tests for git interpret-trailers

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t7513-interpret-trailers.sh | 738 ++
 1 file changed, 738 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..ad36cf8
--- /dev/null
+++ b/t/t7513-interpret-trailers.sh
@@ -0,0 +1,738 @@
+#!/bin/sh
+#
+# Copyright (c) 2013, 2014 Christian Couder
+#
+
+test_description='git interpret-trailers'
+
+. ./test-lib.sh
+
+# When we want one trailing space at the end of each line, let's use sed
+# to make sure that these spaces are not removed by any automatic tool.
+
+test_expect_success 'setup' '
+   : empty 
+   cat basic_message -\EOF 
+   subject
+
+   body
+   EOF
+   cat complex_message_body -\EOF 
+   my subject
+
+   my body which is long
+   and contains some special
+   chars like : = ? !
+
+   EOF
+   sed -e s/ Z\$/ / complex_message_trailers -\EOF 
+   Fixes: Z
+   Acked-by: Z
+   Reviewed-by: Z
+   Signed-off-by: Z
+   EOF
+   cat basic_patch -\EOF
+   ---
+foo.txt | 2 +-
+1 file changed, 1 insertion(+), 1 deletion(-)
+
+   diff --git a/foo.txt b/foo.txt
+   index 0353767..1d91aa1 100644
+   --- a/foo.txt
+   +++ b/foo.txt
+   @@ -1,3 +1,3 @@
+
+   -bar
+   +baz
+
+   --
+   1.9.rc0.11.ga562ddc
+
+   EOF
+'
+
+test_expect_success 'without config' '
+   sed -e s/ Z\$/ / expected -\EOF 
+
+   ack: Peff
+   Reviewed-by: Z
+   Acked-by: Johan
+   EOF
+   git interpret-trailers --trailer ack = Peff --trailer Reviewed-by \
+   --trailer Acked-by: Johan empty actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'without config in another order' '
+   sed -e s/ Z\$/ / expected -\EOF 
+
+   Acked-by: Johan
+   Reviewed-by: Z
+   ack: Peff
+   EOF
+   git interpret-trailers --trailer Acked-by: Johan --trailer 
Reviewed-by \
+   --trailer ack = Peff empty actual 
+   test_cmp expected actual
+'
+
+test_expect_success '--trim-empty without config' '
+   cat expected -\EOF 
+
+   ack: Peff
+   Acked-by: Johan
+   EOF
+   git interpret-trailers --trim-empty --trailer ack=Peff \
+   --trailer Reviewed-by --trailer Acked-by: Johan \
+   --trailer sob: empty actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with config option on the command line' '
+   cat expected -\EOF 
+
+   Acked-by: Johan
+   Reviewed-by: Peff
+   EOF
+   echo Acked-by: Johan | \
+   git -c trailer.Acked-by.ifexists=addifdifferent 
interpret-trailers \
+   --trailer Reviewed-by: Peff --trailer Acked-by: Johan 
actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with config setup' '
+   git config trailer.ack.key Acked-by:  
+   cat expected -\EOF 
+
+   Acked-by: Peff
+   EOF
+   git interpret-trailers --trim-empty --trailer ack = Peff empty 
actual 
+   test_cmp expected actual 
+   git interpret-trailers --trim-empty --trailer Acked-by = Peff empty 
actual 
+   test_cmp expected actual 
+   git interpret-trailers --trim-empty --trailer Acked-by :Peff empty 
actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with config setup and := as separators' '
+   git config trailer.separators := 
+   git config trailer.ack.key Acked-by=  
+   cat expected -\EOF 
+
+   Acked-by= Peff
+   EOF
+   git interpret-trailers --trim-empty --trailer ack = Peff empty 
actual 
+   test_cmp expected actual 
+   git interpret-trailers --trim-empty --trailer Acked-by= Peff empty 
actual 
+   test_cmp expected actual 
+   git interpret-trailers --trim-empty --trailer Acked-by : Peff empty 
actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with config setup and % as separators' '
+   git config trailer.separators % 
+   cat expected -\EOF 
+
+   bug% 42
+   count% 10
+   bug% 422
+   EOF
+   git interpret-trailers --trim-empty --trailer bug = 42 \
+   --trailer count%10 --trailer test: stuff \
+   --trailer bug % 422 empty actual 
+   test_cmp expected actual
+'
+
+test_expect_success 'with % as separators and a message with trailers' '
+   cat special_message -\EOF 
+   Special Message
+
+   bug% 42
+   count% 10
+   bug% 422
+   EOF
+   cat expected

[PATCH v16 09/11] trailer: execute command from 'trailer.name.command'

[Christian Couder]

Let the user specify a command that will give on its standard output
the value to use for the specified trailer.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 trailer.c | 85 ++-
 1 file changed, 84 insertions(+), 1 deletion(-)

diff --git a/trailer.c b/trailer.c
index b0be0d7..8514566 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,5 +1,7 @@
 #include cache.h
 #include string-list.h
+#include run-command.h
+#include string-list.h
 #include trailer.h
 /*
  * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org
@@ -33,6 +35,8 @@ static struct trailer_item *first_conf_item;
 
 static char *separators = :;
 
+#define TRAILER_ARG_STRING $ARG
+
 static int after_or_end(enum action_where where)
 {
return (where == WHERE_AFTER) || (where == WHERE_END);
@@ -78,6 +82,13 @@ static inline int contains_only_spaces(const char *str)
return !*s;
 }
 
+static inline void strbuf_replace(struct strbuf *sb, const char *a, const char 
*b)
+{
+   const char *ptr = strstr(sb-buf, a);
+   if (ptr)
+   strbuf_splice(sb, ptr - sb-buf, strlen(a), b, strlen(b));
+}
+
 static void free_trailer_item(struct trailer_item *item)
 {
free(item-conf.name);
@@ -203,6 +214,63 @@ static struct trailer_item *remove_first(struct 
trailer_item **first)
return item;
 }
 
+static int read_from_command(struct child_process *cp, struct strbuf *buf)
+{
+   if (run_command(cp))
+   return error(running trailer command '%s' failed, 
cp-argv[0]);
+   if (strbuf_read(buf, cp-out, 1024)  1)
+   return error(reading from trailer command '%s' failed, 
cp-argv[0]);
+   strbuf_trim(buf);
+   return 0;
+}
+
+static const char *apply_command(const char *command, const char *arg)
+{
+   struct strbuf cmd = STRBUF_INIT;
+   struct strbuf buf = STRBUF_INIT;
+   struct child_process cp;
+   const char *argv[] = {NULL, NULL};
+   const char *result;
+
+   strbuf_addstr(cmd, command);
+   if (arg)
+   strbuf_replace(cmd, TRAILER_ARG_STRING, arg);
+
+   argv[0] = cmd.buf;
+   memset(cp, 0, sizeof(cp));
+   cp.argv = argv;
+   cp.env = local_repo_env;
+   cp.no_stdin = 1;
+   cp.out = -1;
+   cp.use_shell = 1;
+
+   if (read_from_command(cp, buf)) {
+   strbuf_release(buf);
+   result = xstrdup();
+   } else
+   result = strbuf_detach(buf, NULL);
+
+   strbuf_release(cmd);
+   return result;
+}
+
+static void apply_item_command(struct trailer_item *in_tok, struct 
trailer_item *arg_tok)
+{
+   if (arg_tok-conf.command) {
+   const char *arg;
+   if (arg_tok-value  arg_tok-value[0]) {
+   arg = arg_tok-value;
+   } else {
+   if (in_tok  in_tok-value)
+   arg = xstrdup(in_tok-value);
+   else
+   arg = xstrdup();
+   }
+   arg_tok-value = apply_command(arg_tok-conf.command, arg);
+   free((char *)arg);
+   }
+}
+
 static void apply_arg_if_exists(struct trailer_item *in_tok,
struct trailer_item *arg_tok,
struct trailer_item *on_tok,
@@ -214,16 +282,19 @@ static void apply_arg_if_exists(struct trailer_item 
*in_tok,
free_trailer_item(arg_tok);
break;
case EXISTS_REPLACE:
+   apply_item_command(in_tok, arg_tok);
add_arg_to_input_list(on_tok, arg_tok,
  in_tok_first, in_tok_last);
remove_from_list(in_tok, in_tok_first, in_tok_last);
free_trailer_item(in_tok);
break;
case EXISTS_ADD:
+   apply_item_command(in_tok, arg_tok);
add_arg_to_input_list(on_tok, arg_tok,
  in_tok_first, in_tok_last);
break;
case EXISTS_ADD_IF_DIFFERENT:
+   apply_item_command(in_tok, arg_tok);
if (check_if_different(in_tok, arg_tok, 1))
add_arg_to_input_list(on_tok, arg_tok,
  in_tok_first, in_tok_last);
@@ -231,6 +302,7 @@ static void apply_arg_if_exists(struct trailer_item *in_tok,
free_trailer_item(arg_tok);
break;
case EXISTS_ADD_IF_DIFFERENT_NEIGHBOR:
+   apply_item_command(in_tok, arg_tok);
if (check_if_different(on_tok, arg_tok, 0))
add_arg_to_input_list(on_tok, arg_tok,
  in_tok_first, in_tok_last);
@@ -254,6 +326,7 @@ static void apply_arg_if_missing(struct trailer_item 
**in_tok_first,
case MISSING_ADD

[PATCH v16 11/11] Documentation: add documentation for 'git interpret-trailers'

[Christian Couder]

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 | 314 +++
 command-list.txt |   1 +
 2 files changed, 315 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..81fac3d
--- /dev/null
+++ b/Documentation/git-interpret-trailers.txt
@@ -0,0 +1,314 @@
+git-interpret-trailers(1)
+=
+
+NAME
+
+git-interpret-trailers - help add stuctured information into commit messages
+
+SYNOPSIS
+
+[verse]
+'git interpret-trailers' [--trim-empty] [(--trailer token[(=|:)value])...] 
[file...]
+
+DESCRIPTION
+---
+Help adding 'trailers' lines, that look similar to RFC 822 e-mail
+headers, at the end of the otherwise free-form part of a commit
+message.
+
+This command reads some patches or commit messages from either the
+file arguments or the standard input if no file is specified. Then
+this command applies the arguments passed using the `--trailer`
+option, if any, to the commit message part of each input file. The
+result is emitted on the standard output.
+
+Some configuration variables control the way the `--trailer` arguments
+are applied to each commit message and the way any existing trailer in
+the commit message is changed. They also make it possible to
+automatically add some trailers.
+
+By default, a 'token=value' or 'token:value' argument given
+using `--trailer` will be appended after the existing trailers only if
+the last trailer has a different (token, value) pair (or if there
+is no existing trailer). The token and value parts will be trimmed
+to remove starting and trailing whitespace, and the resulting trimmed
+token and value will appear in the message like this:
+
+
+token: value
+
+
+This means that the trimmed token and value will be separated by
+`': '` (one colon followed by one space).
+
+By default the new trailer will appear at the end of all the existing
+trailers. If there is no existing trailer, the new trailer will appear
+after the commit message part of the ouput, and, if there is no line
+with only spaces at the end of the commit message part, one blank line
+will be added before the new trailer.
+
+Existing trailers are extracted from the input message by looking for
+a group of one or more lines that contain a colon (by default), where
+the group is preceded by one or more empty (or whitespace-only) lines.
+The group must either be at the end of the message or be the last
+non-whitespace lines before a line that starts with '---'. Such three
+minus signs start the patch part of the message.
+
+When reading trailers, there can be whitespaces before and after the
+token, the separator and the value. There can also be whitespaces
+indide the token and the value.
+
+Note that 'trailers' do not follow and are not intended to follow many
+rules for RFC 822 headers. For example they do not follow the line
+folding rules, the encoding rules and probably many other rules.
+
+OPTIONS
+---
+--trim-empty::
+   If the value part of any trailer contains only whitespace,
+   the whole trailer will be removed from the resulting message.
+   This apply to existing trailers as well as new trailers.
+
+--trailer token[(=|:)value]::
+   Specify a (token, value) pair that should be applied as a
+   trailer to the input messages. See the description of this
+   command.
+
+CONFIGURATION VARIABLES
+---
+
+trailer.separators::
+   This option tells which characters are recognized as trailer
+   separators. By default only ':' is recognized as a trailer
+   separator, except that '=' is always accepted on the command
+   line for compatibility with other git commands.
++
+The first character given by this option will be the default character
+used when another separator is not specified in the config for this
+trailer.
++
+For example, if the value for this option is %=$, then only lines
+using the format 'tokensepvalue' with sep containing '%', '='
+or '$' and then spaces will be considered trailers. And '%' will be
+the default separator used, so by default trailers will appear like:
+'token% value' (one percent sign and one space will appear between
+the token and the value).
+
+trailer.where::
+   This option tells where a new trailer will be added.
++
+This can be `end`, which is the default, `start`, `after` or `before`.
++
+If it is `end`, then each new trailer will appear at the end of the
+existing trailers.
++
+If it is `start`, then each new trailer will appear at the start,
+instead of the end, of the existing trailers

[PATCH v16 07/11] trailer: add interpret-trailers command

[Christian Couder]

This patch adds the git interpret-trailers command.
This command uses the previously added process_trailers()
function in trailer.c.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 .gitignore   |  1 +
 Makefile |  1 +
 builtin.h|  1 +
 builtin/interpret-trailers.c | 44 
 git.c|  1 +
 5 files changed, 48 insertions(+)
 create mode 100644 builtin/interpret-trailers.c

diff --git a/.gitignore b/.gitignore
index cf0d191..85593de 100644
--- a/.gitignore
+++ b/.gitignore
@@ -74,6 +74,7 @@
 /git-index-pack
 /git-init
 /git-init-db
+/git-interpret-trailers
 /git-instaweb
 /git-log
 /git-ls-files
diff --git a/Makefile b/Makefile
index ef82972..0b06ae0 100644
--- a/Makefile
+++ b/Makefile
@@ -953,6 +953,7 @@ BUILTIN_OBJS += builtin/hash-object.o
 BUILTIN_OBJS += builtin/help.o
 BUILTIN_OBJS += builtin/index-pack.o
 BUILTIN_OBJS += builtin/init-db.o
+BUILTIN_OBJS += builtin/interpret-trailers.o
 BUILTIN_OBJS += builtin/log.o
 BUILTIN_OBJS += builtin/ls-files.o
 BUILTIN_OBJS += builtin/ls-remote.o
diff --git a/builtin.h b/builtin.h
index 5d91f31..b87df70 100644
--- a/builtin.h
+++ b/builtin.h
@@ -73,6 +73,7 @@ extern int cmd_hash_object(int argc, const char **argv, const 
char *prefix);
 extern int cmd_help(int argc, const char **argv, const char *prefix);
 extern int cmd_index_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_init_db(int argc, const char **argv, const char *prefix);
+extern int cmd_interpret_trailers(int argc, const char **argv, const char 
*prefix);
 extern int cmd_log(int argc, const char **argv, const char *prefix);
 extern int cmd_log_reflog(int argc, const char **argv, const char *prefix);
 extern int cmd_ls_files(int argc, const char **argv, const char *prefix);
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
new file mode 100644
index 000..46838d2
--- /dev/null
+++ b/builtin/interpret-trailers.c
@@ -0,0 +1,44 @@
+/*
+ * Builtin git interpret-trailers
+ *
+ * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org
+ *
+ */
+
+#include cache.h
+#include builtin.h
+#include parse-options.h
+#include string-list.h
+#include trailer.h
+
+static const char * const git_interpret_trailers_usage[] = {
+   N_(git interpret-trailers [--trim-empty] [(--trailer 
token[(=|:)value])...] [file...]),
+   NULL
+};
+
+int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
+{
+   int trim_empty = 0;
+   struct string_list trailers = STRING_LIST_INIT_DUP;
+
+   struct option options[] = {
+   OPT_BOOL(0, trim-empty, trim_empty, N_(trim empty 
trailers)),
+   OPT_STRING_LIST(0, trailer, trailers, N_(trailer),
+   N_(trailer(s) to add)),
+   OPT_END()
+   };
+
+   argc = parse_options(argc, argv, prefix, options,
+git_interpret_trailers_usage, 0);
+
+   if (argc) {
+   int i;
+   for (i = 0; i  argc; i++)
+   process_trailers(argv[i], trim_empty, trailers);
+   } else
+   process_trailers(NULL, trim_empty, trailers);
+
+   string_list_clear(trailers, 0);
+
+   return 0;
+}
diff --git a/git.c b/git.c
index 5ebb32f..e327a90 100644
--- a/git.c
+++ b/git.c
@@ -417,6 +417,7 @@ static struct cmd_struct commands[] = {
{ index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
{ init, cmd_init_db, NO_SETUP },
{ init-db, cmd_init_db, NO_SETUP },
+   { interpret-trailers, cmd_interpret_trailers, RUN_SETUP },
{ log, cmd_log, RUN_SETUP },
{ ls-files, cmd_ls_files, RUN_SETUP },
{ ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY },
-- 
2.1.0.rc0.248.gb91fdbc


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC] git-am: support any number of signatures

[Christian Couder]

From: Junio C Hamano gits...@pobox.com

 Christian Couder christian.cou...@gmail.com writes:
 
 On Sun, Oct 12, 2014 at 11:36 AM, Christian Couder
 christian.cou...@gmail.com wrote:

 With v16 you can easily choose if you want to have the S-o-b in the
 output or not, when there is already one, ...

 By the way, I sent v16 just before the above email, but the series
 still hasn't hit the mailing list.
 Did some of you guys in cc receive something?
 
 I see them and picked them up to replace.

Thanks!
 
 Are these now ready for 'next'?

Yeah, I think so.

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: flatten-merge history

[Christian Couder]

Hi,

On Sat, Oct 25, 2014 at 2:31 PM, Henning Moll newssc...@gmx.de wrote:
 Hi,

 suppose the following history

 P - - - Q - - - - - R   -extern

 A -- - B - - - C - D - - - E   -master
   \   \
   M ...   \ -b1
   \
   W ...   -b2


 Note that master and extern do not have a common parent. Both histories are
 'distinct', they do not share common files, so there can't be any merge
 conflicts. What i want to achieve is this history:

 P - - - Q - - - - - R   -extern

 A -P'- B'- Q'- C'- D'- R'- E'  -master
 \   \
 M'...   \ -b1
 \
 W'...   -b2

 The two histories should be merged in chronological order.
 So while master reflects P-Q-R, b2 should only reflect P-Q and b1 should
 only reflect P.

 All my current attempts (surgery with git replace or interactive rebase
 combined with merging) were not successfull.

Could you tell us why interactive rebase did not work?
If there can't be any merge conflict between both histories, it should
have worked without asking you to resolve any conflict.

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: flatten-merge history

[Christian Couder]

On Sun, Oct 26, 2014 at 4:19 PM, Andreas Schwab sch...@linux-m68k.org wrote:
 Henning Moll newssc...@gmx.de writes:

 1. For P, A is the nearest prior commit on 'master'
 2. on master: git rebase -i A^
 3. change A from pick to edit. save. quit
 4. git merge P
 5. git rebase --continue

 From the perspective of 'master' this worked. But as all of the commits
 have been rewritten, the branches b1 and b2 no longer refer to
 'master'. Branch b2, for example, still branches off at B and not B'.

 You only rebased master, so b1 and b2 were unchanged.  If you want to
 change b1 and b2 you have to rebase them as well.

Yeah. Henning, when interactively rebasing, in our editor, you should
have something like:

pick A
pick P
pick B
pick Q
pick C
pick D
pick R
pick E

which should work without any conflict.
And then you can rebase the b1 and b2 branches on the resulting branch.

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] merge sequencer: turn Conflicts: hint into a comment

[Christian Couder]

On Mon, Oct 27, 2014 at 6:32 PM, Junio C Hamano gits...@pobox.com wrote:
 Jeff King p...@peff.net writes:

 If that is the only casualty, I think it is probably a net-win. We may
 want better tooling around viewing the merge later, but that can wait
 until somebody steps up with a real use case (because even that conflict
 list may not be completely what they want; they may also want the list
 of files that were auto-merged successfully, for example).

 Yup.

 Also Christian's trailer series may want to learn the same trick
 we did to builtin/commit.c in this series, if it does not already
 know about possible trailing comment and blank lines.

The trailer series already tries to ignore comments and blank lines.
This is the relevant function:

/*
 * Return the (0 based) index of the first trailer line or count if
 * there are no trailers. Trailers are searched only in the lines from
 * index (count - 1) down to index 0.
 */
static int find_trailer_start(struct strbuf **lines, int count)
{
int start, only_spaces = 1;

/*
 * Get the start of the trailers by looking starting from the end
 * for a line with only spaces before lines with one separator.
 */
for (start = count - 1; start = 0; start--) {
if (lines[start]-buf[0] == comment_line_char)
continue;
if (contains_only_spaces(lines[start]-buf)) {
if (only_spaces)
continue;
return start + 1;
}
if (strcspn(lines[start]-buf, separators)  lines[start]-len) {
if (only_spaces)
only_spaces = 0;
continue;
}
return count;
}

return only_spaces ? count : 0;
}

But I am not sure sure that it does all of what you do to
builtin/commit.c in the above patch. I will have a look.
Anyway I would be happy to use an existing function or to refactor
some existing code into a shared function 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: Joining historical repository using grafts or replace

[Christian Couder]

Hi,

On Thu, Oct 30, 2014 at 4:39 PM, Dmitry Oksenchuk oksenchu...@gmail.com wrote:
 Hello,

 We're in the middle of conversion of a large CVS repository (20 years,
 70K commits, 1K branches, 10K tags) to Git and considering two
 separate Git repositories: historical with CVS history and working
 created without history from heads of active branches (10 active
 branches). This allows us to have small fast working repository for
 developers who don't want to have full history locally and ability to
 rewrite history in historical repository (for example, to add
 parents to merge commits or to fix conversion mistakes) without
 affecting commit hashes in working repository (the hashes can be
 stored in bug tracker or in the code).

This might be a good idea. Did you already test that the small
repository is really faster than the full repository?

 The first idea was to use grafs to join branch roots in working
 repository with branches in historical repository like in linux
 repository but it seems that grafts are known as a horrible hack (
 http://marc.info/?l=gitm=131127600030310w=2
 http://permalink.gmane.org/gmane.comp.version-control.git/177153 )

 Since Git 1.6.5 replace can also be used to join the histories by
 replacing branch roots in working repository with branch heads in
 historical repository.

 Both grafts and replace will be used locally. Grafts is a bit easier
 to distribute (simple copying, replaces should be created via bash
 script).

First, you might want to have a look at:

http://git-scm.com/book/en/v2/Git-Tools-Replace

as it looks like it describes your use case very well.

 Are there any disadvantages of using grafts and replace? Will both of
 them be supported in future versions of Git?

My opinion is that grafts have no advantage compared to replace refs.

Once you have created your replace refs, they can be managed like
other git refs, so they are easier to distribute.

Basically if you want to get the full history on a computer you just need to do:

git fetch 'refs/replace/*:refs/replace/*'

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: Joining historical repository using grafts or replace

[Christian Couder]

Hi Dmitry,

On Thu, Oct 30, 2014 at 6:41 PM, Dmitry Oksenchuk oksenchu...@gmail.com wrote:
 2014-10-30 19:54 GMT+03:00 Christian Couder christian.cou...@gmail.com:

 This might be a good idea. Did you already test that the small
 repository is really faster than the full repository?

 Yes, because of such amount of refs, push in historical repository
 takes 12 sec, push in working repository takes 0.4 sec, push in
 joined repository takes 2 sec. Local operations with history like
 log and blame work with the same speed in joined repository as in
 historical repository.

What does joined mean? Does it mean joined using grafts? Or joined
using replace refs? Or just the unsplit full repository?

Also what is interesting is if local operations work with the same
speed in the small working repository as in the unsplit full
repository.

 Are there any disadvantages of using grafts and replace? Will both of
 them be supported in future versions of Git?

 My opinion is that grafts have no advantage compared to replace refs.

 Once you have created your replace refs, they can be managed like
 other git refs, so they are easier to distribute.

 Basically if you want to get the full history on a computer you just need to 
 do:

 git fetch 'refs/replace/*:refs/replace/*'

By the way the above should be:

git fetch origin 'refs/replace/*:refs/replace/*'

 That's true but you still need to have another remote with full
 history because it has lots of tags and branches that will be cloned
 by initial clone.

Yeah, you might want to have another remote for that reason, but this
is true with both grafts and replace refs.

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: Joining historical repository using grafts or replace

[Christian Couder]

Hi Dmitry,

On Fri, Oct 31, 2014 at 4:47 PM, Dmitry Oksenchuk oksenchu...@gmail.com wrote:
 Hi Christian,

 On Thu, Oct 30, 2014 at 6:41 PM, Dmitry Oksenchuk oksenchu...@gmail.com 
 wrote:

 Yes, because of such amount of refs, push in historical repository
 takes 12 sec, push in working repository takes 0.4 sec, push in
 joined repository takes 2 sec. Local operations with history like
 log and blame work with the same speed in joined repository as in
 historical repository.

 What does joined mean? Does it mean joined using grafts? Or joined
 using replace refs? Or just the unsplit full repository?

 It's joined using grafts or replace. In both cases performance is the same.

 Also what is interesting is if local operations work with the same
 speed in the small working repository as in the unsplit full
 repository.

 Speed of operations like git diff, git add, git commit is exactly the
 same in both repositories.
 Operations like git log and git blame work much faster in repository
 without history (not surprisingly :)
 For example, git log in small repository takes 0.2 sec, in full
 repository - 0.8 sec. git blame in full repository can take up to 9
 sec for large files with long history.

Ok, thanks for the information. I think it shows that indeed it makes
sense to split your repo.

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: [L10N] Startup of Git 2.2.0 l10n round 1

[Christian Couder]

From: Alexander Shopov a...@kambanaria.org

 Can you please disambiguate message:
 msgid more than one %s
 
 It means that something somewhere was repeated but does not point what
 and where. Perhaps users care about that.

If you configure something like:

[trailer stuff]
key = Stuff
key = Other

You will get:

$ echo | LANG=C git interpret-trailers
warning: more than one trailer.stuff.key

Which means that more than value was configured for the
trailer.stuff.key configuration option. And it makes no sense
because only one should be the canonical one.

 It is now used 3 times (trailer.c:552 trailer.c:557
 builtin/remote.c:288) but points to different things that were
 repeated. It used to mean only that there is a remote' section
 repeated.

In builtin/remote.c the warning is also when more than one value is
given for a configuration option.

Feel free to suggest some more explicit warning in both cases if you
want. (Maybe adding  is configured at the end would be enough.) A
patch would be even 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 4/5] trailer: reuse ignore_non_trailer() to ignore conflict lines

[Christian Couder]

Make sure we look for trailers before any conflict line
by reusing the ignore_non_trailer() function.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh |  2 ++
 trailer.c | 25 ++---
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 1efb880..fed053a 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -232,6 +232,8 @@ test_expect_success 'with message that has comments' '
 
Reviewed-by: Johan
Cc: Peff
+   # last comment
+
EOF
cat basic_patch expected 
git interpret-trailers --trim-empty --trailer Cc: Peff 
message_with_comments actual 
diff --git a/trailer.c b/trailer.c
index f4d51ba..f6aa181 100644
--- a/trailer.c
+++ b/trailer.c
@@ -2,6 +2,7 @@
 #include string-list.h
 #include run-command.h
 #include string-list.h
+#include commit.h
 #include trailer.h
 /*
  * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org
@@ -791,14 +792,24 @@ static int process_input_file(struct strbuf **lines,
  struct trailer_item **in_tok_last)
 {
int count = 0;
-   int patch_start, trailer_start, i;
+   int trailer_start, trailer_end, patch_start, ignore_bytes, i;
+   struct strbuf sb;
 
/* Get the line count */
while (lines[count])
count++;
 
patch_start = find_patch_start(lines, count);
-   trailer_start = find_trailer_start(lines, patch_start);
+
+   /* Get the index of the end of the trailers */
+   for (i = 0; i  patch_start; i++)
+   strbuf_addbuf(sb, lines[i]);
+   ignore_bytes = ignore_non_trailer(sb);
+   for (i = patch_start - 1; i = 0  ignore_bytes  0; i--)
+   ignore_bytes -= lines[i]-len;
+   trailer_end = i + 1;
+
+   trailer_start = find_trailer_start(lines, trailer_end);
 
/* Print lines before the trailers as is */
print_lines(lines, 0, trailer_start);
@@ -807,14 +818,14 @@ static int process_input_file(struct strbuf **lines,
printf(\n);
 
/* Parse trailer lines */
-   for (i = trailer_start; i  patch_start; i++) {
+   for (i = trailer_start; i  trailer_end; i++) {
if (lines[i]-buf[0] != comment_line_char) {
struct trailer_item *new = 
create_trailer_item(lines[i]-buf);
add_trailer_item(in_tok_first, in_tok_last, new);
}
}
 
-   return patch_start;
+   return trailer_end;
 }
 
 static void free_all(struct trailer_item **first)
@@ -831,7 +842,7 @@ void process_trailers(const char *file, int trim_empty, 
struct string_list *trai
struct trailer_item *in_tok_last = NULL;
struct trailer_item *arg_tok_first;
struct strbuf **lines;
-   int patch_start;
+   int trailer_end;
 
/* Default config must be setup first */
git_config(git_trailer_default_config, NULL);
@@ -840,7 +851,7 @@ void process_trailers(const char *file, int trim_empty, 
struct string_list *trai
lines = read_input_file(file);
 
/* Print the lines before the trailers */
-   patch_start = process_input_file(lines, in_tok_first, in_tok_last);
+   trailer_end = process_input_file(lines, in_tok_first, in_tok_last);
 
arg_tok_first = process_command_line_args(trailers);
 
@@ -851,7 +862,7 @@ void process_trailers(const char *file, int trim_empty, 
struct string_list *trai
free_all(in_tok_first);
 
/* Print the lines after the trailers as is */
-   print_lines(lines, patch_start, INT_MAX);
+   print_lines(lines, trailer_end, INT_MAX);
 
strbuf_list_free(lines);
 }
-- 
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/5] trailer: ignore comment lines inside the trailers

[Christian Couder]

Otherwise trailers that are commented out might be
processed. We would also error out if the comment line
char is also a separator.

This means that comments inside a trailer block will
disappear, but that was already the case anyway.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/trailer.c b/trailer.c
index 8514566..761b763 100644
--- a/trailer.c
+++ b/trailer.c
@@ -804,8 +804,10 @@ static int process_input_file(struct strbuf **lines,
 
/* 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);
+   if (lines[i]-buf[0] != comment_line_char) {
+   struct trailer_item *new = 
create_trailer_item(lines[i]-buf);
+   add_trailer_item(in_tok_first, in_tok_last, new);
+   }
}
 
return patch_start;
-- 
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 3/5] commit: make ignore_non_trailer() non static

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/commit.c | 46 --
 commit.c | 46 ++
 commit.h |  3 +++
 3 files changed, 49 insertions(+), 46 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index e3c60dd..cda74e9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -677,52 +677,6 @@ static void adjust_comment_line_char(const struct strbuf 
*sb)
comment_line_char = *p;
 }
 
-/*
- * Inspect sb and determine the true end of the log message, in
- * order to find where to put a new Signed-off-by: line.  Ignored are
- * trailing comment lines and blank lines, and also the traditional
- * Conflicts: block that is not commented out, so that we can use
- * git commit -s --amend on an existing commit that forgot to remove
- * it.
- *
- * Returns the number of bytes from the tail to ignore, to be fed as
- * the second parameter to append_signoff().
- */
-static int ignore_non_trailer(struct strbuf *sb)
-{
-   int boc = 0;
-   int bol = 0;
-   int in_old_conflicts_block = 0;
-
-   while (bol  sb-len) {
-   char *next_line;
-
-   if (!(next_line = memchr(sb-buf + bol, '\n', sb-len - bol)))
-   next_line = sb-buf + sb-len;
-   else
-   next_line++;
-
-   if (sb-buf[bol] == comment_line_char || sb-buf[bol] == '\n') {
-   /* is this the first of the run of comments? */
-   if (!boc)
-   boc = bol;
-   /* otherwise, it is just continuing */
-   } else if (starts_with(sb-buf + bol, Conflicts:\n)) {
-   in_old_conflicts_block = 1;
-   if (!boc)
-   boc = bol;
-   } else if (in_old_conflicts_block  sb-buf[bol] == '\t') {
-   ; /* a pathname in the conflicts block */
-   } else if (boc) {
-   /* the previous was not trailing comment */
-   boc = 0;
-   in_old_conflicts_block = 0;
-   }
-   bol = next_line - sb-buf;
-   }
-   return boc ? sb-len - boc : 0;
-}
-
 static int prepare_to_commit(const char *index_file, const char *prefix,
 struct commit *current_head,
 struct wt_status *s,
diff --git a/commit.c b/commit.c
index 19cf8f9..a54cb9a 100644
--- a/commit.c
+++ b/commit.c
@@ -1640,3 +1640,49 @@ const char *find_commit_header(const char *msg, const 
char *key, size_t *out_len
}
return NULL;
 }
+
+/*
+ * Inspect sb and determine the true end of the log message, in
+ * order to find where to put a new Signed-off-by: line.  Ignored are
+ * trailing comment lines and blank lines, and also the traditional
+ * Conflicts: block that is not commented out, so that we can use
+ * git commit -s --amend on an existing commit that forgot to remove
+ * it.
+ *
+ * Returns the number of bytes from the tail to ignore, to be fed as
+ * the second parameter to append_signoff().
+ */
+int ignore_non_trailer(struct strbuf *sb)
+{
+   int boc = 0;
+   int bol = 0;
+   int in_old_conflicts_block = 0;
+
+   while (bol  sb-len) {
+   char *next_line;
+
+   if (!(next_line = memchr(sb-buf + bol, '\n', sb-len - bol)))
+   next_line = sb-buf + sb-len;
+   else
+   next_line++;
+
+   if (sb-buf[bol] == comment_line_char || sb-buf[bol] == '\n') {
+   /* is this the first of the run of comments? */
+   if (!boc)
+   boc = bol;
+   /* otherwise, it is just continuing */
+   } else if (starts_with(sb-buf + bol, Conflicts:\n)) {
+   in_old_conflicts_block = 1;
+   if (!boc)
+   boc = bol;
+   } else if (in_old_conflicts_block  sb-buf[bol] == '\t') {
+   ; /* a pathname in the conflicts block */
+   } else if (boc) {
+   /* the previous was not trailing comment */
+   boc = 0;
+   in_old_conflicts_block = 0;
+   }
+   bol = next_line - sb-buf;
+   }
+   return boc ? sb-len - boc : 0;
+}
diff --git a/commit.h b/commit.h
index bc68ccb..cd35ac1 100644
--- a/commit.h
+++ b/commit.h
@@ -337,6 +337,9 @@ extern void free_commit_extra_headers(struct 
commit_extra_header *extra);
 extern const char *find_commit_header(const char *msg, const char *key,
  size_t *out_len);
 
+/* Find the end of the log message, the right place for a new trailer. */
+extern int ignore_non_trailer(struct strbuf *sb);
+
 typedef void

[PATCH 5/5] trailer: add test with an old style conflict block

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index fed053a..bd0ab46 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -213,7 +213,7 @@ test_expect_success 'with 2 files arguments' '
 '
 
 test_expect_success 'with message that has comments' '
-   cat basic_message message_with_comments 
+   cat basic_message message_with_comments 
sed -e s/ Z\$/ / message_with_comments -\EOF 
# comment
 
@@ -240,6 +240,36 @@ test_expect_success 'with message that has comments' '
test_cmp expected actual
 '
 
+test_expect_success 'with message that has an old style conflict block' '
+   cat basic_message message_with_comments 
+   sed -e s/ Z\$/ / message_with_comments -\EOF 
+   # comment
+
+   # other comment
+   Cc: Z
+   # yet another comment
+   Reviewed-by: Johan
+   Reviewed-by: Z
+   # last comment
+
+   Conflicts:
+
+   EOF
+   cat basic_message expected 
+   cat expected -\EOF 
+   # comment
+
+   Reviewed-by: Johan
+   Cc: Peff
+   # last comment
+
+   Conflicts:
+
+   EOF
+   git interpret-trailers --trim-empty --trailer Cc: Peff 
message_with_comments actual 
+   test_cmp expected actual
+'
+
 test_expect_success 'with commit complex message and trailer args' '
cat complex_message_body expected 
sed -e s/ Z\$/ / expected -\EOF 
-- 
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 0/5] Small git interpret-trailers fixes

[Christian Couder]

Here is a series to fix a few small bugs in git interpret-trailers.

Patch 1/5 and patch 2/5 are independent from the last 3 patches,
and can be applied to master.

Patches 3/5, 4/5 and 5/5 depend on this series by Junio:

jc/conflict-hint (2014-10-28) 4 commits
  (merged to 'next' on 2014-10-29 at 693250f)

Christian Couder (5):
  trailer: ignore comment lines inside the trailers
  trailer: display a trailer without its trailing newline
  commit: make ignore_non_trailer() non static
  trailer: reuse ignore_non_trailer() to ignore conflict lines
  trailer: add test with an old style conflict block

 builtin/commit.c  | 46 ---
 commit.c  | 46 +++
 commit.h  |  3 +++
 t/t7513-interpret-trailers.sh | 34 +++-
 trailer.c | 39 +---
 5 files changed, 110 insertions(+), 58 deletions(-)

-- 
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 2/5] trailer: display a trailer without its trailing newline

[Christian Couder]

Trailers passed to the parse_trailer() function have
a trailing newline. When erroring out, we should display
the invalid trailer properly, that means without any
trailing newline.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/trailer.c b/trailer.c
index 761b763..f4d51ba 100644
--- a/trailer.c
+++ b/trailer.c
@@ -583,8 +583,12 @@ static int parse_trailer(struct strbuf *tok, struct strbuf 
*val, const char *tra
strbuf_addch(seps, '=');
len = strcspn(trailer, seps.buf);
strbuf_release(seps);
-   if (len == 0)
-   return error(_(empty trailer token in trailer '%s'), trailer);
+   if (len == 0) {
+   struct strbuf sb = STRBUF_INIT;
+   strbuf_addstr(sb, trailer);
+   strbuf_rtrim(sb);
+   return error(_(empty trailer token in trailer '%s'), sb.buf);
+   }
if (len  strlen(trailer)) {
strbuf_add(tok, trailer, len);
strbuf_trim(tok);
-- 
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 v2 0/5] Small git interpret-trailers fixes

[Christian Couder]

Here is a series to fix a few small bugs in git interpret-trailers.

Patch 1/5 and patch 2/5 are independent from the last 3 patches,
and can be applied to master.

Patches 3/5, 4/5 and 5/5 depend on this series by Junio:

jc/conflict-hint (2014-10-28) 4 commits
  (merged to 'next' on 2014-10-29 at 693250f)

Changes since the previous version are:

* use %.*s instead of a strbuf, in patch 2/5

* add new find_trailer_end() function and a call to strbuf_release(),
  in patch 4/5

Thanks to Junio and Peff.

Christian Couder (5):
  trailer: ignore comment lines inside the trailers
  trailer: display a trailer without its trailing newline
  commit: make ignore_non_trailer() non static
  trailer: reuse ignore_non_trailer() to ignore conflict lines
  trailer: add test with an old style conflict block

 builtin/commit.c  | 46 ---
 commit.c  | 46 +++
 commit.h  |  3 +++
 t/t7513-interpret-trailers.sh | 34 +++-
 trailer.c | 46 ---
 5 files changed, 117 insertions(+), 58 deletions(-)

-- 
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 v2 5/5] trailer: add test with an old style conflict block

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index fed053a..bd0ab46 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -213,7 +213,7 @@ test_expect_success 'with 2 files arguments' '
 '
 
 test_expect_success 'with message that has comments' '
-   cat basic_message message_with_comments 
+   cat basic_message message_with_comments 
sed -e s/ Z\$/ / message_with_comments -\EOF 
# comment
 
@@ -240,6 +240,36 @@ test_expect_success 'with message that has comments' '
test_cmp expected actual
 '
 
+test_expect_success 'with message that has an old style conflict block' '
+   cat basic_message message_with_comments 
+   sed -e s/ Z\$/ / message_with_comments -\EOF 
+   # comment
+
+   # other comment
+   Cc: Z
+   # yet another comment
+   Reviewed-by: Johan
+   Reviewed-by: Z
+   # last comment
+
+   Conflicts:
+
+   EOF
+   cat basic_message expected 
+   cat expected -\EOF 
+   # comment
+
+   Reviewed-by: Johan
+   Cc: Peff
+   # last comment
+
+   Conflicts:
+
+   EOF
+   git interpret-trailers --trim-empty --trailer Cc: Peff 
message_with_comments actual 
+   test_cmp expected actual
+'
+
 test_expect_success 'with commit complex message and trailer args' '
cat complex_message_body expected 
sed -e s/ Z\$/ / expected -\EOF 
-- 
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 v2 2/5] trailer: display a trailer without its trailing newline

[Christian Couder]

Trailers passed to the parse_trailer() function often have
a trailing newline. When erroring out, we should display
the invalid trailer properly, that means without any
trailing newline.

Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Jeff King p...@peff.net
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/trailer.c b/trailer.c
index 761b763..219a5a2 100644
--- a/trailer.c
+++ b/trailer.c
@@ -583,8 +583,12 @@ static int parse_trailer(struct strbuf *tok, struct strbuf 
*val, const char *tra
strbuf_addch(seps, '=');
len = strcspn(trailer, seps.buf);
strbuf_release(seps);
-   if (len == 0)
-   return error(_(empty trailer token in trailer '%s'), trailer);
+   if (len == 0) {
+   int l = strlen(trailer);
+   while (l  0  isspace(trailer[l - 1]))
+   l--;
+   return error(_(empty trailer token in trailer '%.*s'), l, 
trailer);
+   }
if (len  strlen(trailer)) {
strbuf_add(tok, trailer, len);
strbuf_trim(tok);
-- 
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 v2 4/5] trailer: reuse ignore_non_trailer() to ignore conflict lines

[Christian Couder]

Make sure we look for trailers before any conflict line
by reusing the ignore_non_trailer() function.

Helped-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t7513-interpret-trailers.sh |  2 ++
 trailer.c | 32 +---
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 1efb880..fed053a 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -232,6 +232,8 @@ test_expect_success 'with message that has comments' '
 
Reviewed-by: Johan
Cc: Peff
+   # last comment
+
EOF
cat basic_patch expected 
git interpret-trailers --trim-empty --trailer Cc: Peff 
message_with_comments actual 
diff --git a/trailer.c b/trailer.c
index 219a5a2..30636d2 100644
--- a/trailer.c
+++ b/trailer.c
@@ -2,6 +2,7 @@
 #include string-list.h
 #include run-command.h
 #include string-list.h
+#include commit.h
 #include trailer.h
 /*
  * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org
@@ -769,6 +770,22 @@ static int find_trailer_start(struct strbuf **lines, int 
count)
return only_spaces ? count : 0;
 }
 
+/* Get the index of the end of the trailers */
+static int find_trailer_end(struct strbuf **lines, int patch_start)
+{
+   struct strbuf sb = STRBUF_INIT;
+   int i, ignore_bytes;
+
+   for (i = 0; i  patch_start; i++)
+   strbuf_addbuf(sb, lines[i]);
+   ignore_bytes = ignore_non_trailer(sb);
+   strbuf_release(sb);
+   for (i = patch_start - 1; i = 0  ignore_bytes  0; i--)
+   ignore_bytes -= lines[i]-len;
+
+   return i + 1;
+}
+
 static int has_blank_line_before(struct strbuf **lines, int start)
 {
for (;start = 0; start--) {
@@ -791,14 +808,15 @@ static int process_input_file(struct strbuf **lines,
  struct trailer_item **in_tok_last)
 {
int count = 0;
-   int patch_start, trailer_start, i;
+   int patch_start, trailer_start, trailer_end, i;
 
/* Get the line count */
while (lines[count])
count++;
 
patch_start = find_patch_start(lines, count);
-   trailer_start = find_trailer_start(lines, patch_start);
+   trailer_end = find_trailer_end(lines, patch_start);
+   trailer_start = find_trailer_start(lines, trailer_end);
 
/* Print lines before the trailers as is */
print_lines(lines, 0, trailer_start);
@@ -807,14 +825,14 @@ static int process_input_file(struct strbuf **lines,
printf(\n);
 
/* Parse trailer lines */
-   for (i = trailer_start; i  patch_start; i++) {
+   for (i = trailer_start; i  trailer_end; i++) {
if (lines[i]-buf[0] != comment_line_char) {
struct trailer_item *new = 
create_trailer_item(lines[i]-buf);
add_trailer_item(in_tok_first, in_tok_last, new);
}
}
 
-   return patch_start;
+   return trailer_end;
 }
 
 static void free_all(struct trailer_item **first)
@@ -831,7 +849,7 @@ void process_trailers(const char *file, int trim_empty, 
struct string_list *trai
struct trailer_item *in_tok_last = NULL;
struct trailer_item *arg_tok_first;
struct strbuf **lines;
-   int patch_start;
+   int trailer_end;
 
/* Default config must be setup first */
git_config(git_trailer_default_config, NULL);
@@ -840,7 +858,7 @@ void process_trailers(const char *file, int trim_empty, 
struct string_list *trai
lines = read_input_file(file);
 
/* Print the lines before the trailers */
-   patch_start = process_input_file(lines, in_tok_first, in_tok_last);
+   trailer_end = process_input_file(lines, in_tok_first, in_tok_last);
 
arg_tok_first = process_command_line_args(trailers);
 
@@ -851,7 +869,7 @@ void process_trailers(const char *file, int trim_empty, 
struct string_list *trai
free_all(in_tok_first);
 
/* Print the lines after the trailers as is */
-   print_lines(lines, patch_start, INT_MAX);
+   print_lines(lines, trailer_end, INT_MAX);
 
strbuf_list_free(lines);
 }
-- 
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 v2 3/5] commit: make ignore_non_trailer() non static

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/commit.c | 46 --
 commit.c | 46 ++
 commit.h |  3 +++
 3 files changed, 49 insertions(+), 46 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index e3c60dd..cda74e9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -677,52 +677,6 @@ static void adjust_comment_line_char(const struct strbuf 
*sb)
comment_line_char = *p;
 }
 
-/*
- * Inspect sb and determine the true end of the log message, in
- * order to find where to put a new Signed-off-by: line.  Ignored are
- * trailing comment lines and blank lines, and also the traditional
- * Conflicts: block that is not commented out, so that we can use
- * git commit -s --amend on an existing commit that forgot to remove
- * it.
- *
- * Returns the number of bytes from the tail to ignore, to be fed as
- * the second parameter to append_signoff().
- */
-static int ignore_non_trailer(struct strbuf *sb)
-{
-   int boc = 0;
-   int bol = 0;
-   int in_old_conflicts_block = 0;
-
-   while (bol  sb-len) {
-   char *next_line;
-
-   if (!(next_line = memchr(sb-buf + bol, '\n', sb-len - bol)))
-   next_line = sb-buf + sb-len;
-   else
-   next_line++;
-
-   if (sb-buf[bol] == comment_line_char || sb-buf[bol] == '\n') {
-   /* is this the first of the run of comments? */
-   if (!boc)
-   boc = bol;
-   /* otherwise, it is just continuing */
-   } else if (starts_with(sb-buf + bol, Conflicts:\n)) {
-   in_old_conflicts_block = 1;
-   if (!boc)
-   boc = bol;
-   } else if (in_old_conflicts_block  sb-buf[bol] == '\t') {
-   ; /* a pathname in the conflicts block */
-   } else if (boc) {
-   /* the previous was not trailing comment */
-   boc = 0;
-   in_old_conflicts_block = 0;
-   }
-   bol = next_line - sb-buf;
-   }
-   return boc ? sb-len - boc : 0;
-}
-
 static int prepare_to_commit(const char *index_file, const char *prefix,
 struct commit *current_head,
 struct wt_status *s,
diff --git a/commit.c b/commit.c
index 19cf8f9..a54cb9a 100644
--- a/commit.c
+++ b/commit.c
@@ -1640,3 +1640,49 @@ const char *find_commit_header(const char *msg, const 
char *key, size_t *out_len
}
return NULL;
 }
+
+/*
+ * Inspect sb and determine the true end of the log message, in
+ * order to find where to put a new Signed-off-by: line.  Ignored are
+ * trailing comment lines and blank lines, and also the traditional
+ * Conflicts: block that is not commented out, so that we can use
+ * git commit -s --amend on an existing commit that forgot to remove
+ * it.
+ *
+ * Returns the number of bytes from the tail to ignore, to be fed as
+ * the second parameter to append_signoff().
+ */
+int ignore_non_trailer(struct strbuf *sb)
+{
+   int boc = 0;
+   int bol = 0;
+   int in_old_conflicts_block = 0;
+
+   while (bol  sb-len) {
+   char *next_line;
+
+   if (!(next_line = memchr(sb-buf + bol, '\n', sb-len - bol)))
+   next_line = sb-buf + sb-len;
+   else
+   next_line++;
+
+   if (sb-buf[bol] == comment_line_char || sb-buf[bol] == '\n') {
+   /* is this the first of the run of comments? */
+   if (!boc)
+   boc = bol;
+   /* otherwise, it is just continuing */
+   } else if (starts_with(sb-buf + bol, Conflicts:\n)) {
+   in_old_conflicts_block = 1;
+   if (!boc)
+   boc = bol;
+   } else if (in_old_conflicts_block  sb-buf[bol] == '\t') {
+   ; /* a pathname in the conflicts block */
+   } else if (boc) {
+   /* the previous was not trailing comment */
+   boc = 0;
+   in_old_conflicts_block = 0;
+   }
+   bol = next_line - sb-buf;
+   }
+   return boc ? sb-len - boc : 0;
+}
diff --git a/commit.h b/commit.h
index bc68ccb..cd35ac1 100644
--- a/commit.h
+++ b/commit.h
@@ -337,6 +337,9 @@ extern void free_commit_extra_headers(struct 
commit_extra_header *extra);
 extern const char *find_commit_header(const char *msg, const char *key,
  size_t *out_len);
 
+/* Find the end of the log message, the right place for a new trailer. */
+extern int ignore_non_trailer(struct strbuf *sb);
+
 typedef void

[PATCH v2 1/5] trailer: ignore comment lines inside the trailers

[Christian Couder]

Otherwise trailers that are commented out might be
processed. We would also error out if the comment line
char is also a separator.

This means that comments inside a trailer block will
disappear, but that was already the case anyway.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 trailer.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/trailer.c b/trailer.c
index 8514566..761b763 100644
--- a/trailer.c
+++ b/trailer.c
@@ -804,8 +804,10 @@ static int process_input_file(struct strbuf **lines,
 
/* 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);
+   if (lines[i]-buf[0] != comment_line_char) {
+   struct trailer_item *new = 
create_trailer_item(lines[i]-buf);
+   add_trailer_item(in_tok_first, in_tok_last, new);
+   }
}
 
return patch_start;
-- 
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 4/5] trailer: reuse ignore_non_trailer() to ignore conflict lines

[Christian Couder]

From: Junio C Hamano gits...@pobox.com
Subject: Re: [PATCH 4/5] trailer: reuse ignore_non_trailer() to ignore conflict 
lines
Date: Fri, 07 Nov 2014 12:27:03 -0800

 Christian Couder chrisc...@tuxfamily.org writes:
 
   * Copyright (c) 2013, 2014 Christian Couder chrisc...@tuxfamily.org
 @@ -791,14 +792,24 @@ static int process_input_file(struct strbuf **lines,
struct trailer_item **in_tok_last)
  {
  int count = 0;
 -int patch_start, trailer_start, i;
 +int trailer_start, trailer_end, patch_start, ignore_bytes, i;
 +struct strbuf sb;
  
  /* Get the line count */
  while (lines[count])
  count++;
  
  patch_start = find_patch_start(lines, count);
 -trailer_start = find_trailer_start(lines, patch_start);
 +
 +/* Get the index of the end of the trailers */
 +for (i = 0; i  patch_start; i++)
 +strbuf_addbuf(sb, lines[i]);
 +ignore_bytes = ignore_non_trailer(sb);
 +for (i = patch_start - 1; i = 0  ignore_bytes  0; i--)
 +ignore_bytes -= lines[i]-len;
 +trailer_end = i + 1;
 
 Looks like there is an impedance mismatch between the caller and the
 callee here.  I can sort of understand why you might want to have an
 array of trailer items, one element per line in the trailer block,
 as that would make it easier on your brain when you have to reorder
 them, insert a new one or a remove an existing one, but you seem to
 be keeping _everything_ in that format, an array of strbuf with one
 strbuf per line, which sounds really wasteful.  The data structure
 might need to be rethoughtbefore this code becomes ready for
 production.
 
 I would have expected it to be more like (1) slurp everything in a
 single strbuf, (2) find the trailer block inside that single strbuf,
 splitting what you read in the previous step into one strbuf for
 stuff before the trailer block, another strbuf for stuff after the
 trailer block and an array of lines in the tailer block, (3) do
 whatever your trailer flipping logic inside that array without
 having to worry about stuff before or after the trailer block and
 then finally (4) spit the whole thing out by concatenating the stuff
 before the trailer block, the stuff in the trailer block and the
 stuff after the trailer block.
 
 Oh well...

I hope it is better now, as I encapsulated the call to
ignore_non_trailer() into a new find_trailer_end() function. There
were already find_trailer_start() and find_patch_start(), and I think
this way we could have a nice high level line oriented API.

Yeah, it won't be as efficient as using only one strbuf and only byte
oriented functions, and it looks much less manly too :-) But over time in
Git we have developed a number of less efficient but quite clean
abstractions like strbuf, argv_array, sha1_array and so on, that we
are quite happy with.

So yeah, with old age we might have lost some manliness and lament the
time when men were really men, and ... Well, let's wait for the
hopefully big party^W conference next year to do that together :-)

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/5] trailer: reuse ignore_non_trailer() to ignore conflict lines

[Christian Couder]

From: Junio C Hamano gits...@pobox.com
Subject: Re: [PATCH v2 4/5] trailer: reuse ignore_non_trailer() to ignore 
conflict lines
Date: Mon, 10 Nov 2014 09:49:34 -0800

 Christian Couder chrisc...@tuxfamily.org writes:
 
 Make sure we look for trailers before any conflict line
 by reusing the ignore_non_trailer() function.

 Helped-by: Junio C Hamano gits...@pobox.com
 Signed-off-by: Christian Couder chrisc...@tuxfamily.org
 ---
 
 It makes sense to unify the logic to decide where the trailer block
 is, I would think.  I however don't think I helped this change in
 any way, not more than maintained the codebase as a solid
 foundation to build new features on, but at that point it would
 apply to any other change and not worth mentioning ;-).

Do you want me to resend the series with only the Helped-by removed?
--
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] replace: forbid replacing an object with one of a different type

[Christian Couder]

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

The doc will be updated in a later patch.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
If this patch is considered useful, I will update the doc and
maybe add tests.

 builtin/replace.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..0246ab3 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char 
*replace_ref,
  int force)
 {
unsigned char object[20], prev[20], repl[20];
+   enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
 
@@ -100,6 +101,14 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
if (check_refname_format(ref, 0))
die('%s' is not a valid ref name., ref);
 
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(repl, NULL);
+   if (obj_type != repl_type)
+   die(Object ref '%s' is of type '%s'\n
+   while replace ref '%s' is of type '%s'.,
+   object_ref, typename(obj_type),
+   replace_ref, typename(repl_type));
+
if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
-- 
1.8.4.rc1.22.g132b1a0

--
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/5] t6050-replace: test that objects are of the same type

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index decdc33..8f631ac 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -263,4 +263,17 @@ test_expect_success 'not just commits' '
test_cmp file.replaced file
 '
 
+test_expect_success 'replaced and replacement objects must be of the same 
type' '
+   test_must_fail git replace mytag $HASH1 2err 
+   grep Object ref '\''mytag'\'' is of type '\''tag'\'' err 
+   grep replace ref '\''$HASH1'\'' is of type '\''commit'\'' err 
+   test_must_fail git replace HEAD^{tree} HEAD~1 2err 
+   grep Object ref '\''HEAD^{tree}'\'' is of type '\''tree'\'' err 
+   grep replace ref '\''HEAD~1'\'' is of type '\''commit'\'' err 
+   BLOB=$(git rev-parse :file) 
+   test_must_fail git replace HEAD^ $BLOB 2err 
+   grep Object ref '\''HEAD^'\'' is of type '\''commit'\'' err 
+   grep replace ref '\''$BLOB'\'' is of type '\''blob'\'' err
+'
+
 test_done
-- 
1.8.4.rc1.24.g13dc82a


--
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 4/5] t6050-replace: add test to clean up all the replace refs

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 8f631ac..4807689 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,4 +276,10 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
grep replace ref '\''$BLOB'\'' is of type '\''blob'\'' err
 '
 
+test_expect_success 'replace ref cleanup' '
+   test -n $(git replace) 
+   git replace -d $(git replace) 
+   test -z $(git replace)
+'
+
 test_done
-- 
1.8.4.rc1.24.g13dc82a


--
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/5] replace: forbid replacing an object with one of a different type

[Christian Couder]

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

There is no case where one object can be replaced with one of a
different type while keeping the history valid, because:

* Annotated tags contain the type of the tagged object.

* The tree/parent lines in commits must be a tree and commits, resp.

* The object types referred to by trees are specified in the 'mode'
  field:
100644 and 100755blob
16   commit
04   tree
  (these are the only valid modes)

* Blobs don't point at anything.

The doc will be updated in a later patch.

Acked-by: Philip Oakley philipoak...@iee.org
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..bc6c245 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char 
*replace_ref,
  int force)
 {
unsigned char object[20], prev[20], repl[20];
+   enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
 
@@ -100,6 +101,15 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
if (check_refname_format(ref, 0))
die('%s' is not a valid ref name., ref);
 
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(repl, NULL);
+   if (obj_type != repl_type)
+   die(Objects must be of the same type.\n
+   Object ref '%s' is of type '%s'\n
+   while replace ref '%s' is of type '%s'.,
+   object_ref, typename(obj_type),
+   replace_ref, typename(repl_type));
+
if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
-- 
1.8.4.rc1.24.g13dc82a


--
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 5/5] Documentation/replace: add Creating Replacement Objects section

[Christian Couder]

There were no hints in the documentation about how to create
replacement objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index aa66d27..736b48c 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -64,6 +64,19 @@ OPTIONS
Typing git replace without arguments, also lists all replace
refs.
 
+CREATING REPLACEMENT OBJECTS
+
+
+linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and
+linkgit:git-rebase[1], among other git commands, can be used to create
+replacement objects from existing objects.
+
+If you want to replace many blobs, trees or commits that are part of a
+string of commits, you may just want to create a replacement string of
+commits and then only replace the commit at the tip of the target
+string of commits with the commit at the tip of the replacement string
+of commits.
+
 BUGS
 
 Comparing blobs or trees that have been replaced with those that
@@ -76,6 +89,9 @@ pending objects.
 
 SEE ALSO
 
+linkgit:git-hash-object[1]
+linkgit:git-filter-branch[1]
+linkgit:git-rebase[1]
 linkgit:git-tag[1]
 linkgit:git-branch[1]
 linkgit:git[1]
-- 
1.8.4.rc1.24.g13dc82a

--
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/5] Documentation/replace: state that objects must be of the same type

[Christian Couder]

A previous patch ensures that both the replaced and the replacement
objects passed to git replace must be of the same type.

While at it state that there is no other restriction on both objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index e0b4057..aa66d27 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -20,6 +20,9 @@ The name of the 'replace' reference is the SHA-1 of the 
object that is
 replaced. The content of the 'replace' reference is the SHA-1 of the
 replacement object.
 
+The replaced object and the replacement object must be of the same type.
+There is no other restriction on them.
+
 Unless `-f` is given, the 'replace' reference must not yet exist.
 
 Replacement references will be used by default by all Git commands
@@ -69,9 +72,7 @@ go back to a replaced commit will move the branch to the 
replacement
 commit instead of the replaced commit.
 
 There may be other problems when using 'git rev-list' related to
-pending objects. And of course things may break if an object of one
-type is replaced by an object of another type (for example a blob
-replaced by a commit).
+pending objects.
 
 SEE ALSO
 
-- 
1.8.4.rc1.24.g13dc82a


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/5] Check replacement object type and minor updates

[Christian Couder]

Here is a reroll of the patch forbiding replacing an bject with one
of a different type (1/5).
It is followed by a documentation update (2/5) and a test (3/5).
I also added for good mesure another test (4/5) I came up with while
developing the previous test, and another doc update related to what
we discussed about creating replacement objects (5/5).

Christian Couder (5):
  replace: forbid replacing an object with one of a different type
  Documentation/replace: state that objects must be of the same type
  t6050-replace: test that objects are of the same type
  t6050-replace: add test to clean up all the replace refs
  Documentation/replace: add Creating Replacement Objects section

 Documentation/git-replace.txt | 23 ---
 builtin/replace.c | 10 ++
 t/t6050-replace.sh| 19 +++
 3 files changed, 49 insertions(+), 3 deletions(-)

-- 
1.8.4.rc1.24.g13dc82a

--
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/5] replace: forbid replacing an object with one of a different type

[Christian Couder]

From: Johannes Sixt j...@kdbg.org

 Am 25.08.2013 15:06, schrieb Christian Couder:
 @@ -100,6 +101,15 @@ static int replace_object(const char *object_ref, const 
 char *replace_ref,
  if (check_refname_format(ref, 0))
  die('%s' is not a valid ref name., ref);
  
 +obj_type = sha1_object_info(object, NULL);
 +repl_type = sha1_object_info(repl, NULL);
 +if (obj_type != repl_type)
 +die(Objects must be of the same type.\n
 +Object ref '%s' is of type '%s'\n
 
 Is it really an Object ref, not just an Object?

Well, it is what is passed to the command line. It is then converted into an
hex sha1 using get_sha1() and then sha1_to_hex().

What about:

die(Objects must be of the same type.\n
'%s' points to a replaced object of type '%s'\n
while '%s' points to a replacement object of type '%s'.,

 BTW, I appreciate your choice of where in the sentence the line breaks are.

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 1/5] replace: forbid replacing an object with one of a different type

[Christian Couder]

From: Johannes Sixt j...@kdbg.org

 Am 25.08.2013 21:44, schrieb Christian Couder:
 What about:
 
  die(Objects must be of the same type.\n
  '%s' points to a replaced object of type '%s'\n
  while '%s' points to a replacement object of type '%s'.,
 
 Much better!

Ok, I will reroll with this change and the change you suggest in the test.

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 1/5] replace: forbid replacing an object with one of a different type

[Christian Couder]

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

There is no case where one object can be replaced with one of a
different type while keeping the history valid, because:

* Annotated tags contain the type of the tagged object.

* The tree/parent lines in commits must be a tree and commits, resp.

* The object types referred to by trees are specified in the 'mode'
  field:
100644 and 100755blob
16   commit
04   tree
  (these are the only valid modes)

* Blobs don't point at anything.

The doc will be updated in a later patch.

Acked-by: Philip Oakley philipoak...@iee.org
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..9a94769 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char 
*replace_ref,
  int force)
 {
unsigned char object[20], prev[20], repl[20];
+   enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
 
@@ -100,6 +101,15 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
if (check_refname_format(ref, 0))
die('%s' is not a valid ref name., ref);
 
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(repl, NULL);
+   if (obj_type != repl_type)
+   die(Objects must be of the same type.\n
+   '%s' points to a replaced object of type '%s'\n
+   while '%s' points to a replacement object of type '%s'.,
+   object_ref, typename(obj_type),
+   replace_ref, typename(repl_type));
+
if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
-- 
1.8.4.rc1.26.gdd51ee9


--
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/5] t6050-replace: add test to clean up all the replace refs

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 5c352c4..05be228 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,4 +276,10 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
grep $BLOB. points to a replacement object of type .blob err
 '
 
+test_expect_success 'replace ref cleanup' '
+   test -n $(git replace) 
+   git replace -d $(git replace) 
+   test -z $(git replace)
+'
+
 test_done
-- 
1.8.4.rc1.26.gdd51ee9


--
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 3/5] t6050-replace: test that objects are of the same type

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index decdc33..5c352c4 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -263,4 +263,17 @@ test_expect_success 'not just commits' '
test_cmp file.replaced file
 '
 
+test_expect_success 'replaced and replacement objects must be of the same 
type' '
+   test_must_fail git replace mytag $HASH1 2err 
+   grep mytag. points to a replaced object of type .tag err 
+   grep $HASH1. points to a replacement object of type .commit err 
+   test_must_fail git replace HEAD^{tree} HEAD~1 2err 
+   grep HEAD^{tree}. points to a replaced object of type .tree err 
+   grep HEAD~1. points to a replacement object of type .commit err 
+   BLOB=$(git rev-parse :file) 
+   test_must_fail git replace HEAD^ $BLOB 2err 
+   grep HEAD^. points to a replaced object of type .commit err 
+   grep $BLOB. points to a replacement object of type .blob err
+'
+
 test_done
-- 
1.8.4.rc1.26.gdd51ee9


--
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 5/5] Documentation/replace: add Creating Replacement Objects section

[Christian Couder]

There were no hints in the documentation about how to create
replacement objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index aa66d27..736b48c 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -64,6 +64,19 @@ OPTIONS
Typing git replace without arguments, also lists all replace
refs.
 
+CREATING REPLACEMENT OBJECTS
+
+
+linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and
+linkgit:git-rebase[1], among other git commands, can be used to create
+replacement objects from existing objects.
+
+If you want to replace many blobs, trees or commits that are part of a
+string of commits, you may just want to create a replacement string of
+commits and then only replace the commit at the tip of the target
+string of commits with the commit at the tip of the replacement string
+of commits.
+
 BUGS
 
 Comparing blobs or trees that have been replaced with those that
@@ -76,6 +89,9 @@ pending objects.
 
 SEE ALSO
 
+linkgit:git-hash-object[1]
+linkgit:git-filter-branch[1]
+linkgit:git-rebase[1]
 linkgit:git-tag[1]
 linkgit:git-branch[1]
 linkgit:git[1]
-- 
1.8.4.rc1.26.gdd51ee9

--
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/5] Documentation/replace: state that objects must be of the same type

[Christian Couder]

A previous patch ensures that both the replaced and the replacement
objects passed to git replace must be of the same type.

While at it state that there is no other restriction on both objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index e0b4057..aa66d27 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -20,6 +20,9 @@ The name of the 'replace' reference is the SHA-1 of the 
object that is
 replaced. The content of the 'replace' reference is the SHA-1 of the
 replacement object.
 
+The replaced object and the replacement object must be of the same type.
+There is no other restriction on them.
+
 Unless `-f` is given, the 'replace' reference must not yet exist.
 
 Replacement references will be used by default by all Git commands
@@ -69,9 +72,7 @@ go back to a replaced commit will move the branch to the 
replacement
 commit instead of the replaced commit.
 
 There may be other problems when using 'git rev-list' related to
-pending objects. And of course things may break if an object of one
-type is replaced by an object of another type (for example a blob
-replaced by a commit).
+pending objects.
 
 SEE ALSO
 
-- 
1.8.4.rc1.26.gdd51ee9


--
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/5] Check replacement object type and minor updates

[Christian Couder]

The only changes compared to the previous version are those
suggested by Johannes:

- the error message is now:

Objects must be of the same type.\n
'%s' points to a replaced object of type '%s'\n
while '%s' points to a replacement object of type '%s'.

- the test uses . instead of '\''

Only patchs 1/5 and 3/5 were changed.

Christian Couder (5):
  replace: forbid replacing an object with one of a different type
  Documentation/replace: state that objects must be of the same type
  t6050-replace: test that objects are of the same type
  t6050-replace: add test to clean up all the replace refs
  Documentation/replace: add Creating Replacement Objects section

 Documentation/git-replace.txt | 23 ---
 builtin/replace.c | 10 ++
 t/t6050-replace.sh| 19 +++
 3 files changed, 49 insertions(+), 3 deletions(-)

-- 
1.8.4.rc1.26.gdd51ee9

--
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 1/5] replace: forbid replacing an object with one of a different type

[Christian Couder]

From: Junio C Hamano gits...@pobox.com

 Thomas Rast tr...@inf.ethz.ch writes:
 
 Hrm, you're right, that's a flaw in my logic.  You could do the same in
 all other cases too, e.g. replace a tree so that an entry is of a
 different type and at the same time change the type of the object
 itself.  You however have to carefully go through all objects that refer
 to the one that was replaced, and fix the type in all of them.

 It still seems an extremely unsafe thing to do with trees...
  ...
 Should we add a --force flag of some sort to allow the user to do this,
 while keeping the normal safety checks?
 
 As long as we do not forbid such an unusual replacement on the
 reading side, we won't break people who are more inventive than we
 are (I am not convinced that we know people's workflow well enough
 to definitively say that no sane workflow, which benefits from being
 able to replace an object with another from a different type,
 exists).
 
 Preventing git replace wrapper from creating such a replacement by
 default will make it harder to do and may reduce mistakes, without
 breaking them too much, I think.

I agree. It is always possible to create replacement refs using git
update-ref if one really wants to.

What about using the following in the commit message or in the
documentation:

--

DISCUSSION

If one object is replaced with one of a different type, the only way
to keep the history valid is to also replace all the other objects
that point to the replaced object. That's because:

* Annotated tags contain the type of the tagged object.

* The tree/parent lines in commits must be a tree and commits, resp.

* The object types referred to by trees are specified in the 'mode'
  field:
100644 and 100755blob
16   commit
04   tree
  (these are the only valid modes)

* Blobs don't point at anything.

But if all the objects that point to an object, called O, are to be
replaced, then in most cases object O probably doesn't need to be
replaced. It's probably sufficient to create the new object, called
O2, that would replace object O and to replace all the objects
pointing to object O with objects pointing to O2.

The only case where someone might really want to replace object 0,
with an object O2 of a different type, and all the objects pointing to
it, is if it's really important, perhaps for external reasons, to have
object O's SHA1 point to O2.

And anyway, if one really wants to do that, it can still be done using
git update-ref.

--

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 1/5] replace: forbid replacing an object with one of a different type

[Christian Couder]

From: Junio C Hamano gits...@pobox.com

 Christian Couder chrisc...@tuxfamily.org writes:
 
 But if all the objects that point to an object, called O, are to be
 replaced, then in most cases object O probably doesn't need to be
 replaced. It's probably sufficient to create the new object, called
 O2, that would replace object O and to replace all the objects
 pointing to object O with objects pointing to O2.
 
 H.
 
 What the above says, with probably and most cases, can easily
 inferred by anybody remotely intelligent, and the only reason to
 have something like the above paragraph would be if it assures that
 the statement holds without these qualifications to weaken it, which
 it doesn't.  I am not sure this paragraph adds much value.
 
 The only case where someone might really want to replace object 0,
 with an object O2 of a different type, and all the objects pointing to
 it, is if it's really important, perhaps for external reasons, to have
 object O's SHA1 point to O2.
 
 The same comment applies here.
 
 And anyway, if one really wants to do that, it can still be done using
 git update-ref.
 
 And I really do not think this sentence is right---you can justify
 with the same sentence to remove git replace wrapper.

Ok, so the commit message (excluding the Acked-by and Signed-off-by)
will be only:

--

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

If one object is replaced with one of a different type, the only way
to keep the history valid is to also replace all the other objects
that point to the replaced object. That's because:

* Annotated tags contain the type of the tagged object.

* The tree/parent lines in commits must be a tree and commits, resp.

* The object types referred to by trees are specified in the 'mode'
  field:
100644 and 100755blob
16   commit
04   tree
  (these are the only valid modes)

* Blobs don't point at anything.

The doc will be updated in a later patch.

--

 The earlier suggestion to bypass the new hand-holding with --force
 is more sensible, I think.

There is already a --force option, but I can add a --force-type in a
another patch.

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 05/11] Documentation/replace: add Creating Replacement Objects section

[Christian Couder]

There were no hints in the documentation about how to create
replacement objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index aa66d27..736b48c 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -64,6 +64,19 @@ OPTIONS
Typing git replace without arguments, also lists all replace
refs.
 
+CREATING REPLACEMENT OBJECTS
+
+
+linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and
+linkgit:git-rebase[1], among other git commands, can be used to create
+replacement objects from existing objects.
+
+If you want to replace many blobs, trees or commits that are part of a
+string of commits, you may just want to create a replacement string of
+commits and then only replace the commit at the tip of the target
+string of commits with the commit at the tip of the replacement string
+of commits.
+
 BUGS
 
 Comparing blobs or trees that have been replaced with those that
@@ -76,6 +89,9 @@ pending objects.
 
 SEE ALSO
 
+linkgit:git-hash-object[1]
+linkgit:git-filter-branch[1]
+linkgit:git-rebase[1]
 linkgit:git-tag[1]
 linkgit:git-branch[1]
 linkgit:git[1]
-- 
1.8.4.rc1.31.g530f5ce.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 v3 00/11] Check replacement object type and minor updates

[Christian Couder]

In this new version of the series, the only change in the 5 first
patches, is a small change in the commit message of patch 1.

Patches from 6/11 to 11/11 are all new:
- 6/11, 7/11 and 8/11 are about bypassing the type check
if -f is used
- 9/11, 10/11 and 11/11 are about adding long option names

Christian Couder (11):
  replace: forbid replacing an object with one of a different type
  Documentation/replace: state that objects must be of the same type
  t6050-replace: test that objects are of the same type
  t6050-replace: add test to clean up all the replace refs
  Documentation/replace: add Creating Replacement Objects section
  replace: bypass the type check if -f option is used
  Documentation/replace: tell that -f option bypasses the type check
  t6050-replace: check that -f option bypasses the type check
  replace: allow long option names
  Documentation/replace: list long option names
  t6050-replace: use some long option names

 Documentation/git-replace.txt | 28 +---
 builtin/replace.c | 16 +---
 t/t6050-replace.sh| 31 ---
 3 files changed, 66 insertions(+), 9 deletions(-)

-- 
1.8.4.rc1.31.g530f5ce.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 v3 01/11] replace: forbid replacing an object with one of a different type

[Christian Couder]

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

If one object is replaced with one of a different type, the only way
to keep the history valid is to also replace all the other objects
that point to the replaced object. That's because:

* Annotated tags contain the type of the tagged object.

* The tree/parent lines in commits must be a tree and commits, resp.

* The object types referred to by trees are specified in the 'mode'
  field:
100644 and 100755blob
16   commit
04   tree
  (these are the only valid modes)

* Blobs don't point at anything.

The doc will be updated in a later patch.

Acked-by: Philip Oakley philipoak...@iee.org
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..9a94769 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char 
*replace_ref,
  int force)
 {
unsigned char object[20], prev[20], repl[20];
+   enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
 
@@ -100,6 +101,15 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
if (check_refname_format(ref, 0))
die('%s' is not a valid ref name., ref);
 
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(repl, NULL);
+   if (obj_type != repl_type)
+   die(Objects must be of the same type.\n
+   '%s' points to a replaced object of type '%s'\n
+   while '%s' points to a replacement object of type '%s'.,
+   object_ref, typename(obj_type),
+   replace_ref, typename(repl_type));
+
if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
-- 
1.8.4.rc1.31.g530f5ce.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 v3 03/11] t6050-replace: test that objects are of the same type

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index decdc33..5c352c4 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -263,4 +263,17 @@ test_expect_success 'not just commits' '
test_cmp file.replaced file
 '
 
+test_expect_success 'replaced and replacement objects must be of the same 
type' '
+   test_must_fail git replace mytag $HASH1 2err 
+   grep mytag. points to a replaced object of type .tag err 
+   grep $HASH1. points to a replacement object of type .commit err 
+   test_must_fail git replace HEAD^{tree} HEAD~1 2err 
+   grep HEAD^{tree}. points to a replaced object of type .tree err 
+   grep HEAD~1. points to a replacement object of type .commit err 
+   BLOB=$(git rev-parse :file) 
+   test_must_fail git replace HEAD^ $BLOB 2err 
+   grep HEAD^. points to a replaced object of type .commit err 
+   grep $BLOB. points to a replacement object of type .blob err
+'
+
 test_done
-- 
1.8.4.rc1.31.g530f5ce.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 v3 04/11] t6050-replace: add test to clean up all the replace refs

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 5c352c4..05be228 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,4 +276,10 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
grep $BLOB. points to a replacement object of type .blob err
 '
 
+test_expect_success 'replace ref cleanup' '
+   test -n $(git replace) 
+   git replace -d $(git replace) 
+   test -z $(git replace)
+'
+
 test_done
-- 
1.8.4.rc1.31.g530f5ce.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 v3 02/11] Documentation/replace: state that objects must be of the same type

[Christian Couder]

A previous patch ensures that both the replaced and the replacement
objects passed to git replace must be of the same type.

While at it state that there is no other restriction on both objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index e0b4057..aa66d27 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -20,6 +20,9 @@ The name of the 'replace' reference is the SHA-1 of the 
object that is
 replaced. The content of the 'replace' reference is the SHA-1 of the
 replacement object.
 
+The replaced object and the replacement object must be of the same type.
+There is no other restriction on them.
+
 Unless `-f` is given, the 'replace' reference must not yet exist.
 
 Replacement references will be used by default by all Git commands
@@ -69,9 +72,7 @@ go back to a replaced commit will move the branch to the 
replacement
 commit instead of the replaced commit.
 
 There may be other problems when using 'git rev-list' related to
-pending objects. And of course things may break if an object of one
-type is replaced by an object of another type (for example a blob
-replaced by a commit).
+pending objects.
 
 SEE ALSO
 
-- 
1.8.4.rc1.31.g530f5ce.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 v3 06/11] replace: bypass the type check if -f option is used

[Christian Couder]

If -f option, which means '--force', is used, we can allow an object
to be replaced with one of a different type, as the user should know
what (s)he is doing.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 9a94769..95736d9 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -103,7 +103,7 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
 
obj_type = sha1_object_info(object, NULL);
repl_type = sha1_object_info(repl, NULL);
-   if (obj_type != repl_type)
+   if (!force  obj_type != repl_type)
die(Objects must be of the same type.\n
'%s' points to a replaced object of type '%s'\n
while '%s' points to a replacement object of type '%s'.,
-- 
1.8.4.rc1.31.g530f5ce.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 v3 08/11] t6050-replace: check that -f option bypasses the type check

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 05be228..0b07a0b 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,6 +276,12 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
grep $BLOB. points to a replacement object of type .blob err
 '
 
+test_expect_success '-f option bypasses the type check' '
+   git replace -f mytag $HASH1 2err 
+   git replace -f HEAD^{tree} HEAD~1 2err 
+   git replace -f HEAD^ $BLOB 2err
+'
+
 test_expect_success 'replace ref cleanup' '
test -n $(git replace) 
git replace -d $(git replace) 
-- 
1.8.4.rc1.31.g530f5ce.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 v3 10/11] Documentation/replace: list long option names

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index a2bd2ee..414000e 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -54,13 +54,16 @@ achieve the same effect as the `--no-replace-objects` 
option.
 OPTIONS
 ---
 -f::
+--force::
If an existing replace ref for the same object exists, it will
be overwritten (instead of failing).
 
 -d::
+--delete::
Delete existing replace refs for the given objects.
 
 -l pattern::
+--list pattern::
List replace refs for objects that match the given pattern (or
all if no pattern is given).
Typing git replace without arguments, also lists all replace
-- 
1.8.4.rc1.31.g530f5ce.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 v3 11/11] t6050-replace: use some long option names

[Christian Couder]

So that they are tested a litlle bit too.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 0b07a0b..5dc26e8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -122,9 +122,9 @@ test_expect_success 'git replace listing and deleting' '
  test $HASH2 = $(git replace -l) 
  test $HASH2 = $(git replace) 
  aa=${HASH2%??} 
- test $HASH2 = $(git replace -l $aa*) 
+ test $HASH2 = $(git replace --list $aa*) 
  test_must_fail git replace -d $R 
- test_must_fail git replace -d 
+ test_must_fail git replace --delete 
  test_must_fail git replace -l -d $HASH2 
  git replace -d $HASH2 
  git show $HASH2 | grep A U Thor 
@@ -147,7 +147,7 @@ test_expect_success 'git replace resolves sha1' '
  git show $HASH2 | grep O Thor 
  test_must_fail git replace $HASH2 $R 
  git replace -f $HASH2 $R 
- test_must_fail git replace -f 
+ test_must_fail git replace --force 
  test $HASH2 = $(git replace)
 '
 
@@ -278,7 +278,7 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
 
 test_expect_success '-f option bypasses the type check' '
git replace -f mytag $HASH1 2err 
-   git replace -f HEAD^{tree} HEAD~1 2err 
+   git replace --force HEAD^{tree} HEAD~1 2err 
git replace -f HEAD^ $BLOB 2err
 '
 
-- 
1.8.4.rc1.31.g530f5ce.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 v3 09/11] replace: allow long option names

[Christian Couder]

It is now standard practice in Git to have both short and long option
names. So let's give a long option name to the git replace options too.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 95736d9..d4d1b75 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -128,9 +128,9 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
 {
int list = 0, delete = 0, force = 0;
struct option options[] = {
-   OPT_BOOLEAN('l', NULL, list, N_(list replace refs)),
-   OPT_BOOLEAN('d', NULL, delete, N_(delete replace refs)),
-   OPT_BOOLEAN('f', NULL, force, N_(replace the ref if it 
exists)),
+   OPT_BOOLEAN('l', list, list, N_(list replace refs)),
+   OPT_BOOLEAN('d', delete, delete, N_(delete replace refs)),
+   OPT_BOOLEAN('f', force, force, N_(replace the ref if it 
exists)),
OPT_END()
};
 
-- 
1.8.4.rc1.31.g530f5ce.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 v3 07/11] Documentation/replace: tell that -f option bypasses the type check

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 736b48c..a2bd2ee 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -21,10 +21,12 @@ replaced. The content of the 'replace' reference is the 
SHA-1 of the
 replacement object.
 
 The replaced object and the replacement object must be of the same type.
-There is no other restriction on them.
+This restriction can be bypassed using `-f`.
 
 Unless `-f` is given, the 'replace' reference must not yet exist.
 
+There is no other restriction on the replaced and replacement objects.
+
 Replacement references will be used by default by all Git commands
 except those doing reachability traversal (prune, pack transfer and
 fsck).
-- 
1.8.4.rc1.31.g530f5ce.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 08/11] t6050-replace: check that -f option bypasses the type check

[Christian Couder]

From: Eric Sunshine sunsh...@sunshineco.com

 On Sat, Aug 31, 2013 at 3:12 PM, Christian Couder
 chrisc...@tuxfamily.org wrote:
 Signed-off-by: Christian Couder chrisc...@tuxfamily.org
 ---
  t/t6050-replace.sh | 6 ++
  1 file changed, 6 insertions(+)

 diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
 index 05be228..0b07a0b 100755
 --- a/t/t6050-replace.sh
 +++ b/t/t6050-replace.sh
 @@ -276,6 +276,12 @@ test_expect_success 'replaced and replacement objects 
 must be of the same type'
 grep $BLOB. points to a replacement object of type .blob err
  '

 +test_expect_success '-f option bypasses the type check' '
 +   git replace -f mytag $HASH1 2err 
 +   git replace -f HEAD^{tree} HEAD~1 2err 
 +   git replace -f HEAD^ $BLOB 2err
 +'
 
 Is there a non-obvious reason you are redirecting stderr to a file in
 this test? Unlike the test added earlier, this one never consults the
 error output. By dropping this apparently unnecessary redirection,
 diagnosis of a regression potentially becomes simpler since any error
 output from git-replace will become visible when the test is run
 verbosely.

You are right! I will drop these redirections.

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 11/11] t6050-replace: use some long option names

[Christian Couder]

From: Philip Oakley philipoak...@iee.org

 So that they are tested a litlle bit too.
 s /litlle/little/

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 05/11] Documentation/replace: add Creating Replacement Objects section

[Christian Couder]

From: Philip Oakley philipoak...@iee.org

 From: Christian Couder chrisc...@tuxfamily.org

 +CREATING REPLACEMENT OBJECTS
 +
 +
 +linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and
 +linkgit:git-rebase[1],
 
 Let's not forget the obvious 'git commit' or 'git merge' on a
 temporary branch for creating a replacement commit.

As it is obvious, and as it is somehow addressed in the below part of
this section, I don't think it is worth talking about git commit or
git merge or git cherry-pick or any other command.
 
 In particular we need to have covered the alternate to a graft of A B
 C (i.e. A is now a merge of B  C) if we are to deprecate grafts with
 any conviction. (https://git.wiki.kernel.org/index.php/GraftPoint)

Adding such an example in a new EXAMPLE section would address this
better. If people agree I will do it in a following patch.

 among other git commands, can be used to create
 +replacement objects from existing objects.
 +
 +If you want to replace many blobs, trees or commits that are part of
 a
 +string of commits, you may just want to create a replacement string
 of
 +commits and then only replace the commit at the tip of the target
 +string of commits with the commit at the tip of the replacement
 string
 +of commits.

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 07/11] Documentation/replace: tell that -f option bypasses the type check

[Christian Couder]

From: Philip Oakley philipoak...@iee.org

 From: Christian Couder chrisc...@tuxfamily.org

 The replaced object and the replacement object must be of the same
 type.
 -There is no other restriction on them.
 +This restriction can be bypassed using `-f`.

 Unless `-f` is given, the 'replace' reference must not yet exist.

 +There is no other restriction on the replaced and replacement
 objects.
 
 Is this trying to allude to the fact that merge commits may be
 exchanged with non-merge commits? I strongly believe that this ability
 to exchange merge and non-merge commits should be stated _explicitly_
 to counteract the false beliefs that are listed out on the internet.

Maybe we can show that in an example. But I think the patch is quite
clear as it is and should be enough.

If we really want to correct some false beliefs, the best would be to
state the truth where those false beliefs are stated.

 It's probably better stated in a separate patch for that explicit
 purpose to avoid mixed messages within this commit.

If people agree, I will add a another patch with an example in an
EXAMPLE section.

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 01/11] replace: forbid replacing an object with one of a different type

[Christian Couder]

From: Philip Oakley philipoak...@iee.org

 Sorry for not replying earlier in the series.
 
 From: Christian Couder chrisc...@tuxfamily.org
 Users replacing an object with one of a different type were not
 prevented to do so, even if it was obvious, and stated in the doc,
 that bad things would result from doing that.

 To avoid mistakes, it is better to just forbid that though.

 If one object is replaced with one of a different type, the only way
 to keep the history valid is to also replace all the other objects
 that point to the replaced object.
 
 Isn't this a recursion problem? Taken in that order one unravels the
 whole DAG.
 
 However if considered in the reverse direction, one can replace an
 existing object within the DAG with a carefully crafted alternative of
 the same type, but which then wrongly references other dangling
 objects which are then replaced by objects which have the right type
 (this last replacement requires -f force).

I am not sure I understand what you are saying.

Anyway in a previous version of this patch I tried to be more explicit
about this, but Junio basically said that he found no value in
discussing this more explicitely...

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 00/11] Check replacement object type and minor updates

[Christian Couder]

The only changes in this new version are:

- removed useless redirections in 8/11
- improved commit message in 11/11 (s/litlle/little/)

I may add examples and more in a new patch series later.

Christian Couder (11):
  replace: forbid replacing an object with one of a different type
  Documentation/replace: state that objects must be of the same type
  t6050-replace: test that objects are of the same type
  t6050-replace: add test to clean up all the replace refs
  Documentation/replace: add Creating Replacement Objects section
  replace: bypass the type check if -f option is used
  Documentation/replace: tell that -f option bypasses the type check
  t6050-replace: check that -f option bypasses the type check
  replace: allow long option names
  Documentation/replace: list long option names
  t6050-replace: use some long option names

 Documentation/git-replace.txt | 28 +---
 builtin/replace.c | 16 +---
 t/t6050-replace.sh| 31 ---
 3 files changed, 66 insertions(+), 9 deletions(-)

-- 
1.8.4.rc1.31.g530f5ce.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 v4 01/11] replace: forbid replacing an object with one of a different type

[Christian Couder]

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

If one object is replaced with one of a different type, the only way
to keep the history valid is to also replace all the other objects
that point to the replaced object. That's because:

* Annotated tags contain the type of the tagged object.

* The tree/parent lines in commits must be a tree and commits, resp.

* The object types referred to by trees are specified in the 'mode'
  field:
100644 and 100755blob
16   commit
04   tree
  (these are the only valid modes)

* Blobs don't point at anything.

The doc will be updated in a later patch.

Acked-by: Philip Oakley philipoak...@iee.org
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..9a94769 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char 
*replace_ref,
  int force)
 {
unsigned char object[20], prev[20], repl[20];
+   enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
 
@@ -100,6 +101,15 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
if (check_refname_format(ref, 0))
die('%s' is not a valid ref name., ref);
 
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(repl, NULL);
+   if (obj_type != repl_type)
+   die(Objects must be of the same type.\n
+   '%s' points to a replaced object of type '%s'\n
+   while '%s' points to a replacement object of type '%s'.,
+   object_ref, typename(obj_type),
+   replace_ref, typename(repl_type));
+
if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
-- 
1.8.4.rc1.31.g530f5ce.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 v4 02/11] Documentation/replace: state that objects must be of the same type

[Christian Couder]

A previous patch ensures that both the replaced and the replacement
objects passed to git replace must be of the same type.

While at it state that there is no other restriction on both objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index e0b4057..aa66d27 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -20,6 +20,9 @@ The name of the 'replace' reference is the SHA-1 of the 
object that is
 replaced. The content of the 'replace' reference is the SHA-1 of the
 replacement object.
 
+The replaced object and the replacement object must be of the same type.
+There is no other restriction on them.
+
 Unless `-f` is given, the 'replace' reference must not yet exist.
 
 Replacement references will be used by default by all Git commands
@@ -69,9 +72,7 @@ go back to a replaced commit will move the branch to the 
replacement
 commit instead of the replaced commit.
 
 There may be other problems when using 'git rev-list' related to
-pending objects. And of course things may break if an object of one
-type is replaced by an object of another type (for example a blob
-replaced by a commit).
+pending objects.
 
 SEE ALSO
 
-- 
1.8.4.rc1.31.g530f5ce.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 v4 08/11] t6050-replace: check that -f option bypasses the type check

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 05be228..622b751 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,6 +276,12 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
grep $BLOB. points to a replacement object of type .blob err
 '
 
+test_expect_success '-f option bypasses the type check' '
+   git replace -f mytag $HASH1 
+   git replace -f HEAD^{tree} HEAD~1 
+   git replace -f HEAD^ $BLOB
+'
+
 test_expect_success 'replace ref cleanup' '
test -n $(git replace) 
git replace -d $(git replace) 
-- 
1.8.4.rc1.31.g530f5ce.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 v4 03/11] t6050-replace: test that objects are of the same type

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index decdc33..5c352c4 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -263,4 +263,17 @@ test_expect_success 'not just commits' '
test_cmp file.replaced file
 '
 
+test_expect_success 'replaced and replacement objects must be of the same 
type' '
+   test_must_fail git replace mytag $HASH1 2err 
+   grep mytag. points to a replaced object of type .tag err 
+   grep $HASH1. points to a replacement object of type .commit err 
+   test_must_fail git replace HEAD^{tree} HEAD~1 2err 
+   grep HEAD^{tree}. points to a replaced object of type .tree err 
+   grep HEAD~1. points to a replacement object of type .commit err 
+   BLOB=$(git rev-parse :file) 
+   test_must_fail git replace HEAD^ $BLOB 2err 
+   grep HEAD^. points to a replaced object of type .commit err 
+   grep $BLOB. points to a replacement object of type .blob err
+'
+
 test_done
-- 
1.8.4.rc1.31.g530f5ce.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 v4 09/11] replace: allow long option names

[Christian Couder]

It is now standard practice in Git to have both short and long option
names. So let's give a long option name to the git replace options too.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 95736d9..d4d1b75 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -128,9 +128,9 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
 {
int list = 0, delete = 0, force = 0;
struct option options[] = {
-   OPT_BOOLEAN('l', NULL, list, N_(list replace refs)),
-   OPT_BOOLEAN('d', NULL, delete, N_(delete replace refs)),
-   OPT_BOOLEAN('f', NULL, force, N_(replace the ref if it 
exists)),
+   OPT_BOOLEAN('l', list, list, N_(list replace refs)),
+   OPT_BOOLEAN('d', delete, delete, N_(delete replace refs)),
+   OPT_BOOLEAN('f', force, force, N_(replace the ref if it 
exists)),
OPT_END()
};
 
-- 
1.8.4.rc1.31.g530f5ce.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 v4 06/11] replace: bypass the type check if -f option is used

[Christian Couder]

If -f option, which means '--force', is used, we can allow an object
to be replaced with one of a different type, as the user should know
what (s)he is doing.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 9a94769..95736d9 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -103,7 +103,7 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
 
obj_type = sha1_object_info(object, NULL);
repl_type = sha1_object_info(repl, NULL);
-   if (obj_type != repl_type)
+   if (!force  obj_type != repl_type)
die(Objects must be of the same type.\n
'%s' points to a replaced object of type '%s'\n
while '%s' points to a replacement object of type '%s'.,
-- 
1.8.4.rc1.31.g530f5ce.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 v4 11/11] t6050-replace: use some long option names

[Christian Couder]

So that they are tested a little bit too.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 622b751..1fe5c1b 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -122,9 +122,9 @@ test_expect_success 'git replace listing and deleting' '
  test $HASH2 = $(git replace -l) 
  test $HASH2 = $(git replace) 
  aa=${HASH2%??} 
- test $HASH2 = $(git replace -l $aa*) 
+ test $HASH2 = $(git replace --list $aa*) 
  test_must_fail git replace -d $R 
- test_must_fail git replace -d 
+ test_must_fail git replace --delete 
  test_must_fail git replace -l -d $HASH2 
  git replace -d $HASH2 
  git show $HASH2 | grep A U Thor 
@@ -147,7 +147,7 @@ test_expect_success 'git replace resolves sha1' '
  git show $HASH2 | grep O Thor 
  test_must_fail git replace $HASH2 $R 
  git replace -f $HASH2 $R 
- test_must_fail git replace -f 
+ test_must_fail git replace --force 
  test $HASH2 = $(git replace)
 '
 
@@ -278,7 +278,7 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
 
 test_expect_success '-f option bypasses the type check' '
git replace -f mytag $HASH1 
-   git replace -f HEAD^{tree} HEAD~1 
+   git replace --force HEAD^{tree} HEAD~1 
git replace -f HEAD^ $BLOB
 '
 
-- 
1.8.4.rc1.31.g530f5ce.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 v4 10/11] Documentation/replace: list long option names

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index a2bd2ee..414000e 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -54,13 +54,16 @@ achieve the same effect as the `--no-replace-objects` 
option.
 OPTIONS
 ---
 -f::
+--force::
If an existing replace ref for the same object exists, it will
be overwritten (instead of failing).
 
 -d::
+--delete::
Delete existing replace refs for the given objects.
 
 -l pattern::
+--list pattern::
List replace refs for objects that match the given pattern (or
all if no pattern is given).
Typing git replace without arguments, also lists all replace
-- 
1.8.4.rc1.31.g530f5ce.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 v4 07/11] Documentation/replace: tell that -f option bypasses the type check

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 736b48c..a2bd2ee 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -21,10 +21,12 @@ replaced. The content of the 'replace' reference is the 
SHA-1 of the
 replacement object.
 
 The replaced object and the replacement object must be of the same type.
-There is no other restriction on them.
+This restriction can be bypassed using `-f`.
 
 Unless `-f` is given, the 'replace' reference must not yet exist.
 
+There is no other restriction on the replaced and replacement objects.
+
 Replacement references will be used by default by all Git commands
 except those doing reachability traversal (prune, pack transfer and
 fsck).
-- 
1.8.4.rc1.31.g530f5ce.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 v4 05/11] Documentation/replace: add Creating Replacement Objects section

[Christian Couder]

There were no hints in the documentation about how to create
replacement objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index aa66d27..736b48c 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -64,6 +64,19 @@ OPTIONS
Typing git replace without arguments, also lists all replace
refs.
 
+CREATING REPLACEMENT OBJECTS
+
+
+linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and
+linkgit:git-rebase[1], among other git commands, can be used to create
+replacement objects from existing objects.
+
+If you want to replace many blobs, trees or commits that are part of a
+string of commits, you may just want to create a replacement string of
+commits and then only replace the commit at the tip of the target
+string of commits with the commit at the tip of the replacement string
+of commits.
+
 BUGS
 
 Comparing blobs or trees that have been replaced with those that
@@ -76,6 +89,9 @@ pending objects.
 
 SEE ALSO
 
+linkgit:git-hash-object[1]
+linkgit:git-filter-branch[1]
+linkgit:git-rebase[1]
 linkgit:git-tag[1]
 linkgit:git-branch[1]
 linkgit:git[1]
-- 
1.8.4.rc1.31.g530f5ce.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 v4 04/11] t6050-replace: add test to clean up all the replace refs

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 5c352c4..05be228 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,4 +276,10 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
grep $BLOB. points to a replacement object of type .blob err
 '
 
+test_expect_success 'replace ref cleanup' '
+   test -n $(git replace) 
+   git replace -d $(git replace) 
+   test -z $(git replace)
+'
+
 test_done
-- 
1.8.4.rc1.31.g530f5ce.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 07/11] Documentation/replace: tell that -f option bypasses the type check

[Christian Couder]

On Mon, Sep 2, 2013 at 11:50 PM, Philip Oakley philipoak...@iee.org wrote:

 From: Christian Couder chrisc...@tuxfamily.org

 You mean something like the following:

 $ cat  ./graft2replace.sh
 #!/bin/bash

 while read orig parents
 do
printf %s git cat-file commit $orig
printf %s  | perl -n -e 'print unless /^parent /'
insn=''
for commit in $parents; do insn=$insn print \parent
 $commit\\n\;; done
printf %s  | perl -n -e 'print; if (/^tree /) { $insn }'
printf %s\n   new_commit.txt
printf %s\n 'REPL=$(git hash-object -t commit -w new_commit.txt)'


 Does `hash-object` do the inverese of `cat-file commit`?

 I didn't find the hash-object(1) man page very informative on that matter
 and a (very) quick look at its code made me think it was just hashing the
 raw contents which wouldn't be what what was wanted.

I agree with Jonathan's suggest to add an EXAMPLE section in
hash-object(1) manpage and maybe a new gitobject(5) manpage too.

You can also find a few examples of how git hash-object can be used in
t/t6050-replace.sh:

For example:

 R=$(git cat-file commit $HASH2 | sed -e s/A U/O/ | git
hash-object -t commit --stdin -w) 
 git cat-file commit $R | grep author O Thor 
 git update-ref refs/replace/$HASH2 $R

printf %s\n git replace $orig \$REPL
 done

 This generates shell instructions from a graft file. Then you only need to
 execute the generated shell instructions.
 For example:

 $ cat graft_file.txt
 5bf34fff3186254d7254583675d10ddf98df989b
 79fe155489351e8af829a3106e7150397c57d863
 dcfbab6bea3df3166503f3084cec2679f10f9e80
 fb5657082148297b61fbca7e64d51c1e7870309a

 $ cat graft_file.txt | ./graft2replace.sh
 git cat-file commit 5bf34fff3186254d7254583675d10ddf98df989b | perl -n -e
 'print unless /^parent /' | perl -n -e 'print; if (/^tree /) {  print
 parent 79fe155489351e8af829a3106e7150397c57d863\n; print parent
 dcfbab6bea3df3166503f3084cec2679f10f9e80\n; }'  new_commit.txt
 REPL=$(git hash-object -t commit -w new_commit.txt)
 git replace 5bf34fff3186254d7254583675d10ddf98df989b $REPL
 git cat-file commit fb5657082148297b61fbca7e64d51c1e7870309a | perl -n -e
 'print unless /^parent /' | perl -n -e 'print; if (/^tree /) {  }' 
 new_commit.txt
 REPL=$(git hash-object -t commit -w new_commit.txt)
 git replace fb5657082148297b61fbca7e64d51c1e7870309a $REPL

 Note that I haven't really tested it.

Also note that it is obviously broken if you have commits with a
commit message that has lines starting with 'parent ' or 'tree '.

 I think we could call it 'git-graft', being the help function/script that
 converts raw grafts to proper object replacements ;-)

I will have a look at improving it, testing it and sending a patch to
put it in contrib/.

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 v4 06/11] replace: bypass the type check if -f option is used

[Christian Couder]

On Wed, Sep 4, 2013 at 10:44 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder chrisc...@tuxfamily.org writes:

 If -f option, which means '--force', is used, we can allow an object
 to be replaced with one of a different type, as the user should know
 what (s)he is doing.

 Signed-off-by: Christian Couder chrisc...@tuxfamily.org
 ---

 This does not matter in a larger picture, but between 1/11 and this
 patch, there is a window where an operation that has been useful in
 some workflows becomes unavailable to the user.

 For future reference, it would be better to do this as a part of
 1/11, to make sure that there always is an escape hatch available to
 the users.

Ok, I will squash patchs 6/11, 7/11 and 8/11 with 1/11, 2/11 and 3/11
respectively.

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 v4 10/11] Documentation/replace: list long option names

[Christian Couder]

On Wed, Sep 4, 2013 at 10:45 PM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder chrisc...@tuxfamily.org writes:

 Signed-off-by: Christian Couder chrisc...@tuxfamily.org
 ---
  Documentation/git-replace.txt | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
 index a2bd2ee..414000e 100644
 --- a/Documentation/git-replace.txt
 +++ b/Documentation/git-replace.txt
 @@ -54,13 +54,16 @@ achieve the same effect as the `--no-replace-objects` 
 option.
  OPTIONS
  ---
  -f::
 +--force::
   If an existing replace ref for the same object exists, it will
   be overwritten (instead of failing).

  -d::
 +--delete::
   Delete existing replace refs for the given objects.

  -l pattern::
 +--list pattern::
   List replace refs for objects that match the given pattern (or
   all if no pattern is given).
   Typing git replace without arguments, also lists all replace

 This should be in the same commit as what adds them.

Ok, I will squash 10/11 into 9/11.

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 v4 03/11] t6050-replace: test that objects are of the same type

[Christian Couder]

From: Junio C Hamano gits...@pobox.com

 Christian Couder chrisc...@tuxfamily.org writes:
  
 +test_expect_success 'replaced and replacement objects must be of the same 
 type' '
 +test_must_fail git replace mytag $HASH1 2err 
 +grep mytag. points to a replaced object of type .tag err 
 +grep $HASH1. points to a replacement object of type .commit err 
 
 Hmm, would these messages ever get translated?  I think it is
 sufficient to make sure that the proposed replacement fails for
 these cases.

Ok, I will get rid of the grep statements.

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 0/7] Check replacement object type and minor updates

[Christian Couder]

Many patchs have been squashed together as Junio suggested.
And in patch 3/7 now no grep is done on the error message.

Christian Couder (7):
  replace: forbid replacing an object with one of a different type
  Documentation/replace: state that objects must be of the same type
  t6050-replace: test that objects are of the same type
  t6050-replace: add test to clean up all the replace refs
  Documentation/replace: add Creating Replacement Objects section
  replace: allow long option names
  t6050-replace: use some long option names

 Documentation/git-replace.txt | 28 +---
 builtin/replace.c | 16 +---
 t/t6050-replace.sh| 25 ++---
 3 files changed, 60 insertions(+), 9 deletions(-)

-- 
1.8.4.rc1.28.ge2684af

--
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] t6050-replace: add test to clean up all the replace refs

[Christian Couder]

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 09bad98..09a2b49 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,4 +276,10 @@ test_expect_success '-f option bypasses the type check' '
git replace -f HEAD^ $BLOB
 '
 
+test_expect_success 'replace ref cleanup' '
+   test -n $(git replace) 
+   git replace -d $(git replace) 
+   test -z $(git replace)
+'
+
 test_done
-- 
1.8.4.rc1.28.ge2684af


--
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: allow long option names

[Christian Couder]

It is now standard practice in Git to have both short and long option
names. So let's give a long option name to the git replace options too.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 3 +++
 builtin/replace.c | 6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index a2bd2ee..414000e 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -54,13 +54,16 @@ achieve the same effect as the `--no-replace-objects` 
option.
 OPTIONS
 ---
 -f::
+--force::
If an existing replace ref for the same object exists, it will
be overwritten (instead of failing).
 
 -d::
+--delete::
Delete existing replace refs for the given objects.
 
 -l pattern::
+--list pattern::
List replace refs for objects that match the given pattern (or
all if no pattern is given).
Typing git replace without arguments, also lists all replace
diff --git a/builtin/replace.c b/builtin/replace.c
index 95736d9..d4d1b75 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -128,9 +128,9 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
 {
int list = 0, delete = 0, force = 0;
struct option options[] = {
-   OPT_BOOLEAN('l', NULL, list, N_(list replace refs)),
-   OPT_BOOLEAN('d', NULL, delete, N_(delete replace refs)),
-   OPT_BOOLEAN('f', NULL, force, N_(replace the ref if it 
exists)),
+   OPT_BOOLEAN('l', list, list, N_(list replace refs)),
+   OPT_BOOLEAN('d', delete, delete, N_(delete replace refs)),
+   OPT_BOOLEAN('f', force, force, N_(replace the ref if it 
exists)),
OPT_END()
};
 
-- 
1.8.4.rc1.28.ge2684af


--
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] Documentation/replace: add Creating Replacement Objects section

[Christian Couder]

There were no hints in the documentation about how to create
replacement objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index d198006..a2bd2ee 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -66,6 +66,19 @@ OPTIONS
Typing git replace without arguments, also lists all replace
refs.
 
+CREATING REPLACEMENT OBJECTS
+
+
+linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and
+linkgit:git-rebase[1], among other git commands, can be used to create
+replacement objects from existing objects.
+
+If you want to replace many blobs, trees or commits that are part of a
+string of commits, you may just want to create a replacement string of
+commits and then only replace the commit at the tip of the target
+string of commits with the commit at the tip of the replacement string
+of commits.
+
 BUGS
 
 Comparing blobs or trees that have been replaced with those that
@@ -78,6 +91,9 @@ pending objects.
 
 SEE ALSO
 
+linkgit:git-hash-object[1]
+linkgit:git-filter-branch[1]
+linkgit:git-rebase[1]
 linkgit:git-tag[1]
 linkgit:git-branch[1]
 linkgit:git[1]
-- 
1.8.4.rc1.28.ge2684af


--
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] t6050-replace: use some long option names

[Christian Couder]

So that they are tested a little bit too.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 09a2b49..7d47984 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -122,9 +122,9 @@ test_expect_success 'git replace listing and deleting' '
  test $HASH2 = $(git replace -l) 
  test $HASH2 = $(git replace) 
  aa=${HASH2%??} 
- test $HASH2 = $(git replace -l $aa*) 
+ test $HASH2 = $(git replace --list $aa*) 
  test_must_fail git replace -d $R 
- test_must_fail git replace -d 
+ test_must_fail git replace --delete 
  test_must_fail git replace -l -d $HASH2 
  git replace -d $HASH2 
  git show $HASH2 | grep A U Thor 
@@ -147,7 +147,7 @@ test_expect_success 'git replace resolves sha1' '
  git show $HASH2 | grep O Thor 
  test_must_fail git replace $HASH2 $R 
  git replace -f $HASH2 $R 
- test_must_fail git replace -f 
+ test_must_fail git replace --force 
  test $HASH2 = $(git replace)
 '
 
@@ -272,7 +272,7 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
 
 test_expect_success '-f option bypasses the type check' '
git replace -f mytag $HASH1 
-   git replace -f HEAD^{tree} HEAD~1 
+   git replace --force HEAD^{tree} HEAD~1 
git replace -f HEAD^ $BLOB
 '
 
-- 
1.8.4.rc1.28.ge2684af

--
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 2/7] Documentation/replace: state that objects must be of the same type

[Christian Couder]

A previous patch ensures that both the replaced and the replacement
objects passed to git replace must be of the same type, except if
-f option is used.

While at it state that there is no other restriction on both objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index e0b4057..d198006 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -20,8 +20,13 @@ The name of the 'replace' reference is the SHA-1 of the 
object that is
 replaced. The content of the 'replace' reference is the SHA-1 of the
 replacement object.
 
+The replaced object and the replacement object must be of the same type.
+This restriction can be bypassed using `-f`.
+
 Unless `-f` is given, the 'replace' reference must not yet exist.
 
+There is no other restriction on the replaced and replacement objects.
+
 Replacement references will be used by default by all Git commands
 except those doing reachability traversal (prune, pack transfer and
 fsck).
@@ -69,9 +74,7 @@ go back to a replaced commit will move the branch to the 
replacement
 commit instead of the replaced commit.
 
 There may be other problems when using 'git rev-list' related to
-pending objects. And of course things may break if an object of one
-type is replaced by an object of another type (for example a blob
-replaced by a commit).
+pending objects.
 
 SEE ALSO
 
-- 
1.8.4.rc1.28.ge2684af


--
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: forbid replacing an object with one of a different type

[Christian Couder]

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

If -f option, which means '--force', is used, we can allow an object
to be replaced with one of a different type, as the user should know
what (s)he is doing.

If one object is replaced with one of a different type, the only way
to keep the history valid is to also replace all the other objects
that point to the replaced object. That's because:

* Annotated tags contain the type of the tagged object.

* The tree/parent lines in commits must be a tree and commits, resp.

* The object types referred to by trees are specified in the 'mode'
  field:
100644 and 100755blob
16   commit
04   tree
  (these are the only valid modes)

* Blobs don't point at anything.

The doc will be updated in a later patch.

Acked-by: Philip Oakley philipoak...@iee.org
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..95736d9 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char 
*replace_ref,
  int force)
 {
unsigned char object[20], prev[20], repl[20];
+   enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
 
@@ -100,6 +101,15 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
if (check_refname_format(ref, 0))
die('%s' is not a valid ref name., ref);
 
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(repl, NULL);
+   if (!force  obj_type != repl_type)
+   die(Objects must be of the same type.\n
+   '%s' points to a replaced object of type '%s'\n
+   while '%s' points to a replacement object of type '%s'.,
+   object_ref, typename(obj_type),
+   replace_ref, typename(repl_type));
+
if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
-- 
1.8.4.rc1.28.ge2684af


--
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] t6050-replace: test that objects are of the same type

[Christian Couder]

and that the -f option bypasses the type check

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index decdc33..09bad98 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -263,4 +263,17 @@ test_expect_success 'not just commits' '
test_cmp file.replaced file
 '
 
+test_expect_success 'replaced and replacement objects must be of the same 
type' '
+   test_must_fail git replace mytag $HASH1 
+   test_must_fail git replace HEAD^{tree} HEAD~1 
+   BLOB=$(git rev-parse :file) 
+   test_must_fail git replace HEAD^ $BLOB
+'
+
+test_expect_success '-f option bypasses the type check' '
+   git replace -f mytag $HASH1 
+   git replace -f HEAD^{tree} HEAD~1 
+   git replace -f HEAD^ $BLOB
+'
+
 test_done
-- 
1.8.4.rc1.28.ge2684af


--
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: Using git-replace in place of grafts -- and publishing .git/refs/replace between repos?

[Christian Couder]

Hi,

On Sat, Sep 15, 2012 at 11:49 PM, David Chanters
david.chant...@googlemail.com wrote:
 Hi,

 On 15 September 2012 18:21, Junio C Hamano gits...@pobox.com wrote:

 Assuming that they do, pushing the replacement ref makes the
 replacing object available in the pushed-into repository, so
 they will *not* rely on your repository.

 This makes sense.  But it is more the mechanics of what happens with
 needing to update the fetch line for the remote in .git/config I am
 more puzzled by.

 For example, if I have two repos -- repoA and repoB, where repoA
 contains the replace refs for repoB -- if I clone both repos with the
 intent of wanting to look at the two histories, what must I do in
 repoA to fetch the replace refs in the first place?

 I've tried:

 [remote origin]
 fetch =
 +refs/replace/*:+refs/heads/*:refs/remotes/origin/*:refs/replace/*

Could you try the following:

[remote origin]
fetch = +refs/replace/*:refs/replace/*
fetch = +refs/heads/*:refs/remotes/origin/*


 But this results in:

 % git pull
 fatal: Invalid refspec
 '+refs/replace/*:+refs/heads/*:refs/remotes/origin/*:refs/replace/*'

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: The GitTogether

[Christian Couder]

Hi,

On Thu, Sep 20, 2012 at 8:53 PM, Sebastian Schuberth
sschube...@gmail.com wrote:
 On 19.09.2012 15:43, Michael Haggerty wrote:

 Is there any news about the proposed gatherings?  I would be quite
 interested in attending the developer meeting.  October is just around
 the corner...what's up?


 I'm also very much interested in attending a gathering Berlin, though
 preferably not in the first week of October. As I'm a local, I could
 probably also help with finding a location if necessary.

In this thread:

https://groups.google.com/forum/?fromgroups=#!topic/repo-discuss/_MCrS4FjZak

there is a message by Shawn with the following:

---

On Sun, Sep 2, 2012 at 12:27 PM, Luca Milanesio
luca.mi...@gmail.com wrote:

 Does anybody have info about the next GitTogether 2012 ?

Google is not hosting GitTogether this year. There may be a Git
gathering in the EU sometime early next year, organized by GitHub.

 (candidate dates ? place ? # of days ?)

 We should plan flights, (and possibly hackathons) in advance in order to get 
 maximum availability :-)

Instead... Google is looking into hosting a 2 day Gerrit user
conference followed immediately by a Gerrit developer hackathon in
early November. Nothing concrete yet, but we are working on it, and
trying to make sure we have enough lead time for people to book an
affordable flight.

---

It is sad that people who know what is or what is not happening are
not taking care of letting people on this list know about it...

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: The GitTogether

[Christian Couder]

On Fri, Sep 21, 2012 at 4:05 PM, Shawn Pearce spea...@spearce.org wrote:
 On Fri, Sep 21, 2012 at 2:20 AM, Christian Couder

 It is sad that people who know what is or what is not happening are
 not taking care of letting people on this list know about it...

 I did not post to this mailing list about the Gerrit Code Review user
 summit because I did not consider it to be on-topic to this list. We
 do not normally discuss Gerrit Code Review here. Most users and
 developers on this list only work on git-core (aka git.git aka the
 thing Junio maintains). Gerrit... is a different animal. :-)

It would have been nice if you had said earlier on this list/thread
that Google chose to host a Gerrit user summit instead of the
traditional GitTogether.

 If you are interested in attending, it is Saturday November 10th and
 Sunday 11th in Mountain View, CA. The user summit is invite only, but
 you may request an invitation at http://goo.gl/5HYlB.

Thanks for the information. I think it is indeed interesting to know about it.

 I have no further information about the potential GitTogether than
 anyone else. IIRC there is a suggestion in this thread about hosting
 something in the EU sometime in early next year, with someone at
 GitHub acting as organizer.

Before I posted what you wrote on the Gerrit mailing list, the only
information people had on this list/thread was about a GitHub proposal
to organize 2 different GitTogether: the developer-centric one in
Berlin
in early October (a few weeks before the Mentor Summit this time) and
the user one in January or February of next year.

 Google chose to run only a Gerrit user summit this year because of the
 mix of attendees at the last GitTogether. The group was about 60-70%
 Gerrit users/admins. We felt it was time to host something specific
 for that audience.

Gerrit users/admins are probably Git users/admins too. But anyway, it
is ok of course for Google to organize whatever it prefers.
I hope GitHub will do as good a job running a GitTogether as Google did.

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: Option for git bisect run to automatically clean up

[Christian Couder]

Hi,

On Thu, Sep 27, 2012 at 6:55 PM, Laszlo Papp lp...@kde.org wrote:
 Hi everybody,

 I have just run into a problem when I had to issue an explicit cleanup for
 tracked files after a configure run in the Qt5 project. I have tried to
 suggest to the people to bring up this idea on the mailing list in order to
 get this further on. Unfortunately I did not have time to do so, especially
 for the follow-up. I have also been told it is not a good way of asking on
 IRC which surprised me a bit, but I am now bringing this up, and I try to
 also make the follow-up. Hope it is ok.

At first I thought that your idea was to have an option to git bisect
run so that a git bisect reset is run automatically after the
bisection is finished.
But I search the IRC log and found the discussion here:

http://colabti.org/irclogger/irclogger_log/git?date=2012-09-27

and I found that you said:

there should be an option for git bisect run which executed the
whatever clean command git has like git clean -fdx./

I understand that, but I wonder what we should do if some people need
a git reset --hard and if some other people need other options than
-dfx.
We would need both a --reset and a --clean, or perhaps even a
--reset[=(hard|mixed|soft|merge|keep) and a --clean[=clean-opts].
And then what if people want to clean only a subdirectory and not everything?
And this does not take into account the fact that many people
will/should clean using make clean or make distclean or rake
clean or something like that, so that a --clean option will not help
them.

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] MALLOC_CHECK: Allow checking to be disabled from config.mak

[Christian Couder]

Hi,

On Mon, Oct 8, 2012 at 3:19 PM, Elia Pinto gitter.spi...@gmail.com wrote:
 Ok. I have found. For me this is not a problem any more these days.
 Was fixed in glibc 2.8.90-9 2008

 * Wed Jul 16 2008 Jakub Jelinek ja...@redhat.com 2.8.90-9
 - update from trunk
   - fix unbuffered vfprintf if writing to the stream fails (#455360)
   - remove useless malloc: using debugging hooks message (#455355)
   - nscd fixes
 (from glibc rpm changelog)

 This is the bugzilla filled and closed
 https://bugzilla.redhat.com/show_bug.cgi?id=455355

 Ramsay, what version of glibc you have and in what distro? thanks

I have the same problem on a RHEL 5.3 (2.6.18-128.el5 #1 SMP Wed Dec
17 11:41:38 EST 2008 x86_64 x86_64 x86_64 GNU/Linux) with
glibc-2.5-34.

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] Fix git p4 sync errors

[Christian Couder]

Hi,

On Thu, Oct 25, 2012 at 4:41 AM, Matt Arsenault arse...@gmail.com wrote:

 On Oct 21, 2012, at 12:06 , Junio C Hamano gits...@pobox.com wrote:

 - Why is it a bug not to pass -s?  How does the bug happen?

 I encountered this one time after using it for months. One day I couldn't git 
 p4 rebase
 with the key error.  I searched for the error and found some version of 
 git-p4 that fixed
 a similar error by adding the -s to describe. Adding the -s fixed the error 
 and
 everything seemed to be working correctly.

The perforce documentation says the following about this flag:

The -s flag omits the diffs of files that were updated.

So if the diffs are not used, there is no downside to use -s.
Maybe the patch should just state this.

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] Documentation/CommunityGuidelines

[Christian Couder]

Hi,

On Thu, Jun 13, 2013 at 12:19 PM, Thomas Adam tho...@xteddy.org wrote:

 So these guidelines gain the community nothing, and only serve to punish
 those who are already following them, without them being written down,
 because the root-cause of the problem is still here, and isn't going to go
 away, no matter how much referring to these guidelines might help.

 That is why I think this is the wrong thing to do.

I don't know if some guidelines will gain the community anything, but
there might be another solution to the current problems in the
community along the following lines:

- decide that later this year (for example next october or september)
there could be a developer
meeting/conference/merge/gittogether/whatever somewhere in North
America

- ask many developers who contributed to Git significantly, including
those involved in the last flamewars, if they could come and if they
would need financial help to come

- ask some companies to sponsor the meeting by providing money, space,
food, beer, accomodation, whatever they want

- maybe ask Git developers or users living neer the meeting place if
the developers coming could crash at their place

- announce the meeting, thanks the sponsors, by the way thanks a lot
GitHub for the git merge 2013 last May in Berlin

- meet, discuss stuff, have a lot of beers together, write stupid joke
patches together and send them to the list

- reimburse the developers who came for their travel and if needed
accomodation expenses

- thank everyone for coming and having a good time together and thank
the sponsors again, by the way thank you guys who came to the git
merge 2013 and thank you again Github, for organizing it and Uber and
Google for sponsoring it

It might not work but it might be a nice try :-)

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: Requirements for integrating a new git subcommand

[Christian Couder]

On Thu, Nov 22, 2012 at 11:11 PM, Eric S. Raymond e...@thyrsus.com wrote:
 Shawn Pearce spea...@spearce.org:
 [Lots of helpful stuff ended by]
  4. How does git help work?  That is, how is a subcommand expected
  to know when it is being called to export its help text?

 IIRC git help foo runs man git-foo.

 OK, that makes sense.

git help can also launch a browser to display the HTML version of
the man page.
It is looking for man pages and HTML docs in the paths given by git
--man-path and git --html-path.

Cheers,
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] svnrdump_sim: start the script with /usr/bin/env python

[Christian Couder]

All the python scripts except contrib/svn-fe/svnrdump_sim.py
start with #!/usr/bin/env python.

This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 contrib/svn-fe/svnrdump_sim.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
index 1cfac4a..d219180 100755
--- a/contrib/svn-fe/svnrdump_sim.py
+++ b/contrib/svn-fe/svnrdump_sim.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 
 Simulates svnrdump by replaying an existing dump from a file, taking care
 of the specified revision range.
-- 
1.7.11.rc3.17.g55b3f8c

--
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] svnrdump_sim: start the script with /usr/bin/env python

[Christian Couder]

On Wed, Nov 28, 2012 at 9:03 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Wed, Nov 28, 2012 at 8:36 AM, Junio C Hamano gits...@pobox.com wrote:
 Christian Couder chrisc...@tuxfamily.org writes:

 All the python scripts except contrib/svn-fe/svnrdump_sim.py
 start with #!/usr/bin/env python.

 This patch fix contrib/svn-fe/svnrdump_sim.py to do the same.

 I suspect you need a bit more than that.

 $ make git-p4
 $ diff -u git-p4.py git-p4

 shows you how we tell the scripts how to find their interpreters
 (that way, there is no need to rely on the existence of
 /usr/bin/env).

 That works if somebody managed to export PYTHON_PATH, which very very
 often is not the case for me.

Yeah, and even if PYTHON_PATH is used, in t9020-remote-svn.sh,
svnrdump.py is used as is.
So if your python is not /usr/bin/python, you cannot just add
something to $PATH to pass the test.

Best regards,
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] Makefile: detect when PYTHON_PATH changes

[Christian Couder]

When make is run the python script are created from *.py files that
are changed to use the python given by PYTHON_PATH. And PYTHON_PATH
is set by default to /usr/bin/python on Linux.

This is nice except when you run make another time setting a
different PYTHON_PATH, because, as the python scripts have already
been created, make finds nothing to do.

The goal of this patch is to detect when the PYTHON_PATH changes and
to create the python scripts again when this happens. To do that we
use the same trick that is done to track prefix, flags and tcl/tk
path. We update a GIT-PYTHON-VARS file with the PYTHON_PATH and check
if it changed.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Makefile | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 4ad6fbd..bd86063 100644
--- a/Makefile
+++ b/Makefile
@@ -2245,7 +2245,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : 
unimplemented.sh
 endif # NO_PERL
 
 ifndef NO_PYTHON
-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX
+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
 $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
$(QUIET_GEN)$(RM) $@ $@+  \
INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
@@ -2636,6 +2636,18 @@ GIT-GUI-VARS: FORCE
 fi
 endif
 
+### Detect Python interpreter path changes
+ifndef NO_PYTHON
+TRACK_VARS = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
+
+GIT-PYTHON-VARS: FORCE
+   @VARS='$(TRACK_VARS)'; \
+   if test x$$VARS != x`cat $@ 2/dev/null` ; then \
+   echo 12 * new Python interpreter location; \
+   echo $$VARS $@; \
+fi
+endif
+
 test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) 
$(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
 
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
-- 
1.8.1.rc1.1.g9537e17

--
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] Makefile: detect when PYTHON_PATH changes

[Christian Couder]

From: Junio C Hamano gits...@pobox.com
Subject: Re: [PATCH] Makefile: detect when PYTHON_PATH changes
Date: Sat, 15 Dec 2012 09:29:48 -0800

 Christian Couder chrisc...@tuxfamily.org writes:
 
 @@ -2636,6 +2636,18 @@ GIT-GUI-VARS: FORCE
  fi
  endif
  
 +### Detect Python interpreter path changes
 +ifndef NO_PYTHON
 +TRACK_VARS = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
 +
 +GIT-PYTHON-VARS: FORCE
 +@VARS='$(TRACK_VARS)'; \
 +if test x$$VARS != x`cat $@ 2/dev/null` ; then \
 +echo 12 * new Python interpreter location; \
 +echo $$VARS $@; \
 +fi
 +endif
 
 Do we have the same issue when you decide to use /usr/local/bin/sh
 after building with /bin/sh set to SHELL_PATH?
 
  - If yes, I presume that there will be follow-up patches to this
one, for SHELL_PATH, PERL_PATH, and TCLTK_PATH (there may be
other languages but I didn't bother to check).  How would the use
of language agnostic looking TRACK_VARS variable in this patch
affect such follow-up patches?

Actually, just above the above hunk, there is:

### Detect Tck/Tk interpreter path changes
ifndef NO_TCLTK
TRACK_VARS = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')

GIT-GUI-VARS: FORCE
@VARS='$(TRACK_VARS)'; \
if test x$$VARS != x`cat $@ 2/dev/null` ; then \
echo 12 * new Tcl/Tk interpreter location; \
echo $$VARS $@; \
fi
endif

But you are right, TRACK_VARS should probably be something like
TRACK_TCLTK when used for TCLTK and TRACK_PYTHON when used for Python.

By the way it looks like GIT-GUI-VARS is not used, so perhaps the
detection of Tck/Tk interpreter path changes above could be removed.

(The detection is done in git-gui/Makefile too.)

About shell, there is the following:

SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
$(gitwebdir_SQ):$(PERL_PATH_SQ)

and then

GIT-SCRIPT-DEFINES: FORCE
@FLAGS='$(SCRIPT_DEFINES)'; \
if test x$$FLAGS != x`cat $@ 2/dev/null` ; then \
echo 12 * new script parameters; \
echo $$FLAGS $@; \
fi


$(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh GIT-SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script)  \
chmod +x $@+  \
mv $@+ $@

$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script)  \
mv $@+ $@

So it looks to me that at least for SHELL_PATH, things are done
properly. 

  - If no (in other words, if we rebuild shell-scripted porcelains
when SHELL_PATH changes), wouldn't it be better to see how it is
done and hook into the same mechanism?

You would like me to just add $(PYTHON_PATH_SQ) in SCRIPT_DEFINES and
then use GIT-SCRIPT-DEFINES instead of GIT-PYTHON-VARS to decide if
python scripts should be rebuilt?

  - If no, and if the approach taken in this patch is better than the
one used to rebuild shell scripts (it may limit the scope of
rebuilding when path changes, or something, but again, I didn't
bother to check), 

Yeah, I think it is better because it limits the scope of rebuilding,
and because nothing is done for Python, while there are some
mechanisms in place for other languages.

 then again follow-up patches to this one to
describe dependencies to build scripts in other languages in a
similar way to this patch would come in the future.  The same
question regarding TRACK_VARS applies in this case.

I agree about TRACK_VARS. About other languages, I will have another
look, but it seems that they already have what they need.

 By the way, 12 is easier to read if written as just 2, I
 think.

Ok I will change this.

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 1/2] Makefile: remove tracking of TCLTK_PATH

[Christian Couder]

It looks like we are tracking the value of TCLTK_PATH in the main
Makefile for no good reason, as this is done in git-gui too and the
GIT-GUI-VARS is not used in the Makefile.

This patch removes the useless code used to do this tracking.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 .gitignore |  1 -
 Makefile   | 14 +-
 2 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/.gitignore b/.gitignore
index f702415..6d69ae1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,7 +1,6 @@
 /GIT-BUILD-OPTIONS
 /GIT-CFLAGS
 /GIT-LDFLAGS
-/GIT-GUI-VARS
 /GIT-PREFIX
 /GIT-SCRIPT-DEFINES
 /GIT-USER-AGENT
diff --git a/Makefile b/Makefile
index 4ad6fbd..585b2eb 100644
--- a/Makefile
+++ b/Makefile
@@ -2624,18 +2624,6 @@ ifdef GIT_PERF_MAKE_OPTS
@echo GIT_PERF_MAKE_OPTS=\''$(subst ','\'',$(subst 
','\'',$(GIT_PERF_MAKE_OPTS)))'\' $@
 endif
 
-### Detect Tck/Tk interpreter path changes
-ifndef NO_TCLTK
-TRACK_VARS = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')
-
-GIT-GUI-VARS: FORCE
-   @VARS='$(TRACK_VARS)'; \
-   if test x$$VARS != x`cat $@ 2/dev/null` ; then \
-   echo 12 * new Tcl/Tk interpreter location; \
-   echo $$VARS $@; \
-fi
-endif
-
 test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) 
$(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
 
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
@@ -2910,7 +2898,7 @@ ifndef NO_TCLTK
$(MAKE) -C gitk-git clean
$(MAKE) -C git-gui clean
 endif
-   $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-GUI-VARS 
GIT-BUILD-OPTIONS
+   $(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
$(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES
 
 .PHONY: all install profile-clean clean strip
-- 
1.8.1.rc1.2.g8740035


--
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/2] Makefile: detect when PYTHON_PATH changes

[Christian Couder]

When make is run, the python scripts are created from *.py files that
are changed to use the python given by PYTHON_PATH. And PYTHON_PATH
is set by default to /usr/bin/python on Linux.

This is nice except when you run make another time setting a
different PYTHON_PATH, because, as the python scripts have already
been created, make finds nothing to do.

The goal of this patch is to detect when the PYTHON_PATH changes and
to create the python scripts again when this happens. To do that we
use the same trick that is done to track other variables like prefix,
flags, tcl/tk path and shell path. We update a GIT-PYTHON-VARS file
with the PYTHON_PATH and check if it changed.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 .gitignore |  1 +
 Makefile   | 16 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 6d69ae1..086c5af 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,7 @@
 /GIT-CFLAGS
 /GIT-LDFLAGS
 /GIT-PREFIX
+/GIT-PYTHON-VARS
 /GIT-SCRIPT-DEFINES
 /GIT-USER-AGENT
 /GIT-VERSION-FILE
diff --git a/Makefile b/Makefile
index 585b2eb..7db8445 100644
--- a/Makefile
+++ b/Makefile
@@ -2245,7 +2245,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : 
unimplemented.sh
 endif # NO_PERL
 
 ifndef NO_PYTHON
-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX
+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
 $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
$(QUIET_GEN)$(RM) $@ $@+  \
INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
@@ -2624,6 +2624,18 @@ ifdef GIT_PERF_MAKE_OPTS
@echo GIT_PERF_MAKE_OPTS=\''$(subst ','\'',$(subst 
','\'',$(GIT_PERF_MAKE_OPTS)))'\' $@
 endif
 
+### Detect Python interpreter path changes
+ifndef NO_PYTHON
+TRACK_PYTHON = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
+
+GIT-PYTHON-VARS: FORCE
+   @VARS='$(TRACK_PYTHON)'; \
+   if test x$$VARS != x`cat $@ 2/dev/null` ; then \
+   echo 12 * new Python interpreter location; \
+   echo $$VARS $@; \
+fi
+endif
+
 test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) 
$(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
 
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
@@ -2899,7 +2911,7 @@ ifndef NO_TCLTK
$(MAKE) -C git-gui clean
 endif
$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
-   $(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES
+   $(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES GIT-PYTHON-VARS
 
 .PHONY: all install profile-clean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
-- 
1.8.1.rc1.2.g8740035

--
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] Makefile: detect when PYTHON_PATH changes

[Christian Couder]

When make is run, the python scripts are created from *.py files that
are changed to use the python given by PYTHON_PATH. And PYTHON_PATH
is set by default to /usr/bin/python on Linux.

This is nice except when you run make another time setting a
different PYTHON_PATH, because, as the python scripts have already
been created, make finds nothing to do.

The goal of this patch is to detect when the PYTHON_PATH changes and
to create the python scripts again when this happens. To do that we
use the same trick that is done to track other variables like prefix,
flags, tcl/tk path and shell path. We update a GIT-PYTHON-VARS file
with the PYTHON_PATH and check if it changed.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 .gitignore |  1 +
 Makefile   | 16 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 6d69ae1..086c5af 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,7 @@
 /GIT-CFLAGS
 /GIT-LDFLAGS
 /GIT-PREFIX
+/GIT-PYTHON-VARS
 /GIT-SCRIPT-DEFINES
 /GIT-USER-AGENT
 /GIT-VERSION-FILE
diff --git a/Makefile b/Makefile
index 585b2eb..7db8445 100644
--- a/Makefile
+++ b/Makefile
@@ -2245,7 +2245,7 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : 
unimplemented.sh
 endif # NO_PERL
 
 ifndef NO_PYTHON
-$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX
+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
 $(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
$(QUIET_GEN)$(RM) $@ $@+  \
INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
@@ -2624,6 +2624,18 @@ ifdef GIT_PERF_MAKE_OPTS
@echo GIT_PERF_MAKE_OPTS=\''$(subst ','\'',$(subst 
','\'',$(GIT_PERF_MAKE_OPTS)))'\' $@
 endif
 
+### Detect Python interpreter path changes
+ifndef NO_PYTHON
+TRACK_PYTHON = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
+
+GIT-PYTHON-VARS: FORCE
+   @VARS='$(TRACK_PYTHON)'; \
+   if test x$$VARS != x`cat $@ 2/dev/null` ; then \
+   echo 12 * new Python interpreter location; \
+   echo $$VARS $@; \
+fi
+endif
+
 test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) 
$(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
 
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
@@ -2899,7 +2911,7 @@ ifndef NO_TCLTK
$(MAKE) -C git-gui clean
 endif
$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
-   $(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES
+   $(RM) GIT-USER-AGENT GIT-PREFIX GIT-SCRIPT-DEFINES GIT-PYTHON-VARS
 
 .PHONY: all install profile-clean clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
-- 
1.8.1.rc1.2.g8740035


--
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