You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <pm...@uklinux.net> on 2001/11/03 18:11:24 UTC

update and revert bugs

Two, possibly related, bugs:

First, given an up-to-date directory containing an out of date file,
update on the file doesn't seem to work:

   $ svn st -uv .
   _               11    .
   _               10    ./bar.c
   Head revision:     11
   $ svn up bar.c
   $ svn st -uv .
   _               11    .
   _               10    ./bar.c
   Head revision:     11

It appears that the file difference does not get reported if the
directory is up to date.


Second, in this situation (file out-of-date in up-to-date directory)
the working copy can be corrupted as follows:

   $ svn del bar.c
   D  bar.c
   $ svn add bar.c
   A          bar.c
   $ svn revert bar.c
   Reverted bar.c
   $ svn st -uv

   svn_error: #21049 : <Filesystem has no such file>
     file not found: filesystem `/home/pm/sw/subversion/repository/trial/db', revision `0', path `bar.c'


I am using a subversion version with my new repository diff code, but
my changes don't touch the update or revert code.

Philip

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: update and revert bugs [PATCH]

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
cmpilato@collab.net writes:
> We should simply NOT set the revision of an added file to '0' in the
> entries file -- the fact that the entry is scheduled for
> addition/replacement *itself* is enough to know that the revision
> field for that entry is bogus, so we don't need to change it to '0'.
> By not changing it to '0', we are able to preserve that revision
> number so that revert can work properly.
> 
> Say I have a directory 'A' at revision 5.  In directory 'A' is a file,
> 'foo', which I edit and commit.  Now 'A' is still at revision 5, but
> 'A/foo' is at 6.  Now, I 'svn rm A/foo', and 'svn add A/foo'.  In the
> current code, 'A/foo' would have a revision of 0.  This sucks.  The
> right way would leave the revision untouched, so that when I 'svn
> revert A/foo' the revision number remains intact, and we aren't lying
> about what revision of 'foo' we have after the revert.

To add to what Mike is saying:

A simple rule is, as long as the text-base is preserved, the revision
number should also be preserved (with other "schedule" flags, such as
`added' or `deleted' telling libsvn_wc whether or not to use that
revision number).

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: update and revert bugs [PATCH]

Posted by cm...@collab.net.
[resending, this time to the whole list...doh!]

> The problem is that the revert command does not remove the
> revision="0" line from the .svn/entries file. It looks like
> fold_entry() should do this but modify_flags does not contain
> SVN_WC__ENTRY_MODIFY_REVISION. It looks like revert_admin_things()
> should be setting it, how about:
> 
> * subversion/libsvn_wc/adm_ops: revert_admin_things(): restore the
>   revision when reverting a replaced file.
> 
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/.svn/text-base/adm_ops.c	Sat Nov  3 17:29:27 2001
> +++ subversion/libsvn_wc/adm_ops.c	Mon Nov  5 15:24:30 2001
> @@ -825,6 +825,10 @@
>            *modify_flags |= SVN_WC__ENTRY_MODIFY_TEXT_TIME;
>            entry->text_time = tstamp;
>          }
> +
> +      /* Restore the revison if reverting a replaced file. */
> +      if (entry->schedule == svn_wc_schedule_replace)
> +        *modify_flags |= SVN_WC__ENTRY_MODIFY_REVISION;
>      }
>  
>    if (entry->conflicted)

This might fix the bug in your particular case, but it's not
completely accurate.  The problem here is that your fix assumes that
a the file that got replaced was at the same revision as it's parent
directory.  This assumption simply cannot be made.

The correct solution, however, does require your patch (as well as
several other changes).  

We should simply NOT set the revision of an added file to '0' in the
entries file -- the fact that the entry is scheduled for
addition/replacement *itself* is enough to know that the revision
field for that entry is bogus, so we don't need to change it to '0'.
By not changing it to '0', we are able to preserve that revision
number so that revert can work properly.

Say I have a directory 'A' at revision 5.  In directory 'A' is a file,
'foo', which I edit and commit.  Now 'A' is still at revision 5, but
'A/foo' is at 6.  Now, I 'svn rm A/foo', and 'svn add A/foo'.  In the
current code, 'A/foo' would have a revision of 0.  This sucks.  The
right way would leave the revision untouched, so that when I 'svn
revert A/foo' the revision number remains intact, and we aren't lying
about what revision of 'foo' we have after the revert.

