On 2009-03-24 15:40:10 +0000, Catalin Marinas wrote:
> Anyway, with apply_treedif() there is no need for the
> temp_index_tree. In the updated patch below, I don't even call
> write_tree() as it isn't needed (to my understanding).
Yes. You're not interested in the result of the patch application, you
just want to know if it succeeded or not.
> Whatever is in the index after the check_merged() function doesn't
> correspond to any tree so it would need to be re-read.
It does correspond to _some_ tree, but as we have no particular reason
to expect that it might be a tree that we're going to need again
immediately, we can set temp_index_tree to None to force re-reading,
rather than pay the cost of write_tree.
When pushing patches, on the other hand, we have good reasons to
believe that the next tree we'll need in the index is the same (or at
least very close to) the one produced in the last merge. Consider the
case of popping a few patches, changing the message on the top patch,
and then pushing the patches back, for example.
> + def check_merged(self, patches):
> + """Return a subset of patches already merged."""
> + merged = []
> + self.temp_index.read_tree(self.stack.head.data.tree)
> + # The self.temp_index is modified by apply_treediff() so force
> + # read_tree() the next time merge() is used.
> + self.temp_index_tree = None
> + for pn in reversed(patches):
> + # check whether patch changes can be reversed in the current index
> + cd = self.patches[pn].data
> + if cd.is_nochange():
> + continue
> + try:
> + self.temp_index.apply_treediff(cd.tree, cd.parent.data.tree,
> + quiet = True)
> + merged.append(pn)
> + except git.MergeException:
> + pass
> + return merged
Some points here:
1. Check if temp_index_tree already contains the tree you want
instead of doing read_tree unconditionally. That is, change
self.temp_index.read_tree(self.stack.head.data.tree)
to
if self.stack.head.data.tree != self.temp_index_tree:
self.temp_index.read_tree(self.stack.head.data.tree)
self.temp_index_tree = self.stack.head.data.tree
2. If apply_treediff fails (raises MergeException), the index wasn't
modified at all (but I guess you knew that, since your code
relies on that fact), so there's no reason to set temp_index_tree
to None in that case. So in order to not clear temp_index_tree
unnecessarily in case none of the patches reverse-apply (a case I
personally encounter frequently, since I leave the -m flag on all
the time), move
self.temp_index_tree = None
to just after (or just before) "merged.append(pn)".
3. Why are empty patches considered not merged?
Other than that, this logic looks fine.
--
Karl Hasselström, kha@xxxxxxxxxxx
www.treskal.com/kalle
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
