[PATCH v5 2/2] test-config: Add tests for the config_set API

[Tanay Abhra]

Expose the `config_set` C API as a set of simple commands in order to
facilitate testing. Add tests for the `config_set` API as well as for
`git_config_get_*()` family for the usual config files.

Signed-off-by: Tanay Abhra 
---
 .gitignore |   1 +
 Makefile   |   1 +
 t/t1308-config-hash.sh | 187 +
 test-config.c  | 127 +
 4 files changed, 316 insertions(+)
 create mode 100755 t/t1308-config-hash.sh
 create mode 100644 test-config.c

diff --git a/.gitignore b/.gitignore
index 42294e5..eeb66e2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -176,6 +176,7 @@
 /gitweb/static/gitweb.js
 /gitweb/static/gitweb.min.*
 /test-chmtime
+/test-config
 /test-ctype
 /test-date
 /test-delta
diff --git a/Makefile b/Makefile
index d503f78..e875070 100644
--- a/Makefile
+++ b/Makefile
@@ -548,6 +548,7 @@ X =
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
+TEST_PROGRAMS_NEED_X += test-config
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
diff --git a/t/t1308-config-hash.sh b/t/t1308-config-hash.sh
new file mode 100755
index 000..eba7102
--- /dev/null
+++ b/t/t1308-config-hash.sh
@@ -0,0 +1,187 @@
+#!/bin/sh
+
+test_description='Test git config-hash API in different settings'
+
+. ./test-lib.sh
+
+test_expect_success 'clear default config' '
+   rm -f .git/config
+'
+
+test_expect_success 'initialize default config' '
+   cat >.git/config << EOF
+   [core]
+   penguin = very blue
+   Movie = BadPhysics
+   UPPERCASE = true
+   MixedCase = true
+   my =
+   foo
+   baz = sam
+   [Cores]
+   WhatEver = Second
+   baz = bar
+   [cores]
+   baz = bat
+   [CORES]
+   baz = ball
+   [my "Foo bAr"]
+   hi = mixed-case
+   [my "FOO BAR"]
+   hi = upper-case
+   [my "foo bar"]
+   hi = lower-case
+   [core]
+   baz = bat
+   baz = hask
+   [lamb]
+   chop = 65
+   head = none
+   [goat]
+   legs = 4
+   head = true
+   skin = false
+   nose = 1
+   horns
+   EOF
+'
+
+test_expect_success 'get value for a simple key' '
+   echo "very blue" >expect &&
+   test-config get_value core.penguin >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'get value for a key with value as an empty string' '
+   echo "" >expect &&
+   test-config get_value core.my >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'get value for a key with value as NULL' '
+   echo "(NULL)" >expect &&
+   test-config get_value core.foo >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'upper case key' '
+   echo "true" >expect &&
+   test-config get_value core.UPPERCASE >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'mixed case key' '
+   echo "true" >expect &&
+   test-config get_value core.MixedCase >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'key and value with mixed case' '
+   echo "BadPhysics" >expect &&
+   test-config get_value core.Movie >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'key with case sensitive subsection' '
+   echo "mixed-case" >expect &&
+   echo "upper-case" >>expect &&
+   echo "lower-case" >>expect &&
+   test-config get_value "my.Foo bAr.hi" >actual &&
+   test-config get_value "my.FOO BAR.hi" >>actual &&
+   test-config get_value "my.foo bar.hi" >>actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'key with case insensitive section header' '
+   echo "ball" >expect &&
+   echo "ball" >>expect &&
+   echo "ball" >>expect &&
+   test-config get_value cores.baz >actual &&
+   test-config get_value Cores.baz >>actual &&
+   test-config get_value CORES.baz >>actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'find value with misspelled key' '
+   echo "Value not found for \"my.fOo Bar.hi\"" >expect &&
+   test_must_fail test-config get_value "my.fOo Bar.hi" >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'find value with the highest priority' '
+   echo hask >expect &&
+   test-config get_value "core.baz">actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'find integer value for a key' '
+   echo 65 >expect &&
+   test-config get_int lamb.chop >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'find integer if value is non parse-able' '
+   echo 65 >expect &&
+   test_must_fail test-config get_int lamb.head >actual &&
+   test_must_fail test_cmp expect actual
+'
+
+test_expect_s

[PATCH v5 0/3] git config cache & special querying api utilizing the cache

[Tanay Abhra]

Hi,

[PATCH V5]: `config_set` now uses a single hashmap. Corrected style nits raised 
in
the thread[7]. Thanks to Junio and Matthieu for their 
suggestions.

[PATCH v4]: Introduced `config_set` construct which points to a ordered set of
config-files cached as hashmaps. Added relevant API functions. For more
details see the documentation. Rewrote the git_config_get* family to use
`config_set` internally. Added tests for both config_set API and 
git_config_get
family. Added type specific API functions which parses the found value 
and
converts it into a specific type.
Most of the changes implemented are the result of discussion in [6].
Thanks to Eric, Ramsay, Junio, Matthieu & Karsten for their suggestions
and review.

[PATCH v3]: Added flag for NULL values that were causing segfaults in some 
cases.
Added test-config for usage examples.
Minor changes and corrections. Refer to discussion thread[5] for more 
details.
Thanks to Matthieu, Jeff and Junio for their valuable suggestions.

[PATCH v2]:Changed the string_list to a struct instead of pointer to a struct.
Added string-list initilization functions.
Minor mistakes corrected acoording to review comments[4]. Thanks to
Eric and Matthieu for their review.

[PATCH V1]:Most of the invaluable suggestions by Eric Sunshine, Torsten 
Bogershausen and
Jeff King has been implemented[1]. Complete rewrite of config_cache*() 
family
using git_config() as hook as suggested by Jeff. Thanks for the review.

[RFC V2]: Improved according to the suggestions by Eric Sunshine and Torsten 
Bogershausen.
Added cache invalidation when config file is changed.[2]
I am using git_config_set_multivar_in_file() as an update hook.

This is patch is for this year's GSoC. My project is
"Git Config API improvements". The link of my proposal is appended below [3].

The aim of this patch series is to generate a cache for querying values from
the config files in a non-callback manner as the current method reads and
parses the config files every time a value is queried for.

The cache is generated from hooking the update_cache function to the current
parsing and callback mechanism in config.c. It is implemented as an hashmap
using the hashmap-api with variables and its corresponding values list as
its members. The values in the list are sorted in order of increasing priority.
The cache is initialised the first time when any of the new query functions is
called. It is invalidated by using git_config_set_multivar_in_file() as an
update hook.

We add two new functions to the config-api, git_config_get_string() and
git_config_get_string_multi() for querying in a non callback manner from
the cache.

[1] http://marc.info/?t=14017206626&r=1&w=2
[2] 
http://git.661346.n2.nabble.com/RFC-PATCH-0-2-Git-config-cache-amp-special-querying-api-utilizing-the-cache-td7611691.html
[3] 
https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing
[4] http://thread.gmane.org/gmane.comp.version-control.git/251073/focus=251369
[5] http://thread.gmane.org/gmane.comp.version-control.git/251704/
[6] http://thread.gmane.org/gmane.comp.version-control.git/252329/
[7] http://marc.info/?t=14042811521&r=1&w=2


Tanay Abhra (2):
  config-hash.c
  test-config

 .gitignore |   1 +
 Documentation/technical/api-config.txt | 134 +++
 Makefile   |   2 +
 cache.h|  34 
 config-hash.c  | 297 +
 config.c   |   3 +
 t/t1308-config-hash.sh | 187 +
 test-config.c  | 127 ++
 8 files changed, 785 insertions(+)
 create mode 100644 config-hash.c
 create mode 100755 t/t1308-config-hash.sh
 create mode 100644 test-config.c

-- 
1.9.0.GIT

--
To unsubscribe from this list: send the line "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/2] add `config_set` API for caching config-like files

[Tanay Abhra]

Currently `git_config()` uses a callback mechanism and file rereads for
config values. Due to this approach, it is not uncommon for the config
files to be parsed several times during the run of a git program, with
different callbacks picking out different variables useful to themselves.

