I can now clarify the desired behaviour, and explain why "--keep-local" shouldn't make any difference. Resolving a conflict is supposed to be a two step process for the user:
1. Choose the desired WC state - e.g. use "svn delete" or something else. 2. Tell Subversion it's resolved - use "svn resolved". Of course there's (supposed to be) a short-cut to most of the simple resolutions that does both steps with one command - "svn resolve --accept=XXX". The point is that "svn delete" shouldn't automatically resolve any conflict, and "svn commit" should not allow a commit that contains a conflict, so there's no reason to think "svn delete" followed by "svn commit" should succeed if there was a conflict. On Tue, 2010-08-24, Julian Foad wrote: [...] > What should happen? > ------------------- > > I think the required changes are: > > * Commit should unconditionally bail out if there are any conflicts > inside a node being committed. No more testing the 'keep_local' flag at > this stage. I'm sure that's the behaviour we should have. The only thing I don't understand is why Neels changed harvest_committables() to ignore conflicts if 'keep_local' is set in r878027. That commit was about fixing some problems with keep_local, but it's not clear to me why this particular change was made. I assumed it was to preserve some 1.6.x behaviour, but I've just tested with a 1.6.x branch build and it doesn't allow the commit. I'm removing that condition, so the commit will fail if there are unresolved conflicts, even inside a directory scheduled for delete. This doesn't cause any test failures except tree_conflicts_tests 17 (which is specifically about this behaviour) and it brings the commit code back into line with 1.6.x in which bail_on_tree_conflicted_children() was called unconditionally. I've committed this in 989189. If Neels or anyone can confirm this diagnosis, that would be great. > * Either the regression test should call "svn resolved" before > attempting the commit, or the "svn delete" that it performs should clear > the conflicts from inside the directory that it deleted. There is no special behaviour that needs to be tested here, so the whole test should go away. (If anything needs testing, it is that "svn delete --keep-local" works the same as "svn delete" except that it keeps the local nodes on disk. Not a tree conflict test.) So I've deleted these two tests: 17 --keep-local del on dir with TCs inside 19 XFAIL --keep-local del on tree-conflicted targets which were added in r878027 and r878028 respectively. The one that wasn't already XFail was failing in single-DB mode, which is what got me started looking at this. I'm also reconsidering the validity of these two tests: 18 XFAIL --force del on dir with TCs inside 20 XFAIL --force del on tree-conflicted targets which were added in r878028 on the basis that "delete --force" followed by "commit" should behave the same way. I may have agreed with that at the time. But why should it? Maybe "delete --force" should clear tree conflicts in the directory being deleted, I don't know, but it certainly shouldn't leave a special intermediate state of "disarmed tree conflict" that "commit" treats as special, which is what these tests are currently expecting. > * None of this behaviour should depend on whether the user wants an > unversioned copy of the node to be kept on disk. > > Neels, in the IRC log below it looks like you had a patch for making > "svn delete" clear tree conflicts when "--force" or "--keep-local" is > given. I don't see why we'd want "--keep-local" to behave specially, > but with "--force", sure, that should clear conflicts. Do you still Actually I don't know that we definitely want "delete --force" to clear tree conflicts. > have the patch? (The paste linked from IRC has expired.) > > <http://colabti.org/irclogger/irclogger_log/svn-dev?date=2010-05-06#l44> - Julian
