On Wed, 2010-08-25, Paul Burba wrote:
> On Tue, Aug 24, 2010 at 7:25 AM, Julian Foad <julian.foad_at_wandisco.com> wrote:
[...]
> Agreed, we simply can't be sure what the user's intention was...I'm
> beginning to be swayed to the idea that restoring the missing subtrees
> may not be the ideal approach...
[...]
> > The pattern that's emerging from my thoughts is: throw an error if
> > logically we need to access the missing working version of the node. If
> > we don't need to access it, just let it be. Never "restore" it unless
> > the user specifically requests so and says what kind of restoration is
> > required.
> >
> > For these reasons I think "merge" should also throw an error if it needs
> > to merge changes into a missing node. (If instead it needs to delete
> > it, then it has the options I mentioned for "svn delete".)
> >
> > But I suspect part of the reason why "restore" seems an attractive
> > option is because Subversion isn't very friendly when "merge" stops with
> > an error. We don't provide any way to resume the same merge,
>
> Quite right about the unfriendliness. Resuming the merge is really a
> hit-or-miss proposition, depending on how much of the merge was done
> successfully prior to encountering the missing subtree. If it
> encountered early, before the application of any diffs, then things
> are ok, but after that things get ugly fast, particularly if the merge
> target originally had any local mods.
>
> It's worth noting again there there are *two* error-out approaches:
>
> i) Throw an error when a subtree the editor needs is found to be
> missing. This is what you are talking about here.
>
> ii) Identify any missing subtrees at the *start* of the merge and
> throw an error before any editor drives are done. Basically we
> disallow merges with missing subtrees due to OS-level deletes.
You're right, I hadn't thought of erroring out before starting the
merge, and that is a much, much better option than erroring out in the
middle.
> > and we
> > don't make it particularly easy to roll back the merge (although that's
> > getting better now that "revert" is, I hope, going to remove nodes that
> > it created by copying), and we don't have any way at all to roll back a
> > merge performed in a non-clean WC. So we're trying to avoid erroring
> > out.
> >
> > Long term, those difficult problems are are the problems we should be
> > looking to solve. Short term, I suppose it's useful to avoid erroring
> > out as much as we can.
>
> Equally bad is the case with trunk right now, where we simply ignore
> missing directories, even if the merge would affect them - see
> http://subversion.tigris.org/issues/show_bug.cgi?id=2915#desc4 for an
> example.
>
> > If that's the important issue, and we recognize
> > it as such, then I could support the idea of "merge" doing this
> > "restore" while other commands don't.
>
> Given what you've said here and thinking anew about merge and missing
> subtrees, I think the best approach might simply be what we currently
> do with files: Skip the missing subtree and record non-inheritable
> mergeinfo so the missing subtree is handled correctly (i.e. the
> mergeinfo reflects the fact that the merge never touched the missing
> subtree).
>
> This has a few things going for it:
>
> A) It's consistent with 1.5-1.6's treatment of missing files.
>
> B) As Daniel hinted at in
> http://svn.haxx.se/dev/archive-2010-08/0189.shtml, it's consistent
> with how merge tracking treats every other type of "missing" subtree
> (e.g. shallow WCs, switches, paths missing due to authz restrictions).
>
> C) It makes no assumptions about what the user intended, it just deals
> with the fact that the subtrees are missing.
>
> D) It allows the merge to complete, no errors mid-merge.
>
> E) It allows the missing subtrees to be restored via update (either
> before or after the merge is committed) and for the merge to be
> repeated. The repeat merge will notice that the merge never touched
> the subtrees and will drive the editor such that only those subtrees
> have the merge repeated.
>
> Any thoughts to this approach?
Another plus:
F) It means the merge algorithm has one less case that can choke it,
which is better than relying on a check to have been done beforehand.
It makes the subroutines easier to re-use (callable by GUIs etc.). And
it could be extremely difficult to make sure that such an external check
exactly matches the merge code in all the corner cases.
The only minus I can think of: In many ways we would serve our users
better by simply not allowing them to get into complex situations and
instead just disallowing such a merge.
But the many advantages seem to outweigh that, and your suggestion
sounds close to perfect.
If you can do that without much additional complexity, +1.
- Julian
Received on 2010-08-26 14:08:19 CEST