nosmart+http interacts badly with http redirects

[Jonathan Lange]

Since we changed the puller to use nosmart+http for all http branches, we've been getting errors like:
    PathNotChild: Path "http://pocoo.org/%7Emitsuhiko/bzr/ruty/.bzr/branch-format" is not a child of path "nosmart+http://pocoo.org/%7Emitsuhiko/bzr/ruty/"

See OOPS-917SMPM113 for a full stack trace.

Given the traceback and that only a few branches have been hit with this, I guess that only affects things with branch references or redirects.

Let's back out the nosmart+ thing and repull the affected branches.

[Michael Hudson-Doyle (mwhudson)]

Seems to be to do with http redirects.

[Michael Hudson-Doyle (mwhudson)]

I think this is a problem in bzr itself -- try 'bzr get nosmart+http://pocoo.org/~mitsuhiko/bzr/ruty' for example.

[Jonathan Lange (jml)]

Workaround submitted to PQM.

[Michael Hudson-Doyle (mwhudson)]

Having chatted a bit about this with Aaron in #bzr, I've got as far as seeing that the code in do_catching_redirections in bzrdir.py to see if the end of the path bzr is being redirected to is the same as the end of the path bzr asked for is a bit screwy.

Maybe transport.get should remember the relpath it was asked for and stash this on the redirect exception. Or something.

[Jonathan Lange (jml)]

The Launchpad workaround has been cherrypicked onto production.

[John A Meinel (jameinel)]

I believe Michael has proposed a change for it on the list, which is pending review and maybe some updates.

[Michael Hudson-Doyle (mwhudson)]

I don't remember submitting any patches for this!

[John A Meinel (jameinel)]

Yeah, I was remembering:
http://thread.gmane.org/gmane.comp.version-control.bazaar-ng.general/44276

bug #129307

I was remembering the wrong thing. (That also has to do with redirection handling and nosmart, though in a different way.)

I guess I'm a bit confused as to what the specific issue is for bzr, then. Is it that the nosmart+ wrapper isn't able to prefix redirected requests? So they appear to be separate URLs?

[Michael Hudson-Doyle (mwhudson)]

Yeah, I don't think it's anything to do with nosmart+ specifically -- "bzr get readonly+http://pocoo.org/~mitsuhiko/bzr/ruty" fails in the same way.

I'm also a bit confused about what the core issue is, but something that definitely should work definitely doesn't, so I think that's a Confirmed rather than Incomplete bug.

[Vincent Ladeuil (vila)]

Long story short: decorators are incompatible with redirects.

Long story:
- only http is concerned by redirects (obviously)
- an http server can redirect urls to anything (not only http), but AFAIK, it will never add back bzr decorators (so, for example, using nosmart+ disable the hpss probing but when redirection occurs, the probing will be done again since we now use a simple http:// scheme)
- bzr never follows redirections *except* in bzrdir.py
- bzrdir redirection handling was meant to be used only at *first* access (which was for the branch format file (which explains the otherwise weird e.target.endswith(relpath)) all other uses are undefined

This should be fixed.

Until then, redirection works if you're lucky (and we have been lucky for a long time)

The FIXME in that part of the code is about the fact that decorators are lost here, which, at the time, meant that an http+urllib, when redirected became an http+pycurl because pycurl was (and still is) the default http transport returned by get_transport(). This now applies equally well for nosmart+ and readonly+ or any decorator: they are lost and their associated semantic with them.

[Vincent Ladeuil (vila)]

Fix available in associated branch, tests appreciated before I submit since I want to fix related bugs in the same submission ( I derogate to the usual one-bug-one-fix rule because several bugs are closely related).