Add a `config_set`, that can be used to construct an in-memory cache for
config-like files that the caller specifies (i.e., files like `.gitmodules`,
`~/.gitconfig` etc.). Add two external functions `git_configset_get_value`
and `git_configset_get_value_multi` for querying from the config sets.
`git_configset_get_value` follows `last one wins` semantic (i.e. if there
are multiple matches for the queried key in the files of the configset the
value returned will be the last entry in `value_list`).
`git_configset_get_value_multi` returns a list of values sorted in order of
increasing priority (i.e. last match will be at the end of the list). Add
type specific query functions like `git_configset_get_bool` and similar.

Add a default `config_set`, `the_config_set` to cache all key-value pairs
read from usual config files (repo specific .git/config, user wide
~/.gitconfig and the global /etc/gitconfig). `the_config_set` uses a
single hashmap populated using `git_config()`.

Add two external functions `git_config_get_value` and 
`git_config_get_value_multi` for querying in a non-callback manner from
`the_config_set`. Also, add type specific query functions that are
implemented as a thin wrapper around the `config_set` API.

Signed-off-by: Tanay Abhra 
---
 Documentation/technical/api-config.txt | 134 +++
 Makefile   |   1 +
 cache.h|  34 
 config-hash.c  | 297 +
 config.c   |   3 +
 5 files changed, 469 insertions(+)
 create mode 100644 config-hash.c

diff --git a/Documentation/technical/api-config.txt 
b/Documentation/technical/api-config.txt
index 230b3a0..bdf86d0 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -77,6 +77,81 @@ To read a specific file in git-config format, use
 `git_config_from_file`. This takes the same callback and data parameters
 as `git_config`.
 
+Querying For Specific Variables
+---
+
+For programs wanting to query for specific variables in a non-callback
+manner, the config API provides two functions `git_config_get_value`
+and `git_config_get_value_multi`. They both read values from an internal
+cache generated previously from reading the config files.
+
+`int git_config_get_value(const char *key, const char **value)`::
+
+   Finds the highest-priority value for the configuration variable `key`,
+   stores the pointer to it in `value` and returns 0. When the
+   configuration variable `key` is not found, returns 1 without touching
+   `value`. The caller should not free or modify `value`, as it is owned
+   by the cache.
+
+`const struct string_list *git_config_get_value_multi(const char *key)`::
+
+   Finds and returns the value list, sorted in order of increasing priority
+   for the configuration variable `key`. When the configuration variable
+   `key` is not found, returns NULL. The caller should not free or modify
+   the returned pointer, as it is owned by the cache.
+
+`void git_config_clear(void)`::
+
+   Resets and invalidate the config cache. 
+
+The config API also provides type specific API functions which do conversion
+as well as retrieval for the queried variable, including:
+
+`int git_config_get_int(const char *key, int *dest)`::
+
+   Finds and parses the value to an integer for the configuration variable
+   `key`. Dies on error; otherwise, stores pointer to the parsed integer in
+   `dest` and returns 0. When the configuration variable `key` is not 
found,
+   returns 1 without touching `dest`.
+
+`int git_config_get_ulong(const char *key, unsigned long *dest)`::
+
+   Similar to `git_config_get_int` but for unsigned longs.
+
+`int git_config_get_int(const char *key, int *dest)`::
+
+   Finds and parses the value into a boolean value, for the configuration
+   variable `key`respecting keywords like "true" and "false". Integer
+   values are converted into true/false values (when they are non-zero or
+   zero, respectively). Other values cause a die(). If parsing is 
successful,
+   stores the pointer to the parsed result in `dest` and returns 0. When 
the
+   configuration variable `key` is not found, returns 1 without touching
+   `dest`.
+
+`int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)`::
+
+   Similar to `git_config_get_bool`, except that integers are copied as-is,
+   and `is_bool` flag is unset.
+
+`int git_config_get_maybe_bool(const char *key, int *dest)`::
+
+   Similar to `git_config_get_bool`, except that it ret

Re: [PATCH v4 3/4] cache-tree: subdirectory tests

[Eric Sunshine]

On Sun, Jul 6, 2014 at 12:06 AM, David Turner  wrote:
> Add tests to confirm that invalidation of subdirectories nether over-

s/nether/neither/

> nor under-invalidates.
>
> Signed-off-by: David Turner 
> ---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] add `config_set` API for caching config-like files

[Ramsay Jones]

On 06/07/14 08:19, Tanay Abhra wrote:
> Currently `git_config()` uses a callback mechanism and file rereads for
> config values. Due to this approach, it is not uncommon for the config
> files to be parsed several times during the run of a git program, with
> different callbacks picking out different variables useful to themselves.
> 
> Add a `config_set`, that can be used to construct an in-memory cache for
> config-like files that the caller specifies (i.e., files like `.gitmodules`,
> `~/.gitconfig` etc.). Add two external functions `git_configset_get_value`
> and `git_configset_get_value_multi` for querying from the config sets.
> `git_configset_get_value` follows `last one wins` semantic (i.e. if there
> are multiple matches for the queried key in the files of the configset the
> value returned will be the last entry in `value_list`).
> `git_configset_get_value_multi` returns a list of values sorted in order of
> increasing priority (i.e. last match will be at the end of the list). Add
> type specific query functions like `git_configset_get_bool` and similar.
> 
> Add a default `config_set`, `the_config_set` to cache all key-value pairs
> read from usual config files (repo specific .git/config, user wide
> ~/.gitconfig and the global /etc/gitconfig). `the_config_set` uses a
> single hashmap populated using `git_config()`.
> 
> Add two external functions `git_config_get_value` and 
> `git_config_get_value_multi` for querying in a non-callback manner from
> `the_config_set`. Also, add type specific query functions that are
> implemented as a thin wrapper around the `config_set` API.
> 
> Signed-off-by: Tanay Abhra 
> ---
>  Documentation/technical/api-config.txt | 134 +++
>  Makefile   |   1 +
>  cache.h|  34 
>  config-hash.c  | 297 
> +
>  config.c   |   3 +
>  5 files changed, 469 insertions(+)
>  create mode 100644 config-hash.c
> 
> diff --git a/Documentation/technical/api-config.txt 
> b/Documentation/technical/api-config.txt
> index 230b3a0..bdf86d0 100644
> --- a/Documentation/technical/api-config.txt
> +++ b/Documentation/technical/api-config.txt
> @@ -77,6 +77,81 @@ To read a specific file in git-config format, use
>  `git_config_from_file`. This takes the same callback and data parameters
>  as `git_config`.
>  
> +Querying For Specific Variables
> +---
> +
> +For programs wanting to query for specific variables in a non-callback
> +manner, the config API provides two functions `git_config_get_value`
> +and `git_config_get_value_multi`. They both read values from an internal
> +cache generated previously from reading the config files.
> +
> +`int git_config_get_value(const char *key, const char **value)`::
> +
> + Finds the highest-priority value for the configuration variable `key`,
> + stores the pointer to it in `value` and returns 0. When the
> + configuration variable `key` is not found, returns 1 without touching
> + `value`. The caller should not free or modify `value`, as it is owned
> + by the cache.
> +
> +`const struct string_list *git_config_get_value_multi(const char *key)`::
> +
> + Finds and returns the value list, sorted in order of increasing priority
> + for the configuration variable `key`. When the configuration variable
> + `key` is not found, returns NULL. The caller should not free or modify
> + the returned pointer, as it is owned by the cache.
> +
> +`void git_config_clear(void)`::
> +
> + Resets and invalidate the config cache. 
> +
> +The config API also provides type specific API functions which do conversion
> +as well as retrieval for the queried variable, including:
> +
> +`int git_config_get_int(const char *key, int *dest)`::
> +
> + Finds and parses the value to an integer for the configuration variable
> + `key`. Dies on error; otherwise, stores pointer to the parsed integer in

... stores the value of the parsed integer in `dest` ...

> + `dest` and returns 0. When the configuration variable `key` is not 
> found,
> + returns 1 without touching `dest`.
> +
> +`int git_config_get_ulong(const char *key, unsigned long *dest)`::
> +
> + Similar to `git_config_get_int` but for unsigned longs.
> +
> +`int git_config_get_int(const char *key, int *dest)`::
---^^^

git_config_get_bool

> +
> + Finds and parses the value into a boolean value, for the configuration
> + variable `key`respecting keywords like "true" and "false". Integer
> + values are converted into true/false values (when they are non-zero or
> + zero, respectively). Other values cause a die(). If parsing is 
> successful,
> + stores the pointer to the parsed result in `dest` and returns 0. When 
> the

... stores the value of the parsed result in `dest` ...

> + configuration variable `key` is not found, returns 1 w

