On Wed, Dec 12, 2012 at 4:46 PM, Michael Dürig <[email protected]> wrote: > Hi, > > Currently the Microkernel contract does not specify a merge policy but is > free to try to merge conflicting changes or throw an exception. I think this > is problematic in various ways: > > 1) Automatic merging may violate the principal of least surprise. It can be > arbitrary complex and still be incorrect wrt. different use cases which need > different merge strategies for the same conflict. > > 2) Furthermore merges should be correctly mirrored in the journal. According > to the Microkernel API: "deleting a node is allowed if the node existed in > the given revision, even if it was deleted in the meantime." So the > following should currently not fail (it does though, see OAK-507): > > String base = mk.getHeadRevision(); > String r1 = mk.commit("-a", base) > String r2 = mk.commit("-a", base) > > At this point retrieving the journal up to revision r2 should only contain a > single -a operation. I'm quite sure this is currently not the case and the > journal will contain two -a operations. One for revision r1 and another for > revision r2.
if that's the case then it's a bug. the journal must IMO contain the exact diff from a revision to its predecessor. cheers stefan > > 3) Throwing an unspecific MicrokernelException leaves the API consumer with > no clue on what caused a commit to fail. Retrying a commit after some client > side conflict resolution becomes a hit and miss. See OAK-442. > > > To address 1) I suggest we define a set of clear cut cases where any > Microkernel implementations MUST merge. For the other cases I'm not sure > whether we should make them MUST NOT, SHOULD NOT or MAY merge. > > To address 2) My preferred solution would be to drop getJournal entirely > from the Microkernel API. However, this means rebasing a branch would need > to go into the Microkernel (OAK-464). Otherwise every merge defined for 1) > would need to take care the journal is adjusted accordingly. > Another possibility here is to leave the journal unadjusted. However then we > need to specify MUST NOT for other merges in 1). Because only then can > clients of the journal know how to interpret the journal (receptively the > conflicts contained therein). > > To address 3) I'd simply derive a more specific exception from > MicroKernelException and throw that in the case of a conflict. See OAK-496. > > Michael
