Hi,
It's been a year since my last commit and I've forgotten about the
internals of the patch code. Sending this patch for review so that I
don't mess something up.
[[[
Fix issue #4049 - SEGV on patch that deletes and skips.
When we do the sanity check of the supplied patch file we set
skipped=TRUE for the files that we're not going to process.
* subversion/libsvn_client/patch.c
(patch_target_info_t): Add SKIPPED field.
(delete_empty_dirs): Don't process targets that are marked skipped.
* subversion/tests/cmdline/patch_tests.py
(patch_delete_and_skip): New.
(test_list): Add new test.
]]]
I could have checked if local_abspath was NULL but I figured it might be
easy to forget to set local_abspath = NULL in one of the many places
were we set the skipped flag.
--
Daniel (dannas)
Index: subversion/tests/cmdline/patch_tests.py
===================================================================
--- subversion/tests/cmdline/patch_tests.py (revision 1199347)
+++ subversion/tests/cmdline/patch_tests.py (arbetskopia)
@@ -3913,6 +3913,84 @@ def patch_dev_null(sbox):
1, # check-props
1) # dry-run
+@Issue(4049)
+def patch_delete_and_skip(sbox):
+ "patch that deletes and skips"
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ patch_file_path = make_patch_path(sbox)
+
+ os.chdir(wc_dir)
+
+ # We need to use abspaths to trigger the segmentation fault.
+ abs = os.path.abspath('.')
+ if sys.platform == 'win32':
+ abs = abs.replace("\\", "/")
+
+ outside_wc = os.path.join(os.pardir, 'X')
+ if sys.platform == 'win32':
+ outside_wc = outside_wc.replace("\\", "/")
+
+ unidiff_patch = [
+ "Index: %s/A/B/E/alpha\n" % abs,
+ "===================================================================\n",
+ "--- %s/A/B/E/alpha\t(revision 1)\n" % abs,
+ "+++ %s/A/B/E/alpha\t(working copy)\n" % abs,
+ "@@ -1 +0,0 @@\n",
+ "-This is the file 'alpha'.\n",
+ "Index: %s/A/B/E/beta\n" % abs,
+ "===================================================================\n",
+ "--- %s/A/B/E/beta\t(revision 1)\n" % abs,
+ "+++ %s/A/B/E/beta\t(working copy)\n" % abs,
+ "@@ -1 +0,0 @@\n",
+ "-This is the file 'beta'.\n",
+ "Index: %s/A/B/E/out-of-reach\n" % abs,
+ "===================================================================\n",
+ "--- %s/iota\t(revision 1)\n" % outside_wc,
+ "+++ %s/iota\t(working copy)\n" % outside_wc,
+ "\n",
+ "Property changes on: iota\n",
+ "___________________________________________________________________\n",
+ "Added: propname\n",
+ "## -0,0 +1 ##\n",
+ "+propvalue\n",
+ ]
+
+ svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
+
+ skipped_path = os.path.join(os.pardir, 'X', 'iota')
+ expected_output = [
+ 'D %s\n' % os.path.join('A', 'B', 'E', 'alpha'),
+ 'D %s\n' % os.path.join('A', 'B', 'E', 'beta'),
+ 'Skipped missing target: \'%s\'\n' % skipped_path,
+ 'D %s\n' % os.path.join('A', 'B', 'E'),
+ 'Summary of conflicts:\n',
+ ' Skipped paths: 1\n'
+ ]
+
+ expected_disk = svntest.main.greek_state.copy()
+ expected_disk.remove('A/B/E/alpha')
+ expected_disk.remove('A/B/E/beta')
+ expected_disk.remove('A/B/E')
+
+ expected_status = svntest.actions.get_virginal_state('.', 1)
+ expected_status.tweak('A/B/E', status='D ')
+ expected_status.tweak('A/B/E/alpha', status='D ')
+ expected_status.tweak('A/B/E/beta', status='D ')
+
+ expected_skip = wc.State('', {skipped_path: Item()})
+
+ svntest.actions.run_and_verify_patch('.', os.path.abspath(patch_file_path),
+ expected_output,
+ expected_disk,
+ expected_status,
+ expected_skip,
+ None, # expected err
+ 1, # check-props
+ 1) # dry-run
+
########################################################################
#Run the tests
@@ -3954,6 +4032,7 @@ test_list = [ None,
patch_reversed_add_with_props,
patch_reversed_add_with_props2,
patch_dev_null,
+ patch_delete_and_skip,
]
if __name__ == '__main__':
Index: subversion/libsvn_client/patch.c
===================================================================
--- subversion/libsvn_client/patch.c (revision 1199347)
+++ subversion/libsvn_client/patch.c (arbetskopia)
@@ -255,6 +255,7 @@ typedef struct patch_target_t {
typedef struct patch_target_info_t {
const char *local_abspath;
svn_boolean_t deleted;
+ svn_boolean_t skipped;
} patch_target_info_t;
@@ -2730,6 +2731,11 @@ delete_empty_dirs(apr_array_header_t *targets_info
SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
target_info = APR_ARRAY_IDX(targets_info, i, patch_target_info_t *);
+
+ /* Skipped dirs will not have a local_abspath set. */
+ if (target_info->skipped)
+ continue;
+
parent = svn_dirent_dirname(target_info->local_abspath, iterpool);
if (apr_hash_get(non_empty_dirs, parent, APR_HASH_KEY_STRING))
@@ -2915,6 +2921,7 @@ apply_patches(void *baton,
target_info->local_abspath = apr_pstrdup(scratch_pool,
target->local_abspath);
target_info->deleted = target->deleted;
+ target_info->skipped = target->skipped;
APR_ARRAY_PUSH(targets_info,
patch_target_info_t *) = target_info;
