You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2010/09/16 15:06:19 UTC
svn commit: r997735 - in /subversion/trunk/subversion: libsvn_client/patch.c
tests/cmdline/patch_tests.py
Author: stsp
Date: Thu Sep 16 13:06:19 2010
New Revision: 997735
URL: http://svn.apache.org/viewvc?rev=997735&view=rev
Log:
Make svn patch match a hunk the normal way around before trying to determine
whether it has already been applied by matching it in reverse.
This fixes a bug where when applying a patch and then applying it again in
reverse, a hunk would be misdetected as "already applied", with the following
type of patch:
Index: foo
===================================================================
--- foo (revision 2)
+++ foo (working copy)
@@ -1 +1,2 @@
alpha
+aaa
This change has a side effect: Instead of causing a "hunk already applied"
message upon duplicate application, repeatedly applying the patch above will
now keep adding "aaa" lines to the file foo, as long as the first line of the
file reads "alpha". Conversely, applying the patch in reverse will keep
removing "aaa" lines following the "alpha" line lines until none are left.
However, this matches the behaviour of Larry Wall's patch implementation,
so we accept it for now. GNU patch detects the duplicate application,
just as Subversion did before this commit. But both Larry's and GNU's patch
implementations correctly apply the above patch in reverse, so the fix for
this has higher priority.
* subversion/tests/cmdline/patch_tests.py
(patch_same_twice): Adjust test expectations to conform to the behaviour
of Larry Wall's patch implementation.
(patch_reverse_revert): New test, which applies a patch and then applies
it again in reverse. This test covers the bug this commit is fixing.
(test_list): Add new test.
* subversion/libsvn_client/patch.c
(get_hunk_info): Try to match the hunk the normal way around before
checking whether it's already been applied.
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=997735&r1=997734&r2=997735&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Thu Sep 16 13:06:19 2010
@@ -1160,54 +1160,55 @@ get_hunk_info(hunk_info_t **hi, patch_ta
else if (original_start > 0 && content_info->stream)
{
svn_linenum_t saved_line = content_info->current_line;
- svn_linenum_t modified_start;
- /* Check if the hunk is already applied.
- * We only check for an exact match here, and don't bother checking
- * for already applied patches with offset/fuzz, because such a
- * check would be ambiguous. */
- if (fuzz == 0)
+ /* Scan for a match at the line where the hunk thinks it
+ * should be going. */
+ SVN_ERR(seek_to_line(content_info, original_start, scratch_pool));
+ if (content_info->current_line != original_start)
{
- modified_start = svn_diff_hunk_get_modified_start(hunk);
- if (modified_start == 0)
- {
- /* Patch wants to delete the file. */
- already_applied = target->locally_deleted;
- }
- else
- {
- SVN_ERR(seek_to_line(content_info, modified_start,
- scratch_pool));
- SVN_ERR(scan_for_match(&matched_line, content_info,
- hunk, TRUE,
- modified_start + 1,
- fuzz, ignore_whitespace, TRUE,
- cancel_func, cancel_baton,
- scratch_pool));
- already_applied = (matched_line == modified_start);
- }
+ /* Seek failed. */
+ matched_line = 0;
}
else
- already_applied = FALSE;
+ SVN_ERR(scan_for_match(&matched_line, content_info, hunk, TRUE,
+ original_start + 1, fuzz,
+ ignore_whitespace, FALSE,
+ cancel_func, cancel_baton,
+ scratch_pool));
- if (! already_applied)
+ if (matched_line != original_start)
{
- /* Scan for a match at the line where the hunk thinks it
- * should be going. */
- SVN_ERR(seek_to_line(content_info, original_start, scratch_pool));
- if (content_info->current_line != original_start)
+ /* Check if the hunk is already applied.
+ * We only check for an exact match here, and don't bother checking
+ * for already applied patches with offset/fuzz, because such a
+ * check would be ambiguous. */
+ if (fuzz == 0)
{
- /* Seek failed. */
- matched_line = 0;
+ svn_linenum_t modified_start;
+
+ modified_start = svn_diff_hunk_get_modified_start(hunk);
+ if (modified_start == 0)
+ {
+ /* Patch wants to delete the file. */
+ already_applied = target->locally_deleted;
+ }
+ else
+ {
+ SVN_ERR(seek_to_line(content_info, modified_start,
+ scratch_pool));
+ SVN_ERR(scan_for_match(&matched_line, content_info,
+ hunk, TRUE,
+ modified_start + 1,
+ fuzz, ignore_whitespace, TRUE,
+ cancel_func, cancel_baton,
+ scratch_pool));
+ already_applied = (matched_line == modified_start);
+ }
}
else
- SVN_ERR(scan_for_match(&matched_line, content_info, hunk, TRUE,
- original_start + 1, fuzz,
- ignore_whitespace, FALSE,
- cancel_func, cancel_baton,
- scratch_pool));
+ already_applied = FALSE;
- if (matched_line != original_start)
+ if (! already_applied)
{
/* Scan the whole file again from the start. */
SVN_ERR(seek_to_line(content_info, 1, scratch_pool));
Modified: subversion/trunk/subversion/tests/cmdline/patch_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/patch_tests.py?rev=997735&r1=997734&r2=997735&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/patch_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/patch_tests.py Thu Sep 16 13:06:19 2010
@@ -2436,7 +2436,14 @@ def patch_same_twice(sbox):
'G %s\n' % os.path.join(wc_dir, 'A', 'D', 'gamma'),
'> hunk @@ -1,1 +1,1 @@ already applied\n',
'G %s\n' % os.path.join(wc_dir, 'iota'),
- '> hunk @@ -1,1 +1,2 @@ already applied\n',
+ # The iota patch inserts a line after the first line in the file,
+ # with no trailing context. Currently, Subversion applies this patch
+ # multiple times, which matches the behaviour of Larry Wall's patch
+ # implementation. A more complicated hunk matching algorithm is needed
+ # to detect the duplicate application in this case. GNU patch does detect
+ # the duplicate application. Should Subversion be taught to detect it,
+ # we need this line here:
+ # '> hunk @@ -1,1 +1,2 @@ already applied\n',
'G %s\n' % os.path.join(wc_dir, 'new'),
'> hunk @@ -0,0 +1,1 @@ already applied\n',
'G %s\n' % os.path.join(wc_dir, 'A', 'mu'),
@@ -2449,6 +2456,10 @@ def patch_same_twice(sbox):
expected_skip = wc.State('', {beta_path : Item()})
+ # See above comment about the iota patch being applied twice.
+ iota_contents += "Some more bytes\n"
+ expected_disk.tweak('iota', contents=iota_contents)
+
svntest.actions.run_and_verify_patch(wc_dir, os.path.abspath(patch_file_path),
expected_output,
expected_disk,
@@ -3141,6 +3152,205 @@ def patch_old_target_names(sbox):
1, # dry-run
"--old-patch-target-names")
+def patch_reverse_revert(sbox):
+ "revert a patch by reverse patching"
+
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ patch_file_path = make_patch_path(sbox)
+ mu_path = os.path.join(wc_dir, 'A', 'mu')
+
+ mu_contents_pre_patch = [
+ "Dear internet user,\n",
+ "\n",
+ "We wish to congratulate you over your email success in our computer\n",
+ "Balloting. This is a Millennium Scientific Electronic Computer Draw\n",
+ "in which email addresses were used. All participants were selected\n",
+ "through a computer ballot system drawn from over 100,000 company\n",
+ "and 50,000,000 individual email addresses from all over the world.\n",
+ "\n",
+ "Your email address drew and have won the sum of 750,000 Euros\n",
+ "( Seven Hundred and Fifty Thousand Euros) in cash credited to\n",
+ "file with\n",
+ " REFERENCE NUMBER: ESP/WIN/008/05/10/MA;\n",
+ " WINNING NUMBER : 14-17-24-34-37-45-16\n",
+ " BATCH NUMBERS :\n",
+ " EULO/1007/444/606/08;\n",
+ " SERIAL NUMBER: 45327\n",
+ "and PROMOTION DATE: 13th June. 2009\n",
+ "\n",
+ "To claim your winning prize, you are to contact the appointed\n",
+ "agent below as soon as possible for the immediate release of your\n",
+ "winnings with the below details.\n",
+ "\n",
+ "Again, we wish to congratulate you over your email success in our\n"
+ "computer Balloting.\n"
+ ]
+
+ # Set mu contents
+ svntest.main.file_write(mu_path, ''.join(mu_contents_pre_patch))
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/mu' : Item(verb='Sending'),
+ })
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.tweak('A/mu', wc_rev=2)
+ svntest.actions.run_and_verify_commit(wc_dir, expected_output,
+ expected_status, None, wc_dir)
+
+ # Apply patch
+
+ unidiff_patch = [
+ "Index: A/D/gamma\n",
+ "===================================================================\n",
+ "--- A/D/gamma\t(revision 1)\n",
+ "+++ A/D/gamma\t(working copy)\n",
+ "@@ -1 +1 @@\n",
+ "-This is the file 'gamma'.\n",
+ "+It is the file 'gamma'.\n",
+ "Index: iota\n",
+ "===================================================================\n",
+ "--- iota\t(revision 1)\n",
+ "+++ iota\t(working copy)\n",
+ "@@ -1 +1,2 @@\n",
+ " This is the file 'iota'.\n",
+ "+Some more bytes\n",
+ "\n",
+ "Index: new\n",
+ "===================================================================\n",
+ "--- new (revision 0)\n",
+ "+++ new (revision 0)\n",
+ "@@ -0,0 +1 @@\n",
+ "+new\n",
+ "\n",
+ "--- A/mu.orig 2009-06-24 15:23:55.000000000 +0100\n",
+ "+++ A/mu 2009-06-24 15:21:23.000000000 +0100\n",
+ "@@ -6,6 +6,9 @@\n",
+ " through a computer ballot system drawn from over 100,000 company\n",
+ " and 50,000,000 individual email addresses from all over the world.\n",
+ " \n",
+ "+It is a promotional program aimed at encouraging internet users;\n",
+ "+therefore you do not need to buy ticket to enter for it.\n",
+ "+\n",
+ " Your email address drew and have won the sum of 750,000 Euros\n",
+ " ( Seven Hundred and Fifty Thousand Euros) in cash credited to\n",
+ " file with\n",
+ "@@ -14,11 +17,8 @@\n",
+ " BATCH NUMBERS :\n",
+ " EULO/1007/444/606/08;\n",
+ " SERIAL NUMBER: 45327\n",
+ "-and PROMOTION DATE: 13th June. 2009\n",
+ "+and PROMOTION DATE: 14th June. 2009\n",
+ " \n",
+ " To claim your winning prize, you are to contact the appointed\n",
+ " agent below as soon as possible for the immediate release of your\n",
+ " winnings with the below details.\n",
+ "-\n",
+ "-Again, we wish to congratulate you over your email success in our\n",
+ "-computer Balloting.\n",
+ "Index: A/B/E/beta\n",
+ "===================================================================\n",
+ "--- A/B/E/beta (revision 1)\n",
+ "+++ A/B/E/beta (working copy)\n",
+ "@@ -1 +0,0 @@\n",
+ "-This is the file 'beta'.\n",
+ ]
+
+ svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))
+
+ gamma_contents = "It is the file 'gamma'.\n"
+ iota_contents = "This is the file 'iota'.\nSome more bytes\n"
+ new_contents = "new\n"
+ mu_contents_post_patch = [
+ "Dear internet user,\n",
+ "\n",
+ "We wish to congratulate you over your email success in our computer\n",
+ "Balloting. This is a Millennium Scientific Electronic Computer Draw\n",
+ "in which email addresses were used. All participants were selected\n",
+ "through a computer ballot system drawn from over 100,000 company\n",
+ "and 50,000,000 individual email addresses from all over the world.\n",
+ "\n",
+ "It is a promotional program aimed at encouraging internet users;\n",
+ "therefore you do not need to buy ticket to enter for it.\n",
+ "\n",
+ "Your email address drew and have won the sum of 750,000 Euros\n",
+ "( Seven Hundred and Fifty Thousand Euros) in cash credited to\n",
+ "file with\n",
+ " REFERENCE NUMBER: ESP/WIN/008/05/10/MA;\n",
+ " WINNING NUMBER : 14-17-24-34-37-45-16\n",
+ " BATCH NUMBERS :\n",
+ " EULO/1007/444/606/08;\n",
+ " SERIAL NUMBER: 45327\n",
+ "and PROMOTION DATE: 14th June. 2009\n",
+ "\n",
+ "To claim your winning prize, you are to contact the appointed\n",
+ "agent below as soon as possible for the immediate release of your\n",
+ "winnings with the below details.\n",
+ ]
+
+ expected_output = [
+ 'U %s\n' % os.path.join(wc_dir, 'A', 'D', 'gamma'),
+ 'U %s\n' % os.path.join(wc_dir, 'iota'),
+ 'A %s\n' % os.path.join(wc_dir, 'new'),
+ 'U %s\n' % os.path.join(wc_dir, 'A', 'mu'),
+ 'D %s\n' % os.path.join(wc_dir, 'A', 'B', 'E', 'beta'),
+ ]
+
+ expected_disk = svntest.main.greek_state.copy()
+ expected_disk.tweak('A/D/gamma', contents=gamma_contents)
+ expected_disk.tweak('iota', contents=iota_contents)
+ expected_disk.add({'new' : Item(contents=new_contents)})
+ expected_disk.tweak('A/mu', contents=''.join(mu_contents_post_patch))
+ expected_disk.remove('A/B/E/beta')
+
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.tweak('A/D/gamma', status='M ')
+ expected_status.tweak('iota', status='M ')
+ expected_status.add({'new' : Item(status='A ', wc_rev=0)})
+ expected_status.tweak('A/mu', status='M ', wc_rev=2)
+ expected_status.tweak('A/B/E/beta', status='D ')
+
+ 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
+
+ # Applying the same patch in reverse should undo local mods
+ expected_output = [
+ 'G %s\n' % os.path.join(wc_dir, 'A', 'D', 'gamma'),
+ 'G %s\n' % os.path.join(wc_dir, 'iota'),
+ 'D %s\n' % os.path.join(wc_dir, 'new'),
+ 'G %s\n' % os.path.join(wc_dir, 'A', 'mu'),
+ 'A %s\n' % os.path.join(wc_dir, 'A', 'B', 'E', 'beta'),
+ ]
+ expected_disk = svntest.main.greek_state.copy()
+ expected_disk.tweak('A/mu', contents=''.join(mu_contents_pre_patch))
+
+ expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
+ expected_status.tweak('A/mu', wc_rev=2)
+
+ ### svn patch should check whether the deleted file has the same
+ ### content as the file added by the patch and revert the deletion
+ ### instead of causing a replacement.
+ expected_status.tweak('A/B/E/beta', status='R ')
+
+ 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
+ '--reverse-diff',
+ '--old-patch-target-names')
+
########################################################################
#Run the tests
@@ -3172,6 +3382,7 @@ test_list = [ None,
patch_prop_with_fuzz,
patch_git_add_file,
patch_old_target_names,
+ patch_reverse_revert,
]
if __name__ == '__main__':