Code review comment for lp:~spiv/bzr/sprout-does-not-reopen-repo

I would almost agree with John about the practical aspect (it works, let's land).

On the other hand, you have already spent a good amount of time on this which will be necessary again if we want to provide a cleaner solution.

But...

267 registry,
268 + repository,
269 revision as _mod_revision,

seems weird. I'd like jelmer's feedback here, but I'm pretty sure we pull controldir out of bzrdir to make sure it was isolated from bzr specifics and having to reintroduce repository here sounds like a step backward.

Also,

10 + # This fallback is already configured. This probably only
11 + # happens because BzrDir.sprout is a horrible mess. To avoid
12 + # confusing _unstack we don't add this a second time.
13 + # XXX: log a warning, maybe?
14 + return

is a sign that you're close to nailing the issue and I think you should ;)

On the overall, we have a 'repository' attribute on the branch object, so having to provide it should only occur when we are creating this object. Do we have that many different ways to create branches that you end up modifying so many signatures ? Doesn't it reveal a deeper issue ?