threads / discuss / 21823

Marking commits as transitory for git bisect?

Subject: Marking commits as transitory for git bisect?

## tl;dr

4 messages between Dec 2, 2009 and Dec 2, 2009.

replies: 3people: 3as markdown or json

David Kastrup· Dec 2, 2009, 09:32 UTC · lore
Hi,

sometimes there are changes which would seem better to restructure into more than one commit, with a non-operative intermediate state.

What I am thinking of is something like
a) change an API (small but highly intricate patch warranting thorough
   line-by-line review to make sure it's fine)
b) adapt all existing callers (really large but utterly trivial patch)

Substructuring this change into two commits may be quite nicer for reviewing and following it.

Except that it breaks git bisect. If there was a way to mark a commit as non-interesting, something which does not necessarily need any new repo features but just a convention like automatically skipping commits that contain the literal string [skip bisect] in the commit message, that would be one way to implement basic functionality like that.

A more thorough approach might also warn against partial cherrypicks or rebases or merges applying just part of one such a combined change.

But the main point is the ability to keep git bisect working on commit combinations with deliberately non-operative transitory stage.

-- 
David Kastrup
Michael J Gruber· Dec 2, 2009, 10:44 UTC · re: David Kastrup · lore

Re: Marking commits as transitory for git bisect?

David Kastrup venit, vidit, dixit 02.12.2009 10:32:
Show 27 quoted lines
> 
> Hi,
> 
> sometimes there are changes which would seem better to restructure into
> more than one commit, with a non-operative intermediate state.
> 
> What I am thinking of is something like
> 
> a) change an API (small but highly intricate patch warranting thorough
>    line-by-line review to make sure it's fine)
> b) adapt all existing callers (really large but utterly trivial patch)
> 
> Substructuring this change into two commits may be quite nicer for
> reviewing and following it.
> 
> Except that it breaks git bisect.  If there was a way to mark a commit
> as non-interesting, something which does not necessarily need any new
> repo features but just a convention like automatically skipping commits
> that contain the literal string [skip bisect] in the commit message,
> that would be one way to implement basic functionality like that.
> 
> A more thorough approach might also warn against partial cherrypicks or
> rebases or merges applying just part of one such a combined change.
> 
> But the main point is the ability to keep git bisect working on commit
> combinations with deliberately non-operative transitory stage.
> 

A quick solution with current git would be "replace": Say, in A-B-C-D you want B and C to be considered an "atom" for bisection. So, "git replace" C by a commit C' which is B+C squashed and has A as its parent: A-C'-D.

Alternatively, if you want this to be distributed more easily and think of it at the time of committing, producing a DAG like

A--C'--D
 \    /
  B--C

with C' as the first parent of D may help during bisection. I.e., you keep the detailed history on the side branch and squash it together on the --first-parent-line, with C,C' being tree-same.

Cheers, Michael

David Kastrup· Dec 2, 2009, 11:01 UTC · re: Michael J Gruber · lore

Re: Marking commits as transitory for git bisect?

Michael J Gruber <git@drmicha.warpmail.net> writes:
Show 16 quoted lines
> David Kastrup venit, vidit, dixit 02.12.2009 10:32:
>> 
>> sometimes there are changes which would seem better to restructure into
>> more than one commit, with a non-operative intermediate state.
>> 
>> What I am thinking of is something like
>> 
>> a) change an API (small but highly intricate patch warranting thorough
>>    line-by-line review to make sure it's fine)
>> b) adapt all existing callers (really large but utterly trivial patch)
>
> A quick solution with current git would be "replace": Say, in
> A-B-C-D
> you want B and C to be considered an "atom" for bisection. So, "git
> replace" C by a commit C' which is B+C squashed and has A as its parent:
> A-C'-D.

That would lose B and C. The idea was to have this available publically.

Show 10 quoted lines
> Alternatively, if you want this to be distributed more easily and think
> of it at the time of committing, producing a DAG like
>
> A--C'--D
>  \    /
>   B--C
>
> with C' as the first parent of D may help during bisection. I.e., you
> keep the detailed history on the side branch and squash it together on
> the --first-parent-line, with C,C' being tree-same.

That would seem to be more or less accomplished by putting B-C on a side-branch, then merge that branch into A (C' basically being the merge commit). So the DAG would be something like

A------D
 \    /
  B--C

If B-C is a local branch, will their history make it through to a remote repository when you push D?

It seems like this workflow could possibly meet my criteria when one has push access to the repo in question. If one passes D upstream via git-format-patch, however, it would appear that just the equivalent of C' makes it upstream, with the B-C history staying local.

Not sure what will happen when upstream pulls D.
Further comments/insights?
-- 
David Kastrup
Junio C Hamano· Dec 2, 2009, 16:49 UTC · re: David Kastrup · lore

Re: Marking commits as transitory for git bisect?

David Kastrup <dak@gnu.org> writes:
> Further comments/insights?

Just make sure that you state in the log message of the commits in the series something like:

 - The way the series is split for easier review has to break X and Y
   and this commit does not even compile;
 - This is the second half of restructuring, and it compiles again;
 - Now A is fully restructured, test X works again, but Y is still broken;
 - This finally completes the series and everything works again.

for _human consumption_. Bisect is not the only thing that needs to know about these commits that are intentionally broken. The reviewers and people who test before accepting the history with these commits in their history need to know about the known breakages, too.

Because you are talking about breakages that are _known_ when you make the series, you can afford to follow this recommendation. Object replacement mechanism is _not_ the answer---it's whole point is that you can add it as an afterthought by (virtually) squashing the series into one.

This way, the initial reviewers and a person who happened to hit such an intentionally broken commit during bisect can deal with the situation exactly the same way. I.e. with "git log".

← back to recent threads