Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 1 | Date: Fri, 19 Dec 2008 00:45:19 -0800 |
| 2 | From: Linus Torvalds <torvalds@linux-foundation.org>, Junio C Hamano <gitster@pobox.com> |
| 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 | Message-ID: <7vocz8a6zk.fsf@gitster.siamese.dyndns.org> |
| 9 | References: <alpine.LFD.2.00.0812181949450.14014@localhost.localdomain> |
Thomas Ackermann | 1797e5c | 2012-10-16 19:25:29 +0200 | [diff] [blame] | 10 | Content-type: text/asciidoc |
| 11 | |
| 12 | How to revert a faulty merge |
| 13 | ============================ |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 14 | |
| 15 | Alan <alan@clueserver.org> said: |
| 16 | |
| 17 | I have a master branch. We have a branch off of that that some |
| 18 | developers are doing work on. They claim it is ready. We merge it |
| 19 | into the master branch. It breaks something so we revert the merge. |
| 20 | They make changes to the code. they get it to a point where they say |
| 21 | it is ok and we merge again. |
| 22 | |
| 23 | When examined, we find that code changes made before the revert are |
| 24 | not in the master branch, but code changes after are in the master |
| 25 | branch. |
| 26 | |
| 27 | and asked for help recovering from this situation. |
| 28 | |
| 29 | The history immediately after the "revert of the merge" would look like |
| 30 | this: |
| 31 | |
| 32 | ---o---o---o---M---x---x---W |
Philip Oakley | 6750f62 | 2016-10-24 22:54:32 +0100 | [diff] [blame] | 33 | / |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 34 | ---A---B |
| 35 | |
| 36 | where A and B are on the side development that was not so good, M is the |
| 37 | merge that brings these premature changes into the mainline, x are changes |
| 38 | unrelated to what the side branch did and already made on the mainline, |
| 39 | and W is the "revert of the merge M" (doesn't W look M upside down?). |
Ramsay Jones | 78bef06 | 2013-10-11 19:24:14 +0100 | [diff] [blame] | 40 | IOW, `"diff W^..W"` is similar to `"diff -R M^..M"`. |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 41 | |
| 42 | Such a "revert" of a merge can be made with: |
| 43 | |
| 44 | $ git revert -m 1 M |
| 45 | |
Mike Ralphson | a1070d4 | 2009-03-03 19:29:22 +0000 | [diff] [blame] | 46 | After the developers of the side branch fix their mistakes, the history |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 47 | may look like this: |
| 48 | |
| 49 | ---o---o---o---M---x---x---W---x |
Philip Oakley | 6750f62 | 2016-10-24 22:54:32 +0100 | [diff] [blame] | 50 | / |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 51 | ---A---B-------------------C---D |
| 52 | |
| 53 | where C and D are to fix what was broken in A and B, and you may already |
| 54 | have some other changes on the mainline after W. |
| 55 | |
| 56 | If you merge the updated side branch (with D at its tip), none of the |
Justin Lebar | a58088a | 2014-03-31 15:11:44 -0700 | [diff] [blame] | 57 | changes made in A or B will be in the result, because they were reverted |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 58 | by W. That is what Alan saw. |
| 59 | |
| 60 | Linus explains the situation: |
| 61 | |
| 62 | Reverting a regular commit just effectively undoes what that commit |
| 63 | did, and is fairly straightforward. But reverting a merge commit also |
| 64 | undoes the _data_ that the commit changed, but it does absolutely |
| 65 | nothing to the effects on _history_ that the merge had. |
| 66 | |
| 67 | So the merge will still exist, and it will still be seen as joining |
| 68 | the two branches together, and future merges will see that merge as |
| 69 | the last shared state - and the revert that reverted the merge brought |
| 70 | in will not affect that at all. |
| 71 | |
| 72 | So a "revert" undoes the data changes, but it's very much _not_ an |
| 73 | "undo" in the sense that it doesn't undo the effects of a commit on |
| 74 | the repository history. |
| 75 | |
| 76 | So if you think of "revert" as "undo", then you're going to always |
| 77 | miss this part of reverts. Yes, it undoes the data, but no, it doesn't |
| 78 | undo history. |
| 79 | |
| 80 | In such a situation, you would want to first revert the previous revert, |
| 81 | which would make the history look like this: |
| 82 | |
| 83 | ---o---o---o---M---x---x---W---x---Y |
Philip Oakley | 6750f62 | 2016-10-24 22:54:32 +0100 | [diff] [blame] | 84 | / |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 85 | ---A---B-------------------C---D |
| 86 | |
| 87 | where Y is the revert of W. Such a "revert of the revert" can be done |
| 88 | with: |
| 89 | |
| 90 | $ git revert W |
| 91 | |
| 92 | This history would (ignoring possible conflicts between what W and W..Y |
Justin Lebar | a58088a | 2014-03-31 15:11:44 -0700 | [diff] [blame] | 93 | changed) be equivalent to not having W or Y at all in the history: |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 94 | |
| 95 | ---o---o---o---M---x---x-------x---- |
Philip Oakley | 6750f62 | 2016-10-24 22:54:32 +0100 | [diff] [blame] | 96 | / |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 97 | ---A---B-------------------C---D |
| 98 | |
| 99 | and merging the side branch again will not have conflict arising from an |
| 100 | earlier revert and revert of the revert. |
| 101 | |
| 102 | ---o---o---o---M---x---x-------x-------* |
Philip Oakley | 6750f62 | 2016-10-24 22:54:32 +0100 | [diff] [blame] | 103 | / / |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 104 | ---A---B-------------------C---D |
| 105 | |
| 106 | Of course the changes made in C and D still can conflict with what was |
| 107 | done by any of the x, but that is just a normal merge conflict. |
| 108 | |
| 109 | On the other hand, if the developers of the side branch discarded their |
| 110 | faulty A and B, and redone the changes on top of the updated mainline |
| 111 | after the revert, the history would have looked like this: |
| 112 | |
| 113 | ---o---o---o---M---x---x---W---x---x |
Philip Oakley | 6750f62 | 2016-10-24 22:54:32 +0100 | [diff] [blame] | 114 | / \ |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 115 | ---A---B A'--B'--C' |
| 116 | |
| 117 | If you reverted the revert in such a case as in the previous example: |
| 118 | |
| 119 | ---o---o---o---M---x---x---W---x---x---Y---* |
Philip Oakley | 6750f62 | 2016-10-24 22:54:32 +0100 | [diff] [blame] | 120 | / \ / |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 121 | ---A---B A'--B'--C' |
| 122 | |
Mike Ralphson | a1070d4 | 2009-03-03 19:29:22 +0000 | [diff] [blame] | 123 | where Y is the revert of W, A' and B' are rerolled A and B, and there may |
Ramsay Jones | 78bef06 | 2013-10-11 19:24:14 +0100 | [diff] [blame] | 124 | also be a further fix-up C' on the side branch. `"diff Y^..Y"` is similar |
| 125 | to `"diff -R W^..W"` (which in turn means it is similar to `"diff M^..M"`), |
| 126 | and `"diff A'^..C'"` by definition would be similar but different from that, |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 127 | because it is a rerolled series of the earlier change. There will be a |
| 128 | lot of overlapping changes that result in conflicts. So do not do "revert |
| 129 | of revert" blindly without thinking.. |
| 130 | |
| 131 | ---o---o---o---M---x---x---W---x---x |
Philip Oakley | 6750f62 | 2016-10-24 22:54:32 +0100 | [diff] [blame] | 132 | / \ |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 133 | ---A---B A'--B'--C' |
| 134 | |
| 135 | In the history with rebased side branch, W (and M) are behind the merge |
| 136 | base of the updated branch and the tip of the mainline, and they should |
| 137 | merge without the past faulty merge and its revert getting in the way. |
| 138 | |
| 139 | To recap, these are two very different scenarios, and they want two very |
| 140 | different resolution strategies: |
| 141 | |
| 142 | - If the faulty side branch was fixed by adding corrections on top, then |
| 143 | doing a revert of the previous revert would be the right thing to do. |
| 144 | |
| 145 | - If the faulty side branch whose effects were discarded by an earlier |
| 146 | revert of a merge was rebuilt from scratch (i.e. rebasing and fixing, |
| 147 | as you seem to have interpreted), then re-merging the result without |
| 148 | doing anything else fancy would be the right thing to do. |
Marc Branchaud | b499549 | 2010-03-24 16:34:04 -0400 | [diff] [blame] | 149 | (See the ADDENDUM below for how to rebuild a branch from scratch |
| 150 | without changing its original branching-off point.) |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 151 | |
| 152 | However, there are things to keep in mind when reverting a merge (and |
| 153 | reverting such a revert). |
| 154 | |
| 155 | For example, think about what reverting a merge (and then reverting the |
| 156 | revert) does to bisectability. Ignore the fact that the revert of a revert |
| 157 | is undoing it - just think of it as a "single commit that does a lot". |
| 158 | Because that is what it does. |
| 159 | |
| 160 | When you have a problem you are chasing down, and you hit a "revert this |
| 161 | merge", what you're hitting is essentially a single commit that contains |
| 162 | all the changes (but obviously in reverse) of all the commits that got |
| 163 | merged. So it's debugging hell, because now you don't have lots of small |
| 164 | changes that you can try to pinpoint which _part_ of it changes. |
| 165 | |
| 166 | But does it all work? Sure it does. You can revert a merge, and from a |
Thomas Ackermann | 2de9b71 | 2013-01-21 20:17:53 +0100 | [diff] [blame] | 167 | purely technical angle, Git did it very naturally and had no real |
Nanako Shiraishi | a128a2c | 2008-12-20 20:27:02 +0900 | [diff] [blame] | 168 | troubles. It just considered it a change from "state before merge" to |
| 169 | "state after merge", and that was it. Nothing complicated, nothing odd, |
| 170 | nothing really dangerous. Git will do it without even thinking about it. |
| 171 | |
| 172 | So from a technical angle, there's nothing wrong with reverting a merge, |
| 173 | but from a workflow angle it's something that you generally should try to |
| 174 | avoid. |
| 175 | |
| 176 | If at all possible, for example, if you find a problem that got merged |
| 177 | into the main tree, rather than revert the merge, try _really_ hard to |
| 178 | bisect the problem down into the branch you merged, and just fix it, or |
| 179 | try to revert the individual commit that caused it. |
| 180 | |
| 181 | Yes, it's more complex, and no, it's not always going to work (sometimes |
| 182 | the answer is: "oops, I really shouldn't have merged it, because it wasn't |
| 183 | ready yet, and I really need to undo _all_ of the merge"). So then you |
| 184 | really should revert the merge, but when you want to re-do the merge, you |
| 185 | now need to do it by reverting the revert. |
Marc Branchaud | b499549 | 2010-03-24 16:34:04 -0400 | [diff] [blame] | 186 | |
| 187 | ADDENDUM |
| 188 | |
| 189 | Sometimes you have to rewrite one of a topic branch's commits *and* you can't |
| 190 | change the topic's branching-off point. Consider the following situation: |
| 191 | |
| 192 | P---o---o---M---x---x---W---x |
| 193 | \ / |
| 194 | A---B---C |
| 195 | |
| 196 | where commit W reverted commit M because it turned out that commit B was wrong |
| 197 | and needs to be rewritten, but you need the rewritten topic to still branch |
| 198 | from commit P (perhaps P is a branching-off point for yet another branch, and |
| 199 | you want be able to merge the topic into both branches). |
| 200 | |
| 201 | The natural thing to do in this case is to checkout the A-B-C branch and use |
| 202 | "rebase -i P" to change commit B. However this does not rewrite commit A, |
| 203 | because "rebase -i" by default fast-forwards over any initial commits selected |
| 204 | with the "pick" command. So you end up with this: |
| 205 | |
| 206 | P---o---o---M---x---x---W---x |
| 207 | \ / |
| 208 | A---B---C <-- old branch |
| 209 | \ |
| 210 | B'---C' <-- naively rewritten branch |
| 211 | |
| 212 | To merge A-B'-C' into the mainline branch you would still have to first revert |
| 213 | commit W in order to pick up the changes in A, but then it's likely that the |
| 214 | changes in B' will conflict with the original B changes re-introduced by the |
| 215 | reversion of W. |
| 216 | |
| 217 | However, you can avoid these problems if you recreate the entire branch, |
| 218 | including commit A: |
| 219 | |
| 220 | A'---B'---C' <-- completely rewritten branch |
| 221 | / |
| 222 | P---o---o---M---x---x---W---x |
| 223 | \ / |
| 224 | A---B---C |
| 225 | |
| 226 | You can merge A'-B'-C' into the mainline branch without worrying about first |
| 227 | reverting W. Mainline's history would look like this: |
| 228 | |
| 229 | A'---B'---C'------------------ |
| 230 | / \ |
| 231 | P---o---o---M---x---x---W---x---M2 |
| 232 | \ / |
| 233 | A---B---C |
| 234 | |
| 235 | But if you don't actually need to change commit A, then you need some way to |
Ralf Wildenhues | 22e5e58 | 2010-08-22 13:12:12 +0200 | [diff] [blame] | 236 | recreate it as a new commit with the same changes in it. The rebase command's |
Marc Branchaud | b499549 | 2010-03-24 16:34:04 -0400 | [diff] [blame] | 237 | --no-ff option provides a way to do this: |
| 238 | |
| 239 | $ git rebase [-i] --no-ff P |
| 240 | |
| 241 | The --no-ff option creates a new branch A'-B'-C' with all-new commits (all the |
| 242 | SHA IDs will be different) even if in the interactive case you only actually |
| 243 | modify commit B. You can then merge this new branch directly into the mainline |
| 244 | branch and be sure you'll get all of the branch's changes. |
| 245 | |
| 246 | You can also use --no-ff in cases where you just add extra commits to the topic |
| 247 | to fix it up. Let's revisit the situation discussed at the start of this howto: |
| 248 | |
| 249 | P---o---o---M---x---x---W---x |
| 250 | \ / |
| 251 | A---B---C----------------D---E <-- fixed-up topic branch |
| 252 | |
| 253 | At this point, you can use --no-ff to recreate the topic branch: |
| 254 | |
| 255 | $ git checkout E |
| 256 | $ git rebase --no-ff P |
| 257 | |
| 258 | yielding |
| 259 | |
| 260 | A'---B'---C'------------D'---E' <-- recreated topic branch |
| 261 | / |
| 262 | P---o---o---M---x---x---W---x |
| 263 | \ / |
| 264 | A---B---C----------------D---E |
| 265 | |
| 266 | You can merge the recreated branch into the mainline without reverting commit W, |
| 267 | and mainline's history will look like this: |
| 268 | |
| 269 | A'---B'---C'------------D'---E' |
| 270 | / \ |
| 271 | P---o---o---M---x---x---W---x---M2 |
| 272 | \ / |
| 273 | A---B---C |