[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Issue #1075 analysis

From: Brian Denny <brian_at_briandenny.net>
Date: 2003-04-20 22:59:45 CEST

I'm working on Issue #1075 (update receiving delete for missing
directory), and I'd like it if somebody who understands the update code
better than I could sanity-check my analysis.

Here's the repro recipe from IZ:
  svnadmin create repo
  svn mkdir -mx file://`pwd`/repo/foo
  svn rm -mx file://`pwd`/repo/foo
  svn co -r1 file://`pwd`/repo wc
  rm -rf wc/foo
  svn up wc
  ../svn/subversion/libsvn_wc/lock.c:422: (apr_err=155005)
  svn: Working copy not locked
  svn: directory not locked (wc/foo)

Studying the code, this is what i think is going on:

In svn_wc_crawl_revisions, missing directories are reported as such by
calling reporter->delete_path -- essentially removing them from the
"source tree" that the update editor will look at to generate deltas.
This works great if the update wants to *restore* the missing directory
-- the editor will see that it's there in the target and not in the
source, and generate a delta to restore it. But if the update wants to
*delete* the missing directory (as in the case of this bug), no delta is
generated because there is no difference between the source and target

By the time we get around to editor->close_edit, the entry for "foo"
should have already been removed from the parent dir's 'entries' file,
but since no delta was generated to delete "foo", the entry is still
there. The "Working copy not locked" error happens in
recursively_tweak_entries, when we try to svn_wc_adm_retrieve for the
missing directory.

So, we have to get rid of the parent dir's entry for "foo".
Off the top of my head, i have two ideas:

(1) I *think* that the editor could "do the right thing" by
    always generating a delta when the source directory is missing --
    i.e., if the directory is present in the target, do an
    add (restore), and if it's absent in the target, do a delete. But
    that would also require some way of signaling a missing path to the
    reporter as distinct from a deleted path (not to mention that i have
    no idea how the missing item should be represented in the transaction
    "source tree").
(2) Do everything as we are doing it, but in recursively_tweak_entries,
    check for a missing directory, and just remove the parent dir's
    entry for it (i.e., assume that any directory which is still missing
    at the end of an update.

Idea #2 seems a *lot* simpler -- but is it too hacky?
I'm open to other ideas. I just barely understand the update code now,
so there may be some clean, simple solution that i'm missing.


p.s., Here's a one-off patch for idea #2:

Index: subversion/libsvn_wc/adm_ops.c
--- subversion/libsvn_wc/adm_ops.c (revision 5680)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -105,12 +105,28 @@
       else if (current_entry->kind == svn_node_dir)
           svn_wc_adm_access_t *child_access;
- const char *child_path
- = svn_path_join (svn_wc_adm_access_path (dirpath), name, subpool);
- SVN_ERR (svn_wc_adm_retrieve (&child_access, dirpath, child_path,
- subpool));
- SVN_ERR (recursively_tweak_entries
- (child_access, child_url, new_rev, subpool));
+ const char *child_path, *path;
+ apr_hash_t *dirents;
+ svn_boolean_t missing;
+ path = svn_wc_adm_access_path (dirpath);
+ svn_io_get_dirents(&dirents, path, subpool);
+ missing = (apr_hash_get (dirents, name, APR_HASH_KEY_STRING)
+ == NULL);
+ if (missing)
+ {
+ svn_wc__entry_remove (entries, name);
+ }
+ else
+ {
+ child_path = svn_path_join (svn_wc_adm_access_path (dirpath),
+ name, subpool);
+ SVN_ERR (svn_wc_adm_retrieve (&child_access, dirpath, child_path,
+ subpool));
+ SVN_ERR (recursively_tweak_entries
+ (child_access, child_url, new_rev, subpool));
+ }

To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Apr 20 22:55:30 2003

This is an archived mail posted to the Subversion Dev mailing list.