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

Re: svn commit: r26953 - branches/svnpatch-diff/subversion/libsvn_client

From: Charles Acknin <charlesacknin_at_gmail.com>
Date: 2007-10-06 17:16:11 CEST

On 10/6/07, David Glasser <glasser@davidglasser.net> wrote:
> A possible better approach is to keep_local *all* file deletions but
> add the file names to a hash, and then after the entire merge is done,
> delete all of the pending deletions?

Yup, in fact I had been going this direction at first but I thought it
didn't look so neat when it came to using patch_cmd_baton (which is
void * edit_baton->diff_cmd_baton) within editor functions (I'm doing
the deletions in close_edit) just because we're in the same file
(unusually both diff_callbacks and delta_editor callbacks are in
libsvn_client/patch.c).

I'm attaching the msglog-less patch of this implementation, please let
me know what you think. Basically what it does is edit_baton holds
the deletions hash (could go into patch_cmd_baton too, as in r26953),
we use keep_local for all deletions, and then close_edit() removes the
schedule-delete files from disk.

[[[
Index: subversion/libsvn_client/patch.c
===================================================================
--- subversion/libsvn_client/patch.c (revision 26952)
+++ subversion/libsvn_client/patch.c (working copy)
@@ -33,6 +33,7 @@
 #include "svn_base64.h"
 #include "svn_string.h"
 #include "svn_hash.h"
+#include "svn_iter.h"
 #include <assert.h>

 #include "svn_private_config.h"
@@ -514,7 +515,7 @@
       /* Passing NULL for the notify_func and notify_baton because
          delete_entry() will do it for us. */
       err = svn_client__wc_delete(mine, parent_access, patch_b->force,
- patch_b->dry_run, FALSE, NULL, NULL,
+ patch_b->dry_run, TRUE, NULL, NULL,
                                   patch_b->ctx, subpool);
       if (err && state)
         {
@@ -746,7 +747,7 @@
         SVN_ERR(svn_wc_adm_retrieve(&parent_access, adm_access, parent_path,
                                     subpool));
         err = svn_client__wc_delete(path, parent_access, patch_b->force,
- patch_b->dry_run, FALSE,
+ patch_b->dry_run, TRUE,
                                     merge_delete_notify_func, &mdb,
                                     patch_b->ctx, subpool);
         if (err && state)
@@ -820,6 +821,12 @@
   svn_wc_notify_func2_t notify_func;
   void *notify_baton;

+ /* The list of paths of schedule-delete entries. All file disk
+ * deletions are held until the patch application is entirely done --
+ * close_edit(). This is to prevent copyfrom files to be missing when
+ * involved in copy or move operations. */
+ apr_hash_t *deletions;
+
   apr_pool_t *pool;
 };

@@ -1140,10 +1147,17 @@
           && (state != svn_wc_notify_state_obstructed))
         {
           action = svn_wc_notify_update_delete;
+
+ /* Use edit_baton's pool, we need those bytes in close_edit(). */
+ const char *wcpath = svn_path_join(eb->target, path, eb->pool);
+ const char *pwcpath = svn_path_join(eb->target, pb->wcpath,
+ eb->pool);
+
+ apr_hash_set(eb->deletions, wcpath, APR_HASH_KEY_STRING, pwcpath);
+
           if (eb->dry_run)
             {
               /* Remember what we _would've_ deleted (issue #2584). */
- const char *wcpath = svn_path_join(eb->target, path, pb->pool);
               apr_hash_set(dry_run_deletions_hash(eb->diff_cmd_baton),
                            wcpath, APR_HASH_KEY_STRING, wcpath);

@@ -1532,6 +1546,31 @@
   return SVN_NO_ERROR;
 }

+static svn_error_t *
+remove_path(void *baton,
+ const void *key,
+ apr_ssize_t klen,
+ void *val,
+ apr_pool_t *pool)
+{
+ const char *path = key;
+ const char *parent_path = val;
+ svn_wc_adm_access_t *adm_access;
+ struct edit_baton *eb = baton;
+ struct patch_cmd_baton *pb = eb->diff_cmd_baton;
+
+ SVN_ERR(get_path_access(&adm_access, eb->adm_access, parent_path,
+ FALSE, pool));
+
+ SVN_ERR(svn_client__wc_delete(path, adm_access, pb->force,
+ pb->dry_run,
+ FALSE, /* do not keep_local now */
+ NULL, NULL, /* no notification */
+ pb->ctx, pool));
+
+ return SVN_NO_ERROR;
+}
+
 /* An editor function. */
 static svn_error_t *
 close_edit(void *edit_baton,
@@ -1539,6 +1578,9 @@
 {
   struct edit_baton *eb = edit_baton;

+ SVN_ERR(svn_iter_apr_hash(NULL, eb->deletions, remove_path,
+ eb, eb->pool));
+
   svn_pool_destroy(eb->pool);

   return SVN_NO_ERROR;
@@ -1567,6 +1609,7 @@
   eb->diff_cmd_baton = patch_cmd_baton;
   eb->notify_func = notify_func;
   eb->notify_baton = notify_baton;
+ eb->deletions = apr_hash_make(subpool);
   eb->pool = subpool;

   tree_editor->open_root = open_root;
]]]

> > + /* The list of paths of schedule-delete entries. See is_removal_safe
>
> It's not a list (lists are ordered).

Sure, though I thought it was OK to talk about hash maps as 'lists' in
common english speaking. (I recall I've seen subversion source code
using this too). Anyway, I'll update the docstring in the above patch
if you think that's worth it.

Charles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 6 17:16:28 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.