r3118 exception in browser

[Alec Thomas]

The unicodification of configuration entries in r3118 seems to break the browser:

Traceback (most recent call last):
  File "/home/athomas/projects/trac/devel/trac/web/main.py", line 300, in dispatch_request
    dispatcher.dispatch(req)
  File "/home/athomas/projects/trac/devel/trac/web/main.py", line 176, in dispatch
    resp = chosen_handler.process_request(req)
  File "/home/athomas/projects/trac/devel/trac/versioncontrol/web_ui/browser.py", line 97, in process_request
    repos = self.env.get_repository(req.authname)
  File "/home/athomas/projects/trac/devel/trac/env.py", line 161, in get_repository
    authname)
  File "/home/athomas/projects/trac/devel/trac/versioncontrol/api.py", line 61, in get_repository
    return self._connector.get_repository(repos_type, repos_dir, authname)
  File "/home/athomas/projects/trac/devel/trac/versioncontrol/svn_fs.py", line 247, in get_repository
    repos = SubversionRepository(dir, authz, self.log)
  File "/home/athomas/projects/trac/devel/trac/versioncontrol/svn_fs.py", line 268, in __init__
    self.path = repos.svn_repos_find_root_path(path, self.pool())
TypeError: argument number 1: a 'char *' is expected, 'unicode(/home/athomas/projects/trac/env/svn)' is received

[Christian Boos]

Oops, but it's strange that I don't have a problem with this…

Can you try this fix:

svn_fs.py

@@ -265 +283 @@
         
         # Remove any trailing slash or else subversion might abort
         path = os.path.normpath(path).replace('\\', '/')
-        self.path = repos.svn_repos_find_root_path(path, self.pool())
+        self.path = repos.svn_repos_find_root_path(_to_svn(path), self.pool())
         if self.path is None:
             raise TracError("%s does not appear to be a Subversion repository." \
                             % path)

[Alec Thomas]

Now I don't get a traceback, but I get:

/home/athomas/projects/trac/env/svn does not appear to be a Subversion repository.

And yeah, it is strange that you don't see this.

[Christian Boos]

Weird, for me (on Windows, so far) it works for 1.2.3 (Python 2.3.5) and 1.3.0 (Python 2.4.2), whether I use a unicode string or not, for this call…

But this function (repos.svn_repos_find_root_path) is known to be a sensitive one, e.g. if you have a trailing slash there, then the application aborts.

Would you mind, er… hardcoding the value in svn_fs.py:

• first using '/home/athomas/projects/trac/env/svn'
• then using u'/home/athomas/projects/trac/env/svn'
• lastly using _to_svn(u'/home/athomas/projects/trac/env/svn')

Thanks!

[Alec Thomas]

Sure thing.

1. Worked.
2. Produced the original error.
3. Produced the does not appear to be a Subversion repository error.

I'm running Python 2.4.2 and SVN 1.2.3.

[Christian Boos]

Well, now all it takes is to understand the difference between 1. and 3. :)

Maybe adding a print type(x), repr(x) with x alternatively 1. and 3. will tell?

Ah, wait, maybe it's a weird gc related issue. Also try to do path = _to_svn(path) before calling repos.svn_repos_find_root_path(path, pool()).

As a side note, for me it also works on Linux w/ Python 2.4.2 and SVN 1.3.0.

[Alec Thomas]

Aha!!

1. <type 'str'> '/home/athomas/projects/trac/env/svn'
   
2. <type 'str'> 'home/athomas/projects/trac/env/svn'

Looks like _to_svn() is being a little too aggressive in its stripping of slashes. The following patch seems to fix the problem:

Index: trac/versioncontrol/svn_fs.py
===================================================================
--- trac/versioncontrol/svn_fs.py       (revision 3121)
+++ trac/versioncontrol/svn_fs.py       (working copy)
@@ -101,7 +101,7 @@
     """Expect a list of `unicode` path components.
     Returns an UTF-8 encoded string suitable for the Subversion python bindings.
     """
-    return '/'.join([path.strip('/') for path in args]).encode('utf-8')
+    return '/' + '/'.join([path.strip('/') for path in args]).encode('utf-8')
     
 def _from_svn(path):
     """Expect an UTF-8 encoded string and transform it to an `unicode` object"""
@@ -265,7 +265,7 @@
         
         # Remove any trailing slash or else subversion might abort
         path = os.path.normpath(path).replace('\\', '/')
-        self.path = repos.svn_repos_find_root_path(path, self.pool())
+        self.path = repos.svn_repos_find_root_path(_to_svn(path), self.pool())
         if self.path is None:
             raise TracError("%s does not appear to be a Subversion repository." \
                             % path)

It's not clear to me whether the same fix should be applied to _normalize_path(), which also strips slashes.

And with that, I've officially doubled my knowledge of the versioncontrol layer :)

[Christian Boos]

Actually that path is the only path that is a filesystem path, so its processing should be a little special (e.g. the volume letters on windows).

I think r3123 should fix this, but let me know.

[Alec Thomas]

Yep, works well for me.

[Alec Thomas]

No issues since this rev.