]>
Commit | Line | Data |
---|---|---|
a128a2cd NS |
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> | |
10 | ||
11 | Alan <alan@clueserver.org> said: | |
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 | ||
a1070d4c | 42 | After the developers of the side branch fix their mistakes, the history |
a128a2cd NS |
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 | ||
a1070d4c | 119 | where Y is the revert of W, A' and B' are rerolled A and B, and there may |
a128a2cd NS |
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. | |
b4995494 MB |
145 | (See the ADDENDUM below for how to rebuild a branch from scratch |
146 | without changing its original branching-off point.) | |
a128a2cd NS |
147 | |
148 | However, there are things to keep in mind when reverting a merge (and | |
149 | reverting such a revert). | |
150 | ||
151 | For example, think about what reverting a merge (and then reverting the | |
152 | revert) does to bisectability. Ignore the fact that the revert of a revert | |
153 | is undoing it - just think of it as a "single commit that does a lot". | |
154 | Because that is what it does. | |
155 | ||
156 | When you have a problem you are chasing down, and you hit a "revert this | |
157 | merge", what you're hitting is essentially a single commit that contains | |
158 | all the changes (but obviously in reverse) of all the commits that got | |
159 | merged. So it's debugging hell, because now you don't have lots of small | |
160 | changes that you can try to pinpoint which _part_ of it changes. | |
161 | ||
162 | But does it all work? Sure it does. You can revert a merge, and from a | |
163 | purely technical angle, git did it very naturally and had no real | |
164 | troubles. It just considered it a change from "state before merge" to | |
165 | "state after merge", and that was it. Nothing complicated, nothing odd, | |
166 | nothing really dangerous. Git will do it without even thinking about it. | |
167 | ||
168 | So from a technical angle, there's nothing wrong with reverting a merge, | |
169 | but from a workflow angle it's something that you generally should try to | |
170 | avoid. | |
171 | ||
172 | If at all possible, for example, if you find a problem that got merged | |
173 | into the main tree, rather than revert the merge, try _really_ hard to | |
174 | bisect the problem down into the branch you merged, and just fix it, or | |
175 | try to revert the individual commit that caused it. | |
176 | ||
177 | Yes, it's more complex, and no, it's not always going to work (sometimes | |
178 | the answer is: "oops, I really shouldn't have merged it, because it wasn't | |
179 | ready yet, and I really need to undo _all_ of the merge"). So then you | |
180 | really should revert the merge, but when you want to re-do the merge, you | |
181 | now need to do it by reverting the revert. | |
b4995494 MB |
182 | |
183 | ADDENDUM | |
184 | ||
185 | Sometimes you have to rewrite one of a topic branch's commits *and* you can't | |
186 | change the topic's branching-off point. Consider the following situation: | |
187 | ||
188 | P---o---o---M---x---x---W---x | |
189 | \ / | |
190 | A---B---C | |
191 | ||
192 | where commit W reverted commit M because it turned out that commit B was wrong | |
193 | and needs to be rewritten, but you need the rewritten topic to still branch | |
194 | from commit P (perhaps P is a branching-off point for yet another branch, and | |
195 | you want be able to merge the topic into both branches). | |
196 | ||
197 | The natural thing to do in this case is to checkout the A-B-C branch and use | |
198 | "rebase -i P" to change commit B. However this does not rewrite commit A, | |
199 | because "rebase -i" by default fast-forwards over any initial commits selected | |
200 | with the "pick" command. So you end up with this: | |
201 | ||
202 | P---o---o---M---x---x---W---x | |
203 | \ / | |
204 | A---B---C <-- old branch | |
205 | \ | |
206 | B'---C' <-- naively rewritten branch | |
207 | ||
208 | To merge A-B'-C' into the mainline branch you would still have to first revert | |
209 | commit W in order to pick up the changes in A, but then it's likely that the | |
210 | changes in B' will conflict with the original B changes re-introduced by the | |
211 | reversion of W. | |
212 | ||
213 | However, you can avoid these problems if you recreate the entire branch, | |
214 | including commit A: | |
215 | ||
216 | A'---B'---C' <-- completely rewritten branch | |
217 | / | |
218 | P---o---o---M---x---x---W---x | |
219 | \ / | |
220 | A---B---C | |
221 | ||
222 | You can merge A'-B'-C' into the mainline branch without worrying about first | |
223 | reverting W. Mainline's history would look like this: | |
224 | ||
225 | A'---B'---C'------------------ | |
226 | / \ | |
227 | P---o---o---M---x---x---W---x---M2 | |
228 | \ / | |
229 | A---B---C | |
230 | ||
231 | But if you don't actually need to change commit A, then you need some way to | |
232 | recreate it as a new commit with the same changes in it. The rebase commmand's | |
233 | --no-ff option provides a way to do this: | |
234 | ||
235 | $ git rebase [-i] --no-ff P | |
236 | ||
237 | The --no-ff option creates a new branch A'-B'-C' with all-new commits (all the | |
238 | SHA IDs will be different) even if in the interactive case you only actually | |
239 | modify commit B. You can then merge this new branch directly into the mainline | |
240 | branch and be sure you'll get all of the branch's changes. | |
241 | ||
242 | You can also use --no-ff in cases where you just add extra commits to the topic | |
243 | to fix it up. Let's revisit the situation discussed at the start of this howto: | |
244 | ||
245 | P---o---o---M---x---x---W---x | |
246 | \ / | |
247 | A---B---C----------------D---E <-- fixed-up topic branch | |
248 | ||
249 | At this point, you can use --no-ff to recreate the topic branch: | |
250 | ||
251 | $ git checkout E | |
252 | $ git rebase --no-ff P | |
253 | ||
254 | yielding | |
255 | ||
256 | A'---B'---C'------------D'---E' <-- recreated topic branch | |
257 | / | |
258 | P---o---o---M---x---x---W---x | |
259 | \ / | |
260 | A---B---C----------------D---E | |
261 | ||
262 | You can merge the recreated branch into the mainline without reverting commit W, | |
263 | and mainline's history will look like this: | |
264 | ||
265 | A'---B'---C'------------D'---E' | |
266 | / \ | |
267 | P---o---o---M---x---x---W---x---M2 | |
268 | \ / | |
269 | A---B---C |