This bug has bitten me three times in the last month or so. After the
second time, I was on the lookout for it... Finally, the third time,
I had a pre-revert WC snapshot. I've reduced it to the minimal
reproduction recipe at the end of this message.
I think it's a moderately serious bug because it undermines the user's
trust in Subversion -- in the reliability of "svn status" and "svn
diff" in particular. When "svn status" and "svn diff" report a
certain set of files as changed, but then "svn revert" reports that it
has reverted an additional file, it is very surprising to the user.
Even though it turns out (as I know now) there is no data loss, it
makes the user *think* there has been data loss. I certainly got a
nasty jolt the first two times it happened to me. Of course, since
there's no way to "undo" the revert, the user can't go back and see
what was changed by the unexpected revert.
In fact, revert is just wrongly reporting that it reverted something
when it did not.
This appears to be the result of the identification of "something has
been reverted" with "something has been written to the log" in
"revert_admin_things". In particular, the removal of the three
conflict entries/files is written to the log, but this does *not*
constitute a reversion.
It seems to me that a reasonable fix would be for revert to output
nothing in the case where it made no changes to the actual file but
maybe removed the conflict info from the entries file, or even removed
the extra suffixed conflict files themselves. Alternatively, if
people feel that it should output something, it could mention that it
did that "conflict cleanup" but with a different message like "Removed
conflict file foo.r123" instead of "Reverted foo". Clearly the
message is wrong, at least. (For some more babbling about how I think
things should work see the comments at the end of the script below.)
I've verified the bug against Fedora Core 4 svn 1.2.3, and trunk
r16714.
A simplistic fix for this bug is to just rip out the "loggy remove" of
the conflict entries/files from libsvn_wc's "revert_admin_things". I
tried that, and it did fix the bug, and "make check" still passed, but
I assume it's not The Right Thing :)
-- cut here --
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 18714)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -1497,6 +1497,7 @@
}
}
+#if 0
/* Remove conflict state (and conflict files), if any.
Handle the three possible text conflict files. */
if (entry->conflict_old)
@@ -1522,6 +1523,7 @@
SVN_ERR(svn_wc__loggy_remove(&log_accum, adm_access,
entry->conflict_wrk, pool));
}
+#endif
/* Remove the prej-file if the entry lists one (and it exists) */
if (entry->prejfile)
-- cut here --
I briefly looked at the testsuite code but did not immediately
understand how to write a testcase for this. I don't know how long it
would take me to do that.
(Also I think a testcase which prevents my bogus patch from passing
the testsuite would be nice too.)
Finally, please note that the book does say (in footnote 4 of the 1.1
version, footnote 5 of the 1.2 nightly, chapter 3) that one *can*
manually remove the conflict files -- which is what I've always done;
I have never once used "svn resolved", nor do I think I should have
to!
Shall I file this as an issue? Any hints as to how to proceed are
welcome.
Thanks,
-Ed
#!/bin/bash
set -ev
svn --version
svnadmin create repos
svn co file://`pwd`/repos wc
touch wc/foo
svn add wc/foo
svn ci wc -m ""
svn up wc
echo "### Cause a conflict." > /dev/null
svn co file://`pwd`/repos wc2
echo "1" > wc/foo
echo "2" > wc2/foo
svn ci wc2 -m ""
svn up wc
echo "### Resolve conflict 'manually'." > /dev/null
echo "2" > wc/foo
rm wc/foo.*
svn st wc
svn diff wc
echo "### Status and diff agree: No changes." > /dev/null
echo "### So revert should not revert anything." > /dev/null
echo "### (We'll snapshot WC here.)" > /dev/null
cp -pR wc wc.save
echo "### Revert (nothing to do, right?)" > /dev/null
svn revert -R wc
echo "### Did it say that it reverted something?" > /dev/null
echo "### If so, the user panics..." > /dev/null
echo "### What was reverted?? Who knows!" > /dev/null
echo "### Luckily, we happen to have a snapshot..." > /dev/null
echo "### So let's see what really did change:" > /dev/null
diff -ur wc.save wc
# It's interesting that only "svn revert" cleans out those conflict
# markers in the entries file.
#
# "svn cleanup" should also clean up these conflict entries. Its help
# text says "cleanup: Recursively clean up the working copy, removing
# locks, resuming unfinished operations, etc.". I tried adding "svn
# cleanup wc" just before "svn revert -R wc" but it did not affect the
# result, ie it did not cleanup the conflict entries.
#
# Perhaps "svn up" should do it too. That would be ideal because, at
# least for me, it is a very frequently executed command, moreso than
# 'revert' and much moreso than 'cleanup'.
#
# Neither "svn status" nor "svn diff" should do it because they should
# be strictly read-only. (Or if they did try to do it, they should
# silently ignore "harmless" failure to update the entries file, to
# avoid regression against a truly read-only WC.)
#
# I wonder if there could be other bugs due to the leftover conflict
# info in there? I did try producing another conflict with the
# previous conflict markers still in the file, but it didn't cause any
# problem AFAICT; the conflict markers were updated properly for the
# second conflict.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Mar 5 21:46:57 2006