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

[PATCH] Fix issue #3532, 'svn update' via ra_serf of locked file in locally deleted dir triggers assert and breaks WC

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Thu, 17 Mar 2011 17:42:05 +0000

Help needed.

This patch succeeds in fixing issue #3532 "'svn update' via ra_serf of
locked file in locally deleted dir triggers assert and breaks WC".

This avoids trying to set a file's permissions if the file does not in
fact exist on disk - e.g. when the incoming change is just an unlock and
the file is locally deleted.

However, this attempt at the patch is ugly. It stats the file to see if
it exists, and I just get the feeling it should be doing it some
"better" way. Something should be different about the structure of the
code - probably the larger tree conflict handling code and read-only bit
twiddling code outside the block that this patch touches - but I can't
see what exactly.

How can it be better?

[[[
* subversion/libsvn_wc/update_editor.c
  (close_file): When the incoming change was an unlock with no text change,
    avoid trying to change the file's permissions if the file doesn't exist
    on disk.
--This line, and those below, will be ignored--

Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c (revision 1082597)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -4297,14 +4297,23 @@ close_file(void *file_baton,
   if (fb->new_text_base_sha1_checksum == NULL
       && lock_state == svn_wc_notify_lock_state_unlocked)
     {
+ svn_node_kind_t disk_kind;
       /* If a lock was removed and we didn't update the text contents, we
          might need to set the file read-only.
 
+ If the file was locally deleted (in which case this update will
+ have caused a tree conflict), then we cannot touch its permissions.
+
          Note: this will also update the executable flag, but ... meh. */
- SVN_ERR(svn_wc__wq_build_sync_file_flags(&work_item, eb->db,
+ SVN_ERR(svn_io_check_path(fb->local_abspath, &disk_kind, pool));
+ SVN_DBG(("up:close_file:build_sync_file_flags(%d, '%s')\n", disk_kind, fb->local_abspath));
+ if (disk_kind == svn_node_file) /* ### or sthg like if (! fb->deleted && ...) ? */
+ {
+ SVN_ERR(svn_wc__wq_build_sync_file_flags(&work_item, eb->db,
                                                fb->local_abspath,
                                                pool, pool));
- all_work_items = svn_wc__wq_merge(all_work_items, work_item, pool);
+ all_work_items = svn_wc__wq_merge(all_work_items, work_item, pool);
+ }
     }
 
   /* Clean up any temporary files. */
@@ -4448,6 +4457,7 @@ close_file(void *file_baton,
           action = svn_wc_notify_update_add;
         }
 
+ SVN_DBG((" notify->action=%d\n", action));
       notify = svn_wc_create_notify(fb->local_abspath, action, pool);
       notify->kind = svn_node_file;
       notify->content_state = content_state;
]]]

- Julian
Received on 2011-03-17 18:42:42 CET

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