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

Re: [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:47:20 +0000

p.s. Maybe if I fix issue #3525 "Locked file which is scheduled for
delete causes tree conflict", this attempt to set a non-existent file
read-only will simply go away.

- Julian

I (Julian Foad) wrote:
> 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:47:59 CET

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