Merge lp:~mnordhoff/loggerhead/380026-fix-serving-branches into lp:loggerhead

[Matt Nordhoff]

[Matt Nordhoff (mnordhoff)]

Serving branches over HTTP had gotten broken when Loggerhead was converted to use transports. This fixes it. (See bug #380026.)

Jelmer proposed an alternate fix 5 days ago in lp:~jelmer/loggerhead/fix-bzrserve, but I didn't notice until after I had written this.

I like my fix better, though, since it returns 404 Not Found instead of a traceback when someone tries to access .bzr/ on a remote branch.

[Matt Nordhoff (mnordhoff)]

...OTOH, Jelmer included a nice TODO comment.

[Matt Nordhoff (mnordhoff)]

Oh, I forgot about one bug I noticed in my fix:

When serving a remote branch, the 404 responses don't get logged properly. It just shows:

DEBUG:paste.httpserver.ThreadPool:Added task (0 tasks queued)
DEBUG:paste.httpserver.ThreadPool:Added task (0 tasks queued)
DEBUG:paste.httpserver.ThreadPool:Added task (0 tasks queued)
DEBUG:paste.httpserver.ThreadPool:Added task (0 tasks queued)

How should I fix that?

[Michael Hudson-Doyle (mwhudson)]

Looks good, and fixes the bug.

The 404s not being logged thing is strange, but happens for other 404s too -- so probably isn't a problem with this branch.

Please land.

review: Approve

=== modified file 'loggerhead/apps/transport.py'
--- loggerhead/apps/transport.py	2009-05-04 20:28:50 +0000
+++ loggerhead/apps/transport.py	2009-05-24 19:14:44 +0000
@@ -82,8 +82,13 @@
         elif environ['PATH_INFO'] == '/favicon.ico':
             return favicon_app(environ, start_response)
         elif '/.bzr/' in environ['PATH_INFO']:
-            app = urlparser.make_static(None, self.transport)
-            return app(environ, start_response)
+            try:
+                path = urlutils.local_path_from_url(self.transport.base)
+            except errors.InvalidURL:
+                raise httpexceptions.HTTPNotFound()
+            else:
+                app = urlparser.make_static(None, path)
+                return app(environ, start_response)
         else:
             return BranchesFromTransportServer(
                 self.transport, self)(environ, start_response)