[merge] win32- executable bits toggling #45010

[Robert Collins]

On Mon, 2006-06-26 at 14:59 -0500, John A Meinel wrote:
> === modified file 'bzrlib/inventory.py'
> --- bzrlib/inventory.py 2006-06-13 16:26:09 +0000
> +++ bzrlib/inventory.py 2006-06-26 19:43:01 +0000
> @@ -692,7 +692,6 @@
>  
>      def _forget_tree_state(self):
>          self.text_sha1 = None
> -        self.executable = None
>  
>      def _snapshot_text(self, file_parents, work_tree,
> commit_builder):
>          """See InventoryEntry._snapshot_text."""
> 
> === modified file 'bzrlib/tests/test_inv.py'
> --- bzrlib/tests/test_inv.py    2006-06-09 07:35:13 +0000
> +++ bzrlib/tests/test_inv.py    2006-06-26 18:25:12 +0000
> @@ -611,6 +611,14 @@
>          self.failUnless(t2.is_executable(a_id), "'a' lost the execute
> bit")
>          self.failIf(t2.is_executable(b_id), "'b' gained an execute
> bit")
>  
> +        # Just do a simple revert without anything changed, and 
> +        # make sure the bits don't swap.
> +        t2.revert([], t2.branch.repository.revision_tree('r3'),
> backups=False)
> +        self.assertEqual(['a', 'b'], [cn for cn,ie in
> t2.inventory.iter_entries()])
> +
> +        self.failUnless(t2.is_executable(a_id), "'a' lost the execute
> bit")
> +        self.failIf(t2.is_executable(b_id), "'b' gained an execute
> bit")
> +
>  

Reverting is a tree function. It seems odd to me to be doing reverts to
test inventories: if we are testing how inventories operation, I would
expect to see calls on inventories.

>  class TestRevert(TestCaseWithTransport):
>  
> 
> === modified file 'bzrlib/tests/test_workingtree.py'
> --- bzrlib/tests/test_workingtree.py    2006-06-20 05:32:16 +0000
> +++ bzrlib/tests/test_workingtree.py    2006-06-26 19:54:31 +0000
> @@ -27,6 +27,7 @@
>  from bzrlib.osutils import pathjoin, getcwd, has_symlinks
>  from bzrlib.tests import TestCaseWithTransport, TestSkipped
>  from bzrlib.trace import mutter
> +from bzrlib.transform import TreeTransform
>  from bzrlib.transport import get_transport
>  from bzrlib.workingtree import (TreeEntry, TreeDirectory, TreeFile,
> TreeLink,
>                                  WorkingTree)
> @@ -202,6 +203,39 @@
>          tree.unlock()
>          self.assertEquals(our_lock.peek(), None)
>  
> +    def test_executable(self):
> +        """Format 3 trees should keep executable=yes in the working
> inventory."""
> +        control =
> bzrdir.BzrDirMetaFormat1().initialize(self.get_url())
> +        control.create_repository()
> +        control.create_branch()
> +        try:
> +            tree =
> workingtree.WorkingTreeFormat3().initialize(control)
> +        except errors.NotLocalUrl:
> +            raise TestSkipped('Not a local URL')
> +        tt = TreeTransform(tree)
> +        tt.new_file('a', tt.root, 'contents of a\n', 'a-xxyy', True)
> +        tt.new_file('b', tt.root, 'contents of b\n', 'b-xxyy', False)
> +        tt.apply()
> +
> +        t = control.get_workingtree_transport(None)
> +        inventory_text = (
> +                '<inventory format="5">\n'
> +                '<file executable="yes" file_id="a-xxyy"
> name="a" />\n'
> +                '<file file_id="b-xxyy" name="b" />\n'
> +                '</inventory>\n'
> +                )
> +        # Check that the executable flag is set in the XML
> +        self.assertEqualDiff(inventory_text,
> t.get('inventory').read())
> +
> +        # Committing shouldn't remove it
> +        tree.commit('first rev')
> +        self.assertEqualDiff(inventory_text,
> t.get('inventory').read())
> +
> +        # And neither should reverting
> +        last_tree =
> tree.branch.repository.revision_tree(tree.last_revision())
> +        tree.revert([], last_tree, backups=False)
> +        self.assertEqualDiff(inventory_text,
> t.get('inventory').read())
> +


I have two comments here. The first is that this test seems to be
testing the internals rather than the desired behaviour: I'd rather have
a test test checks that working with the tree does the right thing. I.e.
do the tree transform, then open the tree fresh and check for the
inventory entry .executable flag, rather than fiddling with the XML. 

Doing that would allow the use of an interface based test, which is my
second comment: preserving the exec bit on win32 seems to be something
we want every tree to do, so there is no reason to have this be a format
specific test, is there? (Perhaps it needs to not run on known-broken,
wont-be-fixed-due-to-backwards-compatability formats to avoid failures)

Possibly the non win32 safe formats should just refuse to run their
tests on win32 completely?

Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060627/4c468ea9/attachment.pgp