[BUG][0.16 blocker] pull doesn't know about '_run_hooks'

[John Arbash Meinel]

John Arbash Meinel wrote:
> I'm trying to sort out the code in question, but I just did:
> 
> $ cd jam-integration
> $ bzr pull ../bzr.dev
> 
> and got (and a traceback):
> 
> bzr: ERROR: exceptions.TypeError: pull() got an unexpected keyword
> argument '_run_hooks'
> 
> I think the problem is that I'm using a checkout of a "bzr+ssh://"
> branch, and the "branch-hooks" updates weren't taught to the new smart
> server code.
> 
> It seems to be that the base implementation does not have "_run_hooks"
> as a parameter, and neither does "RemoteBranch".
> 
> Further, "BzrBranch" also has a "_hook_master" variable. I don't know
> how this should be passed around, since BzrBranch5 does *not* have that
> parameter.
> 
> I think the failure is that Branch.pull() seems to assume that the
> master is of the same format as current, so we expected that we could
> update the local with a private variable and pass it around.
> 
> However, we can't, because master may be a completely different branch
> (it could be an SVN branch, for example).
> 
> The *easy* fix is to just add "_run_hooks" on the RemoteBranch.pull(),
> but that doesn't seem like the correct fix.
> 
> Thoughts?
> 
> John
> =:->

One other thing I wanted to mention...

The reason I feel this is complicated, was I tried to think about how to
write a test for this.

And basically it means creating a local (heavy) checkout of an hpss
branch, and then trying to do local_branch.pull(another_branch). Since
that will trigger the master.pull() bug.

But I don't think we want to create that situation for testing every
function on Branch. Especially since it should also fail for SVNBranch,
which means we would need to test every branch format with a master
branch of the other format.

Which says to me that Branch.pull() is not using the correct api to
access master_branch.pull(). It should either:

1) Promote _run_hooks to being a public parameter, and thus have
interface tests for it in branch_implementations/*

2) Check that the master branch supports the flag, either by catching
the error (ugly ugly ugly), or by having a "supports pull hooks" flag.
And supporting the hook means you support disabling it through _run_hooks().

I don't know how to test this one, though. Maybe a
branch_implementations/* test which checks for "supports pull hooks" and
then verifies that all branches which have the support flag set also
support '_run_bzr'.

John
=:->