threads / patch / 1050

patch, 2 partsFix for git-rev-list --merge-order B ^A (A,B share common base)

Subject: [PATCH 2/2] Fix for git-rev-list --merge-order B ^A (A,B share common base)

## tl;dr

4 messages between Jun 29, 2005 and Jun 30, 2005. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Jon Seymour· Jun 29, 2005, 23:45 UTC · lore

This patch makes --merge-order produce the same list as git-rev-list without --merge-order specified.

In particular, if the graph looks like this:
A
| B
|/ 
C
|
D
The both git-rev-list B ^A and git-rev-list --merge-order will produce B.

The unit test changes in this patch remove use of the --show-breaks flags from certain unit tests. The changed --merge-order behaviour changed the annotation that --show-breaks prints for certain test cases. The new behaviour is reasonable and irrelevant to the intent of the tests so that tests have been changed to eliminate the spurious behaviour.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 epoch.c                         |    8 +++-----
 t/t6001-rev-list-merge-order.sh |   36 ++++++++++++++++++------------------
 2 files changed, 21 insertions(+), 23 deletions(-)
26783f6660e58cc2e988b92050707a6a417cc6ae
Show changes to 2 files +21 −23

epoch.c, t/t6001-rev-list-merge-order.sh

diff --git a/epoch.c b/epoch.c
--- a/epoch.c
+++ b/epoch.c
@@ -585,11 +585,9 @@ int sort_list_in_merge_order(struct comm
 	for (; list; list = list->next) {
 		struct commit *next = list->item;
 
-		if (!(next->object.flags & UNINTERESTING)) {
-			if (!(next->object.flags & DUPCHECK)) {
-				next->object.flags |= DUPCHECK;
-				commit_list_insert(list->item, &reversed);
-			}
+		if (!(next->object.flags & DUPCHECK)) {
+			next->object.flags |= DUPCHECK;
+			commit_list_insert(list->item, &reversed);
 		}
 	}
 
diff --git a/t/t6001-rev-list-merge-order.sh b/t/t6001-rev-list-merge-order.sh
--- a/t/t6001-rev-list-merge-order.sh
+++ b/t/t6001-rev-list-merge-order.sh
@@ -366,34 +366,34 @@ test_output_expect_success "three nodes 
 = root
 EOF
 
-test_output_expect_success "linear prune l2 ^root" 'git-rev-list --merge-order --show-breaks l2 ^root' <<EOF
-= l2
-| l1
-| l0
+test_output_expect_success "linear prune l2 ^root" 'git-rev-list --merge-order l2 ^root' <<EOF
+l2
+l1
+l0
 EOF
 
-test_output_expect_success "linear prune l2 ^l0" 'git-rev-list --merge-order --show-breaks l2 ^l0' <<EOF
-= l2
-| l1
+test_output_expect_success "linear prune l2 ^l0" 'git-rev-list --merge-order l2 ^l0' <<EOF
+l2
+l1
 EOF
 
-test_output_expect_success "linear prune l2 ^l1" 'git-rev-list --merge-order --show-breaks l2 ^l1' <<EOF
-= l2
+test_output_expect_success "linear prune l2 ^l1" 'git-rev-list --merge-order l2 ^l1' <<EOF
+l2
 EOF
 
-test_output_expect_success "linear prune l5 ^a4" 'git-rev-list --merge-order --show-breaks l5 ^a4' <<EOF
-= l5
-| l4
-| l3
+test_output_expect_success "linear prune l5 ^a4" 'git-rev-list --merge-order l5 ^a4' <<EOF
+l5
+l4
+l3
 EOF
 
-test_output_expect_success "linear prune l5 ^l3" 'git-rev-list --merge-order --show-breaks l5 ^l3' <<EOF
-= l5
-| l4
+test_output_expect_success "linear prune l5 ^l3" 'git-rev-list --merge-order l5 ^l3' <<EOF
+l5
+l4
 EOF
 
-test_output_expect_success "linear prune l5 ^l4" 'git-rev-list --merge-order --show-breaks l5 ^l4' <<EOF
-= l5
+test_output_expect_success "linear prune l5 ^l4" 'git-rev-list --merge-order l5 ^l4' <<EOF
+l5
 EOF
 
 test_output_expect_success "max-count 10 - merge order" 'git-rev-list --merge-order --show-breaks --max-count=10 l5' <<EOF
------------
Junio C Hamano· Jun 30, 2005, 00:11 UTC · re: Jon Seymour · lore

Re: [PATCH 2/2] Fix for git-rev-list --merge-order B ^A (A,B share common base)

>>>>> "JS" == Jon Seymour <jon.seymour@gmail.com> writes:
I am puzzled about this part.

JS> The unit test changes in this patch remove use of the --show-breaks JS> flags from certain unit tests. The changed --merge-order behaviour JS> changed the annotation that --show-breaks prints for certain test cases. JS> The new behaviour is reasonable and irrelevant to the intent of the tests JS> so that tests have been changed to eliminate the spurious behaviour.

If the behaviour of --show-breaks subtly changes, and if that changed behaviour is something still acceptable, why not update the test to show the new expected results since you are updating the test anyway?

Showing that "subtle" change in the diff may draw people's attention and would help you to verify that the behaviour change is not something that would be unacceptable to them.

Also if you are changing t6001, could you also merge Mark Allen's BSD portability fix while you are at it?

    Message-ID: <20050628014337.18986.qmail@web41205.mail.yahoo.com>
Jon Seymour· Jun 30, 2005, 01:33 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Fix for git-rev-list --merge-order B ^A (A,B share common base)

On 6/30/05, Junio C Hamano <junkio@cox.net> wrote:
Show 14 quoted lines
> >>>>> "JS" == Jon Seymour <jon.seymour@gmail.com> writes:
> 
> I am puzzled about this part.
> 
> JS> The unit test changes in this patch remove use of the --show-breaks
> JS> flags from certain unit tests. The changed --merge-order behaviour
> JS> changed the annotation that --show-breaks prints for certain test cases.
> JS> The new behaviour is reasonable and irrelevant to the intent of the tests
> JS> so that tests have been changed to eliminate the spurious behaviour.
> 
> If the behaviour of --show-breaks subtly changes, and if that
> changed behaviour is something still acceptable, why not update
> the test to show the new expected results since you are updating
> the test anyway?
I can do it this way, if you prefer. The issue was that the expected output was:
= l2
| l1 
| l0
but became:
^ l2
| l1
| l0

The annotation changes because there is no longer a single head in the start list. There are now multiple heads, it just happens that one of the heads is also a prune point.

> 
> Showing that "subtle" change in the diff may draw people's
> attention and would help you to verify that the behaviour change
> is not something that would be unacceptable to them.
Fair enough, I'll resubmit with a less drastic change to the test case.
Show 7 quoted lines
> 
> Also if you are changing t6001, could you also merge Mark
> Allen's BSD portability fix while you are at it?
> 
>     Message-ID: <20050628014337.18986.qmail@web41205.mail.yahoo.com>
> 
> 
Ok.
jon.
Jon Seymour· Jun 30, 2005, 02:47 UTC · re: Jon Seymour · lore

Re: [PATCH 2/2] Fix for git-rev-list --merge-order B ^A (A,B share common base)

Show 9 quoted lines
> > Also if you are changing t6001, could you also merge Mark
> > Allen's BSD portability fix while you are at it?
> >
> >     Message-ID: <20050628014337.18986.qmail@web41205.mail.yahoo.com>
> >
> >
> 
> Ok.
>

Sorry, forgot to do this. However, Mark's patch should still apply as it is (or will do so with minimal edits), so I'll leave that to Linus' discretion.

jon.
-- 
homepage: http://www.zeta.org.au/~jon/
blog: http://orwelliantremors.blogspot.com/

← back to recent threads