[darcs-devel] repo type witnesses [was: Handle the rebase patch at the repo layer]

[Ganesh Sittampalam]

Hi,

On 22/06/2018 21:15, Benjamin Franksen wrote:

> There are good news on another front though, namely the
> recorded/tentative business. Here are my latest thoughts on the issue.
> 
> We have commands that potentially change the repo and those that don't.
> Those that don't should directly access the recorded state but not the
> tentative state (it may not even exist). Those that do must take the
> repo lock and then call revertRepositoryChanges, which copies the
> recorded to the tentative state.

As an aside we should rename revertRepositoryChanges to
startRepositoryChanges, or provide an alias :-) It's a very confusing
name in this context, though it's the right name if you are abandoning a
transaction.

> From that point on they should read and
> write the tentative state only until they call finalizeRepositoryChanges
> which (atomically) overwrites the recorded with the tentative state.
> 
> The presence of both wR and wT in the Repository type allows to access
> the recorded state after revertRepositoryChanges as well as to access
> the tentative state before revertRepositoryChanges or after
> finalizeRepositoryChanges, i.e. outside of a transaction.
> 
> We fix this by adding a phantom type parameter for the transaction state:
> 
>> {-# LANGUAGE DataKinds, GADTs, TypeFamilies #-}
>> {-# LANGUAGE StandaloneDeriving #-}
>>
>> data Transaction = RO | RW
> 
> RO stands for read-only, RW for read-write. (All names here are
> provisional, of course, bikeshed them to your liking...)

The principle sounds good to me. I was thinking of suggesting it before,
but I wasn't sure if you'd want more type-level tricks.

I don't think the names matter too much as long as we clearly document
them, but the concept of a "RO transaction" which isn't actually a
transaction feels a bit confusing, so my suggestion is NoT | InT.
Anything vaguely reasonable is fine with me.

> The Repository type gets an additional phantom type parameter for the
> transaction state. The wT parameter can be dropped now, and wR stands
> for recorded/tentative state. We add wP for the pending/tentative
> pending state. More parameters should be added as appropriate e.g. for
> the rebase state.
> 
>> data Repository (x :: Transaction) wR wP wU

We might want to do this change in two passes, one that removes wT and
one that adds wP, to avoid any risk of getting the parameters mixed up.
It would mean more churn and I think a mixup is not that likely, but I
think it'd also make the diffs easier to read.

Cheers,

Ganesh