You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2015/10/05 19:16:59 UTC
svn commit: r1706875 - in /subversion/trunk/subversion:
libsvn_client/patch.c tests/cmdline/patch_tests.py
Author: rhuijben
Date: Mon Oct 5 17:16:59 2015
New Revision: 1706875
URL: http://svn.apache.org/viewvc?rev=1706875&view=rev
Log:
Fix the interaction between moves and property changes during 'svn patch'.
Before this patch property changes would be applied to the source of the move,
which would cause an error because it is not possible to set properties on
a deleted file.
To make it easier to test this also fix move issues in the dry run mode.
* subversion/libsvn_client/patch.c
(resolve_target_path): Allow to disable the following of local moves to
allow tree changes on the pre-move target.
(init_patch_target): Allow adding new nodes in place of something that is
moved away. Detect interactions with previously recorded patches.
(send_patch_notification): Don't notify a delete when the add was skipped.
(install_patched_prop_targets): Apply property changes to the right target.
* subversion/tests/cmdline/patch_tests.py
(patch_git_rename): Extend test.
(patch_move_and_change): New test.
(test_list): Add patch_move_and_change.
Modified:
subversion/trunk/subversion/libsvn_client/patch.c
subversion/trunk/subversion/tests/cmdline/patch_tests.py
Modified: subversion/trunk/subversion/libsvn_client/patch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=1706875&r1=1706874&r2=1706875&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Mon Oct 5 17:16:59 2015
@@ -441,6 +441,7 @@ resolve_target_path(patch_target_t *targ
const char *root_abspath,
int strip_count,
svn_boolean_t has_text_changes,
+ svn_boolean_t follow_moves,
svn_wc_context_t *wc_ctx,
const apr_array_header_t *targets_info,
apr_pool_t *result_pool,
@@ -552,12 +553,17 @@ resolve_target_path(patch_target_t *targ
if (target->locally_deleted)
{
- const char *moved_to_abspath;
- const char *op_root_abspath;
+ const char *moved_to_abspath = NULL;
+
+ if (follow_moves
+ && !target_is_added(targets_info, target->local_abspath,
+ scratch_pool))
+ {
+ SVN_ERR(svn_wc__node_was_moved_away(&moved_to_abspath, NULL,
+ wc_ctx, target->local_abspath,
+ result_pool, scratch_pool));
+ }
- SVN_ERR(svn_wc__node_was_moved_away(&moved_to_abspath, &op_root_abspath,
- wc_ctx, target->local_abspath,
- result_pool, scratch_pool));
if (moved_to_abspath)
{
target->local_abspath = moved_to_abspath;
@@ -1009,6 +1015,7 @@ init_patch_target(patch_target_t **patch
patch_target_t *target;
target_content_t *content;
svn_boolean_t has_text_changes = FALSE;
+ svn_boolean_t follow_moves;
has_text_changes = ((patch->hunks && patch->hunks->nelts > 0)
|| patch->binary_patch);
@@ -1031,9 +1038,26 @@ init_patch_target(patch_target_t **patch
target->prop_targets = apr_hash_make(result_pool);
target->operation = patch->operation;
+ if (patch->operation == svn_diff_op_added /* Allow replacing */
+ || patch->operation == svn_diff_op_added)
+ {
+ follow_moves = FALSE;
+ }
+ else if (patch->operation == svn_diff_op_unchanged
+ && patch->hunks && patch->hunks->nelts == 1)
+ {
+ svn_diff_hunk_t *hunk = APR_ARRAY_IDX(patch->hunks, 0,
+ svn_diff_hunk_t *);
+
+ follow_moves = (svn_diff_hunk_get_original_start(hunk) != 0);
+ }
+ else
+ follow_moves = TRUE;
+
SVN_ERR(resolve_target_path(target, choose_target_filename(patch),
root_abspath, strip_count, has_text_changes,
- wc_ctx, targets_info, result_pool, scratch_pool));
+ follow_moves, wc_ctx, targets_info,
+ result_pool, scratch_pool));
*patch_target = target;
if (! target->skipped)
{
@@ -1119,7 +1143,6 @@ init_patch_target(patch_target_t **patch
/* The move target path is either outside of the working
* copy or it is the working copy itself. Skip it. */
target->skipped = TRUE;
- target->local_abspath = NULL;
return SVN_NO_ERROR;
}
}
@@ -1135,7 +1158,7 @@ init_patch_target(patch_target_t **patch
{
/* The target path is outside of the working copy. Skip it. */
target->skipped = TRUE;
- target->local_abspath = NULL;
+ target->move_target_abspath = NULL;
return SVN_NO_ERROR;
}
@@ -1144,7 +1167,9 @@ init_patch_target(patch_target_t **patch
SVN_ERR(svn_wc_read_kind2(&wc_kind, wc_ctx,
target->move_target_abspath,
FALSE, FALSE, scratch_pool));
- if (kind_on_disk != svn_node_none || wc_kind != svn_node_none)
+ if (kind_on_disk != svn_node_none || wc_kind != svn_node_none
+ || target_is_added(targets_info, target->move_target_abspath,
+ scratch_pool))
{
/* The move target path already exists on disk. Skip target. */
target->skipped = TRUE;
@@ -2361,7 +2386,7 @@ send_patch_notification(const patch_targ
svn_pool_destroy(iterpool);
}
- if (target->move_target_abspath)
+ if (!target->skipped && target->move_target_abspath)
{
/* Notify about deletion of move source. */
notify = svn_wc_create_notify(target->local_abspath,
@@ -3258,6 +3283,13 @@ install_patched_prop_targets(patch_targe
{
apr_hash_index_t *hi;
apr_pool_t *iterpool;
+ const char *local_abspath;
+
+ /* Apply properties to a move target if there is one */
+ if (target->move_target_abspath)
+ local_abspath = target->move_target_abspath;
+ else
+ local_abspath = target->local_abspath;
iterpool = svn_pool_create(scratch_pool);
@@ -3281,7 +3313,7 @@ install_patched_prop_targets(patch_targe
if (prop_target->operation == svn_diff_op_deleted)
{
if (! dry_run)
- SVN_ERR(svn_wc_prop_set4(ctx->wc_ctx, target->local_abspath,
+ SVN_ERR(svn_wc_prop_set4(ctx->wc_ctx, local_abspath,
prop_target->name, NULL, svn_depth_empty,
TRUE /* skip_checks */,
NULL /* changelist_filter */,
@@ -3306,7 +3338,7 @@ install_patched_prop_targets(patch_targe
err = svn_wc_canonicalize_svn_prop(&canon_propval,
prop_target->name,
- prop_val, target->local_abspath,
+ prop_val, local_abspath,
target->db_kind,
TRUE, /* ### Skipping checks */
NULL, NULL,
@@ -3314,7 +3346,7 @@ install_patched_prop_targets(patch_targe
}
else
{
- err = svn_wc_prop_set4(ctx->wc_ctx, target->local_abspath,
+ err = svn_wc_prop_set4(ctx->wc_ctx, local_abspath,
prop_target->name, prop_val, svn_depth_empty,
TRUE /* skip_checks */,
NULL /* changelist_filter */,
Modified: subversion/trunk/subversion/tests/cmdline/patch_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/patch_tests.py?rev=1706875&r1=1706874&r2=1706875&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/patch_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/patch_tests.py Mon Oct 5 17:16:59 2015
@@ -4798,8 +4798,10 @@ def patch_git_rename(sbox):
patch_file_path = sbox.get_tempname('my.patch')
svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
- expected_output = [ 'A %s\n' % sbox.ospath('iota2'),
- 'D %s\n' % sbox.ospath('iota')]
+ expected_output = wc.State(wc_dir, {
+ 'iota' : Item(status='D '),
+ 'iota2' : Item(status='A ')
+ })
expected_disk = svntest.main.greek_state.copy()
expected_disk.remove('iota')
expected_disk.add({'iota2' : Item(contents="This is the file 'iota'.\n")})
@@ -4811,7 +4813,36 @@ def patch_git_rename(sbox):
expected_skip = wc.State('', { })
svntest.actions.run_and_verify_patch(wc_dir, patch_file_path,
expected_output, expected_disk,
- expected_status, expected_skip)
+ expected_status, expected_skip,
+ [], True, True)
+
+ # Retry
+ #svntest.actions.run_and_verify_patch(wc_dir, patch_file_path,
+ # expected_output, expected_disk,
+ # expected_status, expected_skip,
+ # [], True, True)
+
+ # Reverse
+ expected_output.tweak('iota', status='A ')
+ expected_output.tweak('iota2', status='D ')
+ expected_disk.remove('iota2')
+ expected_disk.add({
+ 'iota' : Item(contents="This is the file 'iota'.\n"),
+ })
+ expected_status.remove('iota2')
+ expected_status.tweak('iota', moved_to=None, status=' ')
+ svntest.actions.run_and_verify_patch(wc_dir, patch_file_path,
+ expected_output, expected_disk,
+ expected_status, expected_skip,
+ [], True, True,
+ '--reverse-diff')
+
+ # Retry reverse
+ # svntest.actions.run_and_verify_patch(wc_dir, patch_file_path,
+ # expected_output, expected_disk,
+ # expected_status, expected_skip,
+ # [], True, True,
+ # '--reverse-diff')
@Issue(4533)
def patch_hunk_avoid_reorder(sbox):
@@ -7404,6 +7435,84 @@ def patch_with_mergeinfo(sbox):
[], True, True,
'--strip', strip_count)
+def patch_move_and_change(sbox):
+ "patch move and change"
+
+ sbox.build(read_only = True)
+ wc_dir = sbox.wc_dir
+
+ sbox.simple_append('A/B/E/alpha', 'extra\nlines\n')
+ sbox.simple_propset('k', 'v', 'A/B/E/alpha')
+
+ sbox.simple_move('A/B/E/alpha', 'alpha')
+
+ _, diff, _ = svntest.actions.run_and_verify_svn(None, [],
+ 'diff', wc_dir, '--git')
+
+ patch = sbox.get_tempname('move_and_change.patch')
+ svntest.main.file_write(patch, ''.join(diff), mode='wb')
+
+ # Running the diff reversed doesn't work...
+ # We perform the add before reverting the move...
+ expected_output = wc.State(wc_dir, {
+ 'A/B/E/alpha' : Item(status='A '),
+ })
+ expected_skip = wc.State(wc_dir, {
+ 'alpha' : Item(verb='Skipped'),
+ })
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.tweak('A/B/E/alpha', status='R ',
+ moved_to='alpha')
+ expected_status.add({
+ 'alpha' : Item(status='A ', copied='+',
+ moved_from='A/B/E/alpha', wc_rev='-'),
+ })
+ expected_disk = svntest.main.greek_state.copy()
+ expected_disk.add({
+ 'alpha' : Item(contents="This is the file 'alpha'.\nextra\nlines\n",
+ props={'k': 'v'}),
+ })
+ svntest.actions.run_and_verify_patch(wc_dir, patch,
+ expected_output, expected_disk,
+ expected_status, expected_skip,
+ [], True, True,
+ '--reverse-diff')
+
+ # Ok, let's remove the 'delete' portion and try in a clean WC
+ n = diff.index('Index: %s\n' % sbox.path('alpha'))
+ diff = diff[n:]
+ svntest.main.file_write(patch, ''.join(diff), mode='wb')
+
+ sbox.simple_revert('A/B/E/alpha', 'alpha')
+
+ expected_output = wc.State(wc_dir, {
+ 'A/B/E/alpha' : Item(status='D '),
+ 'alpha' : Item(status='A '),
+ })
+ expected_skip = wc.State(wc_dir, {})
+ expected_disk.remove('A/B/E/alpha')
+ expected_status.tweak('A/B/E/alpha', status='D ')
+ svntest.actions.run_and_verify_patch(wc_dir, patch,
+ expected_output, expected_disk,
+ expected_status, expected_skip,
+ [], True, True)
+
+ # Retry won't work yet, but let's try a reverse merge
+ expected_output = wc.State(wc_dir, {
+ 'alpha' : Item(status='D '),
+ 'A/B/E/alpha' : Item(status='A '),
+ })
+ expected_disk.remove('alpha')
+ expected_disk.add({
+ 'A/B/E/alpha' : Item(contents="This is the file 'alpha'.\n"),
+ })
+ expected_status.remove('alpha')
+ expected_status.tweak('A/B/E/alpha', status=' ', moved_to=None)
+ svntest.actions.run_and_verify_patch(wc_dir, patch,
+ expected_output, expected_disk,
+ expected_status, expected_skip,
+ [], True, True,
+ '--reverse-diff')
########################################################################
#Run the tests
@@ -7484,6 +7593,7 @@ test_list = [ None,
patch_symlink_changes,
patch_add_one_line,
patch_with_mergeinfo,
+ patch_move_and_change,
]
if __name__ == '__main__':