RE: FYI

[Denton, Cari-Lee]

From: Denton, Cari-Lee
Sent: July 6, 2014 12:44 AM
To: Denton, Cari-Lee
Subject: FYI

You are picked, e-mail: 
mackenzie.glo...@yahoo.es for Info.
--
To unsubscribe from this list: send the line "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: Idea, Transparent commits, easier "code style" commits

[Andrius Bentkus]

-w looks good for my very specific whitespace case, but imagine you
are adding or removing parenthesis like, it is still codestyle but -w
doesn't cut it anymore.

So yes, I want a flag!

Well, if I think about it, the tools can implement themselves.

They just have to look into the commit message and look for
"#codestylefix" or whatever other string.

On Fri, Jul 4, 2014 at 5:26 PM, Stefan Beller  wrote:
> On 04.07.2014 15:12, Andrius Bentkus wrote:
>> I have worked on projects which only after a while (a year or so)
>> established a consistent code style.
>> After the consensus was established there was still some code left
>> which did not fit the newly established standard.
>> Now the problem is, if I create a new patch to actually fix it, it
>> will pollute the blame history.
>> And most of the projects just reject these kind of patches because of this.
>>
>> Imagine that you would have a type conversion "(int)value" and wanted
>> it to change to "(int) value".
>> The patch will have hundreds of occurrences of this one line changes
>> and will make the git blame look like swiss cheese.
>> It doesn't add much information to the line (you'd rather have
>> technical explanations in the commit) and actually hides all the
>> original comments of the line.
>>
>> So you kinda want to have that style fix patch because inconsistent
>> code style just triggers your OCD, but you can't do anything about it
>> because it doesn't add any value to the program when it executes and
>> actually makes it harder to browse the source code using git blame.
>>
>> My proposal is to add "transparent" commits.
>> If you write git blame these commits will not be shown, instead git
>> blame will show a merged version of the code style commit and the
>> actual commit while only showing the commit id of the original commit.
>>
>> A little visualized example:
>>
>> Imagine your first commit is:
>>
>> 58461d5a float yolo(void *i) {
>> 58461d5a   return (float)*i;
>> 58461d5a }
>>
>> And you want it to change to (float) *i, so you patch it and the blame
>> history looks now like this:
>>
>> 58461d5a float yolo(void *i) {
>> 263da519   return (float) *i;
>> 58461d5a }
>>
>> But what you really want to have when you do a git blame is this:
>>
>> 58461d5a float yolo(void *i) {
>> 58461d5a   return (float)*i;
>> 58461d5a }
>>
>> I hope I expressed myself clearly enough.
>> Maybe this was already proposed, but I couldn't find anything in the 
>> archives.
>
> Check the -w option of blame
> http://git-scm.com/docs/git-blame
> to fix it while blaming.
>
> Or you need to rewrite the history (a bad idea if the history is
> published to collegues or on the internet) and squash your fixes into
> the original commits.
> (see git rebase for that)
>
> But re-reading your mail, you would like to propose a 3rd way?
> So when commiting the fixup, you want to add a flag, which tells git
> it's just a fixup commit, which should not be shown, when blaming, but
> rather their parent for the lines in question.
> That's an interesting idea.
>
> Stefan
>
>
>
--
To unsubscribe from this list: send the line "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: Idea, Transparent commits, easier "code style" commits

[Javier Domingo Cansino]

> They just have to look into the commit message and look for
> "#codestylefix" or whatever other string.

In many projects I have seen, they have a format for commits, such as
"docs: Add support for XXX", "formatting: Space before parethesis and
after comas", "tests: " and so on.

Maybe, being able to specify a RegExp would be the way to go, so that
git blame did actually ignore those commits.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Test failure in t9814-git-p4-rename.sh - my environment or bad test?

[Christoph Bonitz]

Hi,

I'm trying to get the git p4 tests to pass on my machine (OS X
Mavericks) from master before making some changes. I'm experiencing a
test failure in "detect copies" of the rename test.

The test creates file2 with some content, creates a few copies (each
with a commit), then does the following (no git write operations
omitted):
echo "file2" >>file2 &&
cp file2 file10 &&
git add file2 file10 &&
git commit -a -m "Modify and copy file2 to file10" &&
... (some non-write-operations) ...
cp file2 file11 &&
git add file11 &&
git commit -a -m "Copy file2 to file11" &&
git diff-tree -r -C --find-copies-harder HEAD &&
src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
test "$src" = file10 &&

This is where it fails on my machine. The git diff-tree output is this
:100644 100644 22a35c17c4c0779f75142036beef6ccd58525b9c
22a35c17c4c0779f75142036beef6ccd58525b9c C100 file2 file11
so git diff-tree sees file2 as the copy source, not file10. In my
opinion, the diff-tree result is legitimate (at that point, file2,
file10 and file11 are identical). Later in the tests, after making
more copies of file2, the conditions are more flexible, e.g.
test "$src" = file10 || test "$src" = file11 || test "$src" = file12 &&

IMO, the test discounts the legitimate possibility of diff-tree
detecting file2 as source, making unnecessary assumptions about
implementation details. Is this correct, or do I misunderstand the
workings of diff-tree?

I'd be grateful for advice, both on whether this is a bug, and if so,
which branch to base a patch on.

Best regards
Christoph Bonitz
--
To unsubscribe from this list: send the line "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 5/5] Add a little script to compare two make perf runs

[Bert Wesarg]

Hi,

On Sat, Jul 5, 2014 at 1:43 AM, Andi Kleen  wrote:
> From: Andi Kleen 
>
> Signed-off-by: Andi Kleen 
> ---
>  diff-res | 26 ++
>  1 file changed, 26 insertions(+)
>  create mode 100755 diff-res
>
> diff --git a/diff-res b/diff-res
> new file mode 100755
> index 000..90d57be
> --- /dev/null
> +++ b/diff-res
> @@ -0,0 +1,26 @@
> +#!/usr/bin/python
> +# compare two make perf output file
> +# this should be the results only without any header
> +import argparse
> +import math, operator
> +from collections import OrderedDict
> +
> +ap = argparse.ArgumentParser()
> +ap.add_argument('file1', type=argparse.FileType('r'))
> +ap.add_argument('file2', type=argparse.FileType('r'))
> +args = ap.parse_args()
> +
> +cmp = (OrderedDict(), OrderedDict())
> +for f, k in zip((args.file1, args.file2), cmp):
> +for j in f:
> +num = j[59:63]
> +name = j[:59]
> +k[name] = float(num)
> +
> +for j in cmp[0].keys():
> +print j, cmp[1][j] - cmp[0][j]
> +
> +def geomean(l):
> +   return math.pow(reduce(operator.mul, filter(lambda x: x != 0.0, l)), 1.0 
> / len(l))
> +
> +print "geomean %.2f -> %.2f" % (geomean(cmp[0].values()), 
> geomean(cmp[1].values()))

a justification why the geometric mean is used here would increase my
confident significantly.

It calculates wrong values anyway iff there are zeros in the sampling set.

Thanks.

Bert
--
To unsubscribe from this list: send the line "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 5/5] Add a little script to compare two make perf runs

[Andi Kleen]

> a justification why the geometric mean is used here would increase my
> confident significantly.

It's just a standard way to summarize sets of benchmarks. For example SPEC 
uses the same approach.

Anyways the script is not essential to the rest of the profile feedback
feature.  Just ignore it if it's controversal.

-Andi
--
To unsubscribe from this list: send the line "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 5/5] Add a little script to compare two make perf runs

[Bert Wesarg]

On Sun, Jul 6, 2014 at 6:15 PM, Andi Kleen  wrote:
>> a justification why the geometric mean is used here would increase my
>> confident significantly.
>
> It's just a standard way to summarize sets of benchmarks. For example SPEC
> uses the same approach.

No, SPEC would have calculated the geometric mean of the ratios
cmp[1][j] / cmp[0][j]. And this should also only be used under the
assumption that there is a multiplicative correlation.

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

Re: [PATCH v5 1/2] add `config_set` API for caching config-like files

[Matthieu Moy]

Tanay Abhra  writes:

> Add a default `config_set`, `the_config_set` to cache all key-value pairs
> read from usual config files (repo specific .git/config, user wide
> ~/.gitconfig and the global /etc/gitconfig). `the_config_set` uses a
> single hashmap populated using `git_config()`.

