Request for advice - subversion FTBFS in precise

[Colin Watson]

On Fri, Apr 13, 2012 at 09:44:30AM -0400, Scott Kitterman wrote:
> On Friday, April 13, 2012 12:01:45 AM Max Bowsher wrote:
> > Subversion currently FTBFS in precise.
> > 
> > The reason is because apr 1.4.6 changed its hashtable implementation to
> > incorporate randomness into the hash function. This was to prevent
> > possible DoS attacks which are based on feeding a server data which it
> > will put into a hash, which will cause the hash function to perform
> > pathologically badly.
> > 
> > Unfortunately, it's easy to see how this can completely break a
> > testsuite's comparisons against hardcoded expected values - hence the FTBFS.
> > 
> > More interestingly, this change also creates at least one behaviour in
> > Subversion which some might deem a bug: the output of 'svnadmin dump' is
> > no longer identical if performed multiple times on an unchanging
> > repository. Bad news for anyone trying to use it to verify intactness of
> > a repository mirror, for example.
[...]
> I think it would make sense to upload 1.6.18 to oneiric-proposed so it's easy 
> to remove if it doesn't test out well.  svn releases are not consistently 
> regression free.  I don't think reverting an upstream security improvement to 
> make a test suite work is a good idea.  What do svn and apr devs say about 
> this?

I'd been working on making the test suite work again, by backporting a
few upstream patches, but I hadn't quite got the Ruby binding tests
working yet.  I'm more than happy for somebody with more time to work on
this, so I've attached the patch I have so far.

-- 
Colin Watson                                       [cjwatson at ubuntu.com]
-------------- next part --------------
Description: Fix tests to not rely on a specific APR hash order
Origin: upstream, http://svn.apache.org/viewvc?view=revision&revision=1292248
Origin: upstream, http://svn.apache.org/viewvc?view=revision&revision=1292255
Origin: upstream, http://svn.apache.org/viewvc?view=revision&revision=1292260
Last-Update: 2012-04-11

Index: b/subversion/bindings/swig/python/tests/trac/versioncontrol/tests/svn_fs.py
===================================================================
--- a/subversion/bindings/swig/python/tests/trac/versioncontrol/tests/svn_fs.py
+++ b/subversion/bindings/swig/python/tests/trac/versioncontrol/tests/svn_fs.py
@@ -224,30 +224,50 @@
 
     def test_diff_dir_different_revs(self):
         diffs = self.repos.get_deltas('trunk', 4, 'trunk', 8)
-        self._cmp_diff((None, ('trunk/dir1/dir2', 8),
-                        (Node.DIRECTORY, Changeset.ADD)), diffs.next())
-        self._cmp_diff((None, ('trunk/dir1/dir3', 8),
-                        (Node.DIRECTORY, Changeset.ADD)), diffs.next())
-        self._cmp_diff((None, ('trunk/README2.txt', 6),
-                        (Node.FILE, Changeset.ADD)), diffs.next())
-        self._cmp_diff((('trunk/dir2', 4), None,
-                        (Node.DIRECTORY, Changeset.DELETE)), diffs.next())
-        self._cmp_diff((('trunk/dir3', 4), None,
-                        (Node.DIRECTORY, Changeset.DELETE)), diffs.next())
+        expected = [
+          (None, ('trunk/README2.txt', 6),
+           (Node.FILE, Changeset.ADD)),
+          (None, ('trunk/dir1/dir2', 8),
+           (Node.DIRECTORY, Changeset.ADD)),
+          (None, ('trunk/dir1/dir3', 8),
+           (Node.DIRECTORY, Changeset.ADD)),
+          (('trunk/dir2', 4), None,
+           (Node.DIRECTORY, Changeset.DELETE)),
+          (('trunk/dir3', 4), None,
+           (Node.DIRECTORY, Changeset.DELETE)),
+        ]
+        actual = [diffs.next() for i in range(5)]
+        actual = sorted(actual,
+                        key=lambda diff: ((diff[0] or diff[1]).path,
+                                          (diff[0] or diff[1]).rev))
+        self.assertEqual(len(expected), len(actual))
+        for e,a in zip(expected, actual):
+          self._cmp_diff(e,a)
         self.assertRaises(StopIteration, diffs.next)
 
     def test_diff_dir_different_dirs(self):
         diffs = self.repos.get_deltas('trunk', 1, 'branches/v1x', 12)
-        self._cmp_diff((None, ('branches/v1x/dir1', 12),
-                        (Node.DIRECTORY, Changeset.ADD)), diffs.next())
-        self._cmp_diff((None, ('branches/v1x/dir1/dir2', 12),
-                        (Node.DIRECTORY, Changeset.ADD)), diffs.next())
-        self._cmp_diff((None, ('branches/v1x/dir1/dir3', 12),
-                        (Node.DIRECTORY, Changeset.ADD)), diffs.next())
-        self._cmp_diff((None, ('branches/v1x/README.txt', 12),
-                        (Node.FILE, Changeset.ADD)), diffs.next())
-        self._cmp_diff((None, ('branches/v1x/README2.txt', 12),
-                        (Node.FILE, Changeset.ADD)), diffs.next())
+        expected = [
+          (None, ('branches/v1x/README.txt', 12),
+           (Node.FILE, Changeset.ADD)),
+          (None, ('branches/v1x/README2.txt', 12),
+           (Node.FILE, Changeset.ADD)),
+          (None, ('branches/v1x/dir1', 12),
+           (Node.DIRECTORY, Changeset.ADD)),
+          (None, ('branches/v1x/dir1/dir2', 12),
+           (Node.DIRECTORY, Changeset.ADD)),
+          (None, ('branches/v1x/dir1/dir3', 12),
+           (Node.DIRECTORY, Changeset.ADD)),
+        ]
+        actual = [diffs.next() for i in range(5)]
+        actual = sorted(actual, key=lambda diff: (diff[1].path, diff[1].rev))
+        # for e,a in zip(expected, actual):
+        #   t.write("%r\n" % (e,))
+        #   t.write("%r\n" % ((None, (a[1].path, a[1].rev), (a[2], a[3])),) )
+        #   t.write('\n')
+        self.assertEqual(len(expected), len(actual))
+        for e,a in zip(expected, actual):
+          self._cmp_diff(e,a)
         self.assertRaises(StopIteration, diffs.next)
 
     def test_diff_dir_no_change(self):
Index: b/subversion/bindings/swig/python/tests/wc.py
===================================================================
--- a/subversion/bindings/swig/python/tests/wc.py
+++ b/subversion/bindings/swig/python/tests/wc.py
@@ -193,8 +193,9 @@
 
   def test_entries_read(self):
       entries = wc.entries_read(self.wc, True)
-
-      self.assertEqual(['', 'tags', 'branches', 'trunk'], list(entries.keys()))
+      keys = list(entries.keys())
+      keys.sort()
+      self.assertEqual(['', 'branches', 'tags', 'trunk'], keys)
 
   def test_get_ignores(self):
       self.assert_(isinstance(wc.get_ignores(None, self.wc), list))