[merge] win32- executable bits toggling #45010
John Arbash Meinel
john at arbash-meinel.com
Wed Jun 28 14:56:17 BST 2006
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Robert Collins wrote:
> 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.
I was just extending the tests that were there. I can agree that it is
more of a working tree test.
TestExecutable checks that the executable bit stays set across
branch/add/remove+revert/pull/etc. I just added 'no-op revert' since
that was broken on win32.
I suppose the whole test could be put into something like
'working_tree_implementations'.
>
>> 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.
Well, the problem is that checking the executable flag on Linux works.
At least by going through WorkingTree.is_executable(). And the point was
to generate working tree inventories that are the same on Linux as on
Windows. (So if you share your linux tree over SMB, it still works).
So since the low level detail was that the XML was different in the two
cases, I was checking the low level detail.
I think if we read the Inventory and just check the
InventoryFile.executable bits it will work. Since they should be None if
the xml is missing that record.
>
> 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
At this point, I was just trying to get things working again. But I can
work a little bit harder and see if I can get things working even with
older formats.
I'll resubmit later today. Thanks for the recommendations.
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFEooqBJdeBCYSNAAMRAhzqAJ9BH1sJUvFtnciG22ACHqTLaYjNEgCgyO1n
03mezGe9DqEFhsIUCszaxvs=
=5B0j
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list