[merge] fix bundles with binary files

[Martin Pool]

On 11/07/2006, at 7:03 AM, John Arbash Meinel wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> I was able to get a test to fail if I put '\r' into the contents of a
> binary file. I still don't really know why merging the .ico file  
> worked
> for me, but not for Alexander, but I can say the attached test  
> fails if
> you use 'string.splitlines(True)' instead of StringIO 
> (string).readlines().

+1

>
> === modified file bzrlib/bundle/bundle_data.py
> --- bzrlib/bundle/bundle_data.py
> +++ bzrlib/bundle/bundle_data.py
> @@ -723,4 +723,7 @@
>      from bzrlib.iterablefile import IterableFile
>      if file_patch == "":
>          return IterableFile(())
> -    return IterableFile(iter_patched(original,  
> file_patch.splitlines(True)))
> +    # string.splitlines(True) also splits on '\r', but the  
> iter_patched code
> +    # only expects to iterate over '\n' style lines
> +    return IterableFile(iter_patched(original,
> +                StringIO(file_patch).readlines()))

Yes, it's good to have a comment here so someone doesn't "fix" it.


> === modified file bzrlib/tests/test_bundle.py
> --- bzrlib/tests/test_bundle.py
> +++ bzrlib/tests/test_bundle.py
> @@ -610,33 +610,44 @@
>          self.tree1 = BzrDir.create_standalone_workingtree('b1')
>          self.b1 = self.tree1.branch
>          tt = TreeTransform(self.tree1)
> -        tt.new_file('file', tt.root, '\x00\xff', 'binary-1')
> -        tt.new_file('file2', tt.root, '\x00\xff', 'binary-2')
> +
> +        # Add
> +        tt.new_file('file', tt.root, '\x00\n\x00\r\x01\n\x02\r 
> \xff', 'binary-1')
> +        tt.new_file('file2', tt.root, '\x01\n\x02\r\x03\n\x04\r 
> \xff', 'binary-2')
>          tt.apply()
>          self.tree1.commit('add binary', rev_id='b at cset-0-1')
>          self.get_valid_bundle(None, 'b at cset-0-1')
> +
> +        # Delete
>          tt = TreeTransform(self.tree1)
>          trans_id = tt.trans_id_tree_file_id('binary-1')
>          tt.delete_contents(trans_id)
>          tt.apply()
>          self.tree1.commit('delete binary', rev_id='b at cset-0-2')
>          self.get_valid_bundle('b at cset-0-1', 'b at cset-0-2')
> +
> +        # Rename & modify
>          tt = TreeTransform(self.tree1)
>          trans_id = tt.trans_id_tree_file_id('binary-2')
>          tt.adjust_path('file3', tt.root, trans_id)
>          tt.delete_contents(trans_id)
> -        tt.create_file('filecontents\x00', trans_id)
> +        tt.create_file('file\rcontents\x00\n\x00', trans_id)
>          tt.apply()
>          self.tree1.commit('rename and modify binary',  
> rev_id='b at cset-0-3')
>          self.get_valid_bundle('b at cset-0-2', 'b at cset-0-3')
> +
> +        # Modify
>          tt = TreeTransform(self.tree1)
>          trans_id = tt.trans_id_tree_file_id('binary-2')
>          tt.delete_contents(trans_id)
> -        tt.create_file('\x00filecontents', trans_id)
> +        tt.create_file('\x00file\rcontents', trans_id)
>          tt.apply()
>          self.tree1.commit('just modify binary', rev_id='b at cset-0-4')
>          self.get_valid_bundle('b at cset-0-3', 'b at cset-0-4')
>
> +        # Rollup
> +        self.get_valid_bundle(None, 'b at cset-0-4')
> +
>      def test_last_modified(self):
>          self.tree1 = BzrDir.create_standalone_workingtree('b1')
>          self.b1 = self.tree1.branch

This is starting to look like it should be split into several tests  
maybe?

-- 
Martin