Committed in r930662 on my branch svn-patch-improvements.
On Wed, Mar 03, 2010 at 04:25:07PM +0100, Daniel Näslund wrote:
> Hi!
>
> Here's my revised patch. I've commented on the changes inline.
>
> One strange thing: If you compile this you'll get a compiler warning
> about this variable not beeing used:
>
> 1354 apr_array_header_t *deleted_targets;
>
> But if I remove it, no targets will be condensed. It's really silly. Can
> someone explain why a variable that isn't used must be declared in order
> for my code to work. What is the simple logical explanation?
>
> On Mon, Mar 01, 2010 at 04:10:09PM +0100, Stefan Sperling wrote:
> > On Sat, Feb 27, 2010 at 09:09:52PM +0100, Daniel Näslund wrote:
> > > Index: subversion/libsvn_client/patch.c
> > > ===================================================================
> > > --- subversion/libsvn_client/patch.c (revision 916918)
> > > +++ subversion/libsvn_client/patch.c (arbetskopia)
>
> [...]
>
> > > +/* Check if PARENT_DIR_ABSPATH has any versioned or unversioned children
> > > if
> > > + * we ignore the ones in TARGETS_TO_BE_DELETED. Return the answer in
> > > + * EMPTY. */
> > > +static svn_error_t *
> > > +is_dir_empty(svn_boolean_t *empty, const char *parent_dir_abspath,
> > > + svn_client_ctx_t *ctx,
> > > + apr_array_header_t *targets_to_be_deleted,
> > > + apr_pool_t *result_pool, apr_pool_t *scratch_pool)
> > > +{
> > > + struct status_baton btn;
> > > + svn_opt_revision_t revision;
> > > + int i;
> > > +
> > > + btn.existing_targets = apr_array_make(scratch_pool, 0,
> > > + sizeof(patch_target_t *));
> > > + btn.parent_path = parent_dir_abspath;
> > > + btn.result_pool = scratch_pool;
> > > + revision.kind = svn_opt_revision_unspecified;
> > > +
> > > + SVN_ERR(svn_client_status5(NULL, parent_dir_abspath, &revision,
> > > + find_existing_children, &btn,
> > > + svn_depth_immediates, TRUE, FALSE, TRUE,
> > > + FALSE, NULL, ctx, scratch_pool));
> >
> > Not sure if svn_client_status5() is really what we want here.
> > Is there a public or semi-private libsvn_wc function we could
> > use instead? One that doesn't need a client context?
>
> I used svn_wc_walk_status() instead. It does not handle externals but
> treats them as unversioned. No problemos for us, we just want to find
> out if anything exists in a dir.
>
> > > + /* Do we delete all targets? */
> > > + for (i = 0; i < btn.existing_targets->nelts; i++)
> > > + {
> > > + int j;
> > > + const char *found = APR_ARRAY_IDX(btn.existing_targets, i, const
> > > char *);
> > > + svn_boolean_t deleted = FALSE;
> > > +
> > > + for (j = 0; j < targets_to_be_deleted->nelts; j++)
> > > + {
> > > + patch_target_t *to_be_del;
> > > + to_be_del = APR_ARRAY_IDX(targets_to_be_deleted, j,
> > > patch_target_t *);
> > > + if (! strcmp(found, to_be_del->abs_path))
> > > + deleted = TRUE;
> >
> > Are you missing a 'break' here?
>
> Added it. Saves us some iterations.
>
> > > + }
> > > + if (! deleted)
> > > + {
> > > + *empty = FALSE;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + return SVN_NO_ERROR;
> > > +}
> > > +
> > > +/* Add TARGET to the array of targets keyed by their parent dir in
> > > + * TARGETS_TO_BE_DELETED. If there is no array for the parent dir a new
> > > one
> > > + * is created. All allocations are done in RESULT_POOL. */
> > > +static svn_error_t *
> > > +add_target_to_hash_keyed_by_parent_dir(apr_hash_t
> > > *targets_to_be_deleted,
> > > + patch_target_t *target,
> > > + apr_pool_t *result_pool)
> > > +{
> > > + apr_array_header_t * deleted_targets_in_dir;
> > > +
> > > + /* We're using the abs_path of the target. The abs_path is not
> > > + * present if the path is a symlink pointing outside the wc but we
> > > + * know, that's not the case. */
> >
> > The comma should either be removed, or moved: "... wc, but we know ...".
>
> Fixed.
>
> > > + const char *dirname = svn_dirent_dirname(target->abs_path,
> > > + result_pool);
> > > +
> > > + deleted_targets_in_dir = apr_hash_get(targets_to_be_deleted,
> > > + dirname, APR_HASH_KEY_STRING);
> > > +
> > > + if (deleted_targets_in_dir)
> > > + {
> > > + APR_ARRAY_PUSH(deleted_targets_in_dir, patch_target_t *) = target;
> > > +
> > > + apr_hash_set(targets_to_be_deleted, dirname,
> > > + APR_HASH_KEY_STRING, deleted_targets_in_dir);
> > > + }
> > > + else
> > > + {
> > > + apr_array_header_t *new_array;
> > > +
> > > + new_array = apr_array_make(result_pool, 0, sizeof(patch_target_t
> > > *));
> > > + APR_ARRAY_PUSH(new_array, patch_target_t *) = target;
> > > + apr_hash_set(targets_to_be_deleted,
> > > + dirname, APR_HASH_KEY_STRING, new_array);
> > > + }
> > > + return SVN_NO_ERROR;
> > > +}
> > > +
> > > +/* Compare A and B and return an integer greater than, equal to, or less
> > > + * than 0, according to whether A has less subdirs, just as many or more
> > > + * subdir than B. */
> > > +static int
> > > +sort_compare_nr_of_path_elements(const svn_sort__item_t *a,
> > > + const svn_sort__item_t *b)
> >
> > Indentation is off above.
>
> Fixed.
>
> > > + const char *astr, *bstr;
> > > + int n_a, n_b, i;
> > > +
> > > + astr = a->key;
> > > + bstr = b->key;
> > > +
> > > + for (i = 0; i < a->klen; i++)
> > > + if (astr[i] == '/')
> > > + n_a++;
> > > +
> > > + for (i = 0; i < b->klen; i++)
> > > + if (bstr[i] == '/')
> > > + n_b++;
> > > +
> > > + if (n_a > n_b)
> > > + return -1;
> > > + else if (n_a == n_b)
> > > + return 0;
> > > + else
> > > + return 1;
> > > +}
> > > +
> > > +/* If all TARGETS in a dir is to be deleted, we condense those targets
> > > into
> > > + * one, representing their parent dirs. A new array is allocated from
> > > + * RESULT_POOL and returned in TARGETS. */
> >
> > Maybe phrase this as:
> >
> > /* Condense the list of TARGETS to be deleted such that there is
> > * only a single entry single entry for each common parent directory
> > * of deleted targets. Re-allocate TARGETS in RESULT_POOL.
> > * Do temporary allocations in SCRATCH_POOL. */
> >
> > I'm not documenting CTX cause I hope we can get rid of it, see above :)
>
> I took your suggestion straight off.
>
> > Also, is there a way we can get around always allocating a new
> > target arrray?
>
> In my new patch I loop over the targets in the beginning and check for
> targets to be deleted. If none, we're done and can return. Takes more
> iterations but saves some memory.
>
> > > +static svn_error_t *
> > > +maybe_condense_deleted_targets(apr_array_header_t **targets,
> > > + svn_client_ctx_t *ctx, apr_pool_t
> > > *result_pool,
> > > + apr_pool_t *scratch_pool)
> > > +{
> > > + int i;
> > > + apr_array_header_t *new_targets;
> > > + apr_array_header_t *deleted_targets;
> > > + apr_array_header_t *sorted_keys;
> > > + apr_hash_t *targets_to_be_deleted;
> > > +
> > > + new_targets = apr_array_make(result_pool, 0, sizeof(patch_target_t *));
> > > + targets_to_be_deleted = apr_hash_make(result_pool);
> > > +
> > > + /* First we collect the targets that should be deleted ... */
> > > + for (i = 0; i < (*targets)->nelts; i++)
> > > + {
> > > + patch_target_t *target = APR_ARRAY_IDX(*targets, i, patch_target_t
> > > *);
> > > +
> > > + if (! target->deleted)
> > > + APR_ARRAY_PUSH(new_targets, patch_target_t *) = target;
> > > + else
> > > +
> > > SVN_ERR(add_target_to_hash_keyed_by_parent_dir(targets_to_be_deleted,
> > > + target,
> > > result_pool));
> > > + }
> > > +
> > > + /* ... Then we sort the keys to allow us to detect when multiple
> > > subdirs
> > > + * should be deleted. */
> > > + sorted_keys = svn_sort__hash(targets_to_be_deleted,
> > > + sort_compare_nr_of_path_elements,
> > > + scratch_pool);
> > > +
> > > + for (i = 0; i < sorted_keys->nelts ; i++)
> > > + {
> > > + svn_sort__item_t *item;
> > > + svn_boolean_t empty;
> > > + char *key;
> > > + apr_array_header_t *targets_array;
> > > + int j;
> > > +
> > > + item = &APR_ARRAY_IDX(sorted_keys, i, svn_sort__item_t);
> > > + key = (char *) item->key;
> > > + targets_array = (apr_array_header_t *) item->value;
> > > +
> > > + SVN_ERR(is_dir_empty(&empty, key, ctx, targets_array,
> > > + result_pool, scratch_pool));
> > > + if (empty)
> > > + {
> > > + patch_target_t *target = apr_palloc(result_pool,
> > > sizeof(*target));
> > > + target->deleted = TRUE;
> > > + target->kind = svn_node_dir;
> > > + target->abs_path = apr_pstrdup(result_pool, key);
> > > +
> > > + /* Since the dir was empty we'll use a parent_dir target
> > > instead
> > > + * of the child targets. Time to close unused streams! */
> > > + for (j = 0; j < targets_array->nelts; j++)
> > > + {
> > > + patch_target_t *child_target;
> > > +
> > > + child_target = APR_ARRAY_IDX(targets_array, j,
> > > patch_target_t *);
> > > +
> > > + if (child_target->patch)
> > > + SVN_ERR(svn_diff__close_patch(child_target->patch));
> > > + }
> > > +
> > > +
> > > SVN_ERR(add_target_to_hash_keyed_by_parent_dir(targets_to_be_deleted,
> > > + target,
> > > result_pool));
> > > +
> > > + /* Hack! Since the added target of the parent dir has a shorter
> > > + * path, we're guarenteed that it will be inserted later. We do
> > > + * the sort and just continue our iteration. */
> > > + sorted_keys = svn_sort__hash(targets_to_be_deleted,
> > > + sort_compare_nr_of_path_elements,
> > > + scratch_pool);
> > > + }
> > > + else
> > > + {
> > > + /* No empty dir, just add the targets to be deleted */
> > > + for (j = 0; j < targets_array->nelts; j++)
> > > + {
> > > + patch_target_t *target = APR_ARRAY_IDX(targets_array, j,
> > > + patch_target_t *);
> > > + APR_ARRAY_PUSH(new_targets, patch_target_t *) = target;
> > > + }
> > > + }
> > > + }
> > > + *targets = new_targets;
> > > +
> > > + return SVN_NO_ERROR;
> > > +}
> > > +
> > > /* Install a patched TARGET into the working copy at ABS_WC_PATH.
> > > * Use client context CTX to retrieve WC_CTX, and possibly doing
> > > * notifications. If DRY_RUN is TRUE, don't modify the working copy.
> > > @@ -1413,6 +1652,9 @@
> > > }
> > > while (patch);
> > >
> > > + SVN_ERR(maybe_condense_deleted_targets(&targets, btn->ctx,
> > > scratch_pool,
> > > + result_pool));
> >
> > Indentation off above.
>
> Fixed
> > > +
> > > /* Install patched targets into the working copy. */
> > > for (i = 0; i < targets->nelts; i++)
> > > {
> > > @@ -1428,7 +1670,8 @@
> > > SVN_ERR(install_patched_target(target, btn->abs_wc_path,
> > > btn->ctx, btn->dry_run,
> > > iterpool));
> > > SVN_ERR(send_patch_notification(target, btn->ctx, iterpool));
> > > - SVN_ERR(svn_diff__close_patch(target->patch));
> > > + if (target->patch)
> > > + SVN_ERR(svn_diff__close_patch(target->patch));
> >
> > Hmm... this bit looks like it could be committed separately?
>
> Nope, it's needed. If we have a parent dir target, then it does not
> contain any patch. When svn_diff__close_patch() tries to close the
> streams we get a segfault.
>
> Index: subversion/tests/cmdline/patch_tests.py
> ===================================================================
> --- subversion/tests/cmdline/patch_tests.py (revision 918486)
> +++ subversion/tests/cmdline/patch_tests.py (arbetskopia)
> @@ -911,7 +911,108 @@
> None, # expected err
> 1, # check-props
> 1) # dry-run
> +def patch_remove_empty_dir(sbox):
> + "delete the dir if all children is deleted"
>
> + sbox.build()
> + wc_dir = sbox.wc_dir
> +
> + patch_file_path =
> tempfile.mkstemp(dir=os.path.abspath(svntest.main.temp_dir))[1]
> +
> + # Contents of B:
> + # A/B/lamba
> + # A/B/F
> + # A/B/E/{alpha,beta}
> + # Before patching we've deleted F, which means that B is empty after
> patching and
> + # should be removed.
> + #
> + # Contents of H:
> + # A/D/H/{chi,psi,omega}
> + # Before patching, chi has been removed by a non-svn operation which means
> it has
> + # status missing. The patch deletes the other two files but should not
> delete H.
> +
> + unidiff_patch = [
> + "Index: psi\n",
> + "===================================================================\n",
> + "--- A/D/H/psi\t(revision 0)\n",
> + "+++ A/D/H/psi\t(revision 0)\n",
> + "@@ -1 +0,0 @@\n",
> + "-This is the file 'psi'.\n",
> + "Index: omega\n",
> + "===================================================================\n",
> + "--- A/D/H/omega\t(revision 0)\n",
> + "+++ A/D/H/omega\t(revision 0)\n",
> + "@@ -1 +0,0 @@\n",
> + "-This is the file 'omega'.\n",
> + "Index: lambda\n",
> + "===================================================================\n",
> + "--- A/B/lambda\t(revision 0)\n",
> + "+++ A/B/lambda\t(revision 0)\n",
> + "@@ -1 +0,0 @@\n",
> + "-This is the file 'lambda'.\n",
> + "Index: alpha\n",
> + "===================================================================\n",
> + "--- A/B/E/alpha\t(revision 0)\n",
> + "+++ A/B/E/alpha\t(revision 0)\n",
> + "@@ -1 +0,0 @@\n",
> + "-This is the file 'alpha'.\n",
> + "Index: beta\n",
> + "===================================================================\n",
> + "--- A/B/E/beta\t(revision 0)\n",
> + "+++ A/B/E/beta\t(revision 0)\n",
> + "@@ -1 +0,0 @@\n",
> + "-This is the file 'beta'.\n",
> + ]
> +
> + svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
> +
> + F_path = os.path.join(wc_dir, 'A', 'B', 'F')
> + svntest.actions.run_and_verify_svn("Deleting F failed", None, [],
> + 'rm', F_path)
> + svntest.actions.run_and_verify_svn("Update failed", None, [],
> + 'up', wc_dir)
> +
> + # We should be able to handle one path beeing missing.
> + os.remove(os.path.join(wc_dir, 'A', 'D', 'H', 'chi'))
> +
> + expected_output = [
> + 'D %s\n' % os.path.join(wc_dir, 'A', 'B'),
> + 'D %s\n' % os.path.join(wc_dir, 'A', 'D', 'H', 'psi'),
> + 'D %s\n' % os.path.join(wc_dir, 'A', 'D', 'H', 'omega'),
> + ]
> +
> + expected_disk = svntest.main.greek_state.copy()
> + expected_disk.remove('A/D/H/chi')
> + expected_disk.remove('A/D/H/psi')
> + expected_disk.remove('A/D/H/omega')
> + expected_disk.remove('A/B/lambda')
> + expected_disk.remove('A/B/E/alpha')
> + expected_disk.remove('A/B/E/beta')
> +
> + expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
> + expected_status.add({'A/D/H/chi' : Item(status='! ', wc_rev=1)})
> + expected_status.add({'A/D/H/omega' : Item(status='D ', wc_rev=1)})
> + expected_status.add({'A/D/H/psi' : Item(status='D ', wc_rev=1)})
> + expected_status.add({'A/B' : Item(status='D ', wc_rev=1)})
> + expected_status.add({'A/B/E' : Item(status='D ', wc_rev=1)})
> + expected_status.add({'A/B/E/beta' : Item(status='D ', wc_rev=1)})
> + expected_status.add({'A/B/E/alpha' : Item(status='D ', wc_rev=1)})
> + expected_status.add({'A/B/lambda' : Item(status='D ', wc_rev=1)})
> + expected_status.add({'A/B/F' : Item(status='D ', wc_rev=1)})
> +
> + expected_skip = wc.State('', { })
> +
> + svntest.actions.run_and_verify_patch(wc_dir,
> + os.path.abspath(patch_file_path),
> + expected_output,
> + expected_disk,
> + expected_status,
> + expected_skip,
> + None, # expected err
> + 1, # check-props
> + 1) # dry-run
> +
> +
> def patch_reject(sbox):
> "apply a patch which is rejected"
>
> @@ -1363,6 +1464,7 @@
> patch_chopped_leading_spaces,
> patch_strip1,
> patch_add_new_dir,
> + patch_remove_empty_dir,
> patch_reject,
> patch_keywords,
> patch_with_fuzz,
> Index: subversion/libsvn_client/patch.c
> ===================================================================
> --- subversion/libsvn_client/patch.c (revision 918486)
> +++ subversion/libsvn_client/patch.c (arbetskopia)
> @@ -34,6 +34,7 @@
> #include "svn_path.h"
> #include "svn_pools.h"
> #include "svn_props.h"
> +#include "svn_sorts.h"
> #include "svn_subst.h"
> #include "svn_wc.h"
> #include "client.h"
> @@ -1184,6 +1185,268 @@
> return SVN_NO_ERROR;
> }
>
> +/* Baton for find_existing_children() */
> +struct status_baton
> +{
> + apr_array_header_t *existing_targets;
> + const char *parent_path;
> + apr_pool_t *result_pool;
> +};
> +
> +/* Implements svn_wc_status_func4_t. */
> +static svn_error_t *
> +find_existing_children(void *baton,
> + const char *abspath,
> + const svn_wc_status2_t *status,
> + apr_pool_t *pool)
> +{
> + struct status_baton *btn = baton;
> +
> + if (status->text_status != svn_wc_status_none
> + && status->text_status != svn_wc_status_deleted
> + && strcmp(abspath, btn->parent_path))
> + {
> + APR_ARRAY_PUSH(btn->existing_targets,
> + const char *) = apr_pstrdup(btn->result_pool,
> + abspath);
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +/* Check if PARENT_DIR_ABSPATH has any versioned or unversioned children if
> + * we ignore the ones in TARGETS_TO_BE_DELETED. Return the answer in
> + * EMPTY. */
> +static svn_error_t *
> +is_dir_empty(svn_boolean_t *empty, const char *parent_dir_abspath,
> + svn_wc_context_t *wc_ctx,
> + apr_array_header_t *targets_to_be_deleted,
> + apr_pool_t *result_pool, apr_pool_t *scratch_pool)
> +{
> + struct status_baton btn;
> + svn_opt_revision_t revision;
> + int i;
> +
> + btn.existing_targets = apr_array_make(scratch_pool, 0,
> + sizeof(patch_target_t *));
> + btn.parent_path = parent_dir_abspath;
> + btn.result_pool = scratch_pool;
> + revision.kind = svn_opt_revision_unspecified;
> +
> +#if 0
> + SVN_ERR(svn_client_status5(NULL, parent_dir_abspath, &revision,
> + find_existing_children, &btn,
> + svn_depth_immediates, TRUE, FALSE, TRUE,
> + FALSE, NULL, ctx, scratch_pool));
> +#endif
> +
> + SVN_ERR(svn_wc_walk_status(wc_ctx, parent_dir_abspath,
> svn_depth_immediates,
> + TRUE, TRUE, TRUE, NULL, find_existing_children,
> + &btn, NULL, NULL, NULL, NULL, scratch_pool));
> +
> + /* Do we delete all targets? */
> + for (i = 0; i < btn.existing_targets->nelts; i++)
> + {
> + int j;
> + const char *found = APR_ARRAY_IDX(btn.existing_targets, i, const char
> *);
> + svn_boolean_t deleted = FALSE;
> +
> + for (j = 0; j < targets_to_be_deleted->nelts; j++)
> + {
> + patch_target_t *to_be_del;
> + to_be_del = APR_ARRAY_IDX(targets_to_be_deleted, j, patch_target_t
> *);
> + if (! strcmp(found, to_be_del->abs_path))
> + {
> + deleted = TRUE;
> + break;
> + }
> + }
> + if (! deleted)
> + {
> + *empty = FALSE;
> + break;
> + }
> + }
> +
> + return SVN_NO_ERROR;
> +}
> +
> +/* Add TARGET to the array of targets keyed by their parent dir in
> + * TARGETS_TO_BE_DELETED. If there is no array for the parent dir a new one
> + * is created. All allocations are done in RESULT_POOL. */
> +static svn_error_t *
> +add_target_to_hash_keyed_by_parent_dir(apr_hash_t *targets_to_be_deleted,
> + patch_target_t *target,
> + apr_pool_t *result_pool)
> +{
> + apr_array_header_t * deleted_targets_in_dir;
> +
> + /* We're using the abs_path of the target. The abs_path is not
> + * present if the path is a symlink pointing outside the wc but we
> + * know that's not the case. */
> + const char *dirname = svn_dirent_dirname(target->abs_path,
> + result_pool);
> +
> + deleted_targets_in_dir = apr_hash_get(targets_to_be_deleted,
> + dirname, APR_HASH_KEY_STRING);
> +
> + if (deleted_targets_in_dir)
> + {
> + APR_ARRAY_PUSH(deleted_targets_in_dir, patch_target_t *) = target;
> +
> + apr_hash_set(targets_to_be_deleted, dirname,
> + APR_HASH_KEY_STRING, deleted_targets_in_dir);
> + }
> + else
> + {
> + apr_array_header_t *new_array;
> +
> + new_array = apr_array_make(result_pool, 0, sizeof(patch_target_t *));
> + APR_ARRAY_PUSH(new_array, patch_target_t *) = target;
> + apr_hash_set(targets_to_be_deleted,
> + dirname, APR_HASH_KEY_STRING, new_array);
> + }
> + return SVN_NO_ERROR;
> +}
> +
> +/* Compare A and B and return an integer greater than, equal to, or less
> + * than 0, according to whether A has less subdirs, just as many or more
> + * subdir than B. */
> +static int
> +sort_compare_nr_of_path_elements(const svn_sort__item_t *a,
> + const svn_sort__item_t *b)
> +{
> + const char *astr, *bstr;
> + int n_a, n_b, i;
> +
> + astr = a->key;
> + bstr = b->key;
> +
> + for (i = 0; i < a->klen; i++)
> + if (astr[i] == '/')
> + n_a++;
> +
> + for (i = 0; i < b->klen; i++)
> + if (bstr[i] == '/')
> + n_b++;
> +
> + if (n_a > n_b)
> + return -1;
> + else if (n_a == n_b)
> + return 0;
> + else
> + return 1;
> +}
> +
> +/* Condense the list of TARGETS to be deleted such that there is only a
> + * single entry for each common parent directory of deleted targets. Use
> + * WC_CTX for checking if a dir is empty. Re-allocate TARGETS in
> + * RESULT_POOL if deleted targets exists. Do temporary allocations in
> + * SCRATCH_POOL. */
> +static svn_error_t *
> +maybe_condense_deleted_targets(apr_array_header_t **targets,
> + svn_wc_context_t *wc_ctx,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> +{
> + int i;
> + apr_array_header_t *new_targets;
> + apr_array_header_t *deleted_targets;
> + apr_array_header_t *sorted_keys;
> + apr_hash_t *targets_to_be_deleted;
> + svn_boolean_t deleted_targets_exists = FALSE;
> +
> + for (i = 0; i < (*targets)->nelts; i++)
> + {
> + patch_target_t *target = APR_ARRAY_IDX(*targets, i, patch_target_t *);
> + if (target->deleted)
> + deleted_targets_exists = TRUE;
> + }
> +
> + if (! deleted_targets_exists)
> + return SVN_NO_ERROR;
> +
> + /* We have deleted targets, time to do some allocations. */
> + new_targets = apr_array_make(result_pool, 0, sizeof(patch_target_t *));
> + targets_to_be_deleted = apr_hash_make(result_pool);
> +
> + /* First we collect the targets that should be deleted ... */
> + for (i = 0; i < (*targets)->nelts; i++)
> + {
> + patch_target_t *target = APR_ARRAY_IDX(*targets, i, patch_target_t *);
> +
> + if (! target->deleted)
> + APR_ARRAY_PUSH(new_targets, patch_target_t *) = target;
> + else
> +
> SVN_ERR(add_target_to_hash_keyed_by_parent_dir(targets_to_be_deleted,
> + target,
> result_pool));
> + }
> +
> + /* ... Then we sort the keys to allow us to detect when multiple subdirs
> + * should be deleted. */
> + sorted_keys = svn_sort__hash(targets_to_be_deleted,
> + sort_compare_nr_of_path_elements,
> + scratch_pool);
> +
> + for (i = 0; i < sorted_keys->nelts ; i++)
> + {
> + svn_sort__item_t *item;
> + svn_boolean_t empty;
> + char *key;
> + apr_array_header_t *targets_array;
> + int j;
> +
> + item = &APR_ARRAY_IDX(sorted_keys, i, svn_sort__item_t);
> + key = (char *) item->key;
> + targets_array = (apr_array_header_t *) item->value;
> +
> + SVN_ERR(is_dir_empty(&empty, key, wc_ctx, targets_array,
> + result_pool, scratch_pool));
> + if (empty)
> + {
> + patch_target_t *target = apr_palloc(result_pool, sizeof(*target));
> + target->deleted = TRUE;
> + target->kind = svn_node_dir;
> + target->abs_path = apr_pstrdup(result_pool, key);
> +
> + /* Since the dir was empty we'll use a parent_dir target instead
> + * of the child targets. Time to close unused streams! */
> + for (j = 0; j < targets_array->nelts; j++)
> + {
> + patch_target_t *child_target;
> +
> + child_target = APR_ARRAY_IDX(targets_array, j, patch_target_t
> *);
> +
> + if (child_target->patch)
> + SVN_ERR(svn_diff__close_patch(child_target->patch));
> + }
> +
> +
> SVN_ERR(add_target_to_hash_keyed_by_parent_dir(targets_to_be_deleted,
> + target,
> result_pool));
> +
> + /* Hack! Since the added target of the parent dir has a shorter
> + * path, we're guarenteed that it will be inserted later. We do
> + * the sort and just continue our iteration. */
> + sorted_keys = svn_sort__hash(targets_to_be_deleted,
> + sort_compare_nr_of_path_elements,
> + scratch_pool);
> + }
> + else
> + {
> + /* No empty dir, just add the targets to be deleted */
> + for (j = 0; j < targets_array->nelts; j++)
> + {
> + patch_target_t *target = APR_ARRAY_IDX(targets_array, j,
> + patch_target_t *);
> + APR_ARRAY_PUSH(new_targets, patch_target_t *) = target;
> + }
> + }
> + }
> + *targets = new_targets;
> +
> + return SVN_NO_ERROR;
> +}
> +
> /* Install a patched TARGET into the working copy at ABS_WC_PATH.
> * Use client context CTX to retrieve WC_CTX, and possibly doing
> * notifications. If DRY_RUN is TRUE, don't modify the working copy.
> @@ -1438,6 +1701,9 @@
> }
> while (patch);
>
> + SVN_ERR(maybe_condense_deleted_targets(&targets, btn->ctx->wc_ctx,
> + scratch_pool, result_pool));
> +
> /* Install patched targets into the working copy. */
> for (i = 0; i < targets->nelts; i++)
> {
> @@ -1453,7 +1719,8 @@
> SVN_ERR(install_patched_target(target, btn->abs_wc_path,
> btn->ctx, btn->dry_run, iterpool));
> SVN_ERR(send_patch_notification(target, btn->ctx, iterpool));
> - SVN_ERR(svn_diff__close_patch(target->patch));
> + if (target->patch)
> + SVN_ERR(svn_diff__close_patch(target->patch));
> }
>
> svn_pool_destroy(iterpool);
