I have this patch on my TODO list, but I have some minor concerns that I want to look at:
* I'm not sure if we really need all these intermediate states in diff. Perhaps we can return to one of the more generic 'seen' states. Are all these additional options really guaranteed strictly ordered or can they occur in different orders. If that is the case we must return to a generic state.
* There are only a very few testcases for binary patches. Your patch changes one of these, while I've already determined that mode changes have unwanted side effects: a mode change to a not existing path will currently create a 0 byte file.
Adding a new test -perhaps by copying this old test- is probably safer than changing the test to only handle the new route through the state table instead of the old route.
-> This routes back to the first point... multiple intermediate states, requires adding even more tests.
Bert
From: Daniel Shahaf
Sent: woensdag 30 september 2015 00:30
To: dev_at_subversion.apache.org
Subject: Re: [PATCH] mode-changing patches ∩binary patches
Could someone commit this for me, please? It passes tests (as of r1705925).
(I've followed up with infra@ about my account problems, but I don't want this
patch to bitrot while my karma is getting sorted out.)
Thanks!
Daniel
On Mon, Sep 28, 2015 at 06:25:43PM +0000, Daniel Shahaf wrote:
> svn.apache.org doesn't let me commit this, even though it accepts my
> password...
>
> [[[
> patch: Make binary patches and git mode changes coexist.
>
> * subversion/libsvn_diff/parse-diff.c
> (transitions): Add missing transition.
>
> * subversion/tests/cmdline/patch_tests.py
> (patch_binary_file): Tweak the test to trigger the new codepath.
> ]]]
>
> [[[
> Index: subversion/libsvn_diff/parse-diff.c
> ===================================================================
> --- subversion/libsvn_diff/parse-diff.c (revision 1705734)
> +++ subversion/libsvn_diff/parse-diff.c (working copy)
> @@ -2038,6 +2038,7 @@ static struct transition transitions[] =
>
> {"GIT binary patch", state_git_diff_seen, binary_patch_start},
> {"GIT binary patch", state_git_tree_seen, binary_patch_start},
> + {"GIT binary patch", state_git_mode_seen, binary_patch_start},
> };
>
> svn_error_t *
> Index: subversion/tests/cmdline/patch_tests.py
> ===================================================================
> --- subversion/tests/cmdline/patch_tests.py (revision 1705734)
> +++ subversion/tests/cmdline/patch_tests.py (working copy)
> @@ -5659,7 +5659,13 @@ def patch_binary_file(sbox):
> sbox.simple_revert('iota')
>
> tmp = sbox.get_tempname()
> - svntest.main.file_write(tmp, ''.join(diff_output))
> + patch = diff_output[:]
> + patch[3:3] = [
> + "old mode 100644\n",
> + "new mode 100755\n",
> + #"index ...\n",
> + ]
> + svntest.main.file_write(tmp, ''.join(patch))
>
> expected_output = wc.State(wc_dir, {
> 'iota' : Item(status='UU'),
> @@ -5666,7 +5672,8 @@ def patch_binary_file(sbox):
> })
> expected_disk = svntest.main.greek_state.copy()
> expected_disk.tweak('iota',
> - props={'svn:mime-type':'application/binary'},
> + props={'svn:mime-type':'application/binary',
> + 'svn:executable': '*'},
> contents =
> 'This is the file \'iota\'.\n'
> '\0\202\203\204\205\206\207nsomething\nelse\xFF')
> ]]]
Received on 2015-09-30 00:57:29 CEST