"uses a single hashmap" is no longer relevant. Any config_set use a
single hashmap.

(The "populated using git_config()" part is still relevant OTOH)

> +`void git_configset_init(struct config_set *cs)`::
> +
> + Initializes the member variables of config_set `cs`.

I'd say just "initializes the config_set `cs`".

> +/*
> + * Default config_set that contains key-value pairs from the usual set of 
> config
> + * config files (i.e repo specific .git/config, user wide ~/.gitconfig and 
> the
> + * global /etc/gitconfig)

To be technically correct, "user wide ~/.gitconfig, XDG config file and
the global ...".

> +static int add_configset_hash(const char *filename, struct config_set *cs)
> +{
> + int ret = 0;
> + if (!cs->hash_initialized)
> + config_hash_init(&cs->config_hash);
> + cs->hash_initialized = 1;

That would seem more natural to me to have a function config_set_init
instead of config_hash_init, that would set hash_initialized.

config_hash_init is currently a one-liner, so there's no risk of making
it too complex.

> +static int config_hash_add_value(const char *key, const char *value, struct 
> hashmap *config_hash)
> +{
> + struct config_hash_entry *e;
> + e = config_hash_find_entry(key, config_hash);
> +
> + if (!e) {
> + e = xmalloc(sizeof(*e));
> + hashmap_entry_init(e, strhash(key));

Nitpick: you're hashing the same string twice. Once for
config_hash_find_entry, and another here. It would be slightly faster to
allow config_hash_find_entry to return the hashcode (as a by-address
parameter).

But you may want to keep the code as it is for simplicity.

It took me some time to understand why you normalize in
config_hash_find_entry and not here. My understanding is that
config_hash_find_entry is used to query the hashmap from the user API
(so you need normalization), but this particular codepath receives
normalized keys (the comment right below states that for values).

It's probably worth a comment here and/or in config_hash_find_entry.

> + }
> + /*
> +  * Since the values are being fed by git_config*() callback mechanism, 
> they
> +  * are already normalized. So simply add them without any further 
> munging.
> +  */
> + string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);
> +
> + return 0;
> +}

[...]

> +int git_configset_add_file(struct config_set *cs, const char *filename)
> +{
> + return add_configset_hash(filename, cs);
> +}

Why have two functions doing the same, with arguments in different
orders?

I guess it's just historical, and you can keep only one.

> +void git_configset_clear(struct config_set *cs)
> +{
> + struct config_hash_entry *entry;
> + struct hashmap_iter iter;
> + if (!cs->hash_initialized)
> + return;
> +
> + hashmap_iter_init( &cs->config_hash, &iter);

No space after (.

> +int git_config_get_string(const char *key, const char **dest)
> +{
> + git_config_check_init();
> + return git_configset_get_string(&the_config_set, key, dest);
> +}
> +
> +int git_config_get_int(const char *key, int *dest)
> +{
> + git_config_check_init();
> + return git_configset_get_int(&the_config_set, key, dest);
> +}

Reading this list, I initially thought it might be worth factoring this
in a macro. But the list isn't that long, and contains several
special-cases (3 parameters instead of 2). So, don't bother.

OK, we're getting close. v6 should be really good :-).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] test-config: Add tests for the config_set API

[Matthieu Moy]

Tanay Abhra  writes:

> +test_expect_success 'get value for a simple key' '
> + echo "very blue" >expect &&
> + test-config get_value core.penguin >actual &&
> + test_cmp expect actual
> +'

All these tests would greatly benefit from a helper like

test_expect_config () {
echo "1" >expect &&
test-config get_value "$2" >actual &&
test_cmp expect actual
}

Then, all the 3-liners below would become 1-liners.

Should not block inclusion, but may be worth considering.

> +test_expect_success 'get value for a key with value as an empty string' '
> + echo "" >expect &&
> + test-config get_value core.my >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'get value for a key with value as NULL' '
> + echo "(NULL)" >expect &&
> + test-config get_value core.foo >actual &&
> + test_cmp expect actual
> +'
[...]

> +test_expect_success 'key with case sensitive subsection' '
> + echo "mixed-case" >expect &&
> + echo "upper-case" >>expect &&
> + echo "lower-case" >>expect &&
> + test-config get_value "my.Foo bAr.hi" >actual &&
> + test-config get_value "my.FOO BAR.hi" >>actual &&
> + test-config get_value "my.foo bar.hi" >>actual &&
> + test_cmp expect actual
> +'

This would become a 3-liner with my helper.

> +test_expect_success 'key with case insensitive section header' '
> + echo "ball" >expect &&
> + echo "ball" >>expect &&
> + echo "ball" >>expect &&
> + test-config get_value cores.baz >actual &&
> + test-config get_value Cores.baz >>actual &&
> + test-config get_value CORES.baz >>actual &&
> + test_cmp expect actual
> +'

I think you miss a simple case: get_value with a case that doesn't exist
in the config file, like "get_value coreS.baz".

> +test_expect_success 'find value with the highest priority' '
> + echo hask >expect &&
> + test-config get_value "core.baz">actual &&

Space before >.

> diff --git a/test-config.c b/test-config.c

No time for a real review of this file, but from a quick look, it seems
OK.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 2/2] test-config: Add tests for the config_set API

[Matthieu Moy]

Tanay Abhra  writes:

> diff --git a/test-config.c b/test-config.c
> new file mode 100644
> index 000..45ccd0a
> --- /dev/null
> +++ b/test-config.c
> @@ -0,0 +1,127 @@
> +#include "cache.h"
> +#include "hashmap.h"

Useless include, you're not using the hashmap directly.

> +int main(int argc, char **argv)
> +{
> + int i, no_of_files;

Unused no_of_files.

With

CFLAGS += -Wdeclaration-after-statement -Wall -Werror

in config.mak, my git.git refuses to compile with your patch. You should
have similar options for hacking on git.git.

> + if (argc == 3 && !strcmp(argv[1], "get_value")) {

You should do something like

if (argc < 2) {
fprintf(stderr, "Please, provide a command name on the command-line\n");
return 1;
}

before this. Otherwise, some argv[1] below are invalid on mis-use. No
need for thorough checks since it's just a test program, but better
avoid undefined behavior and segfaults anyway...

> + if (!git_config_get_value(argv[2], &v)) {
> + if (!v)
> + printf("(NULL)\n");
> + else
> + printf("%s\n", v);
> + return 0;
> + } else {
> + printf("Value not found for \"%s\"\n", argv[2]);
> + return -1;

Avoid returning negative values from main. Your shell's $? won't see -1,
but most likely 255 or so, but I think it even depends on the OS.

You don't seem to use main's return for anything but error, so 0 =
everything OK; 1 = some error is the common convention.

> + } else {
> + printf("Value not found for \"%s\"\n", argv[2]);
> + return -1;
> + }
> +
> + } else if (!strcmp(argv[1], "configset_get_value_multi")) {

Why a blank line before this "else if" and not before other "else if"s?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 00/28] Support multiple checkouts

[Max Kirillov]

Hi.

What future does this have? Currently it is marked as
"Stalled", but still mergeable with some trivial conflicts
and seem to be working (except some bugs in interaction with
submodules, see below). It would be very nice if this
feature is officially supported.

I also have a comment about how it interacts with submodules.
Would it be more appropriate to mark "modules" as a
per-checkout directory? Because each of the working tree's
submodule is obviously a separated directory in filesystem,
and in most cases (at least in my practice) they are
checked-out to different revisions.

So, currently (before proper linking of submodules checkouts
implemented), if make submodules per-checkout (actually it
appears to somehow work even with current code, maybe
because some submodule code ignores the common_dir), one
could run "git submodule update" if necessary, and get fully
separated clones, which would work normally.

It still may break if submodules are removed, added or
renamed, but this seems to be inevitable until config is
separated to per-checkout and common parts, which I suppose
is a much bigger task.

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

Re: [PATCH v5 2/2] test-config: Add tests for the config_set API

[Ramkumar Ramachandra]

A couple of quick nits.

Tanay Abhra wrote:
> +test_expect_success 'clear default config' '
> +   rm -f .git/config
> +'

Unnecessary; a fresh temporary directory is created for each test run.

> +test_expect_success 'initialize default config' '

You might want to mark this as "setup".
--
To unsubscribe from this list: send the line "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: Test failure in t9814-git-p4-rename.sh - my environment or bad test?

[Pete Wyckoff]

ml.christophbon...@gmail.com wrote on Sun, 06 Jul 2014 16:32 +0200:
> I'm trying to get the git p4 tests to pass on my machine (OS X
> Mavericks) from master before making some changes. I'm experiencing a
> test failure in "detect copies" of the rename test.
> 
> The test creates file2 with some content, creates a few copies (each
> with a commit), then does the following (no git write operations
> omitted):
> echo "file2" >>file2 &&
> cp file2 file10 &&
> git add file2 file10 &&
> git commit -a -m "Modify and copy file2 to file10" &&
> ... (some non-write-operations) ...
> cp file2 file11 &&
> git add file11 &&
> git commit -a -m "Copy file2 to file11" &&
> git diff-tree -r -C --find-copies-harder HEAD &&
> src=$(git diff-tree -r -C --find-copies-harder HEAD | sed 1d | cut -f2) &&
> test "$src" = file10 &&
> 
> This is where it fails on my machine. The git diff-tree output is this
> :100644 100644 22a35c17c4c0779f75142036beef6ccd58525b9c
> 22a35c17c4c0779f75142036beef6ccd58525b9c C100 file2 file11
> so git diff-tree sees file2 as the copy source, not file10. In my
> opinion, the diff-tree result is legitimate (at that point, file2,
> file10 and file11 are identical). Later in the tests, after making
> more copies of file2, the conditions are more flexible, e.g.
> test "$src" = file10 || test "$src" = file11 || test "$src" = file12 &&
> 
> IMO, the test discounts the legitimate possibility of diff-tree
> detecting file2 as source, making unnecessary assumptions about
> implementation details. Is this correct, or do I misunderstand the
> workings of diff-tree?
> 
> I'd be grateful for advice, both on whether this is a bug, and if so,
> which branch to base a patch on.

I think your analysis is correct.  And I agree that later tests
have noticed this ambiguity and added multiple comparisons like
you quote.

I'm not sure how to robustify this.  At least doing the multiple
comparisons should make the tests work again.  The goal of this
series of tests is to make sure that copy detection is working,
not to verify that the correct copy choice was made.  That should
be in other (non-p4) tests.

Do send patches based on Junio's master.  I can ack, and they'll
show up in a future git release.

Thanks!

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

[PATCH v1] rebase -p: Command line option --no-ff is ignored

[Fabian Ruch]

The --no-ff option instructs git-rebase to always recreate commits as
they are being replayed, even if fast-forwards are possible.

However, if git-rebase is asked to recreate merge commits (via the -p
option), it suddenly ignores the --no-ff option and fast-forwards
both normal and merge commits whenever possible.

git-rebase--interactive, which is responsible for recreating merge
commits during a rebase, maintains a variable fast_forward to decide
whether the current replay should be tried as a fast-forward.
Previously, fast_forward was on by default and would get toggled only
if a parent was rewritten or a squash was in effect. Also turn
fast_forward off if --no-ff is in use, which is signalled by
git-rebase through the variable force_rebase.

If --no-ff is not in use, try to fast-forward HEAD using git-reset as
before. In contrast, if --no-ff is in use, replay normal commits
using git-cherry-pick and merge commits using git-merge. Note that
git-rebase--interactive already provides this machinery for enabling
and disabling fast-forwards, controlled by fast_forward being
assigned either t (for boolean true) or f (for boolean false).

As mentioned above, git-rebase--interactive needs to detect when a
squash is in effect. If several commits are squashed into one, each
of them is picked using the git-cherry-pick option -n and they get
all rewritten to the same commit, the squash commit. Previously,
fast_forward was assigned f if and only if -n was specified. This no
longer holds for fast_forward might be turned off due to a use of
--no-ff. To correctly notice squashes, explicitly check for -n.

Add test.

Signed-off-by: Fabian Ruch 
---
Hi,

The code checking force_rebase is copied from pick_one, although
using a ternary operator to initialise fast_forward might be more
readable. Moreover, the code snippet used to detect squash mode is
copied from the f arm of the fast_forward case switch, although the
code base prefers to spell out test(1).

The test recreates a topic branch that merged a second topic branch.
Therefore, the test case tests the recreation of both normal and
merge commits.

Commit b499549 first introduced the --no-ff option to git-rebase and
since then force_rebase seems to respected only by pick_one but not
by its sibling pick_one_preserving_merges. I couldn't find a reason
why. Was pick_one_preserving_merges merely overlooked?

Is it a usability issue that conflicting merges will have to be
resolved again when being replayed now? The same applies to -p and
the replay of merges with rewritten parents. Should the possibly
required resolution be mentioned alongside git-rerere in the
git-rebase manual page?

   Fabian

 git-rebase--interactive.sh|  3 ++-
 t/t3409-rebase-preserve-merges.sh | 12 
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f267d8b..264a768 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -266,10 +266,11 @@ pick_one_preserving_merges () {
;;
esac
sha1=$(git rev-parse $sha1)
+   case "$force_rebase" in '') ;; ?*) fast_forward=f ;; esac
 
