git-format-patch-script bug?

I haven't had a chance to investigate this much yet but I have ran into a peculiar problem.

I was trying to help someone track down a bug that occurred between linux-2.6.12 and linux-2.6.13-rc1. Since it was very much an unknown where the problem was introduced I decided to run git format-patch so I could see what all of the differences were. Then to be certain the patch series worked I started applying them in order.

I didn't get very far when I had a patch conflict.

Looking deeper git-diff-tree is always making the diffs against the parent instead of against the next commit in the series. Which is largely sane. However it does not warn or fail when the parents and the next commit are different, which seems nonintuitive.

Eric

ebiederm@xmission.com (Eric W. Biederman) writes:

Sorry, I offhand do not have a good re-design of what format-patch should do for this purpose; the current design does not try to deal with anything but a linear sequence of commits, primarily because the command was done for preparing individual developer's patch submission.

I find your trying to find where the problem was introduced by reading every single change very laudable. However, for the purpose of "this one was good but somewhere before this one the things got broken, where is it?", I suspect that bisect would be a better fit.

The way to use bisect is very simple. First you make sure you do not have unrecorded changes in your working tree, because bisect procedure would check out sequences of commits for you to test there. Then, you give it two "known to be good" and "known to be bad" commits. The order you give them does not matter, but "good" one _must_ be ancestor of "bad" one (the code currently does not check this):

    $ git bisect start
    $ git bisect good v2.6.12
    $ git bisect bad  v2.6.13-rc1
    Bisecting: 1035 revisions left to test after this

As soon as ou give these two pair, bisect starts thinking, and checks out one revision that is roughly in the halfway between these two good and bad commits. You can, if you feel like, see which one is the first one it wants you to test, like this:

    $ git log --max-count=1 --pretty=short bisect
    commit 15d20bfd606c4b4454aeaa05fc86f77994e48c92
    Author: Domen Puncer <domen@coderock.org>

        [PATCH] ptrace_h8300: condition bugfix

At this point, your working tree has this commit checked out. Compile and test to see if this one is good or bad. If it is good, then you say "good":

    $ git bisect good
    Bisecting: 517 revisions left to test after this

Bisect eliminated about half commits from the original 1000+ suspects to be innocent, and checked out which one it wants you to check next. Compile and test to see if this one is good or bad. If it turns out to be bad, then you say "bad":

    $ git bisect bad
    Bisecting: 266 revisions left to test after this

You go on eliminating about half every time you run one test, and eventually it would find one commit that you may want to examine more deeply by code inspection.

Junio C Hamano <junkio@cox.net> writes:

What format-patch does is currently is fine. If format-patch would simply notice the case and fail gracefully that would be sufficient to avoid giving false impressions.

Depending on the quality of the list from git-rev-list --merge-order I should even be able to achieve what I was attempting, by running git-diff-tree with an ordered list of revisions and two parents on the tree.

Essentially I was attempting an export to quilt operation, which implies a linearization of the changes. As I recall a linearization of the changes is what BK export to CVS was built on. So that case may be worth returning to as there are a lot of interesting cases out there for it.

The other interest tid bit about my experiment was that git-format-patch-script on 2000 diffs was sluggish, certainly slower that the time to perform the write operations. Why it was slow I don't know but it may be worth looking into.

If you can teach the user how to use bisect, and git.  

If we could get gitweb to perform the bisect it would be helpful, or possibly when git settles down and everyone can be counted on to have git on their machine already, bisect would be a help.

Had I been thinking a little more clearly I would have suggested walking through the daily git snapshots as those are at least fairly big steps.

For the moment I think the unexpected behavior I found in git is more interesting then the problem I was actually trying to solve.

Eric

ebiederm@xmission.com (Eric W. Biederman) writes:

Hmph. Since it uses merge-order, We should be able to change it use the tagged output format of rev-list to detect the revision list discontinuity and skip generating diff for such. Or as you suggest just run "diff-tree with the first parent".

I've been wanting to update format-patch to take the commit begin-end pair in the rev-parse format, that is:

    $ git format-patch his..mine

I am reluctant to actually do this right away, because this is an incompatible change from the current format:

    $ git format-patch his mine

The same goes for rebase (and therefore cherry). I could use an ugly heuristics for backward compatibility like "if invoked with exactly two parameters, and there is no prefix ^ nor .. in these two, then use the old interpretation, otherwise give them to rev-parse", but I think this is ugly.

So my question to the list is: do people mind this change?