You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2010/04/04 12:55:26 UTC
svn commit: r930662 - in
/subversion/branches/svn-patch-improvements/subversion:
libsvn_client/patch.c tests/cmdline/patch_tests.py
Author: dannas
Date: Sun Apr 4 10:55:25 2010
New Revision: 930662
URL: http://svn.apache.org/viewvc?rev=930662&view=rev
Log:
Make 'svn patch' able to remove dirs beeing empty after patching, taking
into account both versioned and unversioned files and dirs.
* subversion/tests/cmdline/patch_tests.py
(patch_remove_empty_dir): New.
(test_list): Add new test.
* subversion/libsvn_client/patch.c
(status_baton): New.
(find_existing_children, is_dir_empty,
add_target_to_hash_keyed_by_parent_dir, is_dir_empty,
sort_compare_nr_of_path_elements,
condense_deleted_targets): New functions
(apply_patches): Call condense_deleted_targets(). Check if a
target has a patch associated with it before calling
svn_diff__close_patch() since we're creating targets to delete dirs
and those have no associated patch.
Review by: julianf, stsp, neels
Modified:
subversion/branches/svn-patch-improvements/subversion/libsvn_client/patch.c
subversion/branches/svn-patch-improvements/subversion/tests/cmdline/patch_tests.py
Modified: subversion/branches/svn-patch-improvements/subversion/libsvn_client/patch.c
URL: http://svn.apache.org/viewvc/subversion/branches/svn-patch-improvements/subversion/libsvn_client/patch.c?rev=930662&r1=930661&r2=930662&view=diff
==============================================================================
--- subversion/branches/svn-patch-improvements/subversion/libsvn_client/patch.c (original)
+++ subversion/branches/svn-patch-improvements/subversion/libsvn_client/patch.c Sun Apr 4 10:55:25 2010
@@ -36,6 +36,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"
@@ -1233,6 +1234,250 @@ apply_one_patch(patch_target_t **patch_t
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;
+
+ 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));
+ *empty = TRUE;
+
+ /* 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)
+ {
+ deleted_targets_in_dir = apr_array_make(result_pool, 0,
+ sizeof(patch_target_t *));
+ apr_hash_set(targets_to_be_deleted,
+ dirname, APR_HASH_KEY_STRING, deleted_targets_in_dir);
+ }
+
+ APR_ARRAY_PUSH(deleted_targets_in_dir, patch_target_t *) = target;
+
+ 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. That means that the path with the most subdirs is placed
+ * first. */
+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 = 0, n_b = 0, 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 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 *
+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 *sorted_keys;
+ apr_hash_t *targets_to_be_deleted;
+
+ /* 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 each directory that contains targets to be deleted determine if the
+ * entire dir is empty and in that case mark it to be deleted. Else just
+ * add the individual targets in it to 'new_targets'. */
+ for (i = 0; i < sorted_keys->nelts ; i++)
+ {
+ svn_sort__item_t *item;
+ svn_boolean_t empty;
+ char *parent_dir;
+ apr_array_header_t *targets_array;
+ int j;
+
+ item = &APR_ARRAY_IDX(sorted_keys, i, svn_sort__item_t);
+ parent_dir = (char *) item->key;
+ targets_array = (apr_array_header_t *) item->value;
+
+ SVN_ERR(is_dir_empty(&empty, parent_dir, wc_ctx, targets_array,
+ result_pool, scratch_pool));
+ if (empty)
+ {
+ patch_target_t *parent_target = apr_palloc(result_pool,
+ sizeof(*parent_target));
+ parent_target->deleted = TRUE;
+ parent_target->kind = svn_node_dir;
+ parent_target->abs_path = apr_pstrdup(result_pool, parent_dir);
+
+ /* 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));
+ }
+
+ /* Marking one dir as to be deleted may mean that it's parent dir
+ * will be empty too and thus should be marked for deletion. */
+ SVN_ERR(add_target_to_hash_keyed_by_parent_dir(targets_to_be_deleted,
+ parent_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.
@@ -1505,6 +1750,9 @@ apply_patches(void *baton,
}
while (patch);
+ SVN_ERR(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++)
{
@@ -1512,6 +1760,7 @@ apply_patches(void *baton,
svn_pool_clear(iterpool);
+
if (btn->ctx->cancel_func)
SVN_ERR(btn->ctx->cancel_func(btn->ctx->cancel_baton));
@@ -1520,7 +1769,8 @@ apply_patches(void *baton,
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_ERR(svn_io_file_close(patch_file, iterpool));
Modified: subversion/branches/svn-patch-improvements/subversion/tests/cmdline/patch_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/svn-patch-improvements/subversion/tests/cmdline/patch_tests.py?rev=930662&r1=930661&r2=930662&view=diff
==============================================================================
--- subversion/branches/svn-patch-improvements/subversion/tests/cmdline/patch_tests.py (original)
+++ subversion/branches/svn-patch-improvements/subversion/tests/cmdline/patch_tests.py Sun Apr 4 10:55:25 2010
@@ -997,6 +997,107 @@ def patch_add_new_dir(sbox):
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', 'D', 'H', 'psi'),
+ 'D %s\n' % os.path.join(wc_dir, 'A', 'D', 'H', 'omega'),
+ 'D %s\n' % os.path.join(wc_dir, 'A', 'B'),
+ ]
+
+ 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):
"patch which is rejected"
@@ -2259,6 +2360,7 @@ test_list = [ None,
patch_strip1,
patch_no_index_line,
patch_add_new_dir,
+ patch_remove_empty_dir,
patch_reject,
patch_keywords,
patch_with_fuzz,
Re: svn commit: r930662 - in
/subversion/branches/svn-patch-improvements/subversion:
libsvn_client/patch.c tests/cmdline/patch_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Apr 04, 2010 at 01:20:09PM +0200, Stefan Sperling wrote:
> On Sun, Apr 04, 2010 at 10:55:26AM -0000, dannas@apache.org wrote:
> > Author: dannas
> > Date: Sun Apr 4 10:55:25 2010
> > New Revision: 930662
> >
> > URL: http://svn.apache.org/viewvc?rev=930662&view=rev
> > Log:
> > Make 'svn patch' able to remove dirs beeing empty after patching, taking
> > into account both versioned and unversioned files and dirs.
> >
>
> > +
> > +/* 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. That means that the path with the most subdirs is placed
> > + * first. */
> > +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 = 0, n_b = 0, i;
> > +
> > + astr = a->key;
> > + bstr = b->key;
> > +
> > + for (i = 0; i < a->klen; i++)
> > + if (astr[i] == '/')
> > + n_a++;
>
> Drive-by review (will look more closely later):
Looking more closely, I ended up rewriting a lot of this (I played with
making small changes to make sure I understand the problem, and then I got
carried away :)
Below is a log message and a patch. I moved the code around, partly
because this makes it easier for you to read the new implementation.
I'd rather not rip out a lot of code you wrote without your consent,
so please review this and let me know you what you think.
Thanks,
Stefan
[[[
On the svn-patch-improvements branch, revamp handling of deleted directories.
While not originally intended, this is almost a complete rewrite of the
logic handling directory deletions. Compared to the previous implementation:
- It does not modify lists in place while iterating them.
- It does not add empty directories which can be deleted as 'fake' patch
targets, but deletes them directly, and skips targets which were deleted
together with the parent directories.
- It splits the overall task into more distinct steps, mainly for
clarity to the reader.
* subversion/libsvn_client/patch.c
(patch_target_t): New field PARENT_DIR_DELETED, to trigger special
processing of targets which are within deleted directories.
(init_patch_target): Initialise PARENT_DIR_DELETED.
(is_dir_empty): Rename to ...
(check_dir_empty): ... this, and in addition to the list of deleted
targets, accept a list of directories we know will be removed.
(add_target_to_hash_keyed_by_parent_dir, sort_compare_path_component_count,
condense_deleted_targets): Remove.
(push_if_unique): New helper function, needed because we do not use
a hash table anymore to ensure path uniqueness.
(delete_empty_dirs): New function, which determines which directories will
be empty after patching, deletes them, and marks any targets which are
children of the deleted directories.
(install_patched_target): Skip targets which had their parent dir removed
from above them.
(apply_patches): Call delete_empty_dirs() instead of the removed
condense_deleted_targets(). Do not notify about targets which were
deleted together with their parent directories.
* subversion/tests/cmdline/patch_tests.py
(patch_remove_empty_dir): Rename to ...
(patch_remove_empty_dirs): .. this, and adjust expected output because
notification order has changed.
(test_list): Track renamed function.
]]]
Index: subversion/libsvn_client/patch.c
===================================================================
--- subversion/libsvn_client/patch.c (revision 932869)
+++ subversion/libsvn_client/patch.c (working copy)
@@ -158,6 +158,10 @@ typedef struct {
/* True if the target's parent directory exists. */
svn_boolean_t parent_dir_exists;
+ /* True if the target's parent directory is being deleted
+ * as a result of the patching process. */
+ svn_boolean_t parent_dir_deleted;
+
/* The keywords of the target. */
apr_hash_t *keywords;
@@ -516,6 +520,7 @@ init_patch_target(patch_target_t **patch_target,
target->had_rejects = FALSE;
target->deleted = FALSE;
target->eof = FALSE;
+ target->parent_dir_deleted = FALSE;
target->pool = result_pool;
target->lines = apr_array_make(result_pool, 0,
sizeof(svn_stream_mark_t *));
@@ -1234,241 +1239,6 @@ apply_one_patch(patch_target_t **patch_target, svn
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;
-
- 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));
- *empty = TRUE;
-
- /* 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)
- {
- deleted_targets_in_dir = apr_array_make(result_pool, 0,
- sizeof(patch_target_t *));
- apr_hash_set(targets_to_be_deleted,
- dirname, APR_HASH_KEY_STRING, deleted_targets_in_dir);
- }
-
- APR_ARRAY_PUSH(deleted_targets_in_dir, patch_target_t *) = target;
-
- 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. That means that the path with the most subdirs is placed
- * first. */
-static int
-sort_compare_path_component_count(const svn_sort__item_t *a,
- const svn_sort__item_t *b)
-{
- apr_size_t n_a = 0, n_b = 0;
-
- n_a = svn_path_component_count(a->key);
- n_b = svn_path_component_count(b->key);
-
- if (n_a > n_b)
- return -1;
- else if (n_a == n_b)
- return 0;
- else
- return 1;
-}
-
-/* Condense the list of TARGETS 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 *
-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 *sorted_keys;
- apr_hash_t *targets_to_be_deleted;
-
- /* 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_path_component_count,
- scratch_pool);
-
- /* For each directory that contains targets to be deleted determine if the
- * entire dir is empty and in that case mark it to be deleted. Else just
- * add the individual targets in it to 'new_targets'. */
- for (i = 0; i < sorted_keys->nelts ; i++)
- {
- svn_sort__item_t *item;
- svn_boolean_t empty;
- char *parent_dir;
- apr_array_header_t *targets_array;
- int j;
-
- item = &APR_ARRAY_IDX(sorted_keys, i, svn_sort__item_t);
- parent_dir = (char *) item->key;
- targets_array = (apr_array_header_t *) item->value;
-
- SVN_ERR(is_dir_empty(&empty, parent_dir, wc_ctx, targets_array,
- result_pool, scratch_pool));
- if (empty)
- {
- patch_target_t *parent_target = apr_palloc(result_pool,
- sizeof(*parent_target));
- parent_target->deleted = TRUE;
- parent_target->kind = svn_node_dir;
- parent_target->abs_path = apr_pstrdup(result_pool, parent_dir);
-
- /* 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));
- }
-
- /* Marking one dir as to be deleted may mean that it's parent dir
- * will be empty too and thus should be marked for deletion. */
- SVN_ERR(add_target_to_hash_keyed_by_parent_dir(targets_to_be_deleted,
- parent_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_path_component_count,
- 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.
@@ -1480,7 +1250,7 @@ install_patched_target(patch_target_t *target, con
{
if (target->deleted)
{
- if (! dry_run)
+ if (! dry_run && ! target->parent_dir_deleted)
{
/* Schedule the target for deletion. Suppress
* notification, we'll do it manually in a minute.
@@ -1648,6 +1418,289 @@ install_patched_target(patch_target_t *target, con
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;
+}
+
+/* Indicate in *EMPTY whether the directory at LOCAL_ABSPATH has any
+ * versioned or unversioned children. Consider any DELETED_TARGETS,
+ * as well as paths listed in DELETED_ABSPATHS_LIST (which may be NULL)
+ * as already deleted. Use WC_CTX as the working copy context.
+ * Do temporary allocations in SCRATCH_POOL. */
+static svn_error_t *
+check_dir_empty(svn_boolean_t *empty, const char *local_abspath,
+ svn_wc_context_t *wc_ctx,
+ apr_array_header_t *deleted_targets,
+ apr_array_header_t *deleted_abspath_list,
+ apr_pool_t *scratch_pool)
+{
+ struct status_baton btn;
+ svn_opt_revision_t revision;
+ svn_error_t *err;
+ int i;
+
+ btn.existing_targets = apr_array_make(scratch_pool, 0,
+ sizeof(patch_target_t *));
+ btn.parent_path = local_abspath;
+ btn.result_pool = scratch_pool;
+ revision.kind = svn_opt_revision_unspecified;
+
+ err = svn_wc_walk_status(wc_ctx, local_abspath, svn_depth_immediates,
+ TRUE, TRUE, TRUE, NULL, find_existing_children,
+ &btn, NULL, NULL, NULL, NULL, scratch_pool);
+ if (err)
+ {
+ if (err->apr_err == SVN_ERR_WC_NOT_WORKING_COPY)
+ {
+ /* The patch has deleted the entire working copy. */
+ svn_error_clear(err);
+ *empty = FALSE;
+ return SVN_NO_ERROR;
+ }
+ else
+ svn_error_return(err);
+ }
+
+ *empty = TRUE;
+
+ /* Do we delete all targets? */
+ for (i = 0; i < btn.existing_targets->nelts; i++)
+ {
+ int j;
+ const char *found;
+ svn_boolean_t deleted;
+
+ deleted = FALSE;
+ found = APR_ARRAY_IDX(btn.existing_targets, i, const char *);
+
+ for (j = 0; j < deleted_targets->nelts; j++)
+ {
+ patch_target_t *target;
+
+ target = APR_ARRAY_IDX(deleted_targets, j, patch_target_t *);
+ if (! svn_path_compare_paths(found, target->abs_path))
+ {
+ deleted = TRUE;
+ break;
+ }
+ }
+ if (! deleted && deleted_abspath_list)
+ {
+ for (j = 0; j < deleted_abspath_list->nelts; j++)
+ {
+ const char *abspath;
+
+ abspath = APR_ARRAY_IDX(deleted_abspath_list, j, const char *);
+ if (! svn_path_compare_paths(found, abspath))
+ {
+ deleted = TRUE;
+ break;
+ }
+ }
+ }
+ if (! deleted)
+ {
+ *empty = FALSE;
+ break;
+ }
+ }
+
+ return SVN_NO_ERROR;
+}
+
+/* Push a copy of EMPTY_DIR, allocated in RESULT_POOL, onto the EMPTY_DIRS
+ * array if no directory matching EMPTY_DIR is already in the array. */
+static void
+push_if_unique(apr_array_header_t *empty_dirs, const char *empty_dir,
+ apr_pool_t *result_pool)
+{
+ svn_boolean_t is_unique;
+ int i;
+
+ is_unique = TRUE;
+ for (i = 0; i < empty_dirs->nelts; i++)
+ {
+ const char *empty_dir2;
+
+ empty_dir2 = APR_ARRAY_IDX(empty_dirs, i, const char *);
+ if (strcmp(empty_dir, empty_dir2) == 0)
+ {
+ is_unique = FALSE;
+ break;
+ }
+ }
+
+ if (is_unique)
+ APR_ARRAY_PUSH(empty_dirs, const char *) = apr_pstrdup(result_pool,
+ empty_dir);
+}
+
+/* Delete all directories from the working copy which are left empty
+ * by deleted TARGETS. Use client context CTX.
+ * If DRY_RUN is TRUE, do not modify the working copy.
+ * Do temporary allocations in SCRATCH_POOL. */
+static svn_error_t *
+delete_empty_dirs(apr_array_header_t *targets, svn_client_ctx_t *ctx,
+ svn_boolean_t dry_run, apr_pool_t *scratch_pool)
+{
+ apr_array_header_t *empty_dirs;
+ apr_array_header_t *deleted_targets;
+ apr_pool_t *iterpool;
+ svn_boolean_t again;
+ int i;
+
+ /* Get a list of all deleted targets. */
+ deleted_targets = apr_array_make(scratch_pool, 0, sizeof(patch_target_t *));
+ for (i = 0; i < targets->nelts; i++)
+ {
+ patch_target_t *target;
+
+ target = APR_ARRAY_IDX(targets, i, patch_target_t *);
+ if (target->deleted)
+ APR_ARRAY_PUSH(deleted_targets, patch_target_t *) = target;
+ }
+
+ /* We have nothing to do if there aren't any deleted targets. */
+ if (deleted_targets->nelts == 0)
+ return SVN_NO_ERROR;
+
+ /* Look for empty parent directories of deleted targets. */
+ empty_dirs = apr_array_make(scratch_pool, 0, sizeof(const char *));
+ iterpool = svn_pool_create(scratch_pool);
+ for (i = 0; i < targets->nelts; i++)
+ {
+ svn_boolean_t parent_empty;
+ patch_target_t *target;
+ const char *parent;
+
+ svn_pool_clear(iterpool);
+
+ target = APR_ARRAY_IDX(targets, i, patch_target_t *);
+ parent = svn_dirent_dirname(target->abs_path, iterpool);
+ SVN_ERR(check_dir_empty(&parent_empty, parent, ctx->wc_ctx,
+ deleted_targets, NULL, iterpool));
+ if (parent_empty)
+ {
+ APR_ARRAY_PUSH(empty_dirs, const char *) =
+ apr_pstrdup(scratch_pool, parent);
+ }
+ }
+
+ /* We have nothing to do if there aren't any empty directories. */
+ if (empty_dirs->nelts == 0)
+ {
+ svn_pool_destroy(iterpool);
+ return SVN_NO_ERROR;
+ }
+
+ /* Determine the minimal set of empty directories we need to delete. */
+ do
+ {
+ apr_array_header_t *empty_dirs_copy;
+
+ svn_pool_clear(iterpool);
+
+ /* Rebuild the empty dirs list, replacing empty dirs which have
+ * an empty parent with their parent. */
+ again = FALSE;
+ empty_dirs_copy = apr_array_copy(iterpool, empty_dirs);
+ apr_array_clear(empty_dirs);
+ for (i = 0; i < empty_dirs_copy->nelts; i++)
+ {
+ svn_boolean_t parent_empty;
+ const char *empty_dir;
+ const char *parent;
+
+ empty_dir = APR_ARRAY_IDX(empty_dirs_copy, i, const char *);
+ parent = svn_dirent_dirname(empty_dir, iterpool);
+ SVN_ERR(check_dir_empty(&parent_empty, parent, ctx->wc_ctx,
+ deleted_targets, empty_dirs_copy,
+ iterpool));
+ if (parent_empty)
+ {
+ again = TRUE;
+ push_if_unique(empty_dirs, parent, scratch_pool);
+ }
+ else
+ push_if_unique(empty_dirs, empty_dir, scratch_pool);
+ }
+ }
+ while (again);
+
+ /* Mark all children of empty parent directories. */
+ for (i = 0; i < targets->nelts ; i++)
+ {
+ patch_target_t *target;
+ int j;
+
+ target = APR_ARRAY_IDX(targets, i, patch_target_t *);
+ for (j = 0; j < empty_dirs->nelts; j++)
+ {
+ const char *empty_dir;
+
+ empty_dir = APR_ARRAY_IDX(empty_dirs, j, const char *);
+ if (svn_dirent_is_ancestor(empty_dir, target->abs_path))
+ {
+ target->parent_dir_deleted = TRUE;
+ break;
+ }
+ }
+ }
+
+ /* Finally, delete empty directories. */
+ for (i = 0; i < empty_dirs->nelts; i++)
+ {
+ const char *empty_dir;
+
+ svn_pool_clear(iterpool);
+
+ if (ctx->cancel_func)
+ SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
+
+ empty_dir = APR_ARRAY_IDX(empty_dirs, i, const char *);
+ if (ctx->notify_func2)
+ {
+ svn_wc_notify_t *notify;
+
+ notify = svn_wc_create_notify(empty_dir, svn_wc_notify_delete,
+ iterpool);
+ (*ctx->notify_func2)(ctx->notify_baton2, notify, iterpool);
+ }
+ if (! dry_run)
+ SVN_ERR(svn_wc_delete4(ctx->wc_ctx, empty_dir, FALSE, FALSE,
+ NULL, NULL, /* suppress cancellation */
+ NULL, NULL, /* suppress notification */
+ iterpool));
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* Baton for apply_patches(). */
typedef struct {
/* The path to the patch file. */
@@ -1741,8 +1794,8 @@ apply_patches(void *baton,
}
while (patch);
- SVN_ERR(condense_deleted_targets(&targets, btn->ctx->wc_ctx,
- scratch_pool, result_pool));
+ /* Delete directories which are empty after patching, if any. */
+ SVN_ERR(delete_empty_dirs(targets, btn->ctx, btn->dry_run, scratch_pool));
/* Install patched targets into the working copy. */
for (i = 0; i < targets->nelts; i++)
@@ -1751,7 +1804,6 @@ apply_patches(void *baton,
svn_pool_clear(iterpool);
-
if (btn->ctx->cancel_func)
SVN_ERR(btn->ctx->cancel_func(btn->ctx->cancel_baton));
@@ -1759,7 +1811,8 @@ apply_patches(void *baton,
if (! target->skipped)
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));
+ if (! target->parent_dir_deleted)
+ SVN_ERR(send_patch_notification(target, btn->ctx, iterpool));
if (target->patch)
SVN_ERR(svn_diff_close_patch(target->patch));
}
Index: subversion/tests/cmdline/patch_tests.py
===================================================================
--- subversion/tests/cmdline/patch_tests.py (revision 932894)
+++ subversion/tests/cmdline/patch_tests.py (working copy)
@@ -1003,7 +1003,7 @@ def patch_add_new_dir(sbox):
None, # expected err
1, # check-props
1) # dry-run
-def patch_remove_empty_dir(sbox):
+def patch_remove_empty_dirs(sbox):
"patch deleting all children of a directory"
sbox.build()
@@ -1068,9 +1068,9 @@ def patch_add_new_dir(sbox):
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'),
- 'D %s\n' % os.path.join(wc_dir, 'A', 'B'),
]
expected_disk = svntest.main.greek_state.copy()
@@ -2376,7 +2376,7 @@ test_list = [ None,
patch_strip1,
patch_no_index_line,
patch_add_new_dir,
- patch_remove_empty_dir,
+ patch_remove_empty_dirs,
patch_reject,
patch_keywords,
patch_with_fuzz,
Re: svn commit: r930662 - in
/subversion/branches/svn-patch-improvements/subversion:
libsvn_client/patch.c tests/cmdline/patch_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Apr 04, 2010 at 10:55:26AM -0000, dannas@apache.org wrote:
> Author: dannas
> Date: Sun Apr 4 10:55:25 2010
> New Revision: 930662
>
> URL: http://svn.apache.org/viewvc?rev=930662&view=rev
> Log:
> Make 'svn patch' able to remove dirs beeing empty after patching, taking
> into account both versioned and unversioned files and dirs.
>
> +
> +/* 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. That means that the path with the most subdirs is placed
> + * first. */
> +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 = 0, n_b = 0, i;
> +
> + astr = a->key;
> + bstr = b->key;
> +
> + for (i = 0; i < a->klen; i++)
> + if (astr[i] == '/')
> + n_a++;
Drive-by review (will look more closely later):
Maybe use svn_path_component_count() here?
And rename sort_compare_nr_of_path_elements() to
sort_compare_path_component_count()? (I tend to prefer function names
consisting of proper words...)
Stefan
> +
> + 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;
> +}