I committed this API change (below) in the ctypes-python bindings. Should I be revving these APIs in some way, to maintain backward compatibility, or is this kind of fix considered fair game?
- Julian On Thu, 2011-08-11 at 20:56 +0000, julianf...@apache.org wrote: > Author: julianfoad > Date: Thu Aug 11 20:56:26 2011 > New Revision: 1156826 > > URL: http://svn.apache.org/viewvc?rev=1156826&view=rev > Log: > In the ctypes-python bindings: fix two RemoteRepository methods to return > the Python types they claim to return, and add tests for these and other > methods that had no tests. > > This is an API change in the ctypes-python bindings. > > * subversion/bindings/ctypes-python/csvn/repos.py > (RemoteRepository.list): Create and return a Python dictionary of > svn_dirent_t objects instead of the Hash of pointer objects. > (RemoteRepository.info): Dereference the pointer object and so return a > svn_dirent_t object. > > * subversion/bindings/ctypes-python/test/remoterepos.py > (RemoteRepositoryTestCase): Add tests for the list, info, proplist, > propget and log methods. (The remaining untested method is 'cat', for > which I have not yet been able to write a test that does not crash.) > > Modified: > subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py > subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py > > Modified: subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py?rev=1156826&r1=1156825&r2=1156826&view=diff > ============================================================================== > --- subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py > (original) > +++ subversion/trunk/subversion/bindings/ctypes-python/csvn/repos.py Thu Aug > 11 20:56:26 2011 > @@ -154,7 +154,12 @@ class RemoteRepository(object): > svn_ra_get_dir2(self, dirents.byref(), NULL, NULL, path, > rev, fields, dirents.pool) > self.iterpool.clear() > - return dirents > + # Create a Python dict of svn_dirent_t objects from this Hash of > + # pointers to svn_dirent_t. > + result = {} > + for path, dirent_p in dirents.items(): > + result[path] = dirent_p[0] > + return result > > def cat(self, buffer, path, rev = SVN_INVALID_REVNUM): > """Get PATH@REV and save it to BUFFER. BUFFER must be a Python file > @@ -178,7 +183,7 @@ class RemoteRepository(object): > rev = self.latest_revnum() > svn_ra_stat(self, path, rev, byref(dirent), dirent.pool) > self.iterpool.clear() > - return dirent > + return dirent[0] > > def proplist(self, path, rev = SVN_INVALID_REVNUM): > """Return a dictionary containing the properties on PATH@REV > > Modified: > subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py?rev=1156826&r1=1156825&r2=1156826&view=diff > ============================================================================== > --- subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py > (original) > +++ subversion/trunk/subversion/bindings/ctypes-python/test/remoterepos.py > Thu Aug 11 20:56:26 2011 > @@ -27,6 +27,13 @@ from csvn.core import * > from urllib import pathname2url > from csvn.repos import LocalRepository, RemoteRepository > from stat import * > +from sys import version_info # For Python version check > +if version_info[0] >= 3: > + # Python >=3.0 > + from io import StringIO > +else: > + # Python <3.0 > + from StringIO import StringIO > > repos_location = os.path.join(tempfile.gettempdir(), "svn_test_repos") > repos_url = pathname2url(repos_location) > @@ -64,6 +71,15 @@ class RemoteRepositoryTestCase(unittest. > if os.path.exists(repos_location): > svn_repos_delete(repos_location, Pool()) > > + def svn_dirent_t_assert_equal(self, a, b): > + """Assert that two svn_dirent_t's are equal, ignoring their 'time' > + fields.""" > + self.assertEqual(a.kind, b.kind) > + self.assertEqual(a.size, b.size) > + self.assertEqual(a.has_props, b.has_props) > + self.assertEqual(a.created_rev, b.created_rev) > + self.assertEqual(a.last_author, b.last_author) > + > def test_remote_latest_revnum(self): > self.assertEqual(9, self.repos.latest_revnum()) > > @@ -77,6 +93,50 @@ class RemoteRepositoryTestCase(unittest. > self.assertEqual(svn_node_none, > self.repos.check_path("does_not_compute")) > > + def test_list(self): > + expected = { > + 'README.txt': > + svn_dirent_t(kind=svn_node_file, size=159, has_props=True, > + created_rev=9, last_author=String('bruce')), > + 'ANOTHERREADME.txt': > + svn_dirent_t(kind=svn_node_file, size=66, has_props=False, > + created_rev=4, last_author=String('clark')) } > + found = self.repos.list("trunk") > + self.assertEqual(sorted(found.keys()), sorted(expected.keys())) > + for path in found: > + self.svn_dirent_t_assert_equal(found[path], expected[path]) > + > + def test_info(self): > + e = svn_dirent_t(kind=svn_node_file, size=159, has_props=True, > + created_rev=9, last_author=String('bruce')) > + f = self.repos.info("trunk/README.txt") > + self.svn_dirent_t_assert_equal(f, e) > + > + def test_proplist(self): > + expected = { 'Awesome': 'Yes', > + 'svn:entry:last-author': 'bruce', > + 'svn:entry:committed-rev': '9' } > + found = self.repos.proplist("trunk/README.txt") > + # Check results, ignoring some entry-props > + del found['svn:entry:committed-date'] > + del found['svn:entry:uuid'] > + self.assertEqual(sorted(found.keys()), sorted(expected.keys())) > + for pname in found: > + self.assertEqual(found[pname], expected[pname]) > + > + def test_propget(self): > + found = self.repos.propget("Awesome", "trunk/README.txt") > + self.assertEqual(found, "Yes") > + > + def test_log(self): > + expected = [ (8, 'clark'), > + (9, 'bruce') ] > + for found in self.repos.log(7, 9, ["trunk/README.txt"]): > + (e_rev, e_author) = expected[0] > + self.assertEqual(found.revision, e_rev) > + self.assertEqual(found.author, e_author) > + expected = expected[1:] > + > def test_revprop_list(self): > # Test argument-free case > props = self.repos.revprop_list() > >
