[prev in list] [next in list] [prev in thread] [next in thread]
List: subversion-dev
Subject: [PATCH] 'svn patch' should remove empty dir
From: Daniel =?iso-8859-1?Q?N=E4slund?= <daniel () longitudo ! com>
Date: 2010-02-27 20:09:52
Message-ID: 20100227200952.GA5919 () daniel-laptop
[Download RAW message or body]
Hi!
Puh, 363 lines added. That's the biggest patch I've written so far. Sad
to think that once we have implemented git diff format this code will be
obsolute :).
Reviews welcome!
[[[
Make 'svn patch' able to remove dirs beeing empty after patching, taken
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,
maybe_condense_deleted_targets): New functions
(apply_patches): Call maybe_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.
Patch by: Daniel Näslund <daniel{_AT_}longitudo.com>
Review by: julianf, stsp, neels
]]]
What target fields must be set for the parent dir target? I've just set
deleted, abs_path and deleted. May be a bit hard to maintain. Would be
better with a central initialize_basic_target() func.
In find_existing_children() I've said that if a path has text_status
svn_wc_status{none,deleted} or is the name of the parent_dir we'll
ignore it, all else states means that we have a file or dir at the path.
I haven't taken file externals into account. If a patch says that a
target should be removed I'm assuming the user knows what they're doing.
Daniel
["svn_patch_remove_empty_dirs.diff.txt" (text/plain)]
Index: subversion/tests/cmdline/patch_tests.py
===================================================================
--- subversion/tests/cmdline/patch_tests.py (revision 916918)
+++ 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 916918)
+++ 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"
@@ -1159,6 +1160,244 @@
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_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));
+
+ /* 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;
+ }
+ 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;
+}
+
+/* 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. */
+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));
+
/* 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));
}
svn_pool_destroy(iterpool);
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic