[MERGE] HTTP redirection

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
> After discussions with Robert at NlSprint, John on IRC and Robert
> again on IRC, here is a new attempt at http redirections.

I think this is very close to mergeable.

> - By default http do not redirect silently but instead raise the
>   RedirectedRequest exception,

I'm a little sad to see that removed, because I use transports elsewhere
(e.g. 'bzr patch' from bzrtools) and I'll have to manually handle
redirects now.

> There is one detail I didn't implement: when a redirection
> occurs, we get a new url from the http server. If the original
> transport have a qualifier (+urllib or +pycurl or chroot+ for
> example) and the redirected url use the same scheme that the
> original transport, we should apply it to the new transport. But
> I couldn't find a simple way to take into account all schemes,
> all qualifiers and all decorators.

I see your point here.

>          """
> -        format = BzrDirFormat.find_format(transport)
> +        initial_base = transport.base
> +        redirected = True # to enter the loop
> +        redirections = 0
> +        # If a loop occurs, there is little we can do. So we
> +        # don't try to detect them, just getting out if too much
> +        # redirections occurs. The solution is outside: where the
> +        # loop is defined.
> +        while redirected and redirections < BzrDir.MAX_REDIRECTIONS:
> +            try:
> +                format = BzrDirFormat.find_format(transport)
> +                redirected = False
> +            except errors.RedirectRequested, e:
> +                qualified_source = e.get_source_url()
> +                relpath = transport.relpath(qualified_source)
> +                if not e.target.endswith(relpath):
> +                    # Not redirected to a branch-format, not a branch
> +                    raise errors.NotBranchError(path=e.target)
> +                target = e.target[:-len(relpath)]
> +                note('%s is%s redirected to %s',
> +                     transport.base, e.permanently, target)
> +                # Let's try with a new transport 
> +                qualified_target = e.get_target_url()[:-len(relpath)]
> +                # FIXME: If 'transport' have a qualifier, this
> +                # should be applied again to the new transport
> +                # *iff* the schemes used are the same. It's a bit
> +                # tricky to verify, so I'll punt for now 
> +                # -- vila20070212
> +                transport = get_transport(target)
> +                redirections += 1
> +        if redirected:
> +            # Out of the loop and still redirected ? Either the
> +            # user have kept a very very very old reference to a
> +            # branch or a loop occured in the redirections.
> +            # Nothing we can cure here: tell the user. Note that
> +            # as the user have been informed about each
> +            # redirection, there is no need to issue an
> +            # additional error message.
> +            raise errors.NotBranchError(path=initial_base)

I think this would be more clearly handled as a for..else loop:

for x in range(BzrDir.MAX_REDIRECTIONS):
    try:
        format = BzrDirFormat.find_format(transport)
        break
    except errors.RedirectRequested, e
        qualified_source = e.get_source_url()
        relpath = transport.relpath(qualified_source)
        if not e.target.endswith(relpath):
        # Not redirected to a branch-format, not a branch
        raise errors.NotBranchError(path=e.target)
        target = e.target[:-len(relpath)]
        note('%s is%s redirected to %s',
        transport.base, e.permanently, target)
        # Let's try with a new transport
        qualified_target = e.get_target_url()[:-len(relpath)]
        # FIXME: If 'transport' have a qualifier, this
        # should be applied again to the new transport
        # *iff* the schemes used are the same. It's a bit
        # tricky to verify, so I'll punt for now
        # -- vila20070212
        transport = get_transport(target)
else:
    # Loop exited without resolving redirect ? Either the
    # user has kept a very very very old reference to a
    # branch or a loop occured in the redirections.
    # Nothing we can cure here: tell the user. Note that
    # as the user has been informed about each
    # redirection, there is no need to issue an
    # additional error message.
    raise errors.NotBranchError(path=initial_base)
BzrDir._check_supported(format, _unsupported)
return format.open(transport, _found=True)

While this was exactly what for...else was designed for, you might just
exit early in the try clause.

> @@ -1086,7 +1128,8 @@
>      def probe_transport(klass, transport):
>          """Return the .bzrdir style transport present at URL."""
>          try:
> -            format_string = transport.get(".bzr/branch-format").read()
> +            format_file = transport.get(".bzr/branch-format")
> +            format_string = format_file.read()

^^^ I don't understand this change.

> -# register BzrDirFormat as a control format
> -BzrDirFormat.register_control_format(BzrDirFormat)
> -
> -
>  class BzrDirFormat4(BzrDirFormat):
>      """Bzr dir format 4.
>  
> @@ -1475,6 +1514,11 @@
>      repository_format = property(__return_repository_format, __set_repository_format)
>  
>  
> +# register BzrDirMetaFormat1 as a control format. Our only
> +# concrete control format (BzrDirFormat is an abstract one).
> +BzrDirFormat.register_control_format(BzrDirMetaFormat1)
> +
> +# Register bzr formats
>  BzrDirFormat.register_format(BzrDirFormat4())
>  BzrDirFormat.register_format(BzrDirFormat5())
>  BzrDirFormat.register_format(BzrDirFormat6())

Comment ("our only concrete control format") seems to be contradicted by
registering BzrDirFormat4, 5, 6.

I think we should be testing that redirecting bzrdirs works with
BzrDir.open, i.e. that the resulting transport base takes the redirects
into account.  It probably makes sense to test open,
open_from_transport, open_containing_from_transport, and open_containing.

We should make a clear statement about how open_containing works: does
it follow the initial redirect, or does it keep on altering the initial
url until BzrDir.open succeeds (by following a redirect to a real branch)?

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFF0dZA0F+nu1YWqI0RAqP4AJ9D+JmODynM+8hSQ+ufz9+JLDWYqQCeO0zk
2220ZK7O9s+lW5hzdMvTzS8=
=9KfQ
-----END PGP SIGNATURE-----