You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Eric Gillespie <ep...@pretzelnet.org> on 2007/05/04 21:49:53 UTC

[PATCH] Fix Python digest typemap

I'm not sure where the new test really belongs.  Should i add a
new file for high-level tests of multiple layers?  This one uses
delta, ra, and wc.

Also note the changing revision number problem indicated by
test_get_ancestry.  unittest runs sorts the tests, so what i've
done in another test suite is number the tests
(e.g. test_1_get_ancestry, test_2_commit, ...), so we know the
order in which they run and don't have to keep changing expected
revision numbers around.

[[[
* subversion/bindings/swig/include/svn_types.swg
  (typemap(argout) unsigned char digest[ANY]): Don't convert the digest to
    the hexadecimal cstring representation; callers can do that themselves
    if they so desire.

* subversion/bindings/swig/python/tests/ra.py
  (test_commit_with_delta_driver): Remove.

* subversion/bindings/swig/python/tests/wc.py
  (test_get_ancestry): Bump expected revision, since new test_commit runs
    first.
  (test_commit): New test for working copy commit, superseding
    test_commit_with_delta_driver.  Also covers
    svn.wc.transmit_text_deltas2, svn.wc.process_committed4, and the new
    digest typemap.
]]]

Index: subversion/bindings/swig/include/svn_types.swg
===================================================================
--- subversion/bindings/swig/include/svn_types.swg	(revision 24930)
+++ subversion/bindings/swig/include/svn_types.swg	(working copy)
@@ -986,8 +986,7 @@
 #ifdef SWIGPYTHON
 %typemap(argout) unsigned char digest[ANY]
 {
-  %append_output(PyString_FromString(svn_md5_digest_to_cstring($1,
-                                                               _global_pool)));
+  %append_output(PyString_FromStringAndSize((char *)$1, APR_MD5_DIGESTSIZE));
 }
 #endif
 
Index: subversion/bindings/swig/python/tests/ra.py
===================================================================
--- subversion/bindings/swig/python/tests/ra.py	(revision 24930)
+++ subversion/bindings/swig/python/tests/ra.py	(working copy)
@@ -90,18 +90,6 @@
     child = editor.add_directory("blah", root, None, 0)
     editor.close_edit(edit_baton)
 
-  def test_commit_with_delta_driver(self):
-    def my_callback(info, pool):
-      self.assertEqual(info.revision, fs.youngest_rev(self.fs))
-    (editor, edit_baton) = ra.get_commit_editor2(self.ra_ctx, "log msg", my_callback, None, False)
-
-    paths = ['trunk/newdir']
-    def driver_cb(parent, path, pool):
-      self.assert_(path in paths)
-      return editor.add_directory(path, parent, None, 0)
-    delta.path_driver(editor, edit_baton, -1, paths, driver_cb)
-    editor.close_edit(edit_baton)
-
   def test_do_diff2(self):
 
     class ChangeReceiver(delta.Editor):
Index: subversion/bindings/swig/python/tests/wc.py
===================================================================
--- subversion/bindings/swig/python/tests/wc.py	(revision 24930)
+++ subversion/bindings/swig/python/tests/wc.py	(working copy)
@@ -1,5 +1,6 @@
-import unittest, os, tempfile, shutil, types, setup_path
+import unittest, os, tempfile, shutil, types, setup_path, binascii
 from svn import core, repos, wc, client
+from svn import delta, ra
 from libsvn.core import SubversionException
 
 from trac.versioncontrol.tests.svn_fs import SubversionRepositoryTestSetup, \
@@ -120,7 +121,7 @@
       self.assert_(wc.check_wc(self.path) > 0)
 
   def test_get_ancestry(self):
