[MERGE] Merge directives can now fetch prerequisites from the target branch

[Aaron Bentley]

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

John Arbash Meinel wrote:
> John Arbash Meinel has voted tweak.
> Status is now: Conditionally approved

> ^- info.real_revisions is a plain list. So if we are installing a lot of
> revisions, this will get really slow.

Okay, I'll set-ify it.

> (I'm just
> pretty sure that lots of has_revision() calls is going to be pretty
> sub-optimal, especially with packs.)

With knits, I think it has the same performance characteristics as dict
access.  I guess in theory, we could implement has_revisions using
Graph.get_parents()/get_parents_map()

> Also, considering that a given parent may be referenced more than once,
> doesn't it make sense to make 'missing_revisions' a set?

Sure.

> +                    for missing_revision in missing_revisions:
> +                        target_repo.fetch(target_branch.repository,
> +                                          missing_revision)
> 
> ^- Certainly this is a reason to get multi-fetch implemented.

I'm not sure what you mean by "multi-fetch", but if you mean "fetch with
multiple target revisions", I'm definitely in favour.

> this is also going to play havoc with
> progress bars, and I think in general perform rather sub-optimally. I
> don't know that we have a better api at this point.

Well, maybe we could generate a bundle :-)

> And I'm guessing you
> don't expect many revisions to be missing.

Very much so.

> One way we could at least soften the blow is if we could do a
> topological sort, and then fetch from the children first. Since that
> would mean the parents would already be present, and the fetch() could
> be more of a no-op.
> 
> Just because of how things are entered into the pack file. Would
> reversed(missing_revisions) be sufficient for that? (I think revisions
> in a pack file are added in basic topological order, so they should
> refer to revisions in at least approximate topo order).

Yes, revisions in a v4 bundle are topologically sorted, so that ought to
work.  It wouldn't even be hard to implement a ParentsProvider on
bundles.  (The wonder of tiny interfaces!)

> However, I think you might have repeated the same code 2x. I think you
> did it one time using LBYL, and then rewrote it with EAFP and forgot to
> remove the LBYL.

Eeek.  Yes.  Thank you ever so much for pointing that out.

Thanks for your review.  I'll polish and submit.

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

iD8DBQFHYdM20F+nu1YWqI0RAkRLAJ4v6VJsty7FBSqRJz1pbS1Q8smK2wCfYMy3
RaI2ncY/n8qV1AxQvnmz1aU=
=r10O
-----END PGP SIGNATURE-----