if test -f "$state_dir"/current-commit
then
-   if test "$fast_forward" = t
+   if [ "$1" != "-n" ]
then
while read current_commit
do
diff --git a/t/t3409-rebase-preserve-merges.sh 
b/t/t3409-rebase-preserve-merges.sh
index 8c251c5..838937b 100755
--- a/t/t3409-rebase-preserve-merges.sh
+++ b/t/t3409-rebase-preserve-merges.sh
@@ -81,6 +81,18 @@ test_expect_success 'setup for merge-preserving rebase' \
git commit -a -m "Modify B2"
 '
 
+test_expect_success '--no-ff records new commits' '
+   (
+   cd clone3 &&
+   test_when_finished 'cd clone3 && git checkout topic' &&
+   git checkout -b recreated-topic &&
+   # recreate topic with merged topic2 (branching-off point A1)
+   git rebase -p --no-ff HEAD~2 &&
+   test $(git rev-parse new-topic^) != $(git rev-parse topic^) &&
+   test $(git rev-parse new-topic) != $(git rev-parse topic)
+   )
+'
+
 test_expect_success '--continue works after a conflict' '
(
cd clone2 &&
-- 
2.0.0

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

[PATCH v6 00/10] Add --graft option to git replace

[Christian Couder]

Here is a small series to implement:

git replace [-f] --graft  [...]

This patch series goes on top of the patch series that
implements --edit.

The changes since v5, thanks to Junio, are:

- new patch 1/10 to clean up redirection style in t6050

- new patches 8/10, 9/10 and 10/10 to check mergetags

- add functions to test parents in patch 3/10 and 7/10

- improve testing signed commits in patch 7/10

- improve warning when removing commit signature in
  patch 6/10

Christian Couder (10):
  replace: cleanup redirection style in tests
  replace: add --graft option
  replace: add test for --graft
  Documentation: replace: add --graft option
  contrib: add convert-grafts-to-replace-refs.sh
  replace: remove signature when using --graft
  replace: add test for --graft with signed commit
  commit: add for_each_mergetag()
  replace: check mergetags when using --graft
  replace: add test for --graft with a mergetag

 Documentation/git-replace.txt |  10 +++
 builtin/replace.c | 126 +++-
 commit.c  |  47 +++
 commit.h  |   7 ++
 contrib/convert-grafts-to-replace-refs.sh |  28 +++
 log-tree.c|  15 +---
 t/t6050-replace.sh| 135 --
 7 files changed, 332 insertions(+), 36 deletions(-)
 create mode 100755 contrib/convert-grafts-to-replace-refs.sh

-- 
2.0.0.421.g786a89d.dirty

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

[PATCH v6 01/10] replace: cleanup redirection style in tests

[Christian Couder]

Signed-off-by: Christian Couder 
---
 t/t6050-replace.sh | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 68b3cb2..fb07ad2 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -27,36 +27,36 @@ HASH6=
 HASH7=
 
 test_expect_success 'set up buggy branch' '
- echo "line 1" >> hello &&
- echo "line 2" >> hello &&
- echo "line 3" >> hello &&
- echo "line 4" >> hello &&
+ echo "line 1" >>hello &&
+ echo "line 2" >>hello &&
+ echo "line 3" >>hello &&
+ echo "line 4" >>hello &&
  add_and_commit_file hello "4 lines" &&
  HASH1=$(git rev-parse --verify HEAD) &&
- echo "line BUG" >> hello &&
- echo "line 6" >> hello &&
- echo "line 7" >> hello &&
- echo "line 8" >> hello &&
+ echo "line BUG" >>hello &&
+ echo "line 6" >>hello &&
+ echo "line 7" >>hello &&
+ echo "line 8" >>hello &&
  add_and_commit_file hello "4 more lines with a BUG" &&
  HASH2=$(git rev-parse --verify HEAD) &&
- echo "line 9" >> hello &&
- echo "line 10" >> hello &&
+ echo "line 9" >>hello &&
+ echo "line 10" >>hello &&
  add_and_commit_file hello "2 more lines" &&
  HASH3=$(git rev-parse --verify HEAD) &&
- echo "line 11" >> hello &&
+ echo "line 11" >>hello &&
  add_and_commit_file hello "1 more line" &&
  HASH4=$(git rev-parse --verify HEAD) &&
- sed -e "s/BUG/5/" hello > hello.new &&
+ sed -e "s/BUG/5/" hello >hello.new &&
  mv hello.new hello &&
  add_and_commit_file hello "BUG fixed" &&
  HASH5=$(git rev-parse --verify HEAD) &&
- echo "line 12" >> hello &&
- echo "line 13" >> hello &&
+ echo "line 12" >>hello &&
+ echo "line 13" >>hello &&
  add_and_commit_file hello "2 more lines" &&
  HASH6=$(git rev-parse --verify HEAD) &&
- echo "line 14" >> hello &&
- echo "line 15" >> hello &&
- echo "line 16" >> hello &&
+ echo "line 14" >>hello &&
+ echo "line 15" >>hello &&
+ echo "line 16" >>hello &&
  add_and_commit_file hello "again 3 more lines" &&
  HASH7=$(git rev-parse --verify HEAD)
 '
@@ -95,7 +95,7 @@ test_expect_success 'tag replaced commit' '
 '
 
 test_expect_success '"git fsck" works' '
- git fsck master > fsck_master.out &&
+ git fsck master >fsck_master.out &&
  grep "dangling commit $R" fsck_master.out &&
  grep "dangling tag $(cat .git/refs/tags/mytag)" fsck_master.out &&
  test -z "$(git fsck)"
@@ -217,14 +217,14 @@ test_expect_success 'fetch branch with replacement' '
  (
  cd clone_dir &&
  git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
- git log --pretty=oneline parallel3 > output.txt &&
+ git log --pretty=oneline parallel3 >output.txt &&
  ! grep $PARA3 output.txt &&
- git show $PARA3 > para3.txt &&
+ git show $PARA3 >para3.txt &&
  grep "A U Thor" para3.txt &&
  git fetch origin "refs/replace/*:refs/replace/*" &&
- git log --pretty=oneline parallel3 > output.txt &&
+ git log --pretty=oneline parallel3 >output.txt &&
  grep $PARA3 output.txt &&
- git show $PARA3 > para3.txt &&
+ git show $PARA3 >para3.txt &&
  grep "O Thor" para3.txt
  )
 '
@@ -302,7 +302,7 @@ test_expect_success 'test --format medium' '
echo "$PARA3 -> $S" &&
echo "$MYTAG -> $HASH1"
} | sort >expected &&
-   git replace -l --format medium | sort > actual &&
+   git replace -l --format medium | sort >actual &&
test_cmp expected actual
 '
 
