[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: tree-conflicts: update conflicted, skip vs. error

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Fri, 12 Dec 2008 19:22:13 +0000

Desired behaviour, according to Julian...

* Update and Switch should skip a target if it is a tree conflict victim
itself, or is inside a tree-conflicted directory in the WC.

* Behaviour should not depend on whether the target is explicit or
implicit, nor on whether it is immediate or encountered by recursion.

* Behaviour should not depend on the current directory. (If
implementation difficulties necessitate it, so be it.)

The example WC:

$ svn st
M dir
   C dir/sub
M dir/sub/node

Desired behaviour:

$ svn up dir
Skipped 'dir/sub' because it is tree-conflicted

$ svn up dir/sub
Skipped 'dir/sub' because it is tree-conflicted

$ svn up dir/sub/node
Skipped 'dir/sub/node' because 'dir/sub' is tree-conflicted

Demonstrating independence of CWD:

$ (cd dir && svn up)
Skipped 'sub' because it is tree-conflicted

$ (cd dir && svn up sub)
Skipped 'sub' because it is tree-conflicted

$ (cd dir && svn up sub/node)
Skipped 'sub/node' because 'sub' is tree-conflicted

$ (cd dir/sub && svn up ..)
Skipped '../sub' because it is tree-conflicted

$ (cd dir/sub && svn up)
Skipped '.' because it is tree-conflicted

$ (cd dir/sub && svn up node)
Skipped 'node' because '.' is tree-conflicted

$ (cd dir/sub/node && svn up ../..)
Skipped '../../sub' because it is tree-conflicted

$ (cd dir/sub/node && svn up ..)
Skipped '..' because it is tree-conflicted

$ (cd dir/sub/node && svn up)
Skipped '.' because '..' is tree-conflicted

On Fri, 2008-12-12 at 05:57 +0100, Neels J Hofmeyr wrote:
> Hi tree-conflict folks,
> I'm looking at the problem of what to say when an update is attempted of a)
> a tree conflicted item or of b) an item somewhere within a tree-conflicted
> directory.
> The current stance is that conflicted nodes should be 'Skipped'. This sort
> of applies to a). We reach a tree-conflicted node by recursion:
> $ svn st
> M dir
> C dir/sub/node
> $ svn up dir
> Skipped 'dir/sub/node'
> Now we've got case b), where an *ancestor* dir is in conflict.
> $ svn st
> C dir
> M dir/sub/node
> $ svn up dir/sub/node
> Skipped dir/sub/node
> That's how it is currently specified to look. But, for example, if `dir' was
> removed in the repository, that's pretty much failing to let the user know
> why the update skipped everything.

Yes, but that's a general problem of the message "Skipped X" being a
rather poor indication for the user. In all Subversion's "Skipped"
messages we should aim to indicate why the thing was skipped.

I think it is right to skip in this case.

> Then, there's case c). First cd into the conflicted subtree and then update
> something. Suddenly, when I look through the code, there's no place to
> "skip" such an update. The failure information is available before any
> update stuff even kicked off. When we let the update stuff kick off, it
> issues another failure, namely that we're sort of not in a proper working
> copy...
> $ svn st
> C dir
> M dir/sub/node
> $ cd dir/sub
> $ svn up
> svn: Target path '/.../dir/sub' does not exist
> (depending on the type of tree-conflict?)
> In case a), where we reach a tree-conflicted item by recursion, it's obvious
> to do the 'Skipped' thing as usual.
> But in case c), where we're *in* a directory that is *completely*
> tree-conflicted by an ancestor, it makes sense to me to issue an error
> instead of a 'Skipped':
> svn: Aborting update: '/.../dir' remains in tree-conflict.

That sounds OK for a single-target command... but as you then point out,
for a multi-target update command, it would be better to be consistent
with other cases so. So I'd say this should still say "Skipped".

> In case b), I'm undecided really. We're currently still *outside* a
> tree-conflicted subtree, but some of the *explicit* targets reach into a
> tree-conflicted subtree. Intuitively, I want to issue an error on targets
> within a conflicted subtree, like c). But we probably want to carry out the
> updates on any other, non-conflicted targets issued on the same commandline.
> Maybe an "augmented skip" notification would do for both b) and c), saying
> "Skipped target 'dir/sub/node' because 'dir' remains in tree-conflict".

Yes, definitely. But let's treat this (better message) as a separate
issue and not necessarily try to do this at the same time as ensuring
that targets are actually skipped when there's a conflict.

> A code implication: The place to catch both b) and c) is in an entirely
> different code layer than all other skip notifications. It feels like
> fabricating a skip and having problems with not running whole chunks of code
> although we exit with "success" (haven't tried that yet).

Hmm. Unless you've seen something in particular, I wouldn't expect to
run into major problems.

- Julian

> If c) were implemented to bail, that would also bail on b) cases, and it
> would be some complex fiddling to allow any other targets to go through.
> Do you guys have an opinion on this? That might shed some light.
> Thanks!
> ~Neels
> P.S.:
> The place to catch c) would be for example here:
> subversion/libsvn_client/update.c:210, in svn_client__update_internal().
> or here, where the currently seen error is produced:
> subversion/libsvn_repos/reporter.c:1162 in drive().
> Attached is a trial patch that catches b) and c) by issuing an error in
> svn_client__update_internal(). The patch also re-enables update_tests.py 50
> to show this case.
> Alternatively, find it here:
> http://hofmeyr.de/svn/update-in-conflicted-1.patch
> The place where tree-conflicted ancestry is currently detected, i.e. cases
> a) and b), is deeper within the onion, here:
> subversion/libsvn_wc/update_editor.c:1600, by already_in_a_tree_conflict()

Received on 2008-12-12 20:24:00 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.