You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Mo DeJong <su...@bayarea.net> on 2001/09/18 00:20:17 UTC

Patch for error deleting newely added directory.

Hi all.

In an earlier mail, I mentioned this bug:

% mkdir foo
% svn add foo
% svn remove foo
svn_error: #21026 : <Can't find an entry>
  No default entry in directory `foo'


This problem was showing up in the svn_wc__entries_write method but
it seems that svn_wc__entry_modify was really to blame. It would
call the fold_state_changes method which would notice the move
from added -> deleted and remove the SVN_WC_ENTRY_THIS_DIR
from the entries hashtable. That would blow up in svn_wc__entries_write.

My solution to this problem was to notice the case where the this_dir
entry was removed and make a call to svn_wc_remove_from_revision_control
for the directory that was being "unadded". I also had to check for this
case in svn_wc_delete to avoid an attempt to delete the parent's entry
twice.

Does this seem like a reasonable approach? It fixes the bug for me, but
I wanted to make sure it is "right".

cheers
Mo DeJong


2001-09-17  Mo DeJong  <su...@bayarea.net>

        Fix problem deleting a directory that had been added but was
	not yet committed.

        * subversion/libsvn_wc/entries.c (svn_wc__entry_modify): Check
	for the special case of deleting a directory that has not been
	committed. Call the svn_wc_remove_from_revision_control function
	to delete the administrative subdir in this case.
	* subversion/libsvn_wc/adm_ops.c (svn_wc_delete,
	svn_wc_remove_from_revision_control): Check to see if the
	entry we just marked for deletion was removed in svn_wc_delete.
	Add empty parent dir check to svn_wc_remove_from_revision_control
	so that function accepts short path names.

Index: ./subversion/libsvn_wc/entries.c
===================================================================
--- ./subversion/libsvn_wc/SVN/text-base/entries.c	Mon Sep 10 17:16:48 2001
+++ ./subversion/libsvn_wc/entries.c	Mon Sep 17 16:40:11 2001
@@ -1455,7 +1455,18 @@
                 schedule, existence, conflicted, text_time,
                 prop_time, attributes, pool, ap);
 
-  SVN_ERR (svn_wc__entries_write (entries, path, pool));
+  /* Special case: If we just removed the SVN_WC_ENTRY_THIS_DIR
+     entry then we don't want to write the entry back out.
+     Instead, we want to remove the administrative directory
+     completely since we are dealing with a directory that
+     had been added but was removed before a commit. */
+  if (entry_was_deleted_p
+      && strcmp (name->data, SVN_WC_ENTRY_THIS_DIR) == 0)
+    SVN_ERR (svn_wc_remove_from_revision_control (path,
+                                                  name,
+                                                  FALSE, pool));
+  else
+    SVN_ERR (svn_wc__entries_write (entries, path, pool));
 
   return SVN_NO_ERROR;
 }
Index: ./subversion/libsvn_wc/adm_ops.c
===================================================================
--- ./subversion/libsvn_wc/SVN/text-base/adm_ops.c	Mon Sep 10 17:16:48 2001
+++ ./subversion/libsvn_wc/adm_ops.c	Mon Sep 17 16:48:15 2001
@@ -513,6 +513,7 @@
 {
   svn_stringbuf_t *dir, *basename;
   svn_wc_entry_t *entry;
+  svn_boolean_t dir_unadded = FALSE;
 
   /* Get the entry for the path we are deleting. */
   SVN_ERR (svn_wc_entry (&entry, path, pool));
@@ -528,10 +529,21 @@
 
   if (entry->kind == svn_node_dir)
     {
+      svn_wc_entry_t *new_entry;
+      svn_error_t *err;
       /* Recursively mark a whole tree for deletion. */
       SVN_ERR (mark_tree (path, mark_tree_state_delete, pool));
+
+      /* Get the entry for the path we are deleting. */
+      err = svn_wc_entry (&new_entry, path, pool);
+      if (err)
+        dir_unadded = TRUE; /* We removed a newely added dir */
     }
 
+  /* If we just removed a newely added directory then the parent
+     directories entrt was already removed. */
+  if (! dir_unadded)
+    {
   /* We need to mark this entry for deletion in its parent's entries
      file, so we split off basename from the parent path, then fold in
      the addition of a delete flag. */
@@ -546,6 +558,7 @@
             svn_wc_schedule_delete,
             svn_wc_existence_normal,
             FALSE, 0, 0, NULL, pool, NULL));
+    }
 
   /* Now, call our client feedback function. */
   {
@@ -1032,6 +1045,9 @@
       /* Remove self from parent's entries file */
       svn_path_split (full_path, &parent_dir, &basename,
                       svn_path_local_style, pool);
+      if (svn_path_is_empty (parent_dir, svn_path_local_style))
+        svn_stringbuf_set (parent_dir, ".");
+
       /* ### sanity check:  is parent_dir even a working copy?
          if not, it should not be a fatal error.  we're just removing
          the top of the wc. */

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

Re: Patch for error deleting newely added directory.

Posted by Mo DeJong <su...@bayarea.net>.
On 17 Sep 2001 23:47:36 -0500
cmpilato@collab.net wrote:

> Mo DeJong <su...@bayarea.net> writes:
> 
> > My solution to this problem was to notice the case where the this_dir
> > entry was removed and make a call to svn_wc_remove_from_revision_control
> > for the directory that was being "unadded". I also had to check for this
> > case in svn_wc_delete to avoid an attempt to delete the parent's entry
> > twice.
> > 
> > Does this seem like a reasonable approach? It fixes the bug for me, but
> > I wanted to make sure it is "right".
> 
> Nope, it's wrong.  In fact, it's the *exact* same wrong change that I
> checked in a while ago and then reverted (because of its wrongness). :-)  
> 
> The entries reading/writing interfaces are very low-level, and have no
> business making decisions about what is an what isn't under revision
> control.  Their job is to read/write a hash from/to disk, that's it.

Ok. Well what if we just never call mark_tree() at all? We can detect
the deletion of a newly added directory in the svn_wc_delete() function
and special case it.

How does this look?
Mo


2001-09-18  Mo DeJong  <su...@bayarea.net>

        Fix problem deleting a directory that had been added but was
	not yet committed.

        * subversion/libsvn_wc/entries.c (fold_state_changes): Assert
	if the caller tried to delete a newly added SVN_WC_ENTRY_THIS_DIR
	entry in fold_state_changes. This should never happen since it
	would leave the entries file in an invalid state.
	* subversion/libsvn_wc/adm_ops.c (svn_wc_delete,
	svn_wc_remove_from_revision_control): If a directory has
	be added but not yet committed and it is then deleted, just
	call the svn_wc_remove_from_revision_control method to
	remove the administrative subdirectory. Also add empty
	parent dir check to svn_wc_remove_from_revision_control
	so it works with short path names.

Index: ./subversion/libsvn_wc/entries.c
===================================================================
--- ./subversion/libsvn_wc/SVN/text-base/entries.c	Tue Sep 18 17:45:10 2001
+++ ./subversion/libsvn_wc/entries.c	Tue Sep 18 17:49:39 2001
@@ -1303,8 +1303,11 @@
 
         case svn_wc_schedule_delete:
         case svn_wc_schedule_unadd:
-          /* Not-yet-versioned item being deleted or un-added?  Just
-             remove the entry altogether. */
+          /* Not-yet-versioned item being deleted, Just remove
+             the entry. Check that we are not trying to remove
+             the SVN_WC_ENTRY_THIS_DIR entry as that would
+             leave the entries file in an invalid state. */
+          assert (entry != this_dir_entry);
           apr_hash_set (entries, name->data, name->len, NULL);
           return SVN_NO_ERROR;
         }
Index: ./subversion/libsvn_wc/adm_ops.c
===================================================================
--- ./subversion/libsvn_wc/SVN/text-base/adm_ops.c	Tue Sep 18 17:45:06 2001
+++ ./subversion/libsvn_wc/adm_ops.c	Tue Sep 18 17:59:19 2001
@@ -513,6 +513,7 @@
 {
   svn_stringbuf_t *dir, *basename;
   svn_wc_entry_t *entry;
+  svn_boolean_t dir_unadded = FALSE;
 
   /* Get the entry for the path we are deleting. */
   SVN_ERR (svn_wc_entry (&entry, path, pool));
@@ -528,10 +529,26 @@
 
   if (entry->kind == svn_node_dir)
     {
-      /* Recursively mark a whole tree for deletion. */
-      SVN_ERR (mark_tree (path, mark_tree_state_delete, pool));
+      /* Special case, delete of a newly added dir. */
+      if (entry->schedule == svn_wc_schedule_add)
+        dir_unadded = TRUE;
+      else
+        /* Recursively mark a whole tree for deletion. */
+        SVN_ERR (mark_tree (path, mark_tree_state_delete, pool));
     }
 
+  /* Deleting a directory that has been added but not yet
+     committed is easy, just remove the adminstrative dir. */
+  if (dir_unadded)
+    {
+      svn_stringbuf_t *this_dir =
+        svn_stringbuf_create (SVN_WC_ENTRY_THIS_DIR, pool);
+      SVN_ERR (svn_wc_remove_from_revision_control (path,
+                                                    this_dir,
+                                                    FALSE, pool));
+    }
+  else
+    {
   /* We need to mark this entry for deletion in its parent's entries
      file, so we split off basename from the parent path, then fold in
      the addition of a delete flag. */
@@ -546,6 +563,7 @@
             svn_wc_schedule_delete,
             svn_wc_existence_normal,
             FALSE, 0, 0, NULL, pool, NULL));
+    }
 
   /* Now, call our client feedback function. */
   {
@@ -1032,6 +1050,9 @@
       /* Remove self from parent's entries file */
       svn_path_split (full_path, &parent_dir, &basename,
                       svn_path_local_style, pool);
+      if (svn_path_is_empty (parent_dir, svn_path_local_style))
+        svn_stringbuf_set (parent_dir, ".");
+
       /* ### sanity check:  is parent_dir even a working copy?
          if not, it should not be a fatal error.  we're just removing
          the top of the wc. */




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

Re: Patch for error deleting newely added directory.

Posted by cm...@collab.net.
Mo DeJong <su...@bayarea.net> writes:

> My solution to this problem was to notice the case where the this_dir
> entry was removed and make a call to svn_wc_remove_from_revision_control
> for the directory that was being "unadded". I also had to check for this
> case in svn_wc_delete to avoid an attempt to delete the parent's entry
> twice.
> 
> Does this seem like a reasonable approach? It fixes the bug for me, but
> I wanted to make sure it is "right".

Nope, it's wrong.  In fact, it's the *exact* same wrong change that I
checked in a while ago and then reverted (because of its wrongness). :-)  

The entries reading/writing interfaces are very low-level, and have no
business making decisions about what is an what isn't under revision
control.  Their job is to read/write a hash from/to disk, that's it.

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