[darcs-users] darcs patch: Make Darcs.Repository.Internal compile w... (and 1 more)

[droundy at darcs.net]

I don't think I replied to this email, which has some interesting questions in it.  So I'll send a few replies, which may or may not be useful...

On 8/9/08, Jason Dagit <dagit at codersbase.com> wrote:
>> I'm not really sure what the difference is between tentative, pending,
>> pristine and tentative pristine, tentative pending, etc.  I was hoping you
>> could comment on this, but I forgot to ask.
>>
>> It seems like in Repository p C(r u t), that maybe the pending/tentative
>> state is t, and the pristine state is r?  I'll make that assumption for
>> now
>> and see what type checks.  Take for example:
>> setTentativePending :: forall p C(r u t x y). RepoPatch p => Repository p
>> C(r u t) -> FL Prim C(x y) -> IO ()
>>
>> Am I updating the pending or the tentative state or some hybrid?
>>
>
> I found in another email from you:
>
> read_pending is indeed a tricky function.  It actually goes from the
> recorded to an intermediate state between the recorded and unrecorded
> states.  This intermediate state contains all the unrecorded changes
> that wouldn't show up in a simple diff of slurpies, e.g. adds, renames
> and replace patches.  The details of the pending state are intended to
> be hidden from users of Darcs.Repository, so it is probably best not
> to expose this "pending" state in the Repository type,
>
> Here you were talking about read_pending, but some of it is more general.  I
> thought the pending included adds, renames and replace patches.  Currently a
> repository has 3 phantoms associated with it,
> data Repository (p :: * C(-> * -> *)) C(recordedstate unrecordedstate
> tentativestate) = ...
>
> From what you said about read_pending, and the type we agreed on for it:
> read_pending :: RepoPatch p => Repository p C(r u t) -> IO (Sealed (FL Prim
> C(r)))
>
> Actually, let me remove the seal, because I find that helps to clarify
> things:
> read_pending :: RepoPatch p => Repository p C(r u t) -> IO (FL Prim C(r
> alpha))
>
> Here, I use greek letters for states that are existential.  So the pending
> reads as a sequence of patches coming from the currently recorded state to
> some new state that we are reluctant to track in the Repository object.  How
> is that state different than u, the unrecorded state?  I had been thinking
> of u as the literal state of the working directory.  So I guess the
> difference between u and alpha is that alpha can contain things like adds,
> renames and replace patches?  If so, then what is the tentativestate for?
>
> Perhaps because tentative and pending are such similar words, I had been
> thinking of them as refering to the same thing.  For example, I thought you
> read the pending patchs to get the tentative state.  I was thinking that the
> pending state of the repository was the same as the tentativestate in
> Repository above.  But, if that were true, you would have told me this type
> for read_pending:
> read_pending :: RepoPatch p => Repository p C(r u t) -> IO (FL Prim C(r t))
>
> I think it comes down to the following 3 questions:
> What are the things represented by tentative state?

The tentative state is what will be the recorded state, if we finalize the tentative changes.  Basically, it's an intermediate state that allows us to build up a change gradually (e.g. pulling several patches or removing one patch and replacing it with another), but then atomically update the repository.

> What are the things represented by pending state?

This is the unrecorded changes that can't be determined by diffing the working directory and the pristine state, i.e. replaces, adds, removes and setprefs.  This state shouldn't ever be user-visible, but users can read it from _darcs/patches/pending.

> What are the things represented by unrecorded state?

This is the state of the working directory + pending changes.  It's described by whatsnew.

> These are my attempts and questions of the signatures:
> add_to_pending :: RepoPatch p => Repository p C(r u t) -> FL Prim C(u y) ->
> IO ()
>
> The interesting bit is here:
> add_to_pending repo p =
>     do pend <- get_unrecorded repo
>        make_new_pending repo (pend +>+ p)
>
> Given this signature for get_unrecorded:
> get_unrecorded :: RepoPatch p => Repository p C(r u t) -> IO (FL Prim C(r u))
> Which makes sense to me.  When you get the unrecorded patches they go from
> the recorded state to the unrecorded state.
>
> So, in add_to_pending, that means pend :: C(r u), and thus p must have C(u y). 
> What can that tell us about make_new_pending then?
> make_new_pending :: forall p C(r u t y). RepoPatch p => Repository p C(r u t) -> FL Prim C(r y) -> IO ()
>
> Moving on...
> get_unrecorded_no_look_for_adds :: RepoPatch p => Repository p C(r u t) ->
> IO (FL Prim C(r u))
> get_unrecorded_unsorted :: RepoPatch p => Repository p C(r u t) -> IO (FL
> Prim C(r u))
> get_unrecorded_private :: RepoPatch p => ([DarcsFlag]->[DarcsFlag]) ->
> Repository p C(r u t) -> IO (FL Prim C(r u))
>
> I guess all of the get_unrecorded* functions should have the same witness
> types.  The only question I have, is whether filtering, such as
> _no_look_for_adds, should return IO (FL Prim C(r y)), or IO (FL Prim C(r
> u)).  If you get_unrecorded and get_unrecorded_no_look_for_adds from the
> same repository you can't be sure that return sequences have the same ending
> context.  So, I'll update the filtering versions to return a context of C(r
> y).
>
> This one looks okay to me:
> read_repo :: RepoPatch p => Repository p C(r u t) -> IO (PatchSet p C(r))
>
> Again, looks okay:
> readTentativeRepo :: RepoPatch p => Repository p C(r u t) -> IO (PatchSet p
> C(t))
>
> Not sure, what patch are we looking at...I looked at the function body but
> it didn't give me any insight.
> makePatchLazy :: RepoPatch p => Repository p C(r u t) -> PatchInfoAnd p C(x
> y) -> IO (PatchInfoAnd p C(x y))