I'll add the above comments to the issue.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: update and revert bugs [PATCH]

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Great!  Thanks for tracking this down & writing a patch.

It's issue #548 now, the first part of which is simply to review and
apply the patch.

If I understand it correctly, your patch will fix the revert bug, but
not the first bug you described in your first mail (the one where a
backdated file doesn't get updated correctly).  That one will require
some thoughtful investigation, perhaps discussion as well -- a
backdated file may possibly be a case where we want functionality like
CVS's sticky tags; but not necessarily, as one could have updated the
dir just to get a prop change.

Anyway, both are recorded in that issue for now.

Watch this space for more. :-)

-K

Philip Martin <pm...@uklinux.net> writes:
> Philip Martin <pm...@uklinux.net> writes:
> 
> > Second, in this situation (file out-of-date in up-to-date directory)
> > the working copy can be corrupted as follows:
> 
> This bug occurs in an up-to-date working copy as well.
> 
> > 
> >    $ svn del bar.c
> >    D  bar.c
> >    $ svn add bar.c
> >    A          bar.c
> >    $ svn revert bar.c
> >    Reverted bar.c
> >    $ svn st -uv
> > 
> >    svn_error: #21049 : <Filesystem has no such file>
> >      file not found: filesystem `/home/pm/sw/subversion/repository/trial/db', revision `0', path `bar.c'
> 
> The problem is that the revert command does not remove the
> revision="0" line from the .svn/entries file. It looks like
> fold_entry() should do this but modify_flags does not contain
> SVN_WC__ENTRY_MODIFY_REVISION. It looks like revert_admin_things()
> should be setting it, how about:
> 
> * subversion/libsvn_wc/adm_ops: revert_admin_things(): restore the
>   revision when reverting a replaced file.
> 
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/.svn/text-base/adm_ops.c	Sat Nov  3 17:29:27 2001
> +++ subversion/libsvn_wc/adm_ops.c	Mon Nov  5 15:24:30 2001
> @@ -825,6 +825,10 @@
>            *modify_flags |= SVN_WC__ENTRY_MODIFY_TEXT_TIME;
>            entry->text_time = tstamp;
>          }
> +
> +      /* Restore the revison if reverting a replaced file. */
> +      if (entry->schedule == svn_wc_schedule_replace)
> +        *modify_flags |= SVN_WC__ENTRY_MODIFY_REVISION;
>      }
>  
>    if (entry->conflicted)
> 
> 
> Philip
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: update and revert bugs [PATCH]

Posted by Philip Martin <pm...@uklinux.net>.
Philip Martin <pm...@uklinux.net> writes:

> Second, in this situation (file out-of-date in up-to-date directory)
> the working copy can be corrupted as follows:

This bug occurs in an up-to-date working copy as well.

> 
>    $ svn del bar.c
>    D  bar.c
>    $ svn add bar.c
>    A          bar.c
>    $ svn revert bar.c
>    Reverted bar.c
>    $ svn st -uv
> 
>    svn_error: #21049 : <Filesystem has no such file>
>      file not found: filesystem `/home/pm/sw/subversion/repository/trial/db', revision `0', path `bar.c'

The problem is that the revert command does not remove the
revision="0" line from the .svn/entries file. It looks like
fold_entry() should do this but modify_flags does not contain
SVN_WC__ENTRY_MODIFY_REVISION. It looks like revert_admin_things()
should be setting it, how about:

* subversion/libsvn_wc/adm_ops: revert_admin_things(): restore the
  revision when reverting a replaced file.

Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/.svn/text-base/adm_ops.c	Sat Nov  3 17:29:27 2001
+++ subversion/libsvn_wc/adm_ops.c	Mon Nov  5 15:24:30 2001
@@ -825,6 +825,10 @@
           *modify_flags |= SVN_WC__ENTRY_MODIFY_TEXT_TIME;
           entry->text_time = tstamp;
         }
+
+      /* Restore the revison if reverting a replaced file. */
+      if (entry->schedule == svn_wc_schedule_replace)
+        *modify_flags |= SVN_WC__ENTRY_MODIFY_REVISION;
     }
 
   if (entry->conflicted)


Philip

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org