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

John Arbash Meinel john at arbash-meinel.com
Thu Dec 13 22:08:16 GMT 2007


John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
+                for revision in info.real_revisions:
+                    for parent_id in revision.parent_ids:
+                        if (parent_id not in info.real_revisions and
+                            not target_repo.has_revision(parent_id)):
+                            missing_revisions.append(parent_id)

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

I wish we had a way to check "has_revisions()" instead of just 
has_revision. But I wouldn't block this because of that. (I'm just 
pretty sure that lots of has_revision() calls is going to be pretty 
sub-optimal, especially with packs.)

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


+                if len(missing_revisions) > 0:
+                    target_branch = 
_mod_branch.Branch.open(self.target_branch)
+                    for missing_revision in missing_revisions:
+                        target_repo.fetch(target_branch.repository,
+                                          missing_revision)

^- Certainly this is a reason to get multi-fetch implemented. It is in 
my TODO list, but until then 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. And I'm guessing you 
don't expect many revisions to be missing.

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).


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.

Otherwise it seems fine.

For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C475D5BFA.6010701%40utoronto.ca%3E



More information about the bazaar mailing list