Re: Fold the merge_automatic API into the existing merge_peg API

Now with a patch for discussion. (More below...) Julian Foad wrote: >> Brane told me that while updating the JavaHL API he noticed that the new >> svn_client_merge_automatic() is Yet Another Merge API, and would prefer make >> the existing JavaHL merge API encompass that functionality rather than add >> yet another variant.  So I said if let's see if we can apply that idea to the >> libsvn_client API. >> >> Idea: 'automatic' merge is (in a high level logical sense) most similar to >> a subset of the 'pegged' merge.  So make 'merge_peg' do the automatic merge >> (like calling the automatic merge API) if the params are suitable.  The >> intention is that this should be easier for Subversion client developers to >> understand and for them to DTRT. >> >> API differences between pegged and automatic merge: >> >>                 Pegged merge                 Automatic merge >> >> The 'find'API:  Single merge_peg call does   Separate'find' and 'do'.. >>                 the whole process.           Result of 'find' ... used >>                                              used for 'svn mergeinfo' graph. >> >> Source:         Revision ranges X:Y[,X:Y...] Revision range 0:Y >>                 on branch URL_at_PEG            on branch URL_at_PEG >> >> Target:                                      optional non-WC tgt for 'find' >> >> If mergeinfo:   skip already-merged revs     (same) >>                 record the merge >> >> No mergeinfo:   don't skip revs              error out >>                 don't record >> >> Options differ: allow_mixed_rev              allow_mixed_rev >>                                              allow_local_mods >>                                              allow_switched_subtrees >>                 ignore_mergeinfo >> >>  At that level, it looks close enough to be feasible to embed >> 'automatic' merge inside 'pegged'.  I'm looking closer now. >> >>  Any thoughts? Branko Čibej wrote: > This is pretty much the same conclusion I came to earlier today. I looks > like the easiest thing to do would be to make the _automatic_ APIs > private within libsvn_client, move the selection logic to > svn_client_merge_pegX, and simplify the client implementation. > > The only trouble with that is, as you say, that "svn mergeinfo" relies > on having the automatic_find API available. I think this could be solved > in several ways: > > 1. by splitting the merge-peg into two, as the current automatic-merge; or, > 2. by making only the "do" phase of the automatic merge private, and >     wile leaving the "find" phase public -- but renaming it to something >     more in line with merginfo discovery; or, > 3. somehow exposing the "find" phase through one of the existing (or >     revised) svn_client_mergeinfo_* APIs. > > I'm kind of leaning towards #3, but don't have a sense of how > complicated that would be. Mark Phippard wrote: > There are obviously some benefits for having less API, especially when > it comes time to rev it for something.  That said, as a caller of the > API, it seems nice that the separate "automatic merge" API can provide > a more focused and simpler interface.  For example, the interface does > not need to expose things like a revision range, which should not > apply when doing this kind of merge, but obviously is needed when > doing a cherry-pick merge. > > Looking through a long list of API is hard sometime, but it is also > hard when you want to do something like merge everything in BranchX > and the interface requires passing a bunch of parameters that do not > directly apply. I like the focused API, but I also see how the automatic merge can be considered to fill in a bit of missing functionality that merge_peg ought to provide. Perhaps we can have both.  Teach merge_peg to DTRT in this case, and still have the focused API available for when a client knows it wants an automatic merge.  Is there sufficient merit in that to outweigh the overhead of having to test two similar but different entry points? The attached patch moves the decision to call the 'automatic merge' API from 'svn' into 'svn_merge_peg5()'.  I have run some merge tests and tree conflict tests, but not the whole test suite yet.  Here is the log msg. [[[ Teach svn_client_merge_peg5() to do an automatic merge, and let 'svn merge' rely on that instead of calling the dedicated automatic merge APIs. TODO: Decide whether to keep or make private the 'automatic merge' APIs. TODO: This reduces the verbosity of 'svn merge --verbose'. Consider   doing something about it, perhaps by adding some new notifications for the   notifier callback? * subversion/include/svn_client.h,   subversion/libsvn_client/merge.c   (svn_client_merge_peg5): Do an automatic merge if no revision range     specified. * subversion/svn/merge-cmd.c   (automatic_merge): Delete.   (run_merge): Don't take special action to handle an automatic merge, let     the pegged merge code path handle it. ]]] Thoughts? - Julian