[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1197998 - in /subversion/trunk/subversion: libsvn_client/patch.c tests/cmdline/patch_tests.py

From: Konstantin Kolinko <knst.kolinko_at_gmail.com>
Date: Sun, 6 Nov 2011 03:28:06 +0400

2011/11/6 Daniel Shahaf <d.s_at_daniel.shahaf.name>:
>
>
> On Saturday, November 05, 2011 5:54 PM, stsp_at_apache.org wrote:
>> Author: stsp
>> Date: Sat Nov  5 17:54:31 2011
>> New Revision: 1197998
>>
>> URL: http://svn.apache.org/viewvc?rev=1197998&view=rev
>> Log:
>> Make 'svn patch' ignore "/dev/null" patch target paths to improve
>> compatibility with patches generated by git. Git uses "/dev/null" as the
>> old name for newly added targets, and as the new name for deleted targets.
>>
>> Reported by: Konstantin Kolinko
>> http://svn.haxx.se/users/archive-2011-11/0126.shtml
>>
>> * subversion/libsvn_client/patch.c
>>   (choose_target_filename): If one of the filenames is "/dev/null",
>>    use the other filename.
>>
>> * subversion/tests/cmdline/patch_tests.py
>>   (patch_dev_null, test_list): New test.
>
> Does anything (git, svn, etc) generate "NUL" instead of "/dev/null" when on windows?

First, regarding "NUL": Git and Subversion both use /dev/null -- see below.

Git:
===
The patch that I used to report the issue (see mail thread link above)
was generated by Git on Windows. I am not its author, but I know from
previous discussions and from CRLF line ends in the new file added in
that patch.

It uses /dev/null

Svn:
===
Using svn 1.7.1, on Windows. From the root of Greek tree:
[[[
echo "foo">new
svn add new
svn diff --git
]]]
generates:

[[[
Index: new
===================================================================
diff --git a/trunk/new b/trunk/new
new file mode 10644
--- /dev/null (revision 0)
+++ b/trunk/new (working copy)
@@ -0,0 +1 @@
+foo
]]]

Second, regarding the test case in this commit:

The sample patch in the test case is not fair that it does not match
to what "svn diff --git" or Git generates.

I do not mind keeping this as is, but it needs an additional test or
two -- see below.

Actual patch generated with svn diff --git on Greek tree:
[[[
Index: A/B/E/alpha
===================================================================
diff --git a/trunk/A/B/E/alpha b/trunk/A/B/E/alpha
--- a/trunk/A/B/E/alpha (revision 1)
+++ b/trunk/A/B/E/alpha (working copy)
@@ -1 +1 @@
-This is the file 'A/B/E/alpha'.
+Modified file 'A/B/E/alpha'.
Index: A/B/E/beta
===================================================================
diff --git a/trunk/A/B/E/beta b/trunk/A/B/E/beta
deleted file mode 10644
--- a/trunk/A/B/E/beta (revision 1)
+++ /dev/null (working copy)
@@ -1 +0,0 @@
-This is the file 'A/B/E/beta'.
Index: new
===================================================================
diff --git a/trunk/new b/trunk/new
new file mode 10644
--- /dev/null (revision 0)
+++ b/trunk/new (working copy)
@@ -0,0 +1 @@
+new
]]]

In testcase format that would be:

[[[
  unidiff_patch = [
    "Index: A/B/E/alpha\n",
    "===================================================================\n",
    "diff --git a/trunk/A/B/E/alpha b/trunk/A/B/E/alpha\n",
    "--- a/trunk/A/B/E/alpha\t(revision 1)\n",
    "+++ b/trunk/A/B/E/alpha\t(working copy)\n",
    "@@ -1 +1 @@\n",
    "-This is the file 'A/B/E/alpha'.\n",
    "+Modified file 'A/B/E/alpha'.\n",
    "Index: A/B/E/beta\n",
    "===================================================================\n",
    "diff --git a/trunk/A/B/E/beta b/trunk/A/B/E/beta\n",
    "deleted file mode 10644\n",
    "--- a/trunk/A/B/E/beta\t(revision 1)\n",
    "+++ /dev/null\t(working copy)\n",
    "@@ -1 +0,0 @@\n",
    "-This is the file 'A/B/E/beta'.\n",
    "Index: new\n",
    "===================================================================\n",
    "diff --git a/trunk/new b/trunk/new\n",
    "new file mode 10644\n",
    "--- /dev/null\t(revision 0)\n",
    "+++ b/trunk/new\t(working copy)\n",
    "@@ -0,0 +1 @@\n",
    "+new\n",
  ]

  svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))

  alpha_contents = "Modified file 'A/B/E/alpha'.\n"
  new_contents = "new\n"
  expected_output = [
    'M %s\n' % os.path.join(wc_dir, 'A', 'B', 'E', 'alpha'),
    'D %s\n' % os.path.join(wc_dir, 'A', 'B', 'E', 'beta'),
    'A %s\n' % os.path.join(wc_dir, 'new'),
  ]

  expected_disk = svntest.main.greek_state.copy()
  expected_disk.tweak('A/B/E/alpha', contents=alpha_contents)
  expected_disk.remove('A/B/E/beta')
  expected_disk.add({'new' : Item(contents=new_contents)})

  expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
  expected_status.tweak('A/B/E/alpha', status='M ')
  expected_status.tweak('A/B/E/beta', status='D ')
  expected_status.add({'new' : Item(status='A ', wc_rev=0)})

  expected_skip = wc.State('', { })

# The above to be applied with "--strip 2".
]]]

