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
Received on 2010-08-25 18:00:50 CEST