threads / discuss / 4488

cvsps wierdness

Subject: cvsps wierdness

## tl;dr

4 messages between Jun 12, 2006 and Jun 13, 2006.

replies: 3people: 2as markdown or json

Robin Rosenberg (list subscriber)· Jun 12, 2006, 20:47 UTC · lore

I sometimes get out-of-order imports with git-cvsimport when consecutive commits have the same message. I narrowed it down to a small demo:

#!/bin/bash -x mkdir demo cd demo export CVSROOT=$(pwd)/cvsrepo mkdir $CVSROOT cvs init mkdir $CVSROOT/x cvs co -d sandbox x cd sandbox echo X >k.txt cvs add k.txtsometimes cvs commit -m "A commit" sleep 1 echo a >v.txt cvs add v.txt cvs commit -m "A commit" sleep 1 echo b >v.txt cvs commit -m "A commit" cvsps -x --norc

-- end --

The script creates a small CVS repo with three commits on two files. What's odd is that cvsps lists revision 1.2 of the file v.txt *before* version 1.1, like this:

---------------------
PatchSet 1
Date: 2006/06/13 00:34:15
Author: roro
Branch: HEAD
Tag: (none)
Log:
A commit

Members:
        k.txt:INITIAL->1.1
        v.txt:1.1->1.2

---------------------
PatchSet 2
Date: 2006/06/13 00:34:17
Author: roro
Branch: HEAD
Tag: (none)
Log:
A commit

Members:
        v.txt:INITIAL->1.1

Maybe someone with cvsps insight can spot the error? If you don't get the same error I wouldn't be surprised because I had a similar example that would not repeat itself on both of the machines I tried it. This one however "works" every time (on my machines).

-- robin

Linus Torvalds· Jun 12, 2006, 21:27 UTC · re: Robin Rosenberg (list subscriber) · lore

Re: cvsps wierdness

On Mon, 12 Jun 2006, Robin Rosenberg (list subscriber) wrote:

> 
> The script creates a small CVS repo with three commits on two files. What's 
> odd is that cvsps lists revision 1.2 of the file v.txt *before* version 1.1, 
> like this:

What seems to happen is that the two changes to v.txt are broken up into separate changesets (because they touch the same file), but then the _first_ one is merged with the changeset that contains the k.txt change (because they have the same log message, and roughly the same date).

Then, because it has the earlier date, that combined changeset ends up being considered to be "before" the later one, even though it contains a version of v.txt that is newer.

Does this patch fix it for you (untested - it could result in tons of other trouble, but it basically just says that time ordering is less important than member revision ordering).

I don't think this is strictly correct, btw. I suspect you can still get into strange situations where the changeset merging has resulted in one file ordering one way, and another file ordering the other way.

I really don't think cvsps is really very good about this.

		Linus

---

diff --git a/cvsps.c b/cvsps.c
index 2695a0f..daa93a3 100644
--- a/cvsps.c
+++ b/cvsps.c
@@ -1662,14 +1662,14 @@ static int compare_patch_sets_bytime(con
      * know that insertions are unique at this point.
      */
 
-    diff = ps1->date - ps2->date;
-    if (diff)
-	return (diff < 0) ? -1 : 1;
-
     ret = compare_patch_sets_by_members(ps1, ps2);
     if (ret)
 	return ret;
 
+    diff = ps1->date - ps2->date;
+    if (diff)
+	return (diff < 0) ? -1 : 1;
+
     ret = strcmp(ps1->author, ps2->author);
     if (ret)
 	return ret;

Linus Torvalds· Jun 12, 2006, 21:30 UTC · re: Linus Torvalds · lore

Re: cvsps wierdness

On Mon, 12 Jun 2006, Linus Torvalds wrote:

> 
> I don't think this is strictly correct, btw. I suspect you can still get 
> into strange situations where the changeset merging has resulted in one 
> file ordering one way, and another file ordering the other way.

Btw, I also fear that this could make cvsps noticeably slower, since the patchset comparison code is pretty expensive (O(n*m) in files in each patch-set).

		Linus

Robin Rosenberg (list subscriber)· Jun 13, 2006, 06:06 UTC · re: Linus Torvalds · lore

Re: cvsps wierdness

måndag 12 juni 2006 23:27 skrev Linus Torvalds: [...]

> Does this patch fix it for you (untested - it could result in tons of
> other trouble, but it basically just says that time ordering is less
> important than member revision ordering).

Thanks, it worked on the simple case at least. We'll see about the original full repo later.

>
> I don't think this is strictly correct, btw. I suspect you can still get
> into strange situations where the changeset merging has resulted in one
> file ordering one way, and another file ordering the other way.

Doesn't cvsps's conflict handing simply break up those patches into several patches? More patches is ok. Suboptimal patches can be accepted

-- robin