git-format-patch-script bug?

git-format-patch-script bug?

[Eric W. Biederman]

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

Re: git-format-patch-script bug?

[Junio C Hamano]

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

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

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.

Re: git-format-patch-script bug?

[Eric W. Biederman]

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

> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> 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.  
>
> 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.

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.

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

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

Re: git-format-patch-script bug?

[Junio C Hamano]

ebiederm@xmission.com (Eric W. Biederman) 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.

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?

Re: git-format-patch-script bug?

[Marco Costalba]

Junio C Hamano wrote:

>
>I am reluctant to actually do this right away, because this is
>an incompatible change from the current format:
>
>    $ git format-patch his mine
>


Of course this breaks qgit interface to git-format-patch-script
but if you think it's better this way....


>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?
>

I think it's ugly too, in this early phase of git development 
better go with proper solution then compatibility compromises.

A suggestion I would like to present is if can be useful a
kind of scheduling/list of planned compatibility break features so
 that developers can know in advance when and what will break
 their stuff and users can know when they will need to upgrade.







		
____________________________________________________
Start your day with Yahoo! - make it your home page 
http://www.yahoo.com/r/hs