On Mon, Aug 9, 2010 at 12:54, Paul Burba <ptburba_at_gmail.com> wrote:
> We have several issues related to the question of how revert should
> handle locally added or copied items:
>
> 'svn revert of svn move'
> http://subversion.tigris.org/issues/show_bug.cgi?id=876
>
> 'svn revert should provide a way to delete copied files'
> http://subversion.tigris.org/issues/show_bug.cgi?id=3101
>
> 'Case only renames resulting from merges don't work or break the WC on
> case-insensitive file systems'
> http://subversion.tigris.org/issues/show_bug.cgi?id=3115
>
> With only one exception that I know of[1], revert leaves local
> additions of every stripe as unversioned. My first question is
> simple:
>
> "Do we consider this the correct behavior?"
I haven't read the above bugs, but have come to a "solid" view of how
'svn revert' should behave. When stopping to think about the
*representation* of changes within the working copy, it also (imo)
leads to a better understanding of the operations performed and what a
"revert" of those operations can mean.
I'll try to provide a brief description of my thoughts, then answer
your questions based on that, and then summarize any gaps. Please ask
for more detail, or to argue interpretation!
There are *two* semantic concepts behind revert. The first is "revert
my structural changes" (e.g add/move/copy/delete). The second is
"revert my content changes" (e.g. text edits and prop changes). These
two concepts are conflated today; we make no distinction between them,
and provide no UI or API mechanism to distinguish *what* you would
like to revert.
Content reverts are simple and straight-forward, and can be applied to
any node [which has content changes].
Structural changes *cannot* be applied to "any" node. If you copy a
(sub)tree to a new location, then you cannot revert a *child* of that
copy. Without stretching the brain along some inhuman path, it just
has no meaning. The child arrived as a result of a parent copy. You
can *delete* or *replace* the child using further operations, but an
implied-copy of a child can only be reverted by reverting the ancestor
which is the root of the copied-here operation.
For example,
$ svn cp A/B C
$ svn revert C/D/file
That should error.
$ svn revert C
Should succeed, and undo the copy that was made.
$ edit C/D/file
$ svn revert C/D/file
Should succeed; reverting just local post-copy edits.
There are UI issues with allowing the second to succeed if local mods
are present, yet fail if not. I relegate this to "UI foo".
Conceptually, the second revert should be able to succeed.
Here is the more interesting scenario:
$ svn cp file1 file2
$ edit file2
$ svn revert file2
This is where the two semantics come in. Do you want to revert the
content edits? Or do you want to revert the copy? Questions and UI and
API abound :-)
> A) "Yes! svn revert only reverts the scheduled addition, but leaves
> the item behind as unversioned". In this case the one exception [1]
> becomes a bug which we can fix.
For a schedule-add... yes, this is safest. This content may not exist
anywhere else, so it should remain.
If you're using the term "addition" to also mean "add with history",
then I'll smack you and say you should use the term "copy". A revert
of a copy has restrictions as noted above, and if it also has local
changes, then leaving it behind may be important.
A directory-copy with post-copy propchanges cannot be left behind. It
becomes unversioned, meaning we have nowhere to leave the propchanges
behind. It may be that users first revert the propchanges, *then* they
revert the copy. Dunno. They said "revert", so "losing" information
isn't that big of a deal. They told us to.
> B) "No! svn revert should remove all additions from disk. I asked for
> it after all!". In this case that one exception [1] is the correct
> behavior and we need fix everything else.
Disagree. These additions have no other source, so they shouldn't disappear.
The user said to remove the *scheduled-add*. Not the file. "svn rm"
could remove the file. But a simple revert should not remove anything
from the disk. It should only remove what our metadata said to do with
the file/dir.
> C) Then of course, there is the more complex answer where we allow
> revert to summarily delete additions in some cases, but not in others.
> Perhaps local additions are left as unversioned, but copies are
> deleted. Or perhaps in the latter case we only delete the addition if
> the copy is identical to its source.
No idea what you're saying. If you'are advocating some kind of
non-deterministic behavior of leaving some stuff and rm'ing others...
that's definitely crazy.
> I'm wondering if there is any consensus on the 'A', ''B, or 'C'
> approaches. If 'A' or 'B' then our task is straightforward. If it's
> 'C', then we'll need some more discussion.
>
> I favor 'A'. I know that leaving behind unversioned items post-revert
> can be a bit of a pain in some use cases (e.g. "Oh I merged the wrong
> revs, let's revert and redo the correct merge, hey, what are all these
> skips!?!") but I think this approach is easy to understand and explain
> to users. The workarounds if someone has a problem with this behavior
> are also relatively simple.
The user said "revert my svn add". They didn't say "remove the file".
>...
The rest of the message seemed to be trying to argue what to do around
certain types of situations. Whatever. Revert is about an *operation*.
It does not mean "remove" in any sense.
Reverting a *copy* or a *move* can conceivably remove content from the
filesystem. This is where the *two* semantics come in to help inform
the situation. If there are local edits (post-copy/move), then tossing
the content is very troublesome. If there are no local edits, then
rm'ing is easy. The content that was sitting (uncommitted) on disk is
located elsewhere. When local edits are present, the revert could
conceivably require a switch 'svn revert B --discard-local-edits'. Or
the user could individually revert local edits, then revert the
entirety of B.
Inventing a new revert switch to discard propchanges on B, but NOT
revert its underlying copy/move-here is an exercise for the reader.
Cheers,
-g
Received on 2010-08-10 06:19:36 CEST