git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
@ 2012-04-16  6:26 Michal Kiedrowicz
  2012-04-16 14:57 ` Linus Torvalds
  0 siblings, 1 reply; 24+ messages in thread
From: Michal Kiedrowicz @ 2012-04-16  6:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git

Junio C Hamano <gitster@pobox.com> wrote:

> * lt/octopus-simplify (2012-04-12) 1 commit
>  - Make 'git merge' simplify parents earlier
>
> Octopus merge strategy did not reduce heads that are recorded in the
> final commit.  This was done off-list.

Heh, this seems to fix the issue I reported in [1], except... it
doesn't work for the testcase I posted :).  The problem is that this
commit makes Git 'fast-forward' to the first commit from remoteheads,
not from the reduced heads. See:


	$ git init /tmp/merge
	Initialized empty Git repository in /tmp/merge/.git/
	$ cd /tmp/merge
	$ echo a>>a && git add a && git commit -m first
	[master (root-commit) 7422615] first
	 1 file changed, 1 insertion(+)
	 create mode 100644 a
	$ echo a>>a && git add a && git commit -m second
	[master 0ba02e5] second
	 1 file changed, 1 insertion(+)
	$ echo a>>a && git add a && git commit -m third
	[master 9bd11ac] third
	 1 file changed, 1 insertion(+)
	$ git checkout master~2
	HEAD is now at 7422615... first
	# This is OK:
	$ git merge master master~1 
	Updating 7422615..9bd11ac
	Fast-forward
	 a |    2 ++
	 1 file changed, 2 insertions(+)
	$ git checkout master~2
	Previous HEAD position was 9bd11ac... third
	HEAD is now at 7422615... first
	# This is not OK:
	$ git merge master~1 master 
	Updating 7422615..0ba02e5
	Fast-forward
	 a |    1 +
	 1 file changed, 1 insertion(+)

Ths following patch fixes that.

[1] http://permalink.gmane.org/gmane.comp.version-control.git/190625


Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
(if this fix need a signed-off)
---

diff --git a/builtin/merge.c b/builtin/merge.c
index f5947b9..075c99b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1388,13 +1388,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
                if (verbosity >= 0)
                        printf(_("Updating %s..%s\n"),
                                hex,
-                               find_unique_abbrev(remoteheads->item->object.sha1,
+                               find_unique_abbrev(parents->item->object.sha1,
                                DEFAULT_ABBREV));
                strbuf_addstr(&msg, "Fast-forward");
                if (have_message)
                        strbuf_addstr(&msg,
                                " (no commit created; -m option ignored)");
-               commit = remoteheads->item;
+               commit = parents->item;
                if (!commit) {
                        ret = 1;
                        goto done;

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-16  6:26 What's cooking in git.git (Apr 2012, #05; Thu, 12) Michal Kiedrowicz
@ 2012-04-16 14:57 ` Linus Torvalds
  2012-04-16 17:29   ` Junio C Hamano
  2012-04-16 17:36   ` What's cooking in git.git (Apr 2012, #05; Thu, 12) Junio C Hamano
  0 siblings, 2 replies; 24+ messages in thread
From: Linus Torvalds @ 2012-04-16 14:57 UTC (permalink / raw)
  To: Michal Kiedrowicz; +Cc: Junio C Hamano, git

On Sun, Apr 15, 2012 at 11:26 PM, Michal Kiedrowicz
<michal.kiedrowicz@gmail.com> wrote:
>
> Heh, this seems to fix the issue I reported in [1], except... it
> doesn't work for the testcase I posted :).  The problem is that this
> commit makes Git 'fast-forward' to the first commit from remoteheads,
> not from the reduced heads.

Ack, good catch.

Thinking some more about this thing, I think we have a similar issue
with the "Already up-to-date." thing.

It too had the "only one remote-head" test, which is wrong - what if
you try to do a octopus merge with *two* commits that are different,
and are both in the past? It will fail the "Already up-to-date" test,
and then do a "fast-forward" to the first remote parent, if I read the
code right..

So I think the "Already up-to-date" case should also be fixed, and in
fact it becomes much more natural now that we have finalized the
parents: we just check whether the one remaining parent is the same as
HEAD.

So  Ack on Michal's patch, but I think we also should do the appended
equivalent thing for the fast-forward test on top of it (it's
white-space damaged, sorry).

Trivially tested with

    git merge HEAD^ HEAD^^

which did the wrong thing before, and now works.

                       Linus

---
 builtin/merge.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 016a4dbee3b5..28fb5c9d6ada 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1370,8 +1370,7 @@ int cmd_merge(int argc, const char **argv, const
char *prefix)

        if (!common)
                ; /* No common ancestors found. We need a real merge. */