Another test I would like to see is to use the patch format as generated by Git.
Compare the above with
http://people.apache.org/~markt/patches/2011-11-03-redeploy-trunk-v2.patch
or patches at Github

E.g. for this commit
https://github.com/apache/subversion/commit/49f2818c5d6f856243f029f1dca574ef8ba8a073

its Git patch is shown by adding ".diff" to the above URL:
https://github.com/apache/subversion/commit/49f2818c5d6f856243f029f1dca574ef8ba8a073.diff

I think that Git-inspired variation of the above test will look like
[[[
  unidiff_patch = [
    "diff --git a/A/B/E/alpha b/A/B/E/alpha\n",
    "index 745bf95..d9d94af 100644\n",
    "--- a/A/B/E/alpha\n",
    "+++ b/A/B/E/alpha\n",
    "@@ -1,1 +1,1 @@\n",
    "-This is the file 'A/B/E/alpha'.\n",
    "+Modified file 'A/B/E/alpha'.\n",
    "diff --git a/A/B/E/beta b/A/B/E/beta\n",
    "deleted file mode 100644\n",
    "index c7fc018..0000000\n",
    "--- a/A/B/E/beta\n",
    "+++ /dev/null\n",
    "@@ -1,1 +0,0 @@\n",
    "-This is the file 'A/B/E/beta'.\n",
    "diff --git a/new b/new\n",
    "new file mode 100644\n",
    "index 0000000..63f6544\n",
    "--- /dev/null\n",
    "+++ b/new\n",
    "@@ -0,0 +1,1 @@\n",
    "+new\n",
  ]

  svntest.main.file_write(patch_file_path, ''.join(unidiff_patch))

  alpha_contents = "Modified file 'A/B/E/alpha'.\n"
  new_contents = "new\n"
  expected_output = [
    'M %s\n' % os.path.join(wc_dir, 'A', 'B', 'E', 'alpha'),
    'D %s\n' % os.path.join(wc_dir, 'A', 'B', 'E', 'beta'),
    'A %s\n' % os.path.join(wc_dir, 'new'),
  ]

  expected_disk = svntest.main.greek_state.copy()
  expected_disk.tweak('A/B/E/alpha', contents=alpha_contents)
  expected_disk.remove('A/B/E/beta')
  expected_disk.add({'new' : Item(contents=new_contents)})

  expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
  expected_status.tweak('A/B/E/alpha', status='M ')
  expected_status.tweak('A/B/E/beta', status='D ')
  expected_status.add({'new' : Item(status='A ', wc_rev=0)})

  expected_skip = wc.State('', { })

# The above to be applied with "--strip 1"
]]]

Note that
- headers are a bit different
- additional "index..." line
- file modes are "100644" instead of "10644"; no revision numbers
- paths start with "a/" and not "a/trunk"

Thus it is to be applied with "--strip 1".

I cannot run tests, so there might be some glitches above.

Best regards,
Konstantin Kolinko
Received on 2011-11-06 00:28:37 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.