Re: [MirageOS-devel] Merging in Irmin

[Thomas Leonard <talex5@xxxxxxxxx>]

On 9 April 2015 at 18:22, Thomas Gazagnaire <thomas@xxxxxxxxxxxxxx> wrote:
>>>> BC.make_head :
>>>>   Irmin.config -> ('a -> Irmin.task) -> parents:head list ->
>>>>   msg:'a -> View.t -> ('a -> t) Lwt.t
>
>
> Do you really need a View.t here? How do you build that value? Do you first 
> take a view from somewhere and then tweak it to get the value you want for 
> the merge? Or you just run a sequence of updates?

There are two cases. If the branch already contains a commit:
1. I use View.of_path to get the view.
2. I use BC.of_head to make a temporary branch.
3. I commit to the temporary branch with View.update_path.
4. After testing the commit, I use BC.compare_and_set_head to commit
to the main branch.

If the branch is empty:
1. I use View.empty to get the view.
2. I commit directly to the master branch with View.update_path (no
testing is possible in this case).

This works - it would just be a bit neater if there was a uniform way
to make a commit from a view.

> I've been playing with that API a little bit but that's not very easy to do 
> actually (currently View depends on BC, and adding your function will make 
> the whole thing recursive). I can either:
> 1. add the function in View (ie. `View.make_head: db -> task -> parents:head 
> list -> contents:t -> head`) which is a bit weird but should work;
> 2/ or I can expose a staging area as a HRW store (i.e. without the subpath 
> functions which appear in the View signature), so all updates should go 
> through that module and that will be incompatible with normal views.
> 3. or I can implement immutable views to have a proper staging area using 
> immutable prefix trees.
>
> I'm in favour of 1. for now on and hopefully one day we'll have 3. Is it fine 
> with you?

Sounds good to me. Thanks!

> Thomas
>
>
>>>>
>>>> to implement CueKeeper's API:
>>>>
>>>> module Commit : sig
>>>>   type t
>>>>   val commit :
>>>>     parents:t list ->
>>>>     Staging.t ->
>>>>     msg:string ->
>>>>     t Lwt.t
>>>>
>>>> How can I take two commits, generate a view (manually) with the
>>>> results of my custom merge, and then add the result as a new commit
>>>> with both of the original parents?
>>>
>>> Something like:
>>>
>>> let commit ~parents staging ~msg =
>>>      match staging.Staging.commit with
>>>      | None -> assert false
>>>      | Some t ->
>>>      I.of_head t.c_repo.config t.c_repo.task_maker (id t) >>= fun 
>>> tmp_branch ->
>>>      (* THE ONLY CHANGE *) V.set_parents Staging.view parents;
>>>      V.update_path (tmp_branch msg) I.Key.empty staging.Staging.view >|= 
>>> fun () ->
>>>      {t with c_store = tmp_branch}
>>
>> It doesn't seem to work. I tested with this code (on my "test_merging" 
>> branch):
>>
>> https://github.com/talex5/cuekeeper/commit/4da442f91422bd6b8654922ca6955cf78cd9d83d
>>
>>        Git.Commit.checkout base >>= fun stage ->
>>        Git.Staging.update stage ["foo"] "a" >>= fun () ->
>>        Git.Commit.commit ~parents:[base] ~msg:"A" stage >>= fun a ->
>>        Git.Staging.update stage ["foo"] "b" >>= fun () ->
>>        Git.Commit.commit ~parents:[base] ~msg:"B" stage >>= fun b ->
>>        Git.Staging.update stage ["foo"] "merged" >>= fun () ->
>>        Git.Commit.commit ~parents:[a; b] ~msg:"Merge" stage >>= fun result ->
>>        Git.Branch.fast_forward_to master result >>= function
>>
>> For testing, it uses the Git backend with the directory "test_db".
>> Looking at the result with gitk, I have an initial commit containing
>> "orig" with a direct child "Merge" containing "merged".
>>
>> [ I also had to expose "VIEW.head = S.head" in Irmin:
>> https://github.com/talex5/irmin/commit/d77f81c0786860a58a8a596bca8426d1e1f98661
>> and I modified your code slightly to make "parents" optional and to be
>> a Commit.t, but that shouldn't make any difference I think. ]
>>
>>> I'll see if I can add your `make_head` function.
>>>
>>> Thomas
>>>
>>>>
>>>>>> Currently, I merge to create a new commit, test it, and then do a
>>>>>> fast-forward to update the branch to include the merge if the test
>>>>>> passes. But if I can use custom merge code, then it would be OK to
>>>>>> merge directly to the branch when my merge code returns, since it will
>>>>>> already have had a chance to test it.
>>>>>
>>>>> In the PR, I've also added `Irmin.fast_forward_head` (maybe it should be 
>>>>> `fast_forward_to_head`?) to to that. It returns "false" (and does 
>>>>> nothing) it the new head is not strictly in the future of the current 
>>>>> head.
>>>>
>>>> Thanks! [ As noted elsewhere, it would be useful to distinguish the
>>>> case where it's already up-to-date (success; no futher action needed)
>>>> from the case where it's not an ancestor (failure; retry merge with
>>>> new head). ]
>>>>
>>>>> Let me know if you need something else (I'm still working on the right 
>>>>> way to fix the watch API).
>>>>>
>>>>> Best,
>>>>> Thomas



-- 
Dr Thomas Leonard        http://0install.net/
GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/mirageos-devel