[darcs-users] darcs patch: partial type witnesses in Unrevert

[Jason Dagit]

On Tue, Aug 19, 2008 at 8:23 AM, David Roundy <droundy at darcs.net> wrote:
> On Fri, Aug 15, 2008 at 02:52:41PM -0700, Jason Dagit wrote:
>> On Fri, Aug 15, 2008 at 2:30 PM, David Roundy <droundy at darcs.net> wrote:
>> > I can see two simple workarounds for this:  one would be to simply
>> > pattern match on (h_us:<:NilRL) and leave any other case in the
>> > "impossible" category.  The other would be to simply use concatRL,
>> > which in this case is equivalent to headRL, except in type.  I think
>> > the former possibility is slightly cleaner, as it documents the
>> > stupidity in the interface.  But if my analysis is wrong and there
>> > really is a bug, then you won't have fixed the bug, you'll just have
>> > made it exit with an error message.
>>
>>
>> Even if I use the patten match above, we have the problem that
>> considerMergeToWorking needs to know that both patch sequences start from
>> the unrecorded state.  So, I'm left wondering how to extract that proof from
>> get_common_and_uncommon.  I feed get_common_and_uncommon two sequences, us
>> :: C(() r), and them :: C(() a).  Actually, I'm not confident that them
>> should be a patchset.  I hava feeling it's supposed to be them :: C(a b)
>> when I look at how it was constructed.  But if that's the case then it
>> wouldn't even be safe to call get_common_and_uncommon.  It's created by
>> reading the repository's unrevert then doing a scan_bundle on that to
>> convert to a PatchSet, but I find it hard to believe that scan_bundle
>> returns a patchset.  It seems more likley that scan_bundle returns an RL (RL
>> p)) C(x y), and it's up to the caller to tell scan_bundle the true meaning
>> of those contexts.
>
> Okay, one of your questions is easily answered, which is that
> scan_bundle does correctly return a PatchSet, it just returns a lazy
> patchset that doesn't have all of its elements.  The key is that the
> patch bundle includes the context.  Looking at the code, I just
> noticed that we don't actually return a proper lazy PatchSet in
> scan_bundle, which I should definitely fix (perhaps after the big push
> that I'm doing right now).

Oh okay.  I'll try to fold this in with the changes below.

>
> The other problem is that the type of considerMergeToWorking is wrong,
> which keeps things from matching up.  At least, this is true in my
> copy of the repository, which shows:
>
> considerMergeToWorking :: RepoPatch p
>                       => Repository p C(r u t) -> String -> [DarcsFlag]
>                       -> FL (PatchInfoAnd p) C(u r) -> FL (PatchInfoAnd p) C(u y)
>                       -> IO (Sealed (FL Prim C(u)))
>
> and should be something more like
>
> considerMergeToWorking :: RepoPatch p
>                       => Repository p C(r u t) -> String -> [DarcsFlag]
>                       -> FL (PatchInfoAnd p) C(x r)
>                       -> FL (PatchInfoAnd p) C(x y)
>                       -> IO (Sealed (FL Prim C(u)))

Excellent.  This should cause Repository.Internal to stop working
again, but it should also fix one of the witness mismatches I'm seeing
in every command that calls considerMergeToWorking (and the related
functions like tentativelyAddPatches).

Thanks!
Jason

> But I've gotten behind reviewing your patches, so maybe what I've got
> is out-of-date.

In this case no.

Thanks,
Jason