[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r27396 - in trunk/subversion: libsvn_ra_neon tests/cmdline

From: Eric Gillespie <epg_at_google.com>
Date: 2007-10-26 00:05:34 CEST

epg@tigris.org writes:

> Author: epg
> Date: Thu Oct 25 14:45:19 2007
> New Revision: 27396
>
> Log:
> Fix a little bug introduced in r27190.
>
> * subversion/libsvn_ra_neon/commit.c
> (get_version_url): Don't base RSRC version resource URL on parent if
> the revision numbers do not match.
>
> * subversion/tests/cmdline/copy_tests.py
> (url_to_non_existent_url_path): Match changed error message (see
> also r27215).

This bug doesn't show up via svn_client, obviously. I'm not
entirely clear as to why, but the proximate cause is that
svn_client_commit(rho) opens an ra connection at rho's parent
(G), not at the repository root.

svn ci triggers these resource URLs:
open_root /tmp3t-6oo/!svn/ver/4/public/projects/greek-tree/A/D/G
open_file rho 2 /tmp3t-6oo/!svn/ver/2/public/projects/greek-tree/A/D/G/rho

a commit to rho with ra opened at repository root triggers these:
open_root /tmp3t-6oo/!svn/ver/4/
open_directory ... 4 /tmp3t-6oo/!svn/ver/4/public
open_directory ... 4 /tmp3t-6oo/!svn/ver/4/public/projects
open_directory ... 4 /tmp3t-6oo/!svn/ver/4/public/projects/greek-tree
open_directory ... 4 /tmp3t-6oo/!svn/ver/4/public/projects/greek-tree/A
open_directory ... 4 /tmp3t-6oo/!svn/ver/4/public/projects/greek-tree/A/D
open_directory ... 4 /tmp3t-6oo/!svn/ver/4/public/projects/greek-tree/A/D/G
open_file .../rho 2 /tmp3t-6oo/!svn/ver/4/public/projects/greek-tree/A/D/G/rho

Note that r27190 caused us to open rho at 4 even though what we
have is 2; thus, we get a checksum failure when we try to apply
our text delta.

So, really, r27190 only fixed the authz issue for a narrow case.
Anyone doing what I was doing would run afoul of an authz rule
preventing me from writing to A/D but allowing me to write to
rho. I think we need to fix mod-dav-svn (or whoever) not to make
the assumption that open_directory means I'm going to modify the
directory; it should only reject me on the add_file,
add_directory, delete_entry, or change_dir_prop.

In reviewing my resource URL log for this message, it now occurs
to me that I can probably come up with a test case for
authz_tests that will highlight the problem. What if I change
authz_open_directory to try to commit a change to both A/B/lambda
and A/B/E/mu? Wouldn't that cause it to do this:

open_root A
open_directory A/B
open_directory A/B/E
open_file A/B/E/mu

and thus trigger the authz failure when it tries to open_dir A/B/E?

I'll take a look at that. In the meantime, you can apply this
patch (NOT TO BE COMMITTED) to see the failure.

Run (cd subversion/tests/cmdline && ./davautocheck.sh --no-tests)
and get the URL it prints and then run
TRAC_TESTS_URL=whatever python subversion/bindings/swig/python/tests/wc.py

This is the third or fourth time I've hacked ra-independence into
the Python binding test suite. This time, I tried to do it in a
bit less of a hacky way. I'm going to clean it up again and post
a proposal in a new thread, probably not today.

Index: subversion/bindings/swig/python/tests/trac/versioncontrol/tests/svn_fs.py
===================================================================
--- subversion/bindings/swig/python/tests/trac/versioncontrol/tests/svn_fs.py (revision 27391)
+++ subversion/bindings/swig/python/tests/trac/versioncontrol/tests/svn_fs.py (working copy)
@@ -17,6 +17,7 @@
 import sys
 import tempfile
 import unittest
+from errno import EEXIST
 from urllib import pathname2url
 
 try:
@@ -30,21 +31,24 @@
 from trac.versioncontrol import Changeset, Node
 from trac.versioncontrol.svn_fs import SubversionRepository
 
-REPOS_PATH = tempfile.mktemp("-trac-svnrepos")
-REPOS_URL = pathname2url(REPOS_PATH)
-if REPOS_URL.startswith("///"):
- # Don't add extra slashes if they're already present.
- # (This is important for Windows compatibility).
- REPOS_URL = "file:" + REPOS_URL
-else:
- # If the URL simply starts with '/', we need to add two
- # extra slashes to make it a valid 'file://' URL
- REPOS_URL = "file://" + REPOS_URL
+this_dir = os.path.dirname(__file__)
+REPOS_BASE_DIR = os.path.normpath(
+ os.path.join(this_dir, '..', '..', '..', '..', '..', '..', '..', 'tests',
+ 'cmdline', 'svn-test-work', 'repositories'))
+_REPOS_PATH = None
+_REPOS_URL = None
 
+def REPOS_PATH():
+ return _REPOS_PATH
+def REPOS_URL():
+ return _REPOS_URL
 
+
 class SubversionRepositoryTestSetup(TestSetup):
 
     def setUp(self):
+ global _REPOS_PATH, _REPOS_URL
+
         dumpfile = open(os.path.join(os.path.split(__file__)[0],
                                      'svnrepos.dump'))
 
@@ -52,20 +56,42 @@
         # ensure a fresh start.
         self.tearDown()
 
- r = repos.svn_repos_create(REPOS_PATH, '', '', None, None)
+ try:
+ os.makedirs(REPOS_BASE_DIR)
+ except OSError, e:
+ if e.errno != EEXIST:
+ raise
+ _REPOS_PATH = tempfile.mkdtemp(dir=REPOS_BASE_DIR, prefix="trac-tests-")
+ r = repos.svn_repos_create(_REPOS_PATH, '', '', None, None)
         repos.svn_repos_load_fs2(r, dumpfile, StringIO(),
                                 repos.svn_repos_load_uuid_default, '',
                                 0, 0, None)
 
+ url = os.environ.get('TRAC_TESTS_URL')
+ if url == None:
+ _REPOS_URL = pathname2url(_REPOS_PATH)
+ if _REPOS_URL.startswith("///"):
+ # Don't add extra slashes if they're already present.
+ # (This is important for Windows compatibility).
+ _REPOS_URL = "file:" + _REPOS_URL
+ else:
+ # If the URL simply starts with '/', we need to add two
+ # extra slashes to make it a valid 'file://' URL
+ _REPOS_URL = "file://" + _REPOS_URL
+ else:
+ _REPOS_URL = '/'.join([url, 'svn-test-work/repositories',
+ os.path.basename(_REPOS_PATH)])
+
+
     def tearDown(self):
- if os.path.exists(REPOS_PATH):
- repos.delete(REPOS_PATH)
+ if _REPOS_PATH != None and os.path.exists(_REPOS_PATH):
+ repos.delete(_REPOS_PATH)
 
 
 class SubversionRepositoryTestCase(unittest.TestCase):
 
     def setUp(self):
- self.repos = SubversionRepository(REPOS_PATH, None)
+ self.repos = SubversionRepository(_REPOS_PATH, None)
 
     def tearDown(self):
         self.repos = None
Index: subversion/bindings/swig/python/tests/wc.py
===================================================================
--- subversion/bindings/swig/python/tests/wc.py (revision 27391)
+++ subversion/bindings/swig/python/tests/wc.py (working copy)
@@ -1,11 +1,17 @@
-import unittest, os, tempfile, shutil, types, setup_path, binascii
+import unittest, os, tempfile, shutil, types, setup_path, binascii, re
 from svn import core, repos, wc, client
 from svn import delta, ra
 from libsvn.core import SubversionException
 
+import trac.versioncontrol.tests.svn_fs
 from trac.versioncontrol.tests.svn_fs import SubversionRepositoryTestSetup, \
   REPOS_PATH, REPOS_URL
 
+def run_svn(*args):
+ argv = map(str, args)
+ argv.insert(0, 'subversion/svn/svn -q --username jrandom --password rayjandom')
+ return os.system(' '.join(argv))
+
 class SubversionWorkingCopyTestCase(unittest.TestCase):
   """Test cases for the Subversion working copy layer"""
 
@@ -18,7 +24,7 @@
     SubversionRepositoryTestSetup().setUp()
 
     # Open repository directly for cross-checking
- self.repos = repos.open(REPOS_PATH)
+ self.repos = repos.open(REPOS_PATH())
     self.fs = repos.fs(self.repos)
 
     self.path = core.svn_path_canonicalize(tempfile.mktemp())
@@ -28,8 +34,7 @@
     rev = core.svn_opt_revision_t()
     rev.kind = core.svn_opt_revision_head
 
- client.checkout2(REPOS_URL, self.path, rev, rev, True, True,
- client_ctx)
+ run_svn('co', REPOS_URL(), self.path)
 
     self.wc = wc.adm_open3(None, self.path, True, -1, None)
 
@@ -126,7 +131,7 @@
       self.assert_(wc.check_wc(self.path) > 0)
 
   def test_get_ancestry(self):
- self.assertEqual([REPOS_URL, 12],
+ self.assertEqual([REPOS_URL(), 12],
                        wc.get_ancestry(self.path, self.wc))
 
   def test_status(self):
@@ -256,12 +261,96 @@
     self.assertEquals(entry.cmt_date,
                       core.svn_time_from_cstring(commit_info.date))
 
+ def test_commit_out_of_date(self):
+ readme_path = '%s/trunk/README.txt' % self.path
+ readme_rev = 2
+
+ # Close wc, backdate readme_path, and re-open wc.
+ wc.adm_close(self.wc)
+ run_svn('up', '-r', readme_rev, readme_path)
+ self.wc = wc.adm_open3(None, self.path, True, -1, None)
+
+ # Replace README.txt's contents.
+ fp = open(readme_path, 'w')
+ fp.write('test_commit_out_of_date\n')
+ fp.close()
+
+ # Setup ra_ctx.
+ ra.initialize()
+ callbacks = ra.Callbacks()
+ def open_tmp_file(pool):
+ (fd, fn) = tempfile.mkstemp(dir=self.path)
+ os.close(fd)
+ return fn
+ callbacks.open_tmp_file = open_tmp_file
+ svn_config_dir = os.path.expanduser('~/.subversion')
+ svn_config = core.svn_config_get_config(svn_config_dir)
+ providers = [
+ client.get_simple_provider(),
+ client.get_username_provider(),
+ ]
+ auth_baton = core.svn_auth_open(providers)
+ core.svn_auth_set_parameter(auth_baton,
+ core.SVN_AUTH_PARAM_CONFIG_DIR,
+ svn_config_dir)
+ core.svn_auth_set_parameter(auth_baton,
+ core.SVN_AUTH_PARAM_DEFAULT_USERNAME,
+ 'jrandom')
+ core.svn_auth_set_parameter(auth_baton,
+ core.SVN_AUTH_PARAM_DEFAULT_PASSWORD,
+ 'rayjandom')
+ core.svn_auth_set_parameter(auth_baton,
+ core.SVN_AUTH_PARAM_NON_INTERACTIVE,
+ '')
+ callbacks.auth_baton = auth_baton
+ svn_config_dir = os.path.expanduser('~/.subversion')
+ svn_config = core.svn_config_get_config(svn_config_dir)
+ ra_ctx = ra.open2(REPOS_URL(), callbacks, svn_config, None)
+
+ # Get commit editor.
+ def commit_cb(_commit_info, pool):
+ self.fail('Commit succeeded, should have been out of date.')
+ (editor, edit_baton) = ra.get_commit_editor2(ra_ctx, 'log message',
+ commit_cb,
+ None,
+ False)
+ try:
+ # Drive the commit.
+ try:
+ def driver_cb(parent, path, pool):
+ baton = editor.open_file(path, parent, readme_rev, pool)
+ adm_access = wc.adm_probe_retrieve(self.wc, readme_path, pool)
+ wc.transmit_text_deltas2(readme_path, adm_access, False, editor,
+ baton, pool)
+ return baton
+ delta.path_driver(editor, edit_baton, -1, ['trunk/README.txt'],
+ driver_cb)
+ editor.close_edit(edit_baton)
+ except SubversionException, e:
+ if e.apr_err in [
+ # ra-{local,svn} raise this
+ core.SVN_ERR_FS_TXN_OUT_OF_DATE,
+ # ra-neon (and serf?) raise this
+ core.SVN_ERR_FS_CONFLICT,
+ ]:
+ self.assert_('trunk/README.txt' in e.args[0])
+ else:
+ raise
+ except:
+ try:
+ editor.abort_edit(edit_baton)
+ except:
+ # We already have an exception in progress, not much we can do
+ # about this.
+ pass
+ raise
+
   def tearDown(self):
       wc.adm_close(self.wc)
       core.svn_io_remove_dir(self.path)
 
 def suite():
- return unittest.makeSuite(SubversionWorkingCopyTestCase, 'test')
+ return unittest.makeSuite(SubversionWorkingCopyTestCase, 'test_commit_o')
 
 if __name__ == '__main__':
     runner = unittest.TextTestRunner()

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Oct 26 00:05:50 2007

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.