-       else if (!remoteheads->next && !common->next &&
-                       common->item == remoteheads->item) {
+       else if (!parents->next && parents->item == head_commit) {
                /*
                 * If head can reach all the merge then we are up to date.
                 * but first the most common case of merging one remote.

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-16 14:57 ` Linus Torvalds
@ 2012-04-16 17:29   ` Junio C Hamano
  2012-04-16 17:50     ` Linus Torvalds
  2012-04-16 17:36   ` What's cooking in git.git (Apr 2012, #05; Thu, 12) Junio C Hamano
  1 sibling, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2012-04-16 17:29 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Michal Kiedrowicz, git

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Trivially tested with
>
>     git merge HEAD^ HEAD^^
>
> which did the wrong thing before, and now works.
>
>                        Linus
>
> ---
>  builtin/merge.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 016a4dbee3b5..28fb5c9d6ada 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1370,8 +1370,7 @@ int cmd_merge(int argc, const char **argv, const
> char *prefix)
>
>         if (!common)
>                 ; /* No common ancestors found. We need a real merge. */
> -       else if (!remoteheads->next && !common->next &&
> -                       common->item == remoteheads->item) {
> +       else if (!parents->next && parents->item == head_commit) {

When everybody in remote_heads is an ancestor of the current HEAD,
finalize_parents() would have reduced parents to a single element list
with HEAD on it, and we are "already up-to-date".  Ok.

I wonder if use of remoteheads later in the same function are correct,
though.  We equate "!remoteheads->next" and "We are not doing octopus",
for example.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-16 14:57 ` Linus Torvalds
  2012-04-16 17:29   ` Junio C Hamano
@ 2012-04-16 17:36   ` Junio C Hamano
  2012-04-16 18:02     ` Linus Torvalds
  1 sibling, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2012-04-16 17:36 UTC (permalink / raw)
  To: Michal Kiedrowicz, Linus Torvalds; +Cc: git

Linus Torvalds <torvalds@linux-foundation.org> writes:

> So  Ack on Michal's patch, but I think we also should do the appended
> equivalent thing for the fast-forward test on top of it (it's
> white-space damaged, sorry).
>
> Trivially tested with
>
>     git merge HEAD^ HEAD^^
>
> which did the wrong thing before, and now works.

And it seems to break 6028 ("merge -s ours" and "merge -s subtree"
up-to-date) X-<....

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-16 17:29   ` Junio C Hamano
@ 2012-04-16 17:50     ` Linus Torvalds
  2012-04-16 22:03       ` Junio C Hamano
  2012-04-17 20:34       ` [PATCH 0/4] merge: reduce set of parents consistently Junio C Hamano
  0 siblings, 2 replies; 24+ messages in thread
From: Linus Torvalds @ 2012-04-16 17:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michal Kiedrowicz, git

On Mon, Apr 16, 2012 at 10:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> I wonder if use of remoteheads later in the same function are correct,
> though.  We equate "!remoteheads->next" and "We are not doing octopus",
> for example.

I do think it would generally be a great idea to never use
"remoteheads" at all. An octopus merge that has been simplified to
just two parents isn't really an octopus merge any more.

So I think you're probably right - we should try to avoid using
remoteheads entirely, and any use is suspect

                 Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-16 17:36   ` What's cooking in git.git (Apr 2012, #05; Thu, 12) Junio C Hamano
@ 2012-04-16 18:02     ` Linus Torvalds
  2012-04-16 18:33       ` Linus Torvalds
  0 siblings, 1 reply; 24+ messages in thread
From: Linus Torvalds @ 2012-04-16 18:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michal Kiedrowicz, git

On Mon, Apr 16, 2012 at 10:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> And it seems to break 6028 ("merge -s ours" and "merge -s subtree"
> up-to-date) X-<....

Ugh. I don't even get that far - I get to t3410, which breaks.

(This is with both Michal's and my patch applied)

Oddly, running that test in verbose mode seems to imply that it's the
*rebase* that succeeds, not the merges in that test. Maybe I'm reading
the test results wrong, I didn't really try to understand the test
itself ;(

                        Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-16 18:02     ` Linus Torvalds
@ 2012-04-16 18:33       ` Linus Torvalds
  2012-04-16 21:32         ` Michał Kiedrowicz
  0 siblings, 1 reply; 24+ messages in thread
From: Linus Torvalds @ 2012-04-16 18:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michal Kiedrowicz, git

On Mon, Apr 16, 2012 at 11:02 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Oddly, running that test in verbose mode seems to imply that it's the
> *rebase* that succeeds, not the merges in that test. Maybe I'm reading
> the test results wrong, I didn't really try to understand the test
> itself ;(

Yes, it's the rebase that succeeds. "git log -g" in the trash
directory shows that we ended up successfully rebasing J2:

  commit 5fc34ec1a8ed96664198fefc74121cd052b10861
  Reflog: HEAD@{1} (C O Mitter <committer@example.com>)
  Reflog message: rebase -i (pick): Merge made by the 'recursive' strategy.
  Author: A U Thor <author@example.com>
  Date:   Thu Apr 7 15:28:13 2005 -0700

      J2

while a successful test will fail that.

However, I don't actually see what changed.

Oh - one thing to note is that the *patch* of that successful rebase
is empty. That may be the big clue: we successfully finish the merge
without noticing that it didn't change any state, and we should have
failed it as an empty commit. Hmm?

                   Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-16 18:33       ` Linus Torvalds
@ 2012-04-16 21:32         ` Michał Kiedrowicz
  2012-04-17  1:22           ` Linus Torvalds
  0 siblings, 1 reply; 24+ messages in thread
From: Michał Kiedrowicz @ 2012-04-16 21:32 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git

Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Mon, Apr 16, 2012 at 11:02 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Oddly, running that test in verbose mode seems to imply that it's the
> > *rebase* that succeeds, not the merges in that test. Maybe I'm reading
> > the test results wrong, I didn't really try to understand the test
> > itself ;(
> 
> Yes, it's the rebase that succeeds. "git log -g" in the trash
> directory shows that we ended up successfully rebasing J2:
> 
>   commit 5fc34ec1a8ed96664198fefc74121cd052b10861
>   Reflog: HEAD@{1} (C O Mitter <committer@example.com>)
>   Reflog message: rebase -i (pick): Merge made by the 'recursive' strategy.
>   Author: A U Thor <author@example.com>
>   Date:   Thu Apr 7 15:28:13 2005 -0700
> 
>       J2
> 
> while a successful test will fail that.
> 
> However, I don't actually see what changed.
> 
> Oh - one thing to note is that the *patch* of that successful rebase
> is empty. That may be the big clue: we successfully finish the merge
> without noticing that it didn't change any state, and we should have
> failed it as an empty commit. Hmm?
> 
>                    Linus

So, the difference is that `git merge --no-ff HEAD^` used to work, now
it doesn't because we reduce_heads() only if we allow fast-forward (and
even though there is just one remote we merge with, parents contains
two commits). So what about that trivial patch instead (discarding our
previous patches)?
---
diff --git a/builtin/merge.c b/builtin/merge.c
index 08e01e8..27e0026 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1346,6 +1346,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			allow_trivial = 0;
 	}
 
+	remoteheads = reduce_heads(remoteheads);
+
 	if (!remoteheads->next)
 		common = get_merge_bases(head_commit, remoteheads->item, 1);
 	else {

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-16 17:50     ` Linus Torvalds
@ 2012-04-16 22:03       ` Junio C Hamano
  2012-04-17 20:34       ` [PATCH 0/4] merge: reduce set of parents consistently Junio C Hamano
  1 sibling, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2012-04-16 22:03 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Michal Kiedrowicz, git

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Mon, Apr 16, 2012 at 10:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> I wonder if use of remoteheads later in the same function are correct,
>> though.  We equate "!remoteheads->next" and "We are not doing octopus",
>> for example.
>
> I do think it would generally be a great idea to never use
> "remoteheads" at all. An octopus merge that has been simplified to
> just two parents isn't really an octopus merge any more.
>
> So I think you're probably right - we should try to avoid using
> remoteheads entirely, and any use is suspect

I am still looking at the codepaths involved.  It looks feasible, but
would affect quite a lot of them to deal with many corner cases, I am
afraid.

One of the worst is the "traditional merge format" where format-merge-msg
is called outside "git merge" to prepare the merge message created by "git
pull".  We haven't reduced heads at that stage yet, so the message fed to
us will list what happened on the redundant branches.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-16 21:32         ` Michał Kiedrowicz
@ 2012-04-17  1:22           ` Linus Torvalds
  2012-04-17 18:25             ` [PATCH] git-merge: Reduce heads before trying to merge them Michał Kiedrowicz
  0 siblings, 1 reply; 24+ messages in thread
From: Linus Torvalds @ 2012-04-17  1:22 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: Junio C Hamano, git

2012/4/16 Michał Kiedrowicz <michal.kiedrowicz@gmail.com>:
>
> So, the difference is that `git merge --no-ff HEAD^` used to work, now
> it doesn't because we reduce_heads() only if we allow fast-forward (and
> even though there is just one remote we merge with, parents contains
> two commits). So what about that trivial patch instead (discarding our
> previous patches)?

Yes, I suspect this might work.

And I like how reducing the remoteheads should also automatically fix
the case of the commit messages containing redundant information when
you give the same branch multiple times (assuming you have the whole
merge log thing enabled).

                     Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH] git-merge: Reduce heads before trying to merge them
  2012-04-17  1:22           ` Linus Torvalds
@ 2012-04-17 18:25             ` Michał Kiedrowicz
  2012-04-17 18:52               ` Junio C Hamano
  0 siblings, 1 reply; 24+ messages in thread
From: Michał Kiedrowicz @ 2012-04-17 18:25 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git, Michał Kiedrowicz

This makes us do proper fast-forward merges even for octopus merges,
which could otherwise result in "merge commit" that only had one actual
parent, and should have been a fast-forward.

Odd-case-triggered-by: James Morris <jmorris@namei.org>
Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
---

This is the proposed fix sent as a proper patch with commit message
stolen from Linus and testcases.

I'm not sure if I don't introduce a memleak with the call to
reduce_heads() but other callers seem to not care, just like whole
cmd_merge().

 builtin/merge.c               |    3 +++
 t/t7603-merge-reduce-heads.sh |   19 +++++++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 08e01e8..2d5930f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1346,6 +1346,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			allow_trivial = 0;
 	}
 
+	if (remoteheads->next)
+		remoteheads = reduce_heads(remoteheads);
+
 	if (!remoteheads->next)
 		common = get_merge_bases(head_commit, remoteheads->item, 1);
 	else {
diff --git a/t/t7603-merge-reduce-heads.sh b/t/t7603-merge-reduce-heads.sh
index 7e17eb4..a3b08a6 100755
--- a/t/t7603-merge-reduce-heads.sh
+++ b/t/t7603-merge-reduce-heads.sh
@@ -113,4 +113,23 @@ test_expect_success 'verify merge result' '
 	test $(git rev-parse HEAD^1) = $(git rev-parse E2) &&
 	test $(git rev-parse HEAD^2) = $(git rev-parse I2)
 '
+
+test_expect_success 'fast-forward to redundant refs' '
+	git reset --hard c0 &&
+	git merge c4 c5
+'
+
+test_expect_success 'verify merge result' '
+	test $(git rev-parse HEAD) = $(git rev-parse c5)
+'
+
+test_expect_success 'merge up-to-date redundant refs' '
+	git reset --hard c5 &&
+	git merge c0 c4
+'
+
+test_expect_success 'verify merge result' '
+	test $(git rev-parse HEAD) = $(git rev-parse c5)
+'
+
 test_done
-- 
1.7.8.4

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] git-merge: Reduce heads before trying to merge them
  2012-04-17 18:25             ` [PATCH] git-merge: Reduce heads before trying to merge them Michał Kiedrowicz
@ 2012-04-17 18:52               ` Junio C Hamano
  2012-04-17 20:09                 ` Linus Torvalds
  0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2012-04-17 18:52 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: Linus Torvalds, git

Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:

> diff --git a/builtin/merge.c b/builtin/merge.c
> index 08e01e8..2d5930f 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1346,6 +1346,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  			allow_trivial = 0;
>  	}
>  
> +	if (remoteheads->next)
> +		remoteheads = reduce_heads(remoteheads);
> +

If your current HEAD is an ancestor of one of the commit on that list, the
above does not omit it from the parent list of the resulting merge commit,
but if you performed the same merge while on one of the commit being
merged, your current HEAD will be excluded with reduce_heads(), which
would mean that you will end up recording a different history even though
a merge is supposed to be symmetrical.

In other words, isn't any solution that calls reduce_heads() only on
remoteheads fundamentally wrong and merely papering over the problem?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] git-merge: Reduce heads before trying to merge them
  2012-04-17 18:52               ` Junio C Hamano
@ 2012-04-17 20:09                 ` Linus Torvalds
  2012-04-17 20:48                   ` Junio C Hamano
  0 siblings, 1 reply; 24+ messages in thread
From: Linus Torvalds @ 2012-04-17 20:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michał Kiedrowicz, git

2012/4/17 Junio C Hamano <gitster@pobox.com>:
>
> If your current HEAD is an ancestor of one of the commit on that list, the
> above does not omit it from the parent list of the resulting merge commit,
> but if you performed the same merge while on one of the commit being
> merged, your current HEAD will be excluded with reduce_heads(), which
> would mean that you will end up recording a different history even though
> a merge is supposed to be symmetrical.
>
> In other words, isn't any solution that calls reduce_heads() only on
> remoteheads fundamentally wrong and merely papering over the problem?

I think Michał's patch, together with my original one (but not the
fixups later) is actually the right thing to do.

Michał's patch fixes the "log shown multiple times" problem. It also
turns a certain class of octopus merges into trivial common merges,
which is good.

So I'd suggest:
 - undo the two top commits from lt/octopus-simplify
 - apply Michał's patch on top of the remaining one commit

It's not perfect, and I really think we could simplify things a bit
more here, but I think the two commits together fix the problems in
practice.

Hmm?

                     Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/4] merge: reduce set of parents consistently
  2012-04-16 17:50     ` Linus Torvalds
  2012-04-16 22:03       ` Junio C Hamano
@ 2012-04-17 20:34       ` Junio C Hamano
  2012-04-17 20:34         ` [PATCH 1/4] git-merge: test octopus with redundant parents Junio C Hamano
                           ` (3 more replies)
  1 sibling, 4 replies; 24+ messages in thread
From: Junio C Hamano @ 2012-04-17 20:34 UTC (permalink / raw)
  To: git

So this is my attempt to address "Eek, why does your Octopus record many
irrelevant parents?!?!?!" issue from Linus and Michał (but without reusing
any code from them, except for the initial test).

In the original code, the variable remoteheads held the list of parents
read from the command line, and it was used throughout the code to see how
many parents the resulting merge will have (used to see if we use twohead
or octopus strategy), except that right before we come up with the set of
parents to record in the resulting merge commit, we discarded redundant
ones.

The updated code instead reduces the parents right after we read them.
This even detects the case where our HEAD is an ancestor of one of the
commits being merged, in which case HEAD will not be recorded unless we
are deliberately recording a fast-forward case as a real merge.

Junio C Hamano (3):
  builtin/merge.c: remove "remoteheads" global variable
  builtin/merge.c: collect other parents early
  builtin/merge.c: reduce parents early

Michał Kiedrowicz (1):
  git-merge: test octopus with redundant parents

 builtin/merge.c               |  142 +++++++++++++++++++++++++----------------
 t/t6028-merge-up-to-date.sh   |   17 ++++-
 t/t7602-merge-octopus-many.sh |   10 ++-
 3 files changed, 108 insertions(+), 61 deletions(-)

-- 
1.7.10.332.g1863c

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/4] git-merge: test octopus with redundant parents
  2012-04-17 20:34       ` [PATCH 0/4] merge: reduce set of parents consistently Junio C Hamano
@ 2012-04-17 20:34         ` Junio C Hamano
  2012-04-17 20:34         ` [PATCH 2/4] builtin/merge.c: remove "remoteheads" global variable Junio C Hamano
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2012-04-17 20:34 UTC (permalink / raw)
  To: git; +Cc: Michał Kiedrowicz

From: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>

This happens when git merge is run to merge multiple commits that are
descendants of current HEAD (or are HEAD).  We've hit this while updating
master to origin/master but accidentaly we called (while being on master):

	$ git merge master origin/master

Here is a minimal testcase:

	$ git init a && cd a
	$ echo a >a && git add a
	$ git commit -minitial
	$ echo b >a && git add a
	$ git commit -msecond
	$ git checkout master^

	$ git merge master master
	Fast-forwarding to: master
	Already up-to-date with master
	Merge made by the 'octopus' strategy.
	 a |    2 +-
	  1 files changed, 1 insertions(+), 1 deletions(-)

	$ git cat-file commit HEAD
	tree eebfed94e75e7760540d1485c740902590a00332
	parent bd679e85202280b263e20a57639a142fa14c2c64
	author Michał Kiedrowicz <michal.kiedrowicz@gmail.com> 1329132996 +0100
	committer Michał Kiedrowicz <michal.kiedrowicz@gmail.com> 1329132996 +0100

	Merge branches 'master' and 'master' into HEAD

Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t6028-merge-up-to-date.sh |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/t/t6028-merge-up-to-date.sh b/t/t6028-merge-up-to-date.sh
index a91644e..824fca5 100755
--- a/t/t6028-merge-up-to-date.sh
+++ b/t/t6028-merge-up-to-date.sh
@@ -16,7 +16,12 @@ test_expect_success setup '
 	test_tick &&
 	git commit -m second &&
 	git tag c1 &&
-	git branch test
+	git branch test &&
+	echo third >file &&
+	git add file &&
+	test_tick &&
+	git commit -m third &&
+	git tag c2
 '
 
 test_expect_success 'merge -s recursive up-to-date' '
@@ -74,4 +79,14 @@ test_expect_success 'merge -s subtree up-to-date' '
 
 '
 
+test_expect_failure 'merge fast-forward octopus' '
+
+	git reset --hard c0 &&
+	test_tick &&
+	git merge c1 c2
+	expect=$(git rev-parse c2) &&
+	current=$(git rev-parse HEAD) &&
+	test "$expect" = "$current"
+'
+
 test_done
-- 
1.7.10.332.g1863c

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/4] builtin/merge.c: remove "remoteheads" global variable
  2012-04-17 20:34       ` [PATCH 0/4] merge: reduce set of parents consistently Junio C Hamano
  2012-04-17 20:34         ` [PATCH 1/4] git-merge: test octopus with redundant parents Junio C Hamano
@ 2012-04-17 20:34         ` Junio C Hamano
  2012-04-17 20:34         ` [PATCH 3/4] builtin/merge.c: collect other parents early Junio C Hamano
  2012-04-17 20:34         ` [PATCH 4/4] builtin/merge.c: reduce " Junio C Hamano
  3 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2012-04-17 20:34 UTC (permalink / raw)
  To: git

Instead pass it around starting from the toplevel cmd_merge()
as an explicit parameter.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c |   51 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 5126443..c5ca70b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -52,7 +52,6 @@ static int fast_forward_only, option_edit = -1;
 static int allow_trivial = 1, have_message;
 static int overwrite_ignore = 1;
 static struct strbuf merge_msg = STRBUF_INIT;
-static struct commit_list *remoteheads;
 static struct strategy **use_strategies;
 static size_t use_strategies_nr, use_strategies_alloc;
 static const char **xopts;
@@ -318,7 +317,7 @@ static void finish_up_to_date(const char *msg)
 	drop_save();
 }
 
-static void squash_message(struct commit *commit)
+static void squash_message(struct commit *commit, struct commit_list *remoteheads)
 {
 	struct rev_info rev;
 	struct strbuf out = STRBUF_INIT;
@@ -366,6 +365,7 @@ static void squash_message(struct commit *commit)
 }
 
 static void finish(struct commit *head_commit,
+		   struct commit_list *remoteheads,
 		   const unsigned char *new_head, const char *msg)
 {
 	struct strbuf reflog_message = STRBUF_INIT;
@@ -380,7 +380,7 @@ static void finish(struct commit *head_commit,
 			getenv("GIT_REFLOG_ACTION"), msg);
 	}
 	if (squash) {
-		squash_message(head_commit);
+		squash_message(head_commit, remoteheads);
 	} else {
 		if (verbosity >= 0 && !merge_msg.len)
 			printf(_("No merge message -- not updating HEAD\n"));
@@ -681,6 +681,7 @@ int try_merge_command(const char *strategy, size_t xopts_nr,
 }
 
 static int try_merge_strategy(const char *strategy, struct commit_list *common,
+			      struct commit_list *remoteheads,
 			      struct commit *head, const char *head_arg)
 {
 	int index_fd;
@@ -874,14 +875,14 @@ static void read_merge_msg(struct strbuf *msg)
 		die_errno(_("Could not read from '%s'"), filename);
 }
 
-static void write_merge_state(void);
-static void abort_commit(const char *err_msg)
+static void write_merge_state(struct commit_list *);
+static void abort_commit(struct commit_list *remoteheads, const char *err_msg)
 {
 	if (err_msg)
 		error("%s", err_msg);
 	fprintf(stderr,
 		_("Not committing merge; use 'git commit' to complete the merge.\n"));
-	write_merge_state();
+	write_merge_state(remoteheads);
 	exit(1);
 }
 
@@ -892,7 +893,7 @@ N_("Please enter a commit message to explain why this merge is necessary,\n"
    "Lines starting with '#' will be ignored, and an empty message aborts\n"
    "the commit.\n");
 
-static void prepare_to_commit(void)
+static void prepare_to_commit(struct commit_list *remoteheads)
 {
 	struct strbuf msg = STRBUF_INIT;
 	const char *comment = _(merge_editor_comment);
@@ -905,18 +906,18 @@ static void prepare_to_commit(void)
 		 git_path("MERGE_MSG"), "merge", NULL, NULL);
 	if (option_edit) {
 		if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
-			abort_commit(NULL);
+			abort_commit(remoteheads, NULL);
 	}
 	read_merge_msg(&msg);
 	stripspace(&msg, option_edit);
 	if (!msg.len)
-		abort_commit(_("Empty commit message."));
+		abort_commit(remoteheads, _("Empty commit message."));
 	strbuf_release(&merge_msg);
 	strbuf_addbuf(&merge_msg, &msg);
 	strbuf_release(&msg);
 }
 
-static int merge_trivial(struct commit *head)
+static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 {
 	unsigned char result_tree[20], result_commit[20];
 	struct commit_list *parent = xmalloc(sizeof(*parent));
@@ -927,17 +928,18 @@ static int merge_trivial(struct commit *head)
 	parent->next = xmalloc(sizeof(*parent->next));
 	parent->next->item = remoteheads->item;
 	parent->next->next = NULL;
-	prepare_to_commit();
+	prepare_to_commit(remoteheads);
 	if (commit_tree(&merge_msg, result_tree, parent, result_commit, NULL,
 			sign_commit))
 		die(_("failed to write commit object"));
-	finish(head, result_commit, "In-index merge");
+	finish(head, remoteheads, result_commit, "In-index merge");
 	drop_save();
 	return 0;
 }
 
 static int finish_automerge(struct commit *head,
 			    struct commit_list *common,
+			    struct commit_list *remoteheads,
 			    unsigned char *result_tree,
 			    const char *wt_strategy)
 {
@@ -959,13 +961,13 @@ static int finish_automerge(struct commit *head,
 			pptr = &commit_list_insert(j->item, pptr)->next;
 	}
 	strbuf_addch(&merge_msg, '\n');
-	prepare_to_commit();
+	prepare_to_commit(remoteheads);
 	free_commit_list(remoteheads);
 	if (commit_tree(&merge_msg, result_tree, parents, result_commit,
 			NULL, sign_commit))
 		die(_("failed to write commit object"));
 	strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
-	finish(head, result_commit, buf.buf);
+	finish(head, remoteheads, result_commit, buf.buf);
 	strbuf_release(&buf);
 	drop_save();
 	return 0;
@@ -1070,7 +1072,7 @@ static int setup_with_upstream(const char ***argv)
 	return i;
 }
 
-static void write_merge_state(void)
+static void write_merge_state(struct commit_list *remoteheads)
 {
 	const char *filename;
 	int fd;
@@ -1148,6 +1150,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
 	struct commit_list *common = NULL;
 	const char *best_strategy = NULL, *wt_strategy = NULL;
+	struct commit_list *remoteheads = NULL;
 	struct commit_list **remotes = &remoteheads;
 	void *branch_to_free;
 
@@ -1400,7 +1403,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			goto done;
 		}
 
-		finish(head_commit, commit->object.sha1, msg.buf);
+		finish(head_commit, remoteheads, commit->object.sha1, msg.buf);
 		drop_save();
 		goto done;
 	} else if (!remoteheads->next && common->next)
@@ -1422,7 +1425,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			if (!read_tree_trivial(common->item->object.sha1,
 					       head_commit->object.sha1,
 					       remoteheads->item->object.sha1)) {
-				ret = merge_trivial(head_commit);
+				ret = merge_trivial(head_commit, remoteheads);
 				goto done;
 			}
 			printf(_("Nope.\n"));
@@ -1493,7 +1496,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		wt_strategy = use_strategies[i]->name;
 
 		ret = try_merge_strategy(use_strategies[i]->name,
-					 common, head_commit, head_arg);
+					 common, remoteheads,
+					 head_commit, head_arg);
 		if (!option_commit && !ret) {
 			merge_was_ok = 1;
 			/*
@@ -1535,8 +1539,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * auto resolved the merge cleanly.
 	 */
 	if (automerge_was_ok) {
-		ret = finish_automerge(head_commit, common, result_tree,
-				       wt_strategy);
+		ret = finish_automerge(head_commit, common, remoteheads,
+				       result_tree, wt_strategy);
 		goto done;
 	}
 
@@ -1561,13 +1565,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		restore_state(head_commit->object.sha1, stash);
 		printf(_("Using the %s to prepare resolving by hand.\n"),
 			best_strategy);
-		try_merge_strategy(best_strategy, common, head_commit, head_arg);
+		try_merge_strategy(best_strategy, common, remoteheads,
+				   head_commit, head_arg);
 	}
 
 	if (squash)
-		finish(head_commit, NULL, NULL);
+		finish(head_commit, remoteheads, NULL, NULL);
 	else
-		write_merge_state();
+		write_merge_state(remoteheads);
 
 	if (merge_was_ok)
 		fprintf(stderr, _("Automatic merge went well; "
-- 
1.7.10.332.g1863c

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/4] builtin/merge.c: collect other parents early
  2012-04-17 20:34       ` [PATCH 0/4] merge: reduce set of parents consistently Junio C Hamano
  2012-04-17 20:34         ` [PATCH 1/4] git-merge: test octopus with redundant parents Junio C Hamano
  2012-04-17 20:34         ` [PATCH 2/4] builtin/merge.c: remove "remoteheads" global variable Junio C Hamano
@ 2012-04-17 20:34         ` Junio C Hamano
  2012-04-17 20:34         ` [PATCH 4/4] builtin/merge.c: reduce " Junio C Hamano
  3 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2012-04-17 20:34 UTC (permalink / raw)
  To: git

Move the code around to populate remoteheads list early in the process
before any decision regarding twohead vs octopus and fast-forwardness is
made.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c |   40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index c5ca70b..2cef2f6 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1137,6 +1137,21 @@ static int default_edit_option(void)
 		st_stdin.st_mode == st_stdout.st_mode);
 }
 
