1.7 cherry-pick, revert, rebase

Book can be downloaded from here

You have already seen what merge conflicts (the ones that come about when we are merging branches) look like. There are 3 other operations that can produce conflicts:

• git cherry-pick
• git revert
• git rebase

Even though the basics are the same, each one of the sections of the CB has a different meaning ³⁶ from what they were during a merge.

1.7.1 cherry-pick - Example 12

In git 2.27, this change was introduced:

We can see the way that chunk_ids and chunk_offsets’s sizes are changed to be defined by a macro.

As an experiment, we want to backport this change on top of v2.22.4:

What does the conflict look like? There are two CBs in commit-graph.c. The first CB looks like this:

CB#1

It looks a lot like the conflicts we have seen so far, right? Just like in a merge, there are 3 sections in the CB and I will keep on using the same terms/achronyms we have used so far. The first one is UB, in other words, code as it is on HEAD, in our case, on tag v2.22.4, as that is where we are currently working. Unlike normal merges where we get to see the common ancestor’s code in MB and the other branch’s code in LB, during a cherry-pick operation the other two blocks display something different, as described on the conflict markers. MB shows us the content of the parent of the revision we are trying to cherry-pick³⁷ . LB shows us code as it is on the revision 3be7efcafc, what we are trying to cherry-pick. Even though the MB and LB relate to different concepts from a merge, the way the analysis has to be carried out is the same.

dMU#1

The macro GRAPH_CHUNKID_BASE (present in line 27) was removed.

dML#1

The macro MAX_NUM_CHUNKS was added (line 30).

Resolution#1

I don’t think there’s much difference working from UB or LB.... it’s just a tiny bit simpler to delete things than copying from another place so I will start working from LB:

Then, as dMU is requesting, we remove GRAPH_CHUNKID_BASE.

Remove the other sections of the conflict and the markers and we are done.

And then we go into CB#2 in commit-graph.c: ³⁸

CB#2

dMU#2

The size of chunk_ids and chunk_offsets was changed from 6 to 5.

dML#2

Got rid of the numeric value and replaced it for MAX_NUM_CHUNKS + 1, using the macro we defined on the previous CB.

Resolution#2

In this special case we can keep code from LB as is because the change of 6 for 5 has no incidence on the requirement: We still need to use the macro. So:

And we are done! ... Or so you think! Do you remember the intent of the revision we are trying to cherry-pick? It was related to changing the way we define the size of the arrays to use a macro. This is probably so that if we needed to change the size of the arrays from 6 to 20 in the future, we would only adjust the value of MAX_NUM_CHUNKS from 5 to 19 and it would hit both arrays at the same time. Nice! But we are not applying it on a revision where the size of the arrays is 6. We have arrays of size 5 and it has to remain the same.³⁹ Which means that we need to set up the macro to 4, so that the arrays end up with the correct original value when using the macro. Then we need to go back to the previous CB and we adjust the macro value:

And now we are done!⁴⁰

Question: Would it be OK to leave the macro with 5 and change the arrays in CB#2 to use MAX_NUMS_CHUNKS instead? Like this:

From the plain coding point if view, it might be correct ⁴¹. Code compiles and behaves the same, right? But you are breaking the intent of the revision you are cherry-picking. Just so that you can glance the consequences of this decision, what would happen if, later on, you needed to cherry-pick another revision that uses this same macro? Now that you have broken the values that you are using for it, you will have to do more adjustments on the code than the original intent of the other revisions you were cherry-picking then.... and you might not even get conflicts when you cherry-pick them. Will you be able to keep this change in mind so that you are able to catch those situations later on and correct the code? I know I would.... for the first 30 seconds after I modified the code, but I don’t think I would be able to keep it for much longer. And I’m sure it’s extremely unlikey anyone else will keep this in mind. All in all: You better stick to the original intent. You have been forewarned.

1.7.2 revert - Example 13

When you want to take back the changes introduced by a revision, the operation you have to apply is revert ⁴² . Let’s consider a little case from git.

