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: [email protected] 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') > ]]]
