On Tue, Aug 24, 2010 at 7:25 AM, Julian Foad <julian.foad_at_wandisco.com> wrote:
Hi Julian,
Thanks for taking some time to think about this. Comments below.
> On Fri, 2010-08-20, Paul Burba wrote:
>> I think we can all agree that when a user deletes part of their WC via
>> the OS they have made a mistake of some sort. But which mistake
>> exactly? The obvious answer is that they really intended 'svn del
>> dirX/foo.c'. But possibly they intended something more akin to 'svn
>> up --set-depth empty dirX'. Or maybe it was just a true mistake, and
>> nothing was intended; they expect foo.c to be there!
>
> Or maybe the user *renamed* or *moved* the item to a different path,
> where it still exists complete with its local mods, and when the user
> realizes they should have used "svn mv" instead, they will want to issue
> the as-yet-unavailable "svn mv --retrospectively" command [1], or,
> failing that, they will want to move the item back into place and then
> issue the correct "svn mv".
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...
> So there are several courses of action that the user might want. Is
> there an obvious, simple and "safe" default action that Subversion could
> take in order to resolve each missing item it encounters? I think there
> isn't.
>
>
>> I originally came to this question when dealing with merge tracking
>> and paths missing because they were deleted outside of Subversion, see
>> http://svn.haxx.se/dev/archive-2010-08/0156.shtml.
>>
>> Somewhat tardily I realized the *first* question to be answered is not
>> how merge tracking should deal with such paths, but how should
>> Subversion in general deal with them.
>>
>> I see a few basic approaches:
>>
>> 1) Consistent Approach -- Restore the Missing Paths (unless there is
>> really a compelling reason not too, but the default approach is to
>> restore)
>>
>> WCNG's single DB allows us to simply restore these missing paths as we
>> encounter them (the notable exception being 'svn st' of course, which
>> just reports the missing state).
>
> For a file, the working properties (stored in ACTUAL_NODE table) are
> still known to Subversion, whereas its working text is NOT known to
> Subversion. So Subversion could "restore" the item by:
>
> * reverting the file text and keeping the working properties, or
>
> * reverting the file text and reverting the properties.
>
> For a directory, that applies to each versioned file inside it, as well
> as to the properties on the directories themselves. (Any unversioned
> items inside it would of course be lost.)
>
> Is either of those ways obviously the "right" way to restore? I would
> suggest neither is obviously the right one.
>
>
>> Exhibit A of this behavior: In 1.6.x and 1.7 in multiple DB mode, svn
>> revert skips missing directories. In single-DB mode they are
>> restored.
>
> Here, they are "restored" in the second sense above - i.e. "reverted".
>
>
>> 2) Consistent Approach: Error out
>>
>> If we detect missing paths, simply throw an error asking that the user
>> restore them before proceeding. Obviously for svn st/up/revert, the
>> current behavior is fine, but for svn ci/sw/merge we could follow this
>> route.
>
> I think this is a good approach for ci/sw/merge and in fact most
> commands. It is the simplest approach and that counts for a lot.
>
>> 3) Case-by-Case Approach:
>>
>> Maybe there is *no* consistent approach, and sometimes we will want to
>> restore, but other times we'll do something else. I'll leave what
>> "something else" is as an open question for the moment.
>
> I think consistency is important, but I'm not seeing a single behaviour
> that's suitable for all commands. Let me see...
>
> "svn revert" should of course restore any missing thing back to its last
> versioned state, because that's it's job. (In 1.6 and earlier, it was
> unable to restore missing directories, which was always known as a
> deficiency.)
>
> "svn status" should of course report the problem, because that's its
> job.
>
> "svn delete" could throw an error, or it could ignore the inconsistency
> and just schedule the thing for deletion anyway. (I suppose we could
> consider the latter behaviour to be first restoring and then deleting,
> in one operation.) Same for any other UI that's able to make the node
> go away such as "svn up --set-depth empty PARENT_PATH".
>
> "svn diff" and "svn blame" I think should throw an error if trying to
> access the missing text of the node. (Same for "svn cat -rWORKING" if
> we ever support that.) I don't see any good reason for read-only
> commands such as these to attempt to "fix" a problem.
>
> "svn copy" and "svn move" where the source is missing: throw an error.
> They could perhaps alternatively perform a copy or move in which the
> target working file/dir is still missing, but that's greater complexity
> and I see no benefit.
>
> "svn export": throw an error. It could perhaps alternatively skip the
> missing node, on the basis that its job is to re-create the current
> working state of the WC, but I think we can define 'missing' as an
> invalid (unacceptable) state.
>
> "svn cleanup": throw an error if it needs to access the missing working
> version, else no effect.
>
> I would say "update"'s auto-restoring behaviour is a historical wart. I
> think it was for compatibility with CVS. I won't try to argue that we
> should change it now.
>
> 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.
> 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?
Paul
Received on 2010-08-25 20:00:25 CEST