Yes, this seems right.

> I assumed the working was the unrecorded state.  Is that true?
> applyToWorking :: Patchy p => Repository p1 C(r u t) -> [DarcsFlag] -> p C(u y) -> IO ()

Yes, that's true.

> handle_pend_for_add :: forall p q C(r u t x y). (RepoPatch p, Effect q)
>                     => Repository p C(r u t) -> q C(x y) -> IO ()
> handle_pend_for_add appears to read some tentative patches as the pending,
> removes the (q C(x y)) from that pending and writes the pending out again
> (or is it writing a tentative?).  It seems as though we can't improve this
> type signature.

It might be improvable, but it's not exported, so it's not such a big deal.

> The next 3 seem to be okay:
> tentativelyMergePatches :: 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)))
> 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)))
> tentativelyMergePatches_ :: forall p C(r u t y). RepoPatch p
>                          => MakeChanges
>                          -> 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)))
>
> applyToTentativePristine :: (Effect q, Patchy q) => Repository p C(r u t) -> q C(r y) -> IO ()
> If this applied to pristine I would say, that the patch needs to be in
> context C(r y), but it says it's applying to a tentative pristine.  So does
> that mean it should be q C(t y)?

Hmmm.  I think C(t y) makes sense.  But this is an internal function, which needs to deal with repositories that are in an inconsistent state, so it might be trickier than that.

> These two depend on the semantics of applyToTentativePristine
> tentativelyAddPatch :: RepoPatch p
>                     => Repository p C(r u t) -> [DarcsFlag] -> PatchInfoAnd p C(r y) -> IO ()
> tentativelyAddPatch_ :: RepoPatch p
>                      => UpdatePristine -> Repository p C(r u t) -> [DarcsFlag]
>                      -> PatchInfoAnd p C(r y) -> IO ()
>
> This one looks fine to me:
> tentativelyAddToPending :: forall p C(r u t y). RepoPatch p
>                         => Repository p C(r u t) -> [DarcsFlag] -> FL Prim C(t y) -> IO ()
>
> setTentativePending :: forall p C(r u t y). RepoPatch p => Repository p C(r u t) -> FL Prim C(t y) -> IO ()
> I think what it means is that, pending will become whatever we set it to.  I
> think the tentative part is just there to mean it hasn't been finalized?
> Looking at the internals and the name, I think this is what we want.
>
> I think prepend, since it's almost identical to setTentativePristine should
> have the same type signature:
> prepend :: forall p C(r u t y). RepoPatch p => Repository p C(r u t) -> FL
> Prim C(t y) -> IO ()
>
> These seemed to be okay:
> tentativelyRemovePatches :: RepoPatch p => Repository p C(r u t) ->
> [DarcsFlag]
>                          -> FL (Named p) C(x t) -> IO ()
> tentativelyRemovePatches_ :: RepoPatch p => UpdatePristine
>                           -> Repository p C(r u t) -> [DarcsFlag]
>                           -> FL (Named p) C(x t) -> IO ()
> tentativelyReplacePatches :: RepoPatch p => Repository p C(r u t) ->
> [DarcsFlag]
>                           -> FL (Named p) C(x t) -> IO ()
>
> I think this one is okay:
> remove_from_unrevert_context :: forall p C(r u t x). RepoPatch p
>                              => Repository p C(r u t) -> FL (Named p) C(x t) -> IO ()
>
>
> After making the changes above I still get some type errors.  I'll look into
> those errors, but in the mean time if you could comment on the above
> signatures that would help me a ton!

I've commented on some of these, and you can ask about more.  There's a fundamental problem with adding type checking in this way, which is that we don't yet have a framework for adding type witnesses into IO actions.  i.e. any function that changes the "tentative" state really should change the type witness associate with it, but these functions cannot do this.  To do so would require a new "tricky" monad-like object, as we've discussed in the past.

e.g.

data WIO p C(r u t t') a = WIO (Repository p C(r u t) -> IO a)

(I'm omitting the p below, partly because I forgot about it, and partly because I'm lazy)

(*>>=*) :: WIO C(r u t t') a -> (a -> WIO C(r u t' t'') b) -< WIO C(r u t t'') b
(*>>*) :: WIO C(r u t t') a -> WIO C(r u t' t'') b -> WIO C(r u t t'')
returnW :: a -> WIO C(r u t t) a
returnW = WIO . const . return

io :: IO a -> WIO C(r u t t) a
io = WIO . const

And I wrote more of this email and then lost it.  We'd need to rewrite withGutsOf to accept a WIO as an argument, and we'd make

withRepoLock :: [DarcsFlag] -> (forall p C(r u). RepoPatch p => Repository p C(r u u) -> IO a) -> IO a

and various other changes such as

tentativelyAddPatch :: RepoPatch p => [DarcsFlag] -> PatchInfoAnd p C(t t') -> WIO p C(r u t t') ()

Anyhow, this will be a lot of work...

David