5 ms·

I, too used to wonder about this. But in git merge commits basically break bisect and revert. You can't tell git to only consider feature merges for it's bisect

by patrec 10y ago

I, too used to wonder about this. But in git merge commits basically break bisect and revert. You can't tell git to only consider feature merges for it's bisection (which is normally what you want), nor will revert on a merge commit behave the way people expect. And not everyone enjoys reading long essays about to work around these problems.

cesarb 10y ago
Could you elaborate? I find it hard to believe that merge commits break bisect, since git bisect was developed for use by the Linux kernel developers, and the Linux kernel repository uses merge commits heavily.
- stormbrew 10y ago
  As an advocate for both merges and bisect, even I have to admit that the experience of using bisect over merges can be quite terrible. I would prefer if bisect had a working --first-parent mode, because most of the time that's what I really want. Git-core seems against this, though. Not sure if it's on technical or aesthetic grounds.
- patrec 10y ago
  Let's assume you found a serious bug in production. Probably one of the recently landed feature branches contains the offending change. If you rebase feature branches into master as a single squashed commit, things are straightforward: you run a git bisect HEAD $LAST_KNOWN_GOOD and then you might revert the offending commit and re-deploy before you dig into the problem deeper and fix the bug. If you have a normal merge based workflow, your bisect will likely require some babysitting because git doesn't offer an easy way to just bisect your merge commits into master. Firstly of course there will also be many more commits to consider, most of which you don't really care about at this point and which will slow down the bisect. More importantly some of the intermediate commits from the feature branches will almost certainly be broken, because typically not every single commit even goes through CI. You can work around that by hacking something up to ignore these commits but it's probably gonna be a pain. Once you found the problematic feature you want to back out you then will spend half an hour googling how to revert merge commits. Of course, there are advantages to having the "intermediate" commits available and if git bisect were better designed the experience with merge branches should be strictly better, but in practice it's enough of a pain that people who semantically prefer merges opt for a rebase-based workflow.
  - cyphar 10y ago
    > More importantly some of the intermediate commits from the feature branches will almost certainly be broken, because typically not every single commit even goes through CI. You can work around that by hacking something up to ignore these commits but it's probably gonna be a pain. git bisect skip. Also, if your maintainers aren't doing a `git rebase -x "make test" origin/master` before merging then you're the ones to blame. Not merge commits. > Once you found the problematic feature you want to back out you then will spend half an hour googling how to revert merge commits. `git revert <merge commit>`. Or if by "revert" you mean "change the history to the state it used to be in" you can do `git reset --hard <old HEAD using git log --first-parent>`. > Of course, there are advantages to having the "intermediate" commits available and if git bisect were better designed the experience with merge branches should be strictly better, but in practice it's enough of a pain that people who semantically prefer merges opt for a rebase-based workflow. git bisect is actually quite well designed considering it works given any two commits (even if the histories are different). To be fair, it doesn't handle histories where a bug is fixed and broken multiple times -- but there's no nice way of dealing with that (it's a hard problem to solve without iterating through each commit).
barrkel 10y ago
Merge commits don't break bisect or revert. You may revert to a DAG node that develop (or whatever your source branch) never pointed to, but that's just the reality of developing on branches. Bisect works over a linearization of the DAG, and may similarly test intermediate states that don't make sense. But again, it just reflects the reality of a team of developers working concurrently, sometimes on the same branch, sometimes on different branches.
JoshTriplett 10y ago
Merge commits don't break bisect. People who submit pull requests that don't build, run, and pass tests at every commit break bisect. Rebasing doesn't fix that problem. Don't build a series of commits where a later one fixes an earlier one; organize them into a logical series of changes, such that the project works after each commit.
- falsedan 10y ago
  That's too much work to guard against a rare occurrence. Most junior engineers would think that that workflow is excessively fussy and belt-and-braces (because they haven't seen how useful bisect is).
  - JoshTriplett 10y ago
    I'd love to see some of the major CI systems (like Travis, Appveyor, and Gitlab CI) supporting "build and test every commit" rather than "build and test the final commit".
    - patrec 10y ago
      Agreed, although to do this at acceptable cost you'll often also want a decent build system (a la blaze, that can rebuild and re-test just what needs rebuilding and re-testing) and a lot of CI slaves. That's not a typical setup. Right now build systems don't even do the "build and test the final commit" bit right; it should of course test against a pre-merge (not on the branch) but again that tends to be a pain to set up.
