Summary

Calculate outgoing Mercurial changesets from current revision

Review Request #5066 — Created Dec. 7, 2013 and submitted Jan. 7, 2014, 3:26 p.m.

Information

Owner

indygreg

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

Previously, RBTools calculated outgoing Mercurial changesets by asking
Mercurial about *all* changesets on the current branch not in the
remote. While this works, it's not ideal. This querying is expensive
since it pulls in all changesets, not just ancestors of the eventual
changesets.

With this patch, we now pass "-r ." into hg outgoing. This limits the
lookup to changesets that are ancestors of the working copy. In some
repository configurations (such as my own, where I have bookmarks
tracking thousands of changesets that will never be pushed to the
remote), this drastically speeds up diff computation and makes RBTools
faster.

Testing Done

Thanks, this looks like a good change overall. Unfortunately, it's breaking a unit test (specifically, rbtools.clients.tests:MercurialClientTests.testDiffBranchDiverge). You can run this with nosetests:

$ nosetests -v rbtools.clients.tests:MercurialClientTests.testDiffBranchDiverge

According to the test failure, resulting diff contains more changes than the unit test expects.

Based on the bug post you tweeted earlier today, I don't know if you're up for looking into this
further and making changes. If not, we'll work to get it in.

indygreg Dec. 10, 2013, 6:22 p.m.

I can look into this.

I should know better than to submit a patch without running the tests. I should know better than to submit patches without corresponding tests ;)

chipx86 Dec. 10, 2013, 6:34 p.m.
```
No worries. Thanks!
```

Change Summary:

I updated the patch to make tests work again. Essentially, I retained the behavior of only consulting the current branch instead of all branches. I should have done this from the beginning.

I rebased my changes on top of since-landed refactors.

I added a bunch of rev= arguments to the API. While the only consumer of these functions is currently the "get revisions for len(revspec) == 0" case, it may seem like I could have just hardcoded rev='.' in the final function. However, having flexibility is nice. Besides, I /think/ computation of parent diffs (a feature I'd like to implement) will rely on this argument. If you strip things before commit, I won't feel bad.

Diff:

Revision 2 (+22 -10)

Show changes

rbtools/clients/mercurial.py

Ship it!

Looks great. Thanks!

chipx86 Jan. 7, 2014, 3:15 p.m.

One thing of note is that these changes are on master (staging area for the next 0.6 releases, which is where we're working to improve revision handling all around), whereas the current releases are from release-0.5.x. I can push these to master, but they won't make it into the 0.5.x releases without some massaging. Do you have a strong opinion on getting these into 0.5.x?

indygreg Jan. 7, 2014, 3:49 p.m.

Pushing them to 0.5 isn't high priority for me. I like the refactored code on master. But this patch is trivial enough to uplift to 0.5 if you feel it warrants it.

Status: Closed (submitted)

Change Summary:

Pushed to master (8022815)