-      self.assertEqual([REPOS_URL, 12], 
+      self.assertEqual([REPOS_URL, 13],
                        wc.get_ancestry(self.path, self.wc))
 
   def test_status(self):
@@ -180,6 +181,71 @@
   def test_get_ignores(self):
       self.assert_(isinstance(wc.get_ignores(None, self.wc), list))
 
+  def test_commit(self):
+    # Replace README.txt's contents, using binary mode so we know the
+    # exact contents even on Windows, and therefore the MD5 checksum.
+    readme_path = '%s/trunk/README.txt' % self.path
+    fp = open(readme_path, 'wb')
+    fp.write('hello\n')
+    fp.close()
+
+    # Setup ra_ctx.
+    ra.initialize()
+    callbacks = ra.callbacks2_t()
+    ra_ctx = ra.open2(REPOS_URL, callbacks, None, None)
+
+    # Get commit editor.
+    commit_info = [None]
+    def commit_cb(_commit_info, pool):
+      commit_info[0] = _commit_info
+    (editor, edit_baton) = ra.get_commit_editor2(ra_ctx, 'log message',
+                                                 commit_cb,
+                                                 None,
+                                                 False)
+
+    # Drive the commit.
+    checksum = [None]
+    def driver_cb(parent, path, pool):
+      baton = editor.open_file(path, parent, -1, pool)
+      adm_access = wc.adm_probe_retrieve(self.wc, readme_path, pool)
+      (_, checksum[0]) = wc.transmit_text_deltas2(readme_path, adm_access,
+                                                  False, editor, baton, pool)
+      return baton
+    try:
+      delta.path_driver(editor, edit_baton, -1, ['trunk/README.txt'],
+                        driver_cb)
+      editor.close_edit(edit_baton)
+    except:
+      try:
+        editor.abort_edit(edit_baton)
+      except:
+        # We already have an exception in progress, not much we can do
+        # about this.
+        pass
+      raise
+    (checksum,) = checksum
+    (commit_info,) = commit_info
+
+    # Assert the commit.
+    self.assertEquals(binascii.b2a_hex(checksum),
+                      'b1946ac92492d2347c6235b4d2611184')
+    self.assertEquals(commit_info.revision, 13)
+
+    # Bump working copy state.
+    wc.process_committed4(readme_path,
+                          wc.adm_retrieve(self.wc,
+                                          os.path.dirname(readme_path)),
+                          False, commit_info.revision, commit_info.date,
+                          commit_info.author, None, False, False, checksum)
+
+    # Assert bumped state.
+    entry = wc.entry(readme_path, self.wc, False)
+    self.assertEquals(entry.revision, commit_info.revision)
+    self.assertEquals(entry.schedule, wc.schedule_normal)
+    self.assertEquals(entry.cmt_rev, commit_info.revision)
+    self.assertEquals(entry.cmt_date,
+                      core.svn_time_from_cstring(commit_info.date))
+
   def tearDown(self):
       wc.adm_close(self.wc)
       core.svn_io_remove_dir(self.path)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Fix Python digest typemap

Posted by David James <ja...@cs.toronto.edu>.
On 5/7/07, Eric Gillespie <ep...@pretzelnet.org> wrote:
> > I have a question. Why do you think that the MD5 checksums should be
> > passed in raw format instead of in cstring format? Your change breaks
> > compatibility with any code which previously used the checksum return
> > value, so we should have a good reason if we want to change it.
>
> The old code was the incompatible one; why should the Python
> binding change things like this on you?  To be complete, we'd
> have to wrap the functions that take that raw checksum
> (e.g. svn_wc_process_committed*) to convert the hex string back
> to the raw data; why convert it back and forth?  Python
> applications can always use binascii.b2a_hex to get the
> human-readable hex form.

Ah, ok, I get it. You're basically saying that the old bindings were
effectively broken because the result string couldn't be used by any
Subversion functions which accept checksums as input. In this case
it's in my opinion fine to change the typemap.

At release time, we should probably add a note about this to the
CHANGES file in case somebody depends on the old behavior.

If nobody objects to this change, feel free to commit.

Cheers,

David

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Fix Python digest typemap

Posted by Eric Gillespie <ep...@pretzelnet.org>.
"David James" <ja...@cs.toronto.edu> writes:

> I have a question. Why do you think that the MD5 checksums should be
> passed in raw format instead of in cstring format? Your change breaks
> compatibility with any code which previously used the checksum return
> value, so we should have a good reason if we want to change it.

The old code was the incompatible one; why should the Python
binding change things like this on you?  To be complete, we'd
have to wrap the functions that take that raw checksum
(e.g. svn_wc_process_committed*) to convert the hex string back
to the raw data; why convert it back and forth?  Python
applications can always use binascii.b2a_hex to get the
human-readable hex form.

-- 
Eric Gillespie <*> epg@pretzelnet.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Fix Python digest typemap

Posted by David James <ja...@cs.toronto.edu>.
On 5/4/07, Eric Gillespie <ep...@pretzelnet.org> wrote:
> I'm not sure where the new test really belongs.  Should i add a
> new file for high-level tests of multiple layers?  This one uses
> delta, ra, and wc.

No, this is fine. Generally, you should put tests in the file which is
most affected by the change. Great work on the test!

I have a question. Why do you think that the MD5 checksums should be
passed in raw format instead of in cstring format? Your change breaks
compatibility with any code which previously used the checksum return
value, so we should have a good reason if we want to change it.

Cheers,

David

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org