+static struct commit_list *collect_parents(int argc, const char **argv)
+{
+	int i;
+	struct commit_list *remoteheads = NULL;
+	struct commit_list **remotes = &remoteheads;
+
+	for (i = 0; i < argc; i++) {
+		struct commit *commit = get_merge_parent(argv[i]);
+		if (!commit)
+			die(_("%s - not something we can merge"), argv[i]);
+		remotes = &commit_list_insert(commit, remotes)->next;
+	}
+	*remotes = NULL;
+	return remoteheads;
+}
 
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
@@ -1150,8 +1165,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
 	struct commit_list *common = NULL;
 	const char *best_strategy = NULL, *wt_strategy = NULL;
-	struct commit_list *remoteheads = NULL;
-	struct commit_list **remotes = &remoteheads;
+	struct commit_list *remoteheads, *p;
 	void *branch_to_free;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -1256,6 +1270,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		head_arg = argv[1];
 		argv += 2;
 		argc -= 2;
+		remoteheads = collect_parents(argc, argv);
 	} else if (!head_commit) {
 		struct commit *remote_head;
 		/*
@@ -1271,7 +1286,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (!allow_fast_forward)
 			die(_("Non-fast-forward commit does not make sense into "
 			    "an empty head"));
-		remote_head = get_merge_parent(argv[0]);
+		remoteheads = collect_parents(argc, argv);
+		remote_head = remoteheads->item;
 		if (!remote_head)
 			die(_("%s - not something we can merge"), argv[0]);
 		read_empty(remote_head->object.sha1, 0);
@@ -1289,8 +1305,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * the standard merge summary message to be appended
 		 * to the given message.
 		 */
-		for (i = 0; i < argc; i++)
-			merge_name(argv[i], &merge_names);
+		remoteheads = collect_parents(argc, argv);
+		for (p = remoteheads; p; p = p->next)
+			merge_name(merge_remote_util(p->item)->name, &merge_names);
 
 		if (!have_message || shortlog_len) {
 			struct fmt_merge_msg_opts opts;
@@ -1309,19 +1326,16 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			builtin_merge_options);
 
 	strbuf_addstr(&buf, "merge");
-	for (i = 0; i < argc; i++)
-		strbuf_addf(&buf, " %s", argv[i]);
+	for (p = remoteheads; p; p = p->next)
+		strbuf_addf(&buf, " %s", merge_remote_util(p->item)->name);
 	setenv("GIT_REFLOG_ACTION", buf.buf, 0);
 	strbuf_reset(&buf);
 
-	for (i = 0; i < argc; i++) {
-		struct commit *commit = get_merge_parent(argv[i]);
-		if (!commit)
-			die(_("%s - not something we can merge"), argv[i]);
-		remotes = &commit_list_insert(commit, remotes)->next;
+	for (p = remoteheads; p; p = p->next) {
+		struct commit *commit = p->item;
 		strbuf_addf(&buf, "GITHEAD_%s",
 			    sha1_to_hex(commit->object.sha1));
-		setenv(buf.buf, argv[i], 1);
+		setenv(buf.buf, merge_remote_util(commit)->name, 1);
 		strbuf_reset(&buf);
 		if (!fast_forward_only &&
 		    merge_remote_util(commit) &&
-- 
1.7.10.332.g1863c

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 4/4] builtin/merge.c: reduce parents early
  2012-04-17 20:34       ` [PATCH 0/4] merge: reduce set of parents consistently Junio C Hamano
                           ` (2 preceding siblings ...)
  2012-04-17 20:34         ` [PATCH 3/4] builtin/merge.c: collect other parents early Junio C Hamano
@ 2012-04-17 20:34         ` Junio C Hamano
  3 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2012-04-17 20:34 UTC (permalink / raw)
  To: git

Instead of waiting until we record the parents of resulting merge, reduce
redundant parents (including our HEAD) immediately after reading them.

The change to t7602 illustrates the essence of the effect of this change.
The octopus merge strategy used to be fed with redundant commits only to
discard them as "up-to-date", but we no longer feed such redundant commits
to it and the affected test degenerates to a regular two-head merge.

And obviously the known-to-be-broken test in t7602 is now fixed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c               |   65 +++++++++++++++++++++++++----------------
 t/t6028-merge-up-to-date.sh   |    2 +-
 t/t7602-merge-octopus-many.sh |   10 +++----
 3 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 2cef2f6..20aeca0 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -938,31 +938,22 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 }
 
 static int finish_automerge(struct commit *head,
+			    int head_subsumed,
 			    struct commit_list *common,
 			    struct commit_list *remoteheads,
 			    unsigned char *result_tree,
 			    const char *wt_strategy)
 {
-	struct commit_list *parents = NULL, *j;
+	struct commit_list *parents = NULL;
 	struct strbuf buf = STRBUF_INIT;
 	unsigned char result_commit[20];
 
 	free_commit_list(common);
-	if (allow_fast_forward) {
-		parents = remoteheads;
+	parents = remoteheads;
+	if (!head_subsumed || !allow_fast_forward)
 		commit_list_insert(head, &parents);
-		parents = reduce_heads(parents);
-	} else {
-		struct commit_list **pptr = &parents;
-
-		pptr = &commit_list_insert(head,
-				pptr)->next;
-		for (j = remoteheads; j; j = j->next)
-			pptr = &commit_list_insert(j->item, pptr)->next;
-	}
 	strbuf_addch(&merge_msg, '\n');
 	prepare_to_commit(remoteheads);
-	free_commit_list(remoteheads);
 	if (commit_tree(&merge_msg, result_tree, parents, result_commit,
 			NULL, sign_commit))
 		die(_("failed to write commit object"));
@@ -1137,12 +1128,16 @@ static int default_edit_option(void)
 		st_stdin.st_mode == st_stdout.st_mode);
 }
 
-static struct commit_list *collect_parents(int argc, const char **argv)
+static struct commit_list *collect_parents(struct commit *head_commit,
+					   int *head_subsumed,
+					   int argc, const char **argv)
 {
 	int i;
-	struct commit_list *remoteheads = NULL;
+	struct commit_list *remoteheads = NULL, *parents, *next;
 	struct commit_list **remotes = &remoteheads;
 
+	if (head_commit)
+		remotes = &commit_list_insert(head_commit, remotes)->next;
 	for (i = 0; i < argc; i++) {
 		struct commit *commit = get_merge_parent(argv[i]);
 		if (!commit)
@@ -1150,6 +1145,20 @@ static struct commit_list *collect_parents(int argc, const char **argv)
 		remotes = &commit_list_insert(commit, remotes)->next;
 	}
 	*remotes = NULL;
+
+	parents = reduce_heads(remoteheads);
+
+	*head_subsumed = 1; /* we will flip this to 0 when we find it */
+	for (remoteheads = NULL, remotes = &remoteheads;
+	     parents;
+	     parents = next) {
+		struct commit *commit = parents->item;
+		next = parents->next;
+		if (commit == head_commit)
+			*head_subsumed = 0;
+		else
+			remotes = &commit_list_insert(commit, remotes)->next;
+	}
 	return remoteheads;
 }
 
@@ -1161,7 +1170,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	struct commit *head_commit;
 	struct strbuf buf = STRBUF_INIT;
 	const char *head_arg;
-	int flag, i, ret = 0;
+	int flag, i, ret = 0, head_subsumed;
 	int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;
 	struct commit_list *common = NULL;
 	const char *best_strategy = NULL, *wt_strategy = NULL;
@@ -1270,7 +1279,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		head_arg = argv[1];
 		argv += 2;
 		argc -= 2;
-		remoteheads = collect_parents(argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 	} else if (!head_commit) {
 		struct commit *remote_head;
 		/*
@@ -1286,7 +1295,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (!allow_fast_forward)
 			die(_("Non-fast-forward commit does not make sense into "
 			    "an empty head"));
-		remoteheads = collect_parents(argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 		remote_head = remoteheads->item;
 		if (!remote_head)
 			die(_("%s - not something we can merge"), argv[0]);
@@ -1305,7 +1314,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * the standard merge summary message to be appended
 		 * to the given message.
 		 */
-		remoteheads = collect_parents(argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 		for (p = remoteheads; p; p = p->next)
 			merge_name(merge_remote_util(p->item)->name, &merge_names);
 
@@ -1351,7 +1360,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		option_edit = 0;
 
 	if (!use_strategies) {
-		if (!remoteheads->next)
+		if (!remoteheads)
+			; /* already up-to-date */
+		else if (!remoteheads->next)
 			add_strategies(pull_twohead, DEFAULT_TWOHEAD);
 		else
 			add_strategies(pull_octopus, DEFAULT_OCTOPUS);
@@ -1364,7 +1375,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			allow_trivial = 0;
 	}
 
-	if (!remoteheads->next)
+	if (!remoteheads)
+		; /* already up-to-date */
+	else if (!remoteheads->next)
 		common = get_merge_bases(head_commit, remoteheads->item, 1);
 	else {
 		struct commit_list *list = remoteheads;
@@ -1376,10 +1389,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	update_ref("updating ORIG_HEAD", "ORIG_HEAD", head_commit->object.sha1,
 		   NULL, 0, DIE_ON_ERR);
 
-	if (!common)
+	if (remoteheads && !common)
 		; /* No common ancestors found. We need a real merge. */
-	else if (!remoteheads->next && !common->next &&
-			common->item == remoteheads->item) {
+	else if (!remoteheads ||
+		 (!remoteheads->next && !common->next &&
+		  common->item == remoteheads->item)) {
 		/*
 		 * If head can reach all the merge then we are up to date.
 		 * but first the most common case of merging one remote.
@@ -1553,7 +1567,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * auto resolved the merge cleanly.
 	 */
 	if (automerge_was_ok) {
-		ret = finish_automerge(head_commit, common, remoteheads,
+		ret = finish_automerge(head_commit, head_subsumed,
+				       common, remoteheads,
 				       result_tree, wt_strategy);
 		goto done;
 	}
diff --git a/t/t6028-merge-up-to-date.sh b/t/t6028-merge-up-to-date.sh
index 824fca5..c518e9c 100755
--- a/t/t6028-merge-up-to-date.sh
+++ b/t/t6028-merge-up-to-date.sh
@@ -79,7 +79,7 @@ test_expect_success 'merge -s subtree up-to-date' '
 
 '
 
-test_expect_failure 'merge fast-forward octopus' '
+test_expect_success 'merge fast-forward octopus' '
 
 	git reset --hard c0 &&
 	test_tick &&
diff --git a/t/t7602-merge-octopus-many.sh b/t/t7602-merge-octopus-many.sh
index 5783ebf..7117b57 100755
--- a/t/t7602-merge-octopus-many.sh
+++ b/t/t7602-merge-octopus-many.sh
@@ -70,17 +70,15 @@ test_expect_success 'merge output uses pretty names' '
 '
 
 cat >expected <<\EOF
-Already up-to-date with c4
-Trying simple merge with c5
-Merge made by the 'octopus' strategy.
+Merge made by the 'recursive' strategy.
  c5.c |    1 +
  1 file changed, 1 insertion(+)
  create mode 100644 c5.c
 EOF
 
-test_expect_success 'merge up-to-date output uses pretty names' '
-	git merge c4 c5 >actual &&
-	test_cmp actual expected
+test_expect_success 'merge reduces irrelevant remote heads' '
+	GIT_MERGE_VERBOSITY=0 git merge c4 c5 >actual &&
+	test_cmp expected actual
 '
 
 cat >expected <<\EOF
-- 
1.7.10.332.g1863c

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] git-merge: Reduce heads before trying to merge them
  2012-04-17 20:09                 ` Linus Torvalds
@ 2012-04-17 20:48                   ` Junio C Hamano
  2012-04-18 18:14                     ` Michał Kiedrowicz
  0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2012-04-17 20:48 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Michał Kiedrowicz, git

Linus Torvalds <torvalds@linux-foundation.org> writes:

> 2012/4/17 Junio C Hamano <gitster@pobox.com>:
>>
>> If your current HEAD is an ancestor of one of the commit on that list, the
>> above does not omit it from the parent list of the resulting merge commit,
>> but if you performed the same merge while on one of the commit being
>> merged, your current HEAD will be excluded with reduce_heads(), which
>> would mean that you will end up recording a different history even though
>> a merge is supposed to be symmetrical.
>>
>> In other words, isn't any solution that calls reduce_heads() only on
>> remoteheads fundamentally wrong and merely papering over the problem?
>
> I think Michał's patch, together with my original one (but not the
> fixups later) is actually the right thing to do.
>
> Michał's patch fixes the "log shown multiple times" problem. It also
> turns a certain class of octopus merges into trivial common merges,
> which is good.
>
> So I'd suggest:
>  - undo the two top commits from lt/octopus-simplify
>  - apply Michał's patch on top of the remaining one commit
>
> It's not perfect, and I really think we could simplify things a bit
> more here, but I think the two commits together fix the problems in
> practice.
>
> Hmm?

I was cooking a fix on-and-off since yesterday evening, and sent it out a
few minutes ago. I think the spirit is almost the same as Michał's updated
patch, but it reduces the heads even earlier to catch cases where Michał's
updated patch may misdiagnose arity of the resulting merge due to its use
of remoteheads->next before the list is reduced (namely, the choice of the
default strategy based on how many we are merging).

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] git-merge: Reduce heads before trying to merge them
  2012-04-17 20:48                   ` Junio C Hamano
@ 2012-04-18 18:14                     ` Michał Kiedrowicz
  2012-04-18 20:20                       ` Junio C Hamano
  0 siblings, 1 reply; 24+ messages in thread
From: Michał Kiedrowicz @ 2012-04-18 18:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git

Junio C Hamano <gitster@pobox.com> wrote:
 
> I was cooking a fix on-and-off since yesterday evening, and sent it out a
> few minutes ago. I think the spirit is almost the same as Michał's updated
> patch, but it reduces the heads even earlier to catch cases where Michał's
> updated patch may misdiagnose arity of the resulting merge due to its use
> of remoteheads->next before the list is reduced (namely, the choice of the
> default strategy based on how many we are merging).

I like your patches, especially how reducing heads is now done in
collect_parents() instead of doing it twice (before merging and in
finish_automerge()).  And that you got rid of global remoteheads too.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] git-merge: Reduce heads before trying to merge them
  2012-04-18 18:14                     ` Michał Kiedrowicz
@ 2012-04-18 20:20                       ` Junio C Hamano
  2012-04-19  5:19                         ` Junio C Hamano
  0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2012-04-18 20:20 UTC (permalink / raw)
  To: Michał Kiedrowicz; +Cc: Linus Torvalds, git

Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> wrote:
>  
>> I was cooking a fix on-and-off since yesterday evening, and sent it out a
>> few minutes ago. I think the spirit is almost the same as Michał's updated
>> patch, but it reduces the heads even earlier to catch cases where Michał's
>> updated patch may misdiagnose arity of the resulting merge due to its use
>> of remoteheads->next before the list is reduced (namely, the choice of the
>> default strategy based on how many we are merging).
>
> I like your patches, especially how reducing heads is now done in
> collect_parents() instead of doing it twice (before merging and in
> finish_automerge()).  And that you got rid of global remoteheads too.

Thanks.  Linus also said "Yes, that sounds right (on the road with just my
phone, sorry for the html)" off-list to the series.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] git-merge: Reduce heads before trying to merge them
  2012-04-18 20:20                       ` Junio C Hamano
@ 2012-04-19  5:19                         ` Junio C Hamano
  0 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2012-04-19  5:19 UTC (permalink / raw)
  To: Michał Kiedrowicz, Linus Torvalds; +Cc: git

Junio C Hamano <gitster@pobox.com> writes:

> Michał Kiedrowicz <michal.kiedrowicz@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> wrote:
>>  
>>> I was cooking a fix on-and-off since yesterday evening, and sent it out a
>>> few minutes ago. I think the spirit is almost the same as Michał's updated
>>> patch, but it reduces the heads even earlier to catch cases where Michał's
>>> updated patch may misdiagnose arity of the resulting merge due to its use
>>> of remoteheads->next before the list is reduced (namely, the choice of the
>>> default strategy based on how many we are merging).
>>
>> I like your patches, especially how reducing heads is now done in
>> collect_parents() instead of doing it twice (before merging and in
>> finish_automerge()).  And that you got rid of global remoteheads too.
>
> Thanks.  Linus also said "Yes, that sounds right (on the road with just my
> phone, sorry for the html)" off-list to the series.

There was another bit still missing from my 4-patch series.

"merge" passes the updated test only because it runs the fmt_merge_msg()
internally after the reduction of the remotes, but without this update to
builtin/fmt-merge.msg.c, "pull" doesn't and its merge summary lists
unnecessary heads, because it calls the plumbing version from the script
with full set of possibly redundant parents.

-- >8 --
Subject: [PATCH] fmt-merge-msg: discard needless merge parents

This is used by "git pull" to construct a merge message from list of
remote refs.  When pulling redundant set of refs, however, it did not
filter them even though the merge itself discards them as unnecessary.

Teach the command to do the same for consistency.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fmt-merge-msg.c       |  125 ++++++++++++++++++++++++++++++++++++++---
 t/t7603-merge-reduce-heads.sh |   31 +++++++++-
 2 files changed, 146 insertions(+), 10 deletions(-)

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index c81a7fe..dcc12ea 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -53,7 +53,47 @@ static void init_src_data(struct src_data *data)
 static struct string_list srcs = STRING_LIST_INIT_DUP;
 static struct string_list origins = STRING_LIST_INIT_DUP;
 
-static int handle_line(char *line)
+struct merge_parents {
+	int alloc, nr;
+	struct merge_parent {
+		unsigned char given[20];
+		unsigned char commit[20];
+		unsigned char used;
+	} *item;
+};
+
+/*
+ * I know, I know, this is inefficient, but you won't be pulling and merging
+ * hundreds of heads at a time anyway.
+ */
+static struct merge_parent *find_merge_parent(struct merge_parents *table,
+					      unsigned char *given,
+					      unsigned char *commit)
+{
+	int i;
+	for (i = 0; i < table->nr; i++) {
+		if (given && hashcmp(table->item[i].given, given))
+			continue;
+		if (commit && hashcmp(table->item[i].commit, commit))
+			continue;
+		return &table->item[i];
+	}
+	return NULL;
+}
+
+static void add_merge_parent(struct merge_parents *table,
+			     unsigned char *given,
+			     unsigned char *commit)
+{
+	if (table->nr && find_merge_parent(table, given, commit))
+		return;
+	ALLOC_GROW(table->item, table->nr + 1, table->alloc);
+	hashcpy(table->item[table->nr].given, given);
+	hashcpy(table->item[table->nr].commit, commit);
+	table->nr++;
+}
+
+static int handle_line(char *line, struct merge_parents *merge_parents)
 {
 	int i, len = strlen(line);
 	struct origin_data *origin_data;
@@ -61,6 +101,7 @@ static int handle_line(char *line)
 	struct src_data *src_data;
 	struct string_list_item *item;
 	int pulling_head = 0;
+	unsigned char sha1[20];
 
 	if (len < 43 || line[40] != '\t')
 		return 1;
@@ -71,14 +112,15 @@ static int handle_line(char *line)
 	if (line[41] != '\t')
 		return 2;
 
-	line[40] = 0;
-	origin_data = xcalloc(1, sizeof(struct origin_data));
-	i = get_sha1(line, origin_data->sha1);
-	line[40] = '\t';
-	if (i) {
-		free(origin_data);
+	i = get_sha1_hex(line, sha1);
+	if (i)
 		return 3;
-	}
+
+	if (!find_merge_parent(merge_parents, sha1, NULL))
+		return 0; /* subsumed by other parents */
+
+	origin_data = xcalloc(1, sizeof(struct origin_data));
+	hashcpy(origin_data->sha1, sha1);
 
 	if (line[len - 1] == '\n')
 		line[len - 1] = 0;
@@ -366,6 +408,68 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
 	strbuf_release(&tagbuf);
 }
 
+static struct merge_parents *find_merge_parents(struct strbuf *in, unsigned char *head)
+{
+	struct commit_list *parents, *next;
+	struct commit *head_commit;
+	struct merge_parents *result = xcalloc(1, sizeof(*result));
+	int pos = 0, i, j;
+
+	parents = NULL;
+	while (pos < in->len) {
+		int len;
+		char *p = in->buf + pos;
+		char *newline = strchr(p, '\n');
+		unsigned char sha1[20];
+		struct commit *parent;
+		struct object *obj;
+
+		len = newline ? newline - p : strlen(p);
+		pos += len + !!newline;
+
+		if (len < 43 ||
+		    get_sha1_hex(p, sha1) ||
+		    p[40] != '\t' ||
+		    p[41] != '\t')
+			continue; /* skip not-for-merge */
+		/*
+		 * Do not use get_merge_parent() here; we do not have
+		 * "name" here and we do not want to contaminate its
+		 * util field yet.
+		 */
+		obj = parse_object(sha1);
+		parent = (struct commit *)peel_to_type(NULL, 0, obj, OBJ_COMMIT);
+		if (!parent)
+			continue;
+		commit_list_insert(parent, &parents);
+		add_merge_parent(result, obj->sha1, parent->object.sha1);
+	}
+	head_commit = lookup_commit(head);
+	if (head_commit)
+		commit_list_insert(head_commit, &parents);
+	parents = reduce_heads(parents);
+
+	while (parents) {
+		for (i = 0; i < result->nr; i++)
+			if (!hashcmp(result->item[i].commit,
+				     parents->item->object.sha1))
+				result->item[i].used = 1;
+		next = parents->next;
+		free(parents);
+		parents = next;
+	}
+
+	for (i = j = 0; i < result->nr; i++) {
+		if (result->item[i].used) {
+			if (i != j)
+				result->item[j] = result->item[i];
+			j++;
+		}
+	}
+	result->nr = j;
+	return result;
+}
+
 int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 		  struct fmt_merge_msg_opts *opts)
 {
@@ -373,6 +477,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 	unsigned char head_sha1[20];
 	const char *current_branch;
 	void *current_branch_to_free;
+	struct merge_parents *merge_parents;
 
 	/* get current branch */
 	current_branch = current_branch_to_free =
@@ -382,6 +487,8 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 	if (!prefixcmp(current_branch, "refs/heads/"))
 		current_branch += 11;
 
+	merge_parents = find_merge_parents(in, head_sha1);
+
 	/* get a line */
 	while (pos < in->len) {
 		int len;
@@ -392,7 +499,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 		pos += len + !!newline;
 		i++;
 		p[len] = 0;
-		if (handle_line(p))
+		if (handle_line(p, merge_parents))
 			die ("Error in line %d: %.*s", i, len, p);
 	}
 
diff --git a/t/t7603-merge-reduce-heads.sh b/t/t7603-merge-reduce-heads.sh
index a3b08a6..9894895 100755
--- a/t/t7603-merge-reduce-heads.sh
+++ b/t/t7603-merge-reduce-heads.sh
@@ -57,7 +57,36 @@ test_expect_success 'merge c1 with c2, c3, c4, c5' '
 	test -f c2.c &&
 	test -f c3.c &&
 	test -f c4.c &&
-	test -f c5.c
+	test -f c5.c &&
+	git show --format=%s -s >actual &&
+	! grep c1 actual &&
+	grep c2 actual &&
+	grep c3 actual &&
+	! grep c4 actual &&
+	grep c5 actual
+'
+
+test_expect_success 'pull c2, c3, c4, c5 into c1' '
+	git reset --hard c1 &&
+	git pull . c2 c3 c4 c5 &&
+	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
+	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
+	test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
+	test "$(git rev-parse c5)" = "$(git rev-parse HEAD^4)" &&
+	git diff --exit-code &&
+	test -f c0.c &&
+	test -f c1.c &&
+	test -f c2.c &&
+	test -f c3.c &&
+	test -f c4.c &&
+	test -f c5.c &&
+	git show --format=%s -s >actual &&
+	! grep c1 actual &&
+	grep c2 actual &&
+	grep c3 actual &&
+	! grep c4 actual &&
+	grep c5 actual
 '
 
 test_expect_success 'setup' '
-- 
1.7.10.333.gf16fa

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: What's cooking in git.git (Apr 2012, #05; Thu, 12)
  2012-04-12 22:41 What's cooking in git.git (Apr 2012, #05; Thu, 12) Junio C Hamano
@ 2012-04-15 11:10 ` Clemens Buchacher
  0 siblings, 0 replies; 24+ messages in thread
From: Clemens Buchacher @ 2012-04-15 11:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu, Apr 12, 2012 at 03:41:55PM -0700, Junio C Hamano wrote:
> 
> * cb/http-multi-curl-auth (2012-04-10) 2 commits
>  - fix http auth with multiple curl handles
>  - http auth fails with multiple curl handles
> 
> Fixes http authentication breakage when we keep multiple HTTP requests in
> flight using curl-multi.
> 
> I haven't looked at Peff's follow-up deeply yet.  Perhaps a reroll is
> coming?

I think Peff's follow-up is based on the commits you already have in
cb/http-multi-curl-auth.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* What's cooking in git.git (Apr 2012, #05; Thu, 12)
@ 2012-04-12 22:41 Junio C Hamano
  2012-04-15 11:10 ` Clemens Buchacher
  0 siblings, 1 reply; 24+ messages in thread
From: Junio C Hamano @ 2012-04-12 22:41 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'.

This issue describes the state of my tree in preparation for opening the
post 1.7.10 cycle.  The 'next' branch has been rewound.

You can find the changes described here in the integration branches of the
repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

--------------------------------------------------
[New Topics]

* mh/ref-api (2012-04-10) 13 commits
 - do_for_each_ref(): only iterate over the subtree that was requested
 - refs: store references hierarchically
 - sort_ref_dir(): simplify logic
 - refs.c: rename ref_array -> ref_dir
 - struct ref_entry: nest the value part in a union
 - check_refname_component(): return 0 for zero-length components
 - free_ref_entry(): new function
 - names_conflict(): simplify implementation
 - repack_without_ref(): reimplement using do_for_each_ref_in_array()
 - do_for_each_ref_in_arrays(): new function
 - do_for_each_ref_in_array(): new function
 - refs: manage current_ref within do_one_ref()
 - refs.c: reorder definitions more logically

Allows walking only a sub-hierarchy in refs/*.

Will merge to 'next'.

* jc/maint-blame-minimal (2012-04-11) 1 commit
 - blame: accept --need-minimal

"git blame" started missing quite a few changes from the origin since we
stopped using the diff minimalization by default in v1.7.2 era.

Will merge to 'next'.

* jk/repack-no-explode-objects-from-old-pack (2012-04-11) 1 commit
 - gc: do not explode objects which will be immediately pruned

Will merge to 'next'.

* ld/git-p4-tags-and-labels (2012-04-11) 2 commits
 - git p4: import/export of labels to/from p4
 - git p4: Fixing script editor checks
 (this branch uses pw/git-p4.)

* lt/octopus-simplify (2012-04-12) 1 commit
 - Make 'git merge' simplify parents earlier

Octopus merge strategy did not reduce heads that are recorded in the final
commit.  This was done off-list.

* mr/fetch-call-a-non-branch-a-ref (2012-04-11) 1 commit
 - fetch: Only call a new ref a "branch" if it's under refs/heads/.

The change itself may be sensible, but it exposes a funny in the vicinity
about recursive submodule fetching behaviour.

* nh/empty-rebase (2012-04-11) 3 commits
 - git-cherry-pick: add test to validate new options
 - git-cherry-pick: add --keep-redundant-commits option
 - git-cherry-pick: add allow-empty option

Waiting for a reroll.

* sl/test-wc-l-line-count (2012-04-11) 1 commit
 - tests: modernise style: more uses of test_line_count

Will merge to 'next'.

* zj/upstream-error-message (2012-04-11) 5 commits
 - i18n: mark @{upstream} error messages for translation
 - Be more specific if upstream branch is not fetched
 - Provide better message for barnhc_wiht_tpyo@{u}
 - Provide branch name in error message when using @{u}
 - t1507: add additional tests for @{upstream}

Waiting for a reroll.

* fc/completion-tests (2012-04-12) 1 commit
 - tests: add initial bash completion tests

Will merge to 'next'.

--------------------------------------------------
[Stalled]

* nl/http-proxy-more (2012-04-12) 6 commits
 - fixup? 6e17b8c
 - http: rename HTTP_REAUTH to HTTP_AUTH_RETRY
 - http: Avoid limit of retrying request only twice
 - http: handle proxy authentication failure (error 407)
 - http: handle proxy proactive authentication
 - http: try http_proxy env var when http.proxy config option is not set

The code to talk to http proxies learn to use the same credential
API used to talk to the final http destinations.

Kicked back to 'pu', as it breaks the proxied codepath quite badly.

* lp/maint-diff-three-dash-with-graph (2012-03-20) 3 commits
 - t4202: add test for "log --graph --stat -p" separator lines
 - log --graph: fix break in graph lines
 - log --graph --stat: three-dash separator should come after graph lines

With a review from Zbigniew, I have been expecting that this would be
rerolled, but that didn't happen.

Waiting for a reroll.

* cn/apply-fix-ws-can-lengthen-lines (2012-03-11) 1 commit
 . apply: reallocate the postimage buffer when needed

Attempts to address an ancient bug that dates back to the addition
of an oddball "tab-in-indent" whitespace breakage class that wants
to have longer lines than the original when fixing things up; broken
and results in double-frees.

Will discard shortly if nothing happens.

* nd/columns (2012-03-13) 12 commits
 - column: support grouping entries
 - column: support "denser" mode
 - ls-files: support --column
 - tag: add --column
 - column: support piping stdout to external git-column process
 - status: add --column
 - branch: add --column
 - help: reuse print_columns() for help -a
 - column: add dense layout support
 - column: add columnar layout
 - Stop starting pager recursively
 - Add column layout skeleton and git-column

Modulo minor nits, looked nicer than the previous round and getting closer
to completion but nothing has happened recently.

Will discard shortly if nothing happens.

* ss/git-svn-prompt-sans-terminal (2012-01-04) 3 commits
 - fixup! 15eaaf4
 - git-svn, perl/Git.pm: extend Git::prompt helper for querying users
 - perl/Git.pm: "prompt" helper to honor GIT_ASKPASS and SSH_ASKPASS

The bottom one has been replaced with a rewrite based on comments
from Ævar. The second one needs more work, both in perl/Git.pm and
prompt.c, to give precedence to tty over SSH_ASKPASS when terminal
is available.

Will discard shortly if nothing happens.

* jc/split-blob (2012-04-03) 7 commits
 - chunked-object: streaming checkout
 - chunked-object: fallback checkout codepaths
 - bulk-checkin: support chunked-object encoding
 - bulk-checkin: allow the same data to be multiply hashed
 - new representation types in the packstream
 - packfile: use varint functions
 - varint: make it available outside the context of pack
 (this branch is tangled with jc/index-v4.)

Not ready.

I finished the streaming checkout codepath, but as explained in
127b177 (bulk-checkin: support chunked-object encoding, 2011-11-30),
these are still early steps of a long and painful journey. At least
pack-objects and fsck need to learn the new encoding for the series
to be usable locally, and then index-pack/unpack-objects needs to
learn it to be used remotely.

Given that I heard a lot of noise that people want large files, and
that I was asked by somebody at GitTogether'11 privately for an
advice on how to pay developers (not me) to help adding necessary
support, I am somewhat dissapointed that the original patch series
that was sent almost two months ago still remains here without much
comments and updates from the developer community. I even made the
interface to the logic that decides where to split chunks easily
replaceable, and I deliberately made the logic in the original patch
extremely stupid to entice others, especially the "bup" fanboys, to
come up with a better logic, thinking that giving people an easy
target to shoot for, they may be encouraged to help out. The plan is
not working :-(.

--------------------------------------------------
[Cooking]

* jh/apply-free-patch (2012-04-11) 9 commits
 - apply: document buffer ownership rules across functions
 - apply: tighten constness of line buffer
 - apply: drop unused macro
 - apply: free unused fragments for submodule patch
 - apply: free patch->result
 - apply: release memory for fn_table
 - apply: free patch->{def,old,new}_name fields
 - apply: rename free_patch() to free_patch_list()
 - apply: do not leak patches and fragments

Valgrind reports quite a lot of discarded memory inside apply.

Will merge to 'next'.

* nd/threaded-index-pack (2012-04-11) 3 commits
 - index-pack: support multithreaded delta resolving
 - index-pack: split second pass obj handling into own function
 - compat/win32/pthread.h: Add an pthread_key_delete() implementation

* bw/submodule-sed-solaris (2012-04-09) 1 commit
  (merged to 'next' on 2012-04-12 at 3bf1d83)
 + Avoid bug in Solaris xpg4/sed as used in submodule

* cb/http-multi-curl-auth (2012-04-10) 2 commits
 - fix http auth with multiple curl handles
 - http auth fails with multiple curl handles

Fixes http authentication breakage when we keep multiple HTTP requests in
flight using curl-multi.

I haven't looked at Peff's follow-up deeply yet.  Perhaps a reroll is
coming?

* jk/rebase-i-submodule-conflict-only (2012-04-09) 1 commit
 - rebase -i continue: don't skip commits that only change submodules

Giving "--continue" to a conflicted "rebase -i" session skipped a commit
that only results in changes to submodules.

Will merge to 'next'.

* ms/remote-usage-string (2012-04-09) 1 commit
 - remote: update builtin usage

As an independent follow-up, we may want to rethink how the overall usage
string and subcommand usage strings are maintained.

Will merge to 'next'.

* pw/fast-import-dataref-parsing (2012-04-10) 1 commit
 - fast-import: tighten parsing of datarefs

Will merge to 'next'.

* pw/git-p4 (2012-04-09) 3 commits
 - git p4: use "git p4" directly in tests
 - git p4: update name in script
 - git-p4: move to toplevel
 (this branch is used by ld/git-p4-tags-and-labels.)

Will merge to 'next'.

* rl/show-empty-prefix (2012-04-10) 1 commit
 - rev-parse --show-prefix: add in trailing newline

Will merge to 'next'.

* tr/xdiff-fast-hash (2012-04-09) 1 commit
 - xdiff: load full words in the inner loop of xdl_hash_record

Will merge to 'next'.

* jc/index-v4 (2012-04-04) 10 commits
 - update-index: upgrade/downgrade on-disk index version
 - read-cache.c: write prefix-compressed names in the index
 - read-cache.c: read prefix-compressed names in index on-disk version v4
 - read-cache.c: move code to copy incore to ondisk cache to a helper function
 - read-cache.c: move code to copy ondisk to incore cache to a helper function
 - read-cache.c: report the header version we do not understand
 - read-cache.c: make create_from_disk() report number of bytes it consumed
 - read-cache.c: allow unaligned mapping of the index file
 - cache.h: hide on-disk index details
 - varint: make it available outside the context of pack
 (this branch is tangled with jc/split-blob.)

Trivially shrinks the on-disk size of the index file to save both I/O and
checksum overhead.  The "v4" format represented here may not be what we
eventually want to have, but the preparatory refactoring steps and the way
how backward compatibility is retained should give plenty to learn to GSoC
student-hopefuls.

Will merge to 'next', if nobody objects.

* jk/add-p-skip-conflicts (2012-04-05) 1 commit
  (merged to 'next' on 2012-04-12 at e65ceb1)
 + add--interactive: ignore unmerged entries in patch mode

Excludes conflicted paths from "add -p" processing, as it is not prepared
to handle them.

* bw/spawn-via-shell-path (2012-04-03) 1 commit
  (merged to 'next' on 2012-04-12 at 0ff833b)
 + Use SHELL_PATH from build system in run_command.c:prepare_shell_cmd

"sh" on the user's PATH may be utterly broken on some systems;
consistently use SHELL_PATH even from inside run-command API.

* jc/commit-unedited-template (2012-04-03) 5 commits
  (merged to 'next' on 2012-04-12 at a2e153b)
 + Documentation/git-commit: rephrase the "initial-ness" of templates
 + git-commit.txt: clarify -t requires editing message
 + commit: rephrase the error when user did not touch templated log message
 + commit: do not trigger bogus "has templated message edited" check
 + t7501: test the right kind of breakage

When "git commit --template F" errors out because the user did not touch
the message, it claimed that it aborts due to "empty message", which was
utterly wrong.

* jc/push-upstream-sanity (2012-04-05) 1 commit
  (merged to 'next' on 2012-04-12 at 288e1ad)
 + push: error out when the "upstream" semantics does not make sense

Fix broken 'push to upstream' implementation.  "git push $there" without
refspec, when the current branch is set to push to a remote different from
$there, used to push to $there using the upstream information to a remote
unreleated to $there.

* jk/http-backend-keep-committer-ident-env (2012-03-30) 1 commit
  (merged to 'next' on 2012-04-12 at b2d0136)
 + http-backend: respect existing GIT_COMMITTER_* variables

The smart-http backend used to always override GIT_COMMITTER_* variables
with REMOTE_USER and REMOTE_ADDR.

* mk/gitweb-diff-hl (2012-04-11) 8 commits
 - gitweb: Refinement highlightning in combined diffs
 - gitweb: Highlight interesting parts of diff
 - gitweb: Push formatting diff lines to print_diff_chunk()
 - gitweb: Use print_diff_chunk() for both side-by-side and inline diffs
 - gitweb: Extract print_sidebyside_diff_lines()
 - gitweb: Pass esc_html_hl_regions() options to esc_html()
 - gitweb: esc_html_hl_regions(): Don't create empty <span> elements
 - gitweb: Use descriptive names in esc_html_hl_regions()

Will merge to 'next'.

* it/fetch-pack-many-refs (2012-04-10) 4 commits
 - remote-curl: main test case for the OS command line overflow
 - fetch-pack: test cases for the new --stdin option
 - remote-curl: send the refs to fetch-pack on stdin
 - fetch-pack: new --stdin option to read refs from stdin

Will merge to 'next'.

* jn/debian-customizes-default-editor (2012-04-10) 2 commits
 - var doc: advertise current DEFAULT_PAGER and DEFAULT_EDITOR settings
 - var doc: default editor and pager are configurable at build time

Will merge to 'next'.

* rs/commit-list-sort-in-batch (2012-04-11) 3 commits
 - revision: insert unsorted, then sort in prepare_revision_walk()
 - commit: use mergesort() in commit_list_sort_by_date()
 - add mergesort() for linked lists

Will merge to 'next'.

* hv/submodule-recurse-push (2012-03-30) 3 commits
 - push: teach --recurse-submodules the on-demand option
 - Refactor submodule push check to use string list instead of integer
 - Teach revision walking machinery to walk multiple times sequencially

Will merge to 'next'.

* jk/branch-quiet (2012-03-26) 2 commits
  (merged to 'next' on 2012-04-12 at 7d0a438)
 + teach "git branch" a --quiet option
 + checkout: suppress tracking message with "-q"

Even with "-q"uiet option, "checkout" used to report setting up tracking.
Also "branch" learns "-q"uiet option to squelch informational message.

* jk/run-command-eacces (2012-04-05) 2 commits
  (merged to 'next' on 2012-04-12 at ef3782e)
 + run-command: treat inaccessible directories as ENOENT
 + compat/mingw.[ch]: Change return type of exec functions to int

When PATH contains an unreadable directory, alias expansion code did not
kick in, and failed with an error that said "git-subcmd" was not found.

* jb/am-include (2012-03-28) 1 commit
  (merged to 'next' on 2012-04-12 at ddbf96d)
 + am: support --include option

* jc/am-report-3way (2012-03-28) 1 commit
  (merged to 'next' on 2012-04-12 at cf5d599)
 + am -3: list the paths that needed 3-way fallback

* rs/combine-diff-zero-context-at-the-beginning (2012-03-25) 1 commit
  (merged to 'next' on 2012-04-12 at b4d92e3)
 + combine-diff: fix loop index underflow

Fixes an age old corner case bug in combine diff (only triggered with -U0
and the hunk at the beginning of the file needs to be shown).

* sl/autoconf (2012-03-26) 3 commits
  (merged to 'next' on 2012-04-12 at 68f346d)
 + configure: be more idiomatic
 + configure: avoid some code repetitions thanks to m4_{push,pop}def
 + configure: move definitions of private m4 macros before AC_INIT invocation

Updates our configure.ac to follow a better "autoconf" style.

* wk/gitweb-snapshot-use-if-modified-since (2012-03-30) 3 commits
  (merged to 'next' on 2012-04-12 at 611d318)
 + gitweb: add If-Modified-Since handling to git_snapshot().
 + gitweb: refactor If-Modified-Since handling
 + gitweb: add `status` headers to git_feed() responses.

Makes 'snapshot' request to "gitweb" honor If-Modified-Since: header,
based on the commit date.

* jk/diff-no-rename-empty (2012-03-23) 4 commits
  (merged to 'next' on 2012-04-12 at d1d95c5)
 + merge-recursive: don't detect renames of empty files
 + teach diffcore-rename to optionally ignore empty content
 + make is_empty_blob_sha1 available everywhere
 + drop casts from users EMPTY_TREE_SHA1_BIN

Forbids rename detection logic from matching two empty files as renames
during merge-recursive to prevent mismerges.

* th/difftool-diffall (2012-04-04) 8 commits
 - difftool: print list of valid tools with '--tool-help'
 - difftool: teach difftool to handle directory diffs
 - difftool: eliminate setup_environment function
 - difftool: stop appending '.exe' to git
 - difftool: remove explicit change of PATH
 - difftool: exit(0) when usage is printed
 - difftool: add '--no-gui' option
 - difftool: parse options using Getopt::Long

Rolls the two-directory-diff logic from diffall script (in contrib/) into
"git difftool" framework. 

Still being reworked.

* jc/maint-clean-nested-worktree-in-subdir (2012-03-15) 2 commits
  (merged to 'next' on 2012-04-12 at 280a6e5)
 + clean: preserve nested git worktree in subdirectories
 + remove_dir_recursively(): Add flag for skipping removal of toplevel dir
 (this branch is tangled with jh/notes-merge-in-git-dir-worktree.)

Originally merged to 'next' on 2012-03-20

"git clean -d -f" (not "-d -f -f") is supposed to protect nested working
trees of independent git repositories that exist in the current project
working tree from getting removed, but the protection applied only to such
working trees that are at the top-level of the current project by mistake.

* ct/advise-push-default (2012-04-12) 3 commits
  (merged to 'next' on 2012-04-12 at b0613cd)
 + Fix httpd tests that broke when non-ff push advice changed
 + clean up struct ref's nonfastforward field
 + push: Provide situational hints for non-fast-forward errors

Breaks down the cases in which "git push" fails due to non-ff into three
categories, and gives separate advise messages.  This should be a good
change regardless of mm/push-default-switch-warning topic.

* nl/rebase-i-cheat-sheet (2012-03-20) 1 commit
  (merged to 'next' on 2012-04-12 at cd76fcb)
 + rebase -i: remind that the lines are top-to-bottom

Originally merged to 'next' on 2012-03-20

* da/difftool-test (2012-03-19) 1 commit
  (merged to 'next' on 2012-04-11 at f245c01)
 + t7800: Test difftool passing arguments to diff

Makes sure "difftool" options can be given in any order.

Will merge to 'master'.

* jh/notes-merge-in-git-dir-worktree (2012-03-15) 4 commits
  (merged to 'next' on 2012-04-11 at 6042ef5)
 + notes-merge: Don't remove .git/NOTES_MERGE_WORKTREE; it may be the user's cwd
 + notes-merge: use opendir/readdir instead of using read_directory()
 + t3310: illustrate failure to "notes merge --commit" inside $GIT_DIR/
 + remove_dir_recursively(): Add flag for skipping removal of toplevel dir
 (this branch is tangled with jc/maint-clean-nested-worktree-in-subdir.)

Originally merged to 'next' on 2012-03-20

Running "notes merge --commit" failed to perform correctly when run
from any directory inside $GIT_DIR/.  When "notes merge" stops with
conflicts, $GIT_DIR/NOTES_MERGE_WORKTREE is the place a user edits
to resolve it.

Will merge to 'master'.

* jn/diffstat-tests (2012-03-13) 7 commits
  (merged to 'next' on 2012-04-11 at 7365e41)
 + diffstat summary line varies by locale: miscellany
 + test: use numstat instead of diffstat in binary-diff test
 + test: use --numstat instead of --stat in "git stash show" tests
 + test: test cherry-pick functionality and output separately
 + test: modernize funny-names test style
 + test: use numstat instead of diffstat in funny-names test
 + test: use test_i18ncmp when checking --stat output

Originally merged to 'next' on 2012-03-20

Some tests checked the "diff --stat" output when they do not have to,
which unnecessarily made things harder to verify under GETTEXT_POISON.

Will merge to 'master'.

* tr/maint-word-diff-regex-sticky (2012-03-14) 3 commits
  (merged to 'next' on 2012-04-11 at 24d14de)
 + diff: tweak a _copy_ of diff_options with word-diff
 + diff: refactor the word-diff setup from builtin_diff_cmd
 + t4034: diff.*.wordregex should not be "sticky" in --word-diff

Originally merged to 'next' on 2012-03-20

The regexp configured with wordregex was incorrectly reused across
files.

Will merge to 'master'.

* zj/test-cred-helper-nicer-prove (2012-03-15) 2 commits
  (merged to 'next' on 2012-04-11 at 9169614)
 + t0303: resurrect commit message as test documentation
 + t0303: immediately bail out w/o GIT_TEST_CREDENTIAL_HELPER

Originally merged to 'next' on 2012-03-20

Minor improvement to t0303.
Will merge to 'master'.

* jc/commit-hook-authorship (2012-03-11) 3 commits
  (merged to 'next' on 2012-04-11 at 15ec4d4)
 + commit: pass author/committer info to hooks
 + t7503: does pre-commit-hook learn authorship?
 + ident.c: add split_ident_line() to parse formatted ident line
 (this branch is tangled with jc/run-hook-env-1.)

Originally merged to 'next' on 2012-03-12

"git commit --author=$name" did not tell the name that was being
recorded in the resulting commit to hooks, even though it does do so
when the end user overrode the authorship via the "GIT_AUTHOR_NAME"
environment variable.

Will merge to 'master'.

* jc/run-hook-env-1 (2012-03-11) 3 commits
 - run_hook(): enhance the interface to pass arbitrary environment
 + t7503: does pre-commit-hook learn authorship?
 + ident.c: add split_ident_line() to parse formatted ident line
 (this branch is tangled with jc/commit-hook-authorship.)

Not urgent.

Updates run_hook() API to be much less specific to "commit".  It would
only be useful if people start doing more interesting things with hooks.

* jc/diff-algo-cleanup (2012-02-19) 2 commits
  (merged to 'next' on 2012-04-11 at f2c6b9a)
 + xdiff: PATIENCE/HISTOGRAM are not independent option bits
 + xdiff: remove XDL_PATCH_* macros

Originally merged to 'next' on 2012-03-15

Resurrects the preparatory clean-up patches from another topic that was
discarded, as this would give a saner foundation to build on diff.algo
configuration option series.

Will merge to 'master'.

* rs/unpack-trees-leakfix (2012-04-10) 2 commits
  (merged to 'next' on 2012-04-12 at 6958b54)
 + unpack-trees: plug minor memory leak
 + unpack-trees: don't perform any index operation if we're not merging

Originally merged to 'next' on 2012-03-07

* mm/push-default-switch-warning (2012-03-09) 1 commit
 - push: start warning upcoming default change for push.default

This resurrects an ancient patch I wrote during a discussion we had in the
1.6.3-1.6.4 era.  This should probably come after ct/advise-push-default
topic and at that point the advise messages need to be rephrased, taking
the future default change into account.

Will kick back to 'pu'.

* jc/fmt-merge-msg-people (2012-03-13) 1 commit
  (merged to 'next' on 2012-04-12 at 4c2031c)
 + fmt-merge-msg: show those involved in a merged series

The "fmt-merge-msg" command learns to list the primary contributors
involved in the side topic you are merging.

* nd/stream-more (2012-03-07) 7 commits
  (merged to 'next' on 2012-04-11 at b17684d)
 + update-server-info: respect core.bigfilethreshold
 + fsck: use streaming API for writing lost-found blobs
 + show: use streaming API for showing blobs
 + parse_object: avoid putting whole blob in core
 + cat-file: use streaming API to print blobs
 + Add more large blob test cases
 + streaming: make streaming-write-entry to be more reusable

Originally merged to 'next' on 2012-03-07

Use API to read blob data in smaller chunks in more places to
reduce the memory footprint.  In general, looked fairly good.

Will merge to 'master'.

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2012-04-19  5:20 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-16  6:26 What's cooking in git.git (Apr 2012, #05; Thu, 12) Michal Kiedrowicz
2012-04-16 14:57 ` Linus Torvalds
2012-04-16 17:29   ` Junio C Hamano
2012-04-16 17:50     ` Linus Torvalds
2012-04-16 22:03       ` Junio C Hamano
2012-04-17 20:34       ` [PATCH 0/4] merge: reduce set of parents consistently Junio C Hamano
2012-04-17 20:34         ` [PATCH 1/4] git-merge: test octopus with redundant parents Junio C Hamano
2012-04-17 20:34         ` [PATCH 2/4] builtin/merge.c: remove "remoteheads" global variable Junio C Hamano
2012-04-17 20:34         ` [PATCH 3/4] builtin/merge.c: collect other parents early Junio C Hamano
2012-04-17 20:34         ` [PATCH 4/4] builtin/merge.c: reduce " Junio C Hamano
2012-04-16 17:36   ` What's cooking in git.git (Apr 2012, #05; Thu, 12) Junio C Hamano
2012-04-16 18:02     ` Linus Torvalds
2012-04-16 18:33       ` Linus Torvalds
2012-04-16 21:32         ` Michał Kiedrowicz
2012-04-17  1:22           ` Linus Torvalds
2012-04-17 18:25             ` [PATCH] git-merge: Reduce heads before trying to merge them Michał Kiedrowicz
2012-04-17 18:52               ` Junio C Hamano
2012-04-17 20:09                 ` Linus Torvalds
2012-04-17 20:48                   ` Junio C Hamano
2012-04-18 18:14                     ` Michał Kiedrowicz
2012-04-18 20:20                       ` Junio C Hamano
2012-04-19  5:19                         ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2012-04-12 22:41 What's cooking in git.git (Apr 2012, #05; Thu, 12) Junio C Hamano
2012-04-15 11:10 ` Clemens Buchacher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).