From git repo, checkout tag v2.26.2. We will revert revision 22a69fda197f. Let me show you what the revision that I want to revert did originally to builtin/rebase.c:

The revision we want to revert changed the original calls to OPT_BOOL() for defining allow-empty-message to OPT_BOOL_F() and adjusted the parameters. That means that if we want to revert this revision, we need to end up with the original OPT_BOOL() call to define allow-empty-message in place after the revert is finished, right? Let’s try:

There is a CB in builtin/rebase.c:

This is not too different from what we saw in cherry-pick, right? UB is the way the code is on v2.26.2, as that is the revision we started working from. The interesting part is in the other two blocks. You can see from the markers that MB is the way code looked on the revision we are trying to revert (in other words, the code the way it looked after the revision was applied) and LB is the way code looks on its parent revision (in other words, the way the code looked before) ⁴³. This means that we, just like in the merges and cherry-picks we have already solved, need to go through dMU and dML without any changes in methodology so it’s business as usual.

dMU

The way keep-empty is defined is changed from a call to OPT_BOOL() in line 482 to a hand-made definition in lines 474-477.

dML

The way allow-empty-message is defined is changed from a call to OPT_BOOL_F() in lines 483-485 to OPT_BOOL() in lines 488-489, which also removed PARSE_OPT_HIDDEN as a parameter to the new call.

Resolution

I will stay working on UB, just because the way keep-empty was changed from the MB is bigger than the change that was done for allow-empty-message in dML. Then I will change the macro call from OPT_BOOL_F() to OPT_BOOL() (lines 478-480):

Remove PARSE_OPT_HIDDEN as a parameter (present on line 480):

Reposition the second line for the argument on the second line of the macro call to align with the position of the call (line 479):

And now we are done with the reversion:

And we can see how we ended up with a call to OPT_BOOL() to define allow-empty-message, just what we wanted to get with the reversion.

Just so that you can see that the same rules apply when analyzing a reversion, try solving the conflict working from LB and bring over the changes introduced in dMU. You should end up with the same code.

1.7.3 rebase - Example 14

Let’s play a little bit. From git repo, checkout revision ce9baf234f. Let’s take a look at a section of builtin/push.c on this revision:

Take a close look at the way <refname>:<expect> and recurse-submodules are defined. <refname>:<expect> is defined using a struct by hand in lines 549-552 and recurse-submodules is defined with a call to OPT_CALLBACK() in lines 553-554.

Let’s ask git to rebase on top of 6652716200⁴⁴ . You should see a CB related to the section of builtin/push.c that I showed you before:

You remember the revision you were on when you ran rebase? It was on ce9baf234f, right? Look at UB. Does it look the way it was in ce9baf234f? No? Ok. How about LB? Oh, it’s there! And then, what is on UB? You remember that I said that UB always contains code as it is in HEAD? Well, that is correct, even in this case. The trick that you have to realize is that when you run a rebase git first checks out the base branch for the rebase and then starts to apply the revisions that you ask to rebase on top of the base branch.

So, in our current example, in UB you will see the code as it is in 6652716200 and in LB you will see the code from ce9baf234f. In MB you will see the parent of the revision that is being rebased at the time, so in this case it will hold the parent of ce9baf234f. You can see the details about the revision used in MB and LB in the conflict markers, just in case.

Now that we have cleared that up, we continue with the analysis as we have done so far, no changes:

dMU

The way that <refname>:<expect> is being defined is from by-hand in lines 559-562 to a call to OPT_CALLBACK_F() in lines 552-554.

dML

The way recurse-submodules is being defined is changed from by hand in lines 563-565 to a call to OPT_CALLBACK() in lines 571-572.

Resolution

I don’t see much difference between working from UB and LB. Let’s start working from UB:

Then we modify the way that recurse-submodules is defined as in dML:

And we are done:

We add the file to index, nothing else is pending and so we can continue with the rebase ⁴⁵ .

1.7.4 Tips

Regarless of the operation being done (merge, cherry-pick, revert, rebase), always use the same technique to solve conflicts: dMU/dML, Resolution.

Copyright 2020 Edmundo Carmona Antoranz