[darcs-devel] [patch1731] store rebase patch at the repo layer

[Ben Franksen]

Ben Franksen <ben.franksen at online.de> added the comment:

Am 17.11.18 um 10:44 schrieb Ganesh Sittampalam:
>> * add PatchInfoAndG which is polymorphic in the named patch type
> 
> Fine.
> 
> It wasn't immediately obvious to me why this led to needing new
> constraints (Eq2, PatchListFormat) but it's not really a big deal.

We abstract over the type of patch that is contained in a PatchInfoAnd.
Where previously this was fixed at (Named p) we now have only p. Named p
has instances for Eq and PatchListFormat but p in general has not. What
we usually have is RepoPatch p but that doesn't have these two as
superclasses.

>> * add command 'rebase upgrade'
> 
>>      -- FIXME inlining this action below where it is used
>>      -- results in lots of ambiguous type variable errors
>>      -- which is rather strange behavior of ghc IMHO
> 
> Indeed, that's really weird. I played a bit more and it's the
> *presence* of the update_repo definition that matters; you can inline it but
> leave the definition itself, and also cut it down to
> 
>>        PatchSet ts (mapRL_RL (fmapPIAP W.fromRebasing) wps')
> 
> and there's still no type error. But I didn't manage to simply further from
> that expression or work out what's going on. I thought maybe I'd be able to
> provoke the error by leaving the definition and messing around with the
> monomorphism restriction ([No]{MonomorphismRestriction, MonoLocalBinds}) but
> didn't get anywhere.

Yeah, I remember I did a few experiments along that line myself before
giving up. If we manage to cut this down to a stand-alone example we can
ask on @ghc-users what (TF) is going on here...

>> * reliably fail if we detect that an old-style rebase is in progress
> 
> Fine (again I haven't checked every line).

That one took me a lot longer to get right than I expected.

>>   * cleanups in D.P.Named.Wrapped and D.P.Rebase.Container
> 
>> -- The witnesses are such that a @Suspended@ appears to have no effect.
>> -- This is obsolete now and is also a semantically wrong. It is kept only
>> -- because I (bf) am too lazy to fix it.
> 
> Can you explain this a bit more? I guess this is only wanted for
> upgrading old-style rebases, but I didn't see why the witness model 
> for the  old-style rebases would have changed.

Ah, no it wouldn't. I wrote that comment before I realized that we need
to be able to read old-style rebase repos so we can upgrade them. That
means the comment is no longer correct, it should say "if we didn't need
the ability to read old-style rebase, then ...".  BTW, I am glad I was
too lazy to "fix" the witnesses because I would have had to change them
back to implement rebase upgrade...

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