@@ -314,7 +314,7 @@ test_expect_success 'test --format long' '
echo "$PARA3 (commit) -> $S (commit)" &&
echo "$MYTAG (tag) -> $HASH1 (commit)"
} | sort >expected &&
-   git replace --format=long | sort > actual &&
+   git replace --format=long | sort >actual &&
test_cmp expected actual
 '
 
-- 
2.0.0.421.g786a89d.dirty


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

[PATCH v6 02/10] replace: add --graft option

[Christian Couder]

The usage string for this option is:

git replace [-f] --graft  [...]

First we create a new commit that is the same as 
except that its parents are [...]

Then we create a replace ref that replace  with
the commit we just created.

With this new option, it should be straightforward to
convert grafts to replace refs.

Signed-off-by: Christian Couder 
Signed-off-by: Junio C Hamano 
---
 builtin/replace.c | 74 ++-
 1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 1bb491d..ad47237 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -17,6 +17,7 @@
 static const char * const git_replace_usage[] = {
N_("git replace [-f]  "),
N_("git replace [-f] --edit "),
+   N_("git replace [-f] --graft  [...]"),
N_("git replace -d ..."),
N_("git replace [--format=] [-l []]"),
NULL
@@ -294,6 +295,66 @@ static int edit_and_replace(const char *object_ref, int 
force)
return replace_object_sha1(object_ref, old, "replacement", new, force);
 }
 
