[merge][0.11] Transform._set_mode stats the wrong file

[John Arbash Meinel]

Aaron Bentley wrote:
> John Arbash Meinel wrote:
>>> Attached is a patch which fixes:
>>> https://launchpad.net/products/bzr/+bug/56549
>>>
>>> And includes a test case.
>>>
>>> I'm not super happy with the test case, but it was the easiest way to
>>> fight my way through all the layering.
> 
> I've attached an alternative test.  What do you think?
> 
>>> Perhaps Aaron was thinking that rw permissions should be preserved as
>>> much as possible. Though because of the bug, that wasn't really happening.
> 
> That was the reason.  I don't think changing the contents of a file
> should alter its file mode.
> 
>>> +            transform.create_file('bar2 contents\n', bar2_id, mode_id=bar1_id)
> 
> It's unusual to use the mode_id parameter.  The reason you see it in
> revert is because revert's backups should have the same mode as the new
> contents.  So I'd rather see delete_contents(bar1_id),
> create_contents(bar1_id.)
> 
> But +1.  Do whatever you think is right wrt the test cases.  Keep one,
> or both, if you like.
> 
> Aaron


...

 class TestTreeTransform(TestCaseInTempDir):

@@ -535,6 +540,51 @@
         self.assertTrue(wt.is_executable('soc'))
         self.assertTrue(wt.is_executable('sac'))

+    def test_preserve_mode(self):
+        """File mode is preserved when replacing content"""
+        if sys.platform == 'win32':
+            raise TestSkipped('chmod has no effect on win32')
+        transform, root = self.get_transform()
+        transform.new_file('file1', root, 'contents', 'file1-id', True)
+        transform.apply()
+        self.assertTrue(self.wt.is_executable('file1-id'))
+        transform, root = self.get_transform()
+        file1_id = transform.trans_id_tree_file_id('file1-id')
+        transform.delete_contents(file1_id)
+        transform.create_file('contents2', file1_id)
+        transform.apply()
+        self.assertTrue(self.wt.is_executable('file1-id'))
+

Does this test fail when the patch is not present? The problem with
'executable' is I tried a few tests using that, but they always passed
because executability was handled by a different section.

I would prefer to not be hijacking 'os.stat()', I just couldn't find
another way to do it.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060921/68ea0490/attachment.pgp