|
| 1 | +Date: Fri, 19 Dec 2008 00:45:19 -0800 |
| 2 | +From: Linus Torvalds < [email protected]>, Junio C Hamano < [email protected]> |
| 3 | +Subject: Re: Odd merge behaviour involving reverts |
| 4 | +Abstract: Sometimes a branch that was already merged to the mainline |
| 5 | + is later found to be faulty. Linus and Junio give guidance on |
| 6 | + recovering from such a premature merge and continuing development |
| 7 | + after the offending branch is fixed. |
| 8 | + |
| 9 | + |
| 10 | + |
| 11 | + |
| 12 | + |
| 13 | + I have a master branch. We have a branch off of that that some |
| 14 | + developers are doing work on. They claim it is ready. We merge it |
| 15 | + into the master branch. It breaks something so we revert the merge. |
| 16 | + They make changes to the code. they get it to a point where they say |
| 17 | + it is ok and we merge again. |
| 18 | + |
| 19 | + When examined, we find that code changes made before the revert are |
| 20 | + not in the master branch, but code changes after are in the master |
| 21 | + branch. |
| 22 | + |
| 23 | +and asked for help recovering from this situation. |
| 24 | + |
| 25 | +The history immediately after the "revert of the merge" would look like |
| 26 | +this: |
| 27 | + |
| 28 | + ---o---o---o---M---x---x---W |
| 29 | + / |
| 30 | + ---A---B |
| 31 | + |
| 32 | +where A and B are on the side development that was not so good, M is the |
| 33 | +merge that brings these premature changes into the mainline, x are changes |
| 34 | +unrelated to what the side branch did and already made on the mainline, |
| 35 | +and W is the "revert of the merge M" (doesn't W look M upside down?). |
| 36 | +IOW, "diff W^..W" is similar to "diff -R M^..M". |
| 37 | + |
| 38 | +Such a "revert" of a merge can be made with: |
| 39 | + |
| 40 | + $ git revert -m 1 M |
| 41 | + |
| 42 | +After the develpers of the side branch fixes their mistakes, the history |
| 43 | +may look like this: |
| 44 | + |
| 45 | + ---o---o---o---M---x---x---W---x |
| 46 | + / |
| 47 | + ---A---B-------------------C---D |
| 48 | + |
| 49 | +where C and D are to fix what was broken in A and B, and you may already |
| 50 | +have some other changes on the mainline after W. |
| 51 | + |
| 52 | +If you merge the updated side branch (with D at its tip), none of the |
| 53 | +changes made in A nor B will be in the result, because they were reverted |
| 54 | +by W. That is what Alan saw. |
| 55 | + |
| 56 | +Linus explains the situation: |
| 57 | + |
| 58 | + Reverting a regular commit just effectively undoes what that commit |
| 59 | + did, and is fairly straightforward. But reverting a merge commit also |
| 60 | + undoes the _data_ that the commit changed, but it does absolutely |
| 61 | + nothing to the effects on _history_ that the merge had. |
| 62 | + |
| 63 | + So the merge will still exist, and it will still be seen as joining |
| 64 | + the two branches together, and future merges will see that merge as |
| 65 | + the last shared state - and the revert that reverted the merge brought |
| 66 | + in will not affect that at all. |
| 67 | + |
| 68 | + So a "revert" undoes the data changes, but it's very much _not_ an |
| 69 | + "undo" in the sense that it doesn't undo the effects of a commit on |
| 70 | + the repository history. |
| 71 | + |
| 72 | + So if you think of "revert" as "undo", then you're going to always |
| 73 | + miss this part of reverts. Yes, it undoes the data, but no, it doesn't |
| 74 | + undo history. |
| 75 | + |
| 76 | +In such a situation, you would want to first revert the previous revert, |
| 77 | +which would make the history look like this: |
| 78 | + |
| 79 | + ---o---o---o---M---x---x---W---x---Y |
| 80 | + / |
| 81 | + ---A---B-------------------C---D |
| 82 | + |
| 83 | +where Y is the revert of W. Such a "revert of the revert" can be done |
| 84 | +with: |
| 85 | + |
| 86 | + $ git revert W |
| 87 | + |
| 88 | +This history would (ignoring possible conflicts between what W and W..Y |
| 89 | +changed) be equivalent to not having W nor Y at all in the history: |
| 90 | + |
| 91 | + ---o---o---o---M---x---x-------x---- |
| 92 | + / |
| 93 | + ---A---B-------------------C---D |
| 94 | + |
| 95 | +and merging the side branch again will not have conflict arising from an |
| 96 | +earlier revert and revert of the revert. |
| 97 | + |
| 98 | + ---o---o---o---M---x---x-------x-------* |
| 99 | + / / |
| 100 | + ---A---B-------------------C---D |
| 101 | + |
| 102 | +Of course the changes made in C and D still can conflict with what was |
| 103 | +done by any of the x, but that is just a normal merge conflict. |
| 104 | + |
| 105 | +On the other hand, if the developers of the side branch discarded their |
| 106 | +faulty A and B, and redone the changes on top of the updated mainline |
| 107 | +after the revert, the history would have looked like this: |
| 108 | + |
| 109 | + ---o---o---o---M---x---x---W---x---x |
| 110 | + / \ |
| 111 | + ---A---B A'--B'--C' |
| 112 | + |
| 113 | +If you reverted the revert in such a case as in the previous example: |
| 114 | + |
| 115 | + ---o---o---o---M---x---x---W---x---x---Y---* |
| 116 | + / \ / |
| 117 | + ---A---B A'--B'--C' |
| 118 | + |
| 119 | +where Y is the revert of W, A' and B'are rerolled A and B, and there may |
| 120 | +also be a further fix-up C' on the side branch. "diff Y^..Y" is similar |
| 121 | +to "diff -R W^..W" (which in turn means it is similar to "diff M^..M"), |
| 122 | +and "diff A'^..C'" by definition would be similar but different from that, |
| 123 | +because it is a rerolled series of the earlier change. There will be a |
| 124 | +lot of overlapping changes that result in conflicts. So do not do "revert |
| 125 | +of revert" blindly without thinking.. |
| 126 | + |
| 127 | + ---o---o---o---M---x---x---W---x---x |
| 128 | + / \ |
| 129 | + ---A---B A'--B'--C' |
| 130 | + |
| 131 | +In the history with rebased side branch, W (and M) are behind the merge |
| 132 | +base of the updated branch and the tip of the mainline, and they should |
| 133 | +merge without the past faulty merge and its revert getting in the way. |
| 134 | + |
| 135 | +To recap, these are two very different scenarios, and they want two very |
| 136 | +different resolution strategies: |
| 137 | + |
| 138 | + - If the faulty side branch was fixed by adding corrections on top, then |
| 139 | + doing a revert of the previous revert would be the right thing to do. |
| 140 | + |
| 141 | + - If the faulty side branch whose effects were discarded by an earlier |
| 142 | + revert of a merge was rebuilt from scratch (i.e. rebasing and fixing, |
| 143 | + as you seem to have interpreted), then re-merging the result without |
| 144 | + doing anything else fancy would be the right thing to do. |
| 145 | + |
| 146 | +However, there are things to keep in mind when reverting a merge (and |
| 147 | +reverting such a revert). |
| 148 | + |
| 149 | +For example, think about what reverting a merge (and then reverting the |
| 150 | +revert) does to bisectability. Ignore the fact that the revert of a revert |
| 151 | +is undoing it - just think of it as a "single commit that does a lot". |
| 152 | +Because that is what it does. |
| 153 | + |
| 154 | +When you have a problem you are chasing down, and you hit a "revert this |
| 155 | +merge", what you're hitting is essentially a single commit that contains |
| 156 | +all the changes (but obviously in reverse) of all the commits that got |
| 157 | +merged. So it's debugging hell, because now you don't have lots of small |
| 158 | +changes that you can try to pinpoint which _part_ of it changes. |
| 159 | + |
| 160 | +But does it all work? Sure it does. You can revert a merge, and from a |
| 161 | +purely technical angle, git did it very naturally and had no real |
| 162 | +troubles. It just considered it a change from "state before merge" to |
| 163 | +"state after merge", and that was it. Nothing complicated, nothing odd, |
| 164 | +nothing really dangerous. Git will do it without even thinking about it. |
| 165 | + |
| 166 | +So from a technical angle, there's nothing wrong with reverting a merge, |
| 167 | +but from a workflow angle it's something that you generally should try to |
| 168 | +avoid. |
| 169 | + |
| 170 | +If at all possible, for example, if you find a problem that got merged |
| 171 | +into the main tree, rather than revert the merge, try _really_ hard to |
| 172 | +bisect the problem down into the branch you merged, and just fix it, or |
| 173 | +try to revert the individual commit that caused it. |
| 174 | + |
| 175 | +Yes, it's more complex, and no, it's not always going to work (sometimes |
| 176 | +the answer is: "oops, I really shouldn't have merged it, because it wasn't |
| 177 | +ready yet, and I really need to undo _all_ of the merge"). So then you |
| 178 | +really should revert the merge, but when you want to re-do the merge, you |
| 179 | +now need to do it by reverting the revert. |
0 commit comments