[darcs-devel] [issue1026] Darcs trouble

[Jason Dagit]

Jason Dagit <dagit at codersbase.com> added the comment:

On Fri, Aug 22, 2008 at 4:59 AM, Simon Peyton-Jones <bugs at darcs.net> wrote:
>
> New submission from Simon Peyton-Jones <simonpj at microsoft.com>:
>
> I am continuing to have trouble with Darcs.  Since the darcs community is now active again, I'll try to continue to report my troubles.  I cannot promise that I am reporting distinct problems.
>
> On Windows, using darcs2.
>
> Simon
>
> See the trace below.  I'm using the darcs-all script to pull in patches from a peer directory.  I suspect (but I do not know) that the script isn't doing the Right Thing (see the line in red below).  But regardless, darcs should not crash with "bug in get_extra".   Doing so does not induce confidence.

For some reason the red has been stripped out, but in this case it's
still quite clear you're having a problem.

> sh-2.04$ darcs2 pull c:/simonpj/darcs//HEAD --repodir libraries/array
> Pulling from "c:/simonpj/darcs/HEAD"...
> darcs2.exe: bug in get_extra commuting patch:

Eric,

Weren't we discussing last week the circumstances under which a user
could get a bug in get_extra?  What did we decide at that time?  This
is what I recall (pasting realvant parts of the code):

-- | Returns a sub-sequence from @patches@, where all the elements of
@common@ have
-- been removed by commuting them out.
get_extra :: RepoPatch p => [PatchInfo] -- ^ @common@
          -> RL (PatchInfoAnd p) C(u x) -- ^ @patches@
          -> Either MissingPatch (FlippedSeal (RL (PatchInfoAnd p)) C(y))

              case commuteFL (p :> skpd) of
                Just (skipped_patch' :> p') -> do
                    FlippedSeal x <- get_extra_aux (return
skipped_patch') common pps
                    return $ flipSeal (info hp `piap` p' :<: x)
                -- Failure to commute indicates a bug because it means
                -- that a patch was interspersed between the common
                -- patches.  This should only happen if that patch was
                -- commuted there.  This uses 2 properties:
                -- 1) commute is its own inverse
                -- 2) if patches commute in one adjacent context then
                --    they commute in any context where they are
                --    adjacent
                Nothing -> errorDoc $ text "bug in get_extra commuting patch:"
                           $$ human_friendly (info hp)

Triggering this bug basically means, your repositories merged together
at some point in the past, but now the patches fail to commute like
they used to.  This code be a bug in commutation, but I think it's
more likely to be due to index or patch corruption in one or both
copies of the repository.

Thinking about the above, I guess another way this could happen is if
the user amend records a patch and then later tries to merge their
repo with one containing the original patch?

I think we should try printing the patch in skpd that didn't commute
with p.  This could be done by writing a function (untested, but
should be right; and more elegant combining of the Maybe and Either
monads is welcome):
commuteFL2 :: Commute p => (p :> FL p) C(x y) -> Either (Sealed2 p)
((FL p :> p) C(x y))
commuteFL2 (p :> NilFL) = return (NilFL :> p)
commuteFL2 (q :> p :>: ps) = case commute (q :> p) of
                             Just (p' :> q') ->
                               case commuteFL2 (q' :> ps) of
                               Right (ps' :> p'') -> return (p' :>: ps' :> q'')
                               p'' -> p''
                             Nothing -> fail $ seal2 p

Then we could see not only the patch that should have commuted, but
the patch it failed to commute with.  If this is an amend-record
problem it might make it really obvious that it is.  Otherwise, it may
just help with debugging.

Yet another debugging trick to apply is to figure out if this is
happening in the first or second call to get_extra in the function
gcau_simple.  It's especially interesting if it happens in one and not
the other.  I think this could be implemented via a catch in
gcau_simple, but if not we can rewrite get_extra to return the error
message too.  Then we try to compare the patches in question to see if
we can tell why the commute failed.

Another possibility is that we're wrong about the assumptions above
some how, but that is trickery to understand.

We may need to fold David into the discussion on this one.

Jason

----------
title: pull => bug in get_extra commuting patch (2.x) -> Darcs trouble

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/issue1026>
__________________________________