+static void replace_parents(struct strbuf *buf, int argc, const char **argv)
+{
+   struct strbuf new_parents = STRBUF_INIT;
+   const char *parent_start, *parent_end;
+   int i;
+
+   /* find existing parents */
+   parent_start = buf->buf;
+   parent_start += 46; /* "tree " + "hex sha1" + "\n" */
+   parent_end = parent_start;
+
+   while (starts_with(parent_end, "parent "))
+   parent_end += 48; /* "parent " + "hex sha1" + "\n" */
+
+   /* prepare new parents */
+   for (i = 1; i < argc; i++) {
+   unsigned char sha1[20];
+   if (get_sha1(argv[i], sha1) < 0)
+   die(_("Not a valid object name: '%s'"), argv[i]);
+   lookup_commit_or_die(sha1, argv[i]);
+   strbuf_addf(&new_parents, "parent %s\n", sha1_to_hex(sha1));
+   }
+
+   /* replace existing parents with new ones */
+   strbuf_splice(buf, parent_start - buf->buf, parent_end - parent_start,
+ new_parents.buf, new_parents.len);
+
+   strbuf_release(&new_parents);
+}
+
+static int create_graft(int argc, const char **argv, int force)
+{
+   unsigned char old[20], new[20];
+   const char *old_ref = argv[0];
+   struct commit *commit;
+   struct strbuf buf = STRBUF_INIT;
+   const char *buffer;
+   unsigned long size;
+
+   if (get_sha1(old_ref, old) < 0)
+   die(_("Not a valid object name: '%s'"), old_ref);
+   commit = lookup_commit_or_die(old, old_ref);
+
+   buffer = get_commit_buffer(commit, &size);
+   strbuf_add(&buf, buffer, size);
+   unuse_commit_buffer(commit, buffer);
+
+   replace_parents(&buf, argc, argv);
+
+   if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+   die(_("could not write replacement commit for: '%s'"), old_ref);
+
+   strbuf_release(&buf);
+
+   if (!hashcmp(old, new))
+   return error("new commit is the same as the old one: '%s'", 
sha1_to_hex(old));
+
+   return replace_object_sha1(old_ref, old, "replacement", new, force);
+}
+
 int cmd_replace(int argc, const char **argv, const char *prefix)
 {
int force = 0;
@@ -303,12 +364,14 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
MODE_LIST,
MODE_DELETE,
MODE_EDIT,
+   MODE_GRAFT,
MODE_REPLACE
} cmdmode = MODE_UNSPECIFIED;
struct option options[] = {
OPT_CMDMODE('l', "list", &cmdmode, N_("list replace refs"), 
MODE_LIST),
OPT_CMDMODE('d', "delete", &cmdmode, N_("delete replace refs"), 
MODE_DELETE),
OPT_CMDMODE('e', "edit", &cmdmode, N_("edit existing object"), 
MODE_EDIT),
+   OPT_CMDMODE('g', "graft", &cmdmode, N_("change a commit's 
parents"), MODE_GRAFT),
OPT_BOOL('f', "force", &force, N_("replace the ref if it 
exists")),
OPT_STRING(0, "format", &format, N_("format"), N_("use this 
format")),
OPT_END()
@@ -325,7 +388,10 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
usage_msg_opt("--format cannot be used when not listing",
  git_replace_usage, options);
 
-   if (force && cmdmode != MODE_REPLACE && cmdmode != MODE_EDIT)
+   if (force &&
+   cmdmode != MODE_REPLACE &&
+   cmdmode != MODE_EDIT &&
+   cmdmode != MODE_GRAFT)
usage_msg_opt("-f only makes sense when writing a replacement",
  git_replace_usage, options);
 
@@ -348,6 +414,12 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
  git_replace_usage, options);
return edit_and_replace(argv[0], force);
 
+   case MODE_GRAFT:
+   if (

[PATCH v6 06/10] replace: remove signature when using --graft

[Christian Couder]

It could be misleading to keep a signature in a
replacement commit, so let's remove it.

Note that there should probably be a way to sign
the replacement commit created when using --graft,
but this can be dealt with in another commit or
patch series.

Signed-off-by: Christian Couder 
---
 builtin/replace.c |  5 +
 commit.c  | 34 ++
 commit.h  |  2 ++
 3 files changed, 41 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index ad47237..cc29ef2 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -344,6 +344,11 @@ static int create_graft(int argc, const char **argv, int 
force)
 
replace_parents(&buf, argc, argv);
 
+   if (remove_signature(&buf)) {
+   warning(_("the original commit '%s' has a gpg signature."), 
old_ref);
+   warning(_("the signature will be removed in the replacement 
commit!"));
+   }
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_("could not write replacement commit for: '%s'"), old_ref);
 
diff --git a/commit.c b/commit.c
index fb7897c..54e157d 100644
--- a/commit.c
+++ b/commit.c
@@ -1177,6 +1177,40 @@ int parse_signed_commit(const struct commit *commit,
return saw_signature;
 }
 
+int remove_signature(struct strbuf *buf)
+{
+   const char *line = buf->buf;
+   const char *tail = buf->buf + buf->len;
+   int in_signature = 0;
+   const char *sig_start = NULL;
+   const char *sig_end = NULL;
+
+   while (line < tail) {
+   const char *next = memchr(line, '\n', tail - line);
+   next = next ? next + 1 : tail;
+
+   if (in_signature && line[0] == ' ')
+   sig_end = next;
+   else if (starts_with(line, gpg_sig_header) &&
+line[gpg_sig_header_len] == ' ') {
+   sig_start = line;
+   sig_end = next;
+   in_signature = 1;
+   } else {
+   if (*line == '\n')
+   /* dump the whole remainder of the buffer */
+   next = tail;
+   in_signature = 0;
+   }
+   line = next;
+   }
+
+   if (sig_start)
+   strbuf_remove(buf, sig_start - buf->buf, sig_end - sig_start);
+
+   return sig_start != NULL;
+}
+
 static void handle_signed_tag(struct commit *parent, struct 
commit_extra_header ***tail)
 {
struct merge_remote_desc *desc;
diff --git a/commit.h b/commit.h
index 2e1492a..4234dae 100644
--- a/commit.h
+++ b/commit.h
@@ -327,6 +327,8 @@ struct commit *get_merge_parent(const char *name);
 
 extern int parse_signed_commit(const struct commit *commit,
   struct strbuf *message, struct strbuf 
*signature);
+extern int remove_signature(struct strbuf *buf);
+
 extern void print_commit_list(struct commit_list *list,
  const char *format_cur,
  const char *format_last);
-- 
2.0.0.421.g786a89d.dirty


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

[PATCH v6 05/10] contrib: add convert-grafts-to-replace-refs.sh

[Christian Couder]

This patch adds into contrib/ an example script to convert
grafts from an existing grafts file into replace refs using
the new --graft option of "git replace".

While at it let's mention this new script in the
"git replace" documentation for the --graft option.

Signed-off-by: Christian Couder 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-replace.txt |  4 +++-
 contrib/convert-grafts-to-replace-refs.sh | 28 
 2 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100755 contrib/convert-grafts-to-replace-refs.sh

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 491875e..e1be2e6 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -79,7 +79,9 @@ OPTIONS
content as  except that its parents will be
[...] instead of 's parents. A replacement ref
is then created to replace  with the newly created
-   commit.
+   commit. See contrib/convert-grafts-to-replace-refs.sh for an
+   example script based on this option that can convert grafts to
+   replace refs.
 
 -l ::
 --list ::
diff --git a/contrib/convert-grafts-to-replace-refs.sh 
b/contrib/convert-grafts-to-replace-refs.sh
new file mode 100755
index 000..0cbc917
--- /dev/null
+++ b/contrib/convert-grafts-to-replace-refs.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# You should execute this script in the repository where you
+# want to convert grafts to replace refs.
+
+GRAFTS_FILE="${GIT_DIR:-.git}/info/grafts"
+
+. $(git --exec-path)/git-sh-setup
+
+test -f "$GRAFTS_FILE" || die "Could not find graft file: '$GRAFTS_FILE'"
+
+grep '^[^# ]' "$GRAFTS_FILE" |
+while read definition
+do
+   if test -n "$definition"
+   then
+   echo "Converting: $definition"
+   git replace --graft $definition ||
+   die "Conversion failed for: $definition"
+   fi
+done
+
+mv "$GRAFTS_FILE" "$GRAFTS_FILE.bak" ||
+   die "Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak'"
+
+echo "Success!"
+echo "All the grafts in '$GRAFTS_FILE' have been converted to replace refs!"
+echo "The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak'"
-- 
2.0.0.421.g786a89d.dirty


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

[PATCH v6 10/10] replace: add test for --graft with a mergetag

[Christian Couder]

Signed-off-by: Christian Couder 
---
 t/t6050-replace.sh | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 15fd541..3bb8d06 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -416,4 +416,26 @@ test_expect_success GPG '--graft with a signed commit' '
git replace -d $HASH8
 '
 
+test_expect_success GPG 'set up a merge commit with a mergetag' '
+   git reset --hard HEAD &&
+   git checkout -b test_branch HEAD~2 &&
+   echo "line 1 from test branch" >>hello &&
+   echo "line 2 from test branch" >>hello &&
+   git add hello &&
+   test_tick &&
+   git commit -m "hello: 2 more lines from a test branch" &&
+   HASH9=$(git rev-parse --verify HEAD) &&
+   git tag -s -m "tag for testing with a mergetag" test_tag HEAD &&
+   git checkout master &&
+   git merge -s ours test_tag &&
+   HASH10=$(git rev-parse --verify HEAD) &&
+   git cat-file commit $HASH10 | grep "^mergetag object"
+'
+
+test_expect_success GPG '--graft on a commit with a mergetag' '
+   test_must_fail git replace --graft $HASH10 $HASH8^1 &&
+   git replace --graft $HASH10 $HASH8^1 $HASH9 &&
+   git replace -d $HASH10
+'
+
 test_done
-- 
2.0.0.421.g786a89d.dirty

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

[PATCH v6 04/10] Documentation: replace: add --graft option

[Christian Couder]

Signed-off-by: Christian Couder 
Signed-off-by: Junio C Hamano 
---
 Documentation/git-replace.txt | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 61461b9..491875e 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git replace' [-f]  
 'git replace' [-f] --edit 
+'git replace' [-f] --graft  [...]
 'git replace' -d ...
 'git replace' [--format=] [-l []]
 
@@ -73,6 +74,13 @@ OPTIONS
newly created object. See linkgit:git-var[1] for details about
how the editor will be chosen.
 
+--graft  [...]::
+   Create a graft commit. A new commit is created with the same
+   content as  except that its parents will be
+   [...] instead of 's parents. A replacement ref
+   is then created to replace  with the newly created
+   commit.
+
 -l ::
 --list ::
List replace refs for objects that match the given pattern (or
-- 
2.0.0.421.g786a89d.dirty


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

[PATCH v6 07/10] replace: add test for --graft with signed commit

[Christian Couder]

Signed-off-by: Christian Couder 
---
 t/t6050-replace.sh | 25 +
 1 file changed, 25 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d80a89e..15fd541 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -7,6 +7,7 @@ test_description='Tests replace refs functionality'
 exec >hello &&
+   echo "line 18" >>hello &&
+   git add hello &&
+   test_tick &&
+   git commit --quiet -S -m "hello: 2 more lines in a signed commit" &&
+   HASH8=$(git rev-parse --verify HEAD) &&
+   git verify-commit $HASH8
+'
+
+test_expect_success GPG '--graft with a signed commit' '
+   git cat-file commit $HASH8 >orig &&
+   git replace --graft $HASH8 &&
+   git cat-file commit $HASH8 >repl &&
+   commit_buffer_contains_parents $HASH8 &&
+   commit_has_parents $HASH8 &&
+   test_must_fail git verify-commit $HASH8 &&
+   sed -n -e "/^tree /p" -e "/^author /p" -e "/^committer /p" orig 
>expected &&
+   echo >>expected &&
+   sed -e "/^$/q" repl >actual &&
+   test_cmp expected actual &&
+   git replace -d $HASH8
+'
+
 test_done
-- 
2.0.0.421.g786a89d.dirty


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

[PATCH v6 03/10] replace: add test for --graft

[Christian Couder]

Signed-off-by: Christian Couder 
Signed-off-by: Junio C Hamano 
---
 t/t6050-replace.sh | 40 
 1 file changed, 40 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index fb07ad2..d80a89e 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -18,6 +18,33 @@ add_and_commit_file()
 git commit --quiet -m "$_file: $_msg"
 }
 
+commit_buffer_contains_parents()
+{
+git cat-file commit "$1" >payload &&
+sed -n -e '/^$/q' -e '/^parent /p' actual &&
+shift &&
+: >expected &&
+for _parent
+do
+   echo "parent $_parent" >>expected || return 1
+done &&
+test_cmp actual expected
+}
+
+commit_has_parents()
+{
+_parent_number=1
+_commit="$1"
+shift &&
+for _parent
+do
+   _found=$(git rev-parse --verify $_commit^$_parent_number) || return 1
+   test "$_found" = "$_parent" || return 1
+   _parent_number=$(( $_parent_number + 1 ))
+done &&
+test_must_fail git rev-parse --verify $_commit^$_parent_number
+}
+
 HASH1=
 HASH2=
 HASH3=
@@ -351,4 +378,17 @@ test_expect_success 'replace ref cleanup' '
test -z "$(git replace)"
 '
 
+test_expect_success '--graft with and without already replaced object' '
+   test $(git log --oneline | wc -l) = 7 &&
+   git replace --graft $HASH5 &&
+   test $(git log --oneline | wc -l) = 3 &&
+   commit_buffer_contains_parents $HASH5 &&
+   commit_has_parents $HASH5 &&
+   test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 &&
+   git replace --force -g $HASH5 $HASH4 $HASH3 &&
+   commit_buffer_contains_parents $HASH5 $HASH4 $HASH3 &&
+   commit_has_parents $HASH5 $HASH4 $HASH3 &&
+   git replace -d $HASH5
+'
+
 test_done
-- 
2.0.0.421.g786a89d.dirty


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

[PATCH v6 09/10] replace: check mergetags when using --graft

[Christian Couder]

When using --graft, with a mergetag in the original
commit, we should check that the commit pointed to by
the mergetag is still a parent of then new commit we
create, otherwise the mergetag could be misleading.

If the commit pointed to by the mergetag is no more
a parent of the new commit, we could remove the
mergetag, but in this case there is a good chance
that the title or other elements of the commit might
also be misleading. So let's just error out and
suggest to use --edit instead on the commit.

Signed-off-by: Christian Couder 
---
 builtin/replace.c | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index cc29ef2..2290529 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -13,6 +13,7 @@
 #include "refs.h"
 #include "parse-options.h"
 #include "run-command.h"
+#include "tag.h"
 
 static const char * const git_replace_usage[] = {
N_("git replace [-f]  "),
@@ -325,6 +326,50 @@ static void replace_parents(struct strbuf *buf, int argc, 
const char **argv)
strbuf_release(&new_parents);
 }
 
+struct check_mergetag_data {
+   int argc;
+   const char **argv;
+};
+
+static void check_one_mergetag(struct commit *commit,
+  struct commit_extra_header *extra,
+  void *data)
+{
+   struct check_mergetag_data *mergetag_data = (struct check_mergetag_data 
*)data;
+   const char *ref = mergetag_data->argv[0];
+   unsigned char tag_sha1[20];
+   struct tag *tag;
+   int i;
+
+   hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), tag_sha1);
+   tag = lookup_tag(tag_sha1);
+   if (!tag)
+   die(_("bad mergetag in commit '%s'"), ref);
+   if (parse_tag_buffer(tag, extra->value, extra->len))
+   die(_("malformed mergetag in commit '%s'"), ref);
+
+   /* iterate over new parents */
+   for (i = 1; i < mergetag_data->argc; i++) {
+   unsigned char sha1[20];
+   if (get_sha1(mergetag_data->argv[i], sha1) < 0)
+   die(_("Not a valid object name: '%s'"), 
mergetag_data->argv[i]);
+   if (!hashcmp(tag->tagged->sha1, sha1))
+   return; /* found */
+   }
+
+   die(_("original commit '%s' contains mergetag '%s' that is discarded; "
+ "use --edit instead of --graft"), ref, sha1_to_hex(tag_sha1));
+}
+
+static void check_mergetags(struct commit *commit, int argc, const char **argv)
+{
+   struct check_mergetag_data mergetag_data;
+
+   mergetag_data.argc = argc;
+   mergetag_data.argv = argv;
+   for_each_mergetag(check_one_mergetag, commit, &mergetag_data);
+}
+
 static int create_graft(int argc, const char **argv, int force)
 {
unsigned char old[20], new[20];
@@ -349,6 +394,8 @@ static int create_graft(int argc, const char **argv, int 
force)
warning(_("the signature will be removed in the replacement 
commit!"));
}
 
+   check_mergetags(commit, argc, argv);
+
if (write_sha1_file(buf.buf, buf.len, commit_type, new))
die(_("could not write replacement commit for: '%s'"), old_ref);
 
-- 
2.0.0.421.g786a89d.dirty


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

[PATCH v6 08/10] commit: add for_each_mergetag()

[Christian Couder]

In the same way as there is for_each_ref() to
iterate on refs, it might be useful to have
for_each_mergetag() to iterate on the mergetags
of a given commit.

Signed-off-by: Christian Couder 
---
 commit.c   | 13 +
 commit.h   |  5 +
 log-tree.c | 15 ---
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/commit.c b/commit.c
index 54e157d..0f83ff7 100644
--- a/commit.c
+++ b/commit.c
@@ -1348,6 +1348,19 @@ struct commit_extra_header 
*read_commit_extra_headers(struct commit *commit,
return extra;
 }
 
+void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
+{
+   struct commit_extra_header *extra, *to_free;
+
+   to_free = read_commit_extra_headers(commit, NULL);
+   for (extra = to_free; extra; extra = extra->next) {
+   if (strcmp(extra->key, "mergetag"))
+   continue; /* not a merge tag */
+   fn(commit, extra, data);
+   }
+   free_commit_extra_headers(to_free);
+}
+
 static inline int standard_header_field(const char *field, size_t len)
 {
return ((len == 4 && !memcmp(field, "tree ", 5)) ||
diff --git a/commit.h b/commit.h
index 4234dae..c81ba85 100644
--- a/commit.h
+++ b/commit.h
@@ -312,6 +312,11 @@ extern struct commit_extra_header 
*read_commit_extra_headers(struct commit *, co
 
 extern void free_commit_extra_headers(struct commit_extra_header *extra);
 
+typedef void (*each_mergetag_fn)(struct commit *commit, struct 
commit_extra_header *extra,
+void *cb_data);
+
+extern void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void 
*data);
+
 struct merge_remote_desc {
struct object *obj; /* the named object, could be a tag */
const char *name;
diff --git a/log-tree.c b/log-tree.c
index 10e6844..706ed4c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -413,10 +413,11 @@ static int is_common_merge(const struct commit *commit)
&& !commit->parents->next->next);
 }
 
-static void show_one_mergetag(struct rev_info *opt,
+static void show_one_mergetag(struct commit *commit,
  struct commit_extra_header *extra,
- struct commit *commit)
+ void *data)
 {
+   struct rev_info *opt = (struct rev_info *)data;
unsigned char sha1[20];
struct tag *tag;
struct strbuf verify_message;
@@ -463,15 +464,7 @@ static void show_one_mergetag(struct rev_info *opt,
 
 static void show_mergetag(struct rev_info *opt, struct commit *commit)
 {
-   struct commit_extra_header *extra, *to_free;
-
-   to_free = read_commit_extra_headers(commit, NULL);
-   for (extra = to_free; extra; extra = extra->next) {
-   if (strcmp(extra->key, "mergetag"))
-   continue; /* not a merge tag */
-   show_one_mergetag(opt, extra, commit);
-   }
-   free_commit_extra_headers(to_free);
+   for_each_mergetag(show_one_mergetag, commit, opt);
 }
 
 void show_log(struct rev_info *opt)
-- 
2.0.0.421.g786a89d.dirty


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