threads / discuss / 24222

Patching bad commits

Subject: Patching bad commits

## tl;dr

4 messages between Jun 28, 2010 and Jul 5, 2010.

replies: 3people: 4as markdown or json

Michael Witten· Jun 28, 2010, 22:03 UTC · lore

Sometimes bisecting is less than useful because a number of commits fail to build.

How about implementing something SIMILAR to "git notes" (perhaps even using "git notes") to store and apply patches for such bad commits?

For instance, bisecting could transparently apply any patches that are necessary to make a bad commit actually buildable; such patches could be distributed as usual for the benefit of others.

I'm thinking that a "patch note" could have 2 forms:
    * A plain patch file (or maybe a git tree of patches).
    * A reference to another commit that should be checked
      out instead.
Is this a worthwhile idea?

Sincerely, Michael Witten

Avery Pennarun· Jun 28, 2010, 22:23 UTC · re: Michael Witten · lore

Re: Patching bad commits

On Mon, Jun 28, 2010 at 6:03 PM, Michael Witten <mfwitten@gmail.com> wrote:
Show 6 quoted lines
> Sometimes bisecting is less than useful because a number of commits
> fail to build.
>
> How about implementing something SIMILAR to "git notes" (perhaps
> even using "git notes") to store and apply patches for such bad
> commits?

You could probably use the (relatively new) 'git replace' feature for this. Just replace the broken tree object with a new one that doesn't have the bug. I've never tried it, but it does seem like it should work.

That said, this idea would require you to manually patch every single commit that ever had a problem. I'm not sure there's much benefit to that compared to the overwhelming cost. I think it might be more useful - and much easier -to just have a permanent "always skip these commits when bisecting" list. (I don't think such a feature exists yet though.)

Btw, in recent versions of git, 'git bisect skip' is pretty smart and seeks around in the tree in such a way that it doesn't hurt so much when you have a batch of bad commits somewhere in the middle. So you may find that it's really not that bad to just use 'git bisect skip' occasionally if your version of git is relatively new and your history isn't absolutely littered with crap.

Speaking of littering your history with crap, the an ounce of prevention is worth a pound of cure: make sure all your commits pass unit tests before committing, and you'll rarely have this problem anyway :) That's obviously easier said than done however.

Have fun,
Avery
Antriksh Pany· Jul 5, 2010, 08:37 UTC · re: Avery Pennarun · lore

Re: Patching bad commits

On Tue, Jun 29, 2010 at 3:53 AM, Avery Pennarun <apenwarr@gmail.com> wrote:
> On Mon, Jun 28, 2010 at 6:03 PM, Michael Witten <mfwitten@gmail.com> wrote:
>> Sometimes bisecting is less than useful because a number of commits
>> fail to build.
>>
You could automate the process of skipping certain commits when build fails.
  make || git bisect skip
Automatically mark commits as bad when your test fails:
  ./test || git bisect bad
Otherwise 'git bisect good' is what you do.

I think you can put these together in a loop that works automatically without much user intervention.

Hope this helps.

Cheers Antriksh Pany

Sverre Rabbelier· Jun 28, 2010, 22:24 UTC · re: Michael Witten · lore

Re: Patching bad commits

Heya,
On Tue, Jun 29, 2010 at 00:03, Michael Witten <mfwitten@gmail.com> wrote:
> How about implementing something SIMILAR to "git notes" (perhaps
> even using "git notes") to store and apply patches for such bad
> commits?

Definitely use 'git notes' for this, I'd say this is a perfect use case for it. Just store the patches in their own notes ref and you're good to go.

> For instance, bisecting could transparently apply any patches that
> are necessary to make a bad commit actually buildable; such patches
> could be distributed as usual for the benefit of others.

Something like that should probably be guarded by a switch/configuration value, but that sounds very useful.

>    * A plain patch file (or maybe a git tree of patches).

I guess in this form 'git bisect' could just call 'git am' on that tree (regardless of whether it has 1 or more patches), which would apply them in the usual order? It might be useful to instead stream the contents of the note to 'git am', but I could imagine it being useful to have the patches on disk (so that you can edit them, or look at them when applying fails).

>    * A reference to another commit that should be checked
>      out instead.

I'm not sure how useful that is, if you're just going to check out another commit you might as well 'git bisect skip' the current one? Well, I guess if the other commit is outside the bisect range but is applied on top of the commit under inspection it makes sense. So makes sense, only problem I see is that if that commit is on another branch it might not be available locally.

> Is this a worthwhile idea?

I like this idea a lot, it's always bothered me that some commits can just come up during git bisect as being untestable. The tricky part will be to ensure that the patches you apply don't affect the bug ;).

-- 
Cheers,

Sverre Rabbelier

← back to recent threads