[MERGE][RFC] switch for heavyweight checkouts

Ian Clatworthy ian.clatworthy at internode.on.net
Fri Dec 7 06:34:31 GMT 2007


John Arbash Meinel wrote:

> I'm happy to see switch for heavy checkouts implemented (as then I might
> actually start using it, we shall see, but then again, maybe I should keep
> paying the build_tree price so that I'm motivated to fix it).

Thanks for the review.

> Having thought about it a bit, I think I agree that to switch from a missing
> branch you should supply '--force'.
> 
> There is a subtle but important difference from "the branch moved, point at
> this new location" versus "I'm done with this, and want to work on something
> else." By silently ignoring a missing branch, the "I'm done with this" may not
> really mean that.

Done.

> ^- I'm very happy to see set_reference be a method/class method rather than
> having it in switch.py.
> 
> I didn't see any direct tests of it, though. A simple smoke test is enough
> (just so that if someone changes the api, deletes it, etc, it tells us).

Done.

> +            if not force and _any_local_commits(b, bound_branch):
> 
> ^- I would tend to not pass the url, and just have the helper use
> "b.get_master_branch()" instead.

Done.

> Also, should we be passing around possible transports?

Done. I've submitted the tweaks to pqm but I'd appreciate it if you
could double-check what I did here. I think my changes are safe but
perhaps I missed some API calls which would benefit from this.

> +            remote_graph = other_branch.repository.get_revision_graph(
> +                other_last_rev)
> +            if last_rev not in remote_graph:
> +                return True
> 
> ^- Using get_revision_graph() is slow. You should instead do:
> 
> graph = this_branch.repository.get_graph(other_branch.repository)
> if graph.is_ancestor(last_rev, other_last_rev):
>   return True
> 
> This can look at just the tip of the graph. And using "this.get_graph(other)"
> will allow us to use a StackedParentsProvider that allows us to pull revisions
> we have from the local side, rather than downloading all of the revision graph
> from the remote side.

Sweet. FWIW, I copied that code from bind() in branch.py. :-( It's a
little more complex but perhaps it could benefit in much the same way?

Ian C.



More information about the bazaar mailing list