| Submitter | Martin von Zweigbergk |
|---|---|
| Date | March 26, 2015, 5:31 p.m. |
| Message ID | <4dd5ebf2778b1ec0f967.1427391060@martinvonz.mtv.corp.google.com> |
| Download | mbox | patch |
| Permalink | /patch/8295/ |
| State | Superseded |
| Headers | show |
Comments
On Thu, Mar 26, 2015 at 1:31 PM, Martin von Zweigbergk < martinvonz@google.com> wrote: > # HG changeset patch > # User Martin von Zweigbergk <martinvonz@google.com> > # Date 1427388141 25200 > # Thu Mar 26 09:42:21 2015 -0700 > # Node ID 4dd5ebf2778b1ec0f967d28e1f752906360602ae > # Parent c99862a283585e40e907b9116a7736de6a3f4369 > test-manifest.py: separate out test for byte 22 of nodeid > How much of a perf lose is it for treemanifest to trim the nodeid to no more than 21 bytes? I think I'd rather see the behavior be consistent... > > When assigning a 22-byte hash to a nodeid in a manifest, manifestdict > drops the 22nd byte, while treemanifest keeps it. Let's keep this test > separate and make it pass whether the 22nd byte gets stored or > dropped. > > diff -r c99862a28358 -r 4dd5ebf2778b tests/test-manifest.py > --- a/tests/test-manifest.py Wed Mar 25 14:13:46 2015 -0700 > +++ b/tests/test-manifest.py Thu Mar 26 09:42:21 2015 -0700 > @@ -127,10 +127,6 @@ > self.assertEqual([('bar/baz/qux.py', binascii.unhexlify(HASH_2)), > ('foo', binascii.unhexlify(HASH_1) + 'a')], > list(m.iteritems())) > - # Sometimes it even tries a 22-byte fake hash, but we can > - # return 21 and it'll work out > - m['foo'] = want + '+' > - self.assertEqual(want, m['foo']) > # make sure the suffix survives a copy > match = matchmod.match('', '', ['re:foo']) > m2 = m.matches(match) > @@ -148,6 +144,14 @@ > self.assertEqual({'foo': ((want, f), (h, ''))}, m.diff(clean)) > self.assertEqual({'foo': ((h, ''), (want, f))}, clean.diff(m)) > > + def test22ByteNode(self): > + m = parsemanifest(A_SHORT_MANIFEST) > + h = binascii.unhexlify(HASH_3) > + # Sometimes it even tries a 22-byte fake hash, but we can > + # keep 21 and it'll work out > + m['foo'] = h + 'a+' > + self.assertIn(m['foo'], [h + 'a', h + 'a+']) > + > def testMatchException(self): > m = parsemanifest(A_SHORT_MANIFEST) > match = matchmod.match('', '', ['re:.*']) >
On Thu, Mar 26, 2015 at 10:34 AM Augie Fackler <augie@google.com> wrote: > On Thu, Mar 26, 2015 at 1:31 PM, Martin von Zweigbergk < > martinvonz@google.com> wrote: > >> # HG changeset patch >> # User Martin von Zweigbergk <martinvonz@google.com> >> # Date 1427388141 25200 >> # Thu Mar 26 09:42:21 2015 -0700 >> # Node ID 4dd5ebf2778b1ec0f967d28e1f752906360602ae >> # Parent c99862a283585e40e907b9116a7736de6a3f4369 >> test-manifest.py: separate out test for byte 22 of nodeid >> > > How much of a perf lose is it for treemanifest to trim the nodeid to no > more than 21 bytes? I think I'd rather see the behavior be consistent... > Probably not even noticeably much. I'll send a V2. > > >> >> When assigning a 22-byte hash to a nodeid in a manifest, manifestdict >> drops the 22nd byte, while treemanifest keeps it. Let's keep this test >> separate and make it pass whether the 22nd byte gets stored or >> dropped. >> >> diff -r c99862a28358 -r 4dd5ebf2778b tests/test-manifest.py >> --- a/tests/test-manifest.py Wed Mar 25 14:13:46 2015 -0700 >> +++ b/tests/test-manifest.py Thu Mar 26 09:42:21 2015 -0700 >> @@ -127,10 +127,6 @@ >> self.assertEqual([('bar/baz/qux.py', binascii.unhexlify(HASH_2)), >> ('foo', binascii.unhexlify(HASH_1) + 'a')], >> list(m.iteritems())) >> - # Sometimes it even tries a 22-byte fake hash, but we can >> - # return 21 and it'll work out >> - m['foo'] = want + '+' >> - self.assertEqual(want, m['foo']) >> # make sure the suffix survives a copy >> match = matchmod.match('', '', ['re:foo']) >> m2 = m.matches(match) >> @@ -148,6 +144,14 @@ >> self.assertEqual({'foo': ((want, f), (h, ''))}, m.diff(clean)) >> self.assertEqual({'foo': ((h, ''), (want, f))}, clean.diff(m)) >> >> + def test22ByteNode(self): >> + m = parsemanifest(A_SHORT_MANIFEST) >> + h = binascii.unhexlify(HASH_3) >> + # Sometimes it even tries a 22-byte fake hash, but we can >> + # keep 21 and it'll work out >> + m['foo'] = h + 'a+' >> + self.assertIn(m['foo'], [h + 'a', h + 'a+']) >> + >> def testMatchException(self): >> m = parsemanifest(A_SHORT_MANIFEST) >> match = matchmod.match('', '', ['re:.*']) >> >
Patch
diff -r c99862a28358 -r 4dd5ebf2778b tests/test-manifest.py --- a/tests/test-manifest.py Wed Mar 25 14:13:46 2015 -0700 +++ b/tests/test-manifest.py Thu Mar 26 09:42:21 2015 -0700 @@ -127,10 +127,6 @@ self.assertEqual([('bar/baz/qux.py', binascii.unhexlify(HASH_2)), ('foo', binascii.unhexlify(HASH_1) + 'a')], list(m.iteritems())) - # Sometimes it even tries a 22-byte fake hash, but we can - # return 21 and it'll work out - m['foo'] = want + '+' - self.assertEqual(want, m['foo']) # make sure the suffix survives a copy match = matchmod.match('', '', ['re:foo']) m2 = m.matches(match) @@ -148,6 +144,14 @@ self.assertEqual({'foo': ((want, f), (h, ''))}, m.diff(clean)) self.assertEqual({'foo': ((h, ''), (want, f))}, clean.diff(m)) + def test22ByteNode(self): + m = parsemanifest(A_SHORT_MANIFEST) + h = binascii.unhexlify(HASH_3) + # Sometimes it even tries a 22-byte fake hash, but we can + # keep 21 and it'll work out + m['foo'] = h + 'a+' + self.assertIn(m['foo'], [h + 'a', h + 'a+']) + def testMatchException(self): m = parsemanifest(A_SHORT_MANIFEST) match = matchmod.match('', '', ['re:.*'])