- patrec 10y ago
  In practice rebase heavy-workflows squash a lot of commits away; in fact I think the standard workflow with phabricator is sqashing everything in the feature branch into a single commit and then rebasing that in. This has obvious downsides, but it means it's easy to revert and bisect works, which probably does in many cases outweight the downsides.
  - 10y ago

by patrec 10y ago

cesarb 10y ago

Could you elaborate? I find it hard to believe that merge commits break bisect, since git bisect was developed for use by the Linux kernel developers, and the Linux kernel repository uses merge commits heavily.

stormbrew 10y ago
As an advocate for both merges and bisect, even I have to admit that the experience of using bisect over merges can be quite terrible. I would prefer if bisect had a working --first-parent mode, because most of the time that's what I really want. Git-core seems against this, though. Not sure if it's on technical or aesthetic grounds.
patrec 10y ago
Let's assume you found a serious bug in production. Probably one of the recently landed feature branches contains the offending change. If you rebase feature branches into master as a single squashed commit, things are straightforward: you run a git bisect HEAD $LAST_KNOWN_GOOD and then you might revert the offending commit and re-deploy before you dig into the problem deeper and fix the bug. If you have a normal merge based workflow, your bisect will likely require some babysitting because git doesn't offer an easy way to just bisect your merge commits into master. Firstly of course there will also be many more commits to consider, most of which you don't really care about at this point and which will slow down the bisect. More importantly some of the intermediate commits from the feature branches will almost certainly be broken, because typically not every single commit even goes through CI. You can work around that by hacking something up to ignore these commits but it's probably gonna be a pain. Once you found the problematic feature you want to back out you then will spend half an hour googling how to revert merge commits. Of course, there are advantages to having the "intermediate" commits available and if git bisect were better designed the experience with merge branches should be strictly better, but in practice it's enough of a pain that people who semantically prefer merges opt for a rebase-based workflow.
- cyphar 10y ago
  > More importantly some of the intermediate commits from the feature branches will almost certainly be broken, because typically not every single commit even goes through CI. You can work around that by hacking something up to ignore these commits but it's probably gonna be a pain. git bisect skip. Also, if your maintainers aren't doing a `git rebase -x "make test" origin/master` before merging then you're the ones to blame. Not merge commits. > Once you found the problematic feature you want to back out you then will spend half an hour googling how to revert merge commits. `git revert <merge commit>`. Or if by "revert" you mean "change the history to the state it used to be in" you can do `git reset --hard <old HEAD using git log --first-parent>`. > Of course, there are advantages to having the "intermediate" commits available and if git bisect were better designed the experience with merge branches should be strictly better, but in practice it's enough of a pain that people who semantically prefer merges opt for a rebase-based workflow. git bisect is actually quite well designed considering it works given any two commits (even if the histories are different). To be fair, it doesn't handle histories where a bug is fixed and broken multiple times -- but there's no nice way of dealing with that (it's a hard problem to solve without iterating through each commit).

barrkel 10y ago

Merge commits don't break bisect or revert. You may revert to a DAG node that develop (or whatever your source branch) never pointed to, but that's just the reality of developing on branches. Bisect works over a linearization of the DAG, and may similarly test intermediate states that don't make sense. But again, it just reflects the reality of a team of developers working concurrently, sometimes on the same branch, sometimes on different branches.

JoshTriplett 10y ago

Merge commits don't break bisect. People who submit pull requests that don't build, run, and pass tests at every commit break bisect. Rebasing doesn't fix that problem. Don't build a series of commits where a later one fixes an earlier one; organize them into a logical series of changes, such that the project works after each commit.

falsedan 10y ago
That's too much work to guard against a rare occurrence. Most junior engineers would think that that workflow is excessively fussy and belt-and-braces (because they haven't seen how useful bisect is).
- JoshTriplett 10y ago
  I'd love to see some of the major CI systems (like Travis, Appveyor, and Gitlab CI) supporting "build and test every commit" rather than "build and test the final commit".
  - patrec 10y ago
    Agreed, although to do this at acceptable cost you'll often also want a decent build system (a la blaze, that can rebuild and re-test just what needs rebuilding and re-testing) and a lot of CI slaves. That's not a typical setup. Right now build systems don't even do the "build and test the final commit" bit right; it should of course test against a pre-merge (not on the branch) but again that tends to be a pain to set up.
patrec 10y ago
In practice rebase heavy-workflows squash a lot of commits away; in fact I think the standard workflow with phabricator is sqashing everything in the feature branch into a single commit and then rebasing that in. This has obvious downsides, but it means it's easy to revert and bisect works, which probably does in many cases outweight the downsides.
- 10y ago