[darcs-devel] darcs patch

[David Roundy]

On Sat, Nov 27, 2004 at 11:13:28AM +0100, Tomasz Zielonka wrote:
> On Fri, Nov 26, 2004 at 03:34:18PM -0500, Ian Lynagh wrote:
> > It takes a while before it prompts you at all. I think this is due to
> > time spent comparing patches for equality when doing things like
> > is_patch_first. Given we have unique names in this case (the filename
> > in _darcs/patches) I think these should be used for the comparison,
> > only falling back to == if we don't have a name.

Indeed, I think that we ought to be able to

{
hunk ./Patch.lhs 151
              deriving (Ord)
 
 instance Eq Patch where
-    (NamedP n1 d1 p1) == (NamedP n2 d2 p2) = n1 == n2 && d1 == d2 && p1 == p2
+    (NamedP n1 d1 p1) == (NamedP n2 d2 p2) = n1 == n2
     (Move a b) == (Move c d) = a == c && b == d
     (DP d1 p1) == (DP d2 p2) = d1 == d2 && p1 == p2
     (FP f1 fp1) == (FP f2 fp2) = f1 == f2 && fp1 == fp2
}

with safety.  The only instance in which the d1 == d2 and p1 == p2 would
have an effect is if there is a bug in darcs or a corrupt repository (in
which case they wouldn't necesarily cause more "correct" behavior than if
they were left out..

> Isn't it possible to have patches with the same identity but a different
> representation, as a result of commutation? Should the be equal?

It is a bug to compare patches which have different context, since two
*different* patches could give a false positive.  So your concern shouldn't
be an issue.

That said, PatchChoices.is_patch_first *does* compare patches which have
different context, because it is using the equality to test which of the
finite set of patches (it is known that the patch is *one* of the set) is
being queried.  So there is no danger of false inequality, only of false
equality.

On Fri, Nov 26, 2004 at 03:34:18PM -0500, Ian Lynagh wrote:
> Just realised something - is this even valid? Given the commutating that
> goes on, can't we get false positives where we have two patches that
> look identical?

One could indeed confuse PatchChoices by creating two patches with the same
representation, such as

darcs mv foo bar
darcs mv bar baz
darcs mv one foo
darcs mv foo bar

I think you'd have trouble selecting patches after this.  :(

> Unless I'm confused, to fix this (if it's a bug), and also to get the
> above speedup when doing record (i.e. where we don't have names yet), we
> could instead tuple patches with unique names (e.g. Integers) at the
> start of the patch selection and use these for the equality checks.

Yes, this would be a good idea.  Somehow, when I wrote that code, it didn't
occur to me that users could create multiple identical patches.
-- 
David Roundy
http://www.darcs.net