1

We have scenario where for any application config changes in Prod, we create 2 PRs: 1 for implementation and another for rollback. We create these in advance. Then, we get them reviewed and approved.

We don't have anyone to review the PR during the change window. So, we have to create PR in advance and get approvals.

Let's say, master branch contains a config file app.properties with below 2 properties: test.base.certificate=abcd
test.base.key=abcd

We create the implementation PR as below:

  1. create implementation branch from master.
  2. Update the properties with new values as below:
    test.base.certificate=efgh
    test.base.key=efgh
  3. Commit the changes.
  4. Push the changes to remote.

We create the rollback PR as below:

  1. Create rollback branch from master.
  2. Update the properties with new values as below.
    test.base.certificate=efgh
    test.base.key=efgh
  3. Commit the changes.
  4. Revert to old property values:
    test.base.certificate=abcd
    test.base.key=abcd
  5. Commit the changes.
  6. Push the changes to remote.

Rollback PR doesn't show any differences in app.properties. This is because after the 2nd commit, there is no difference between the file in master and rollback branch.

During the Prod change window, we first merge the implementation PR and test. If any issues during testing, we merge the rollback PR to revert the changes.

I had to perform the rollback activity first time recently. After merging the rollback PR, I observed that the app.properties in the master branch still had the new property values:
test.base.certificate=efgh
test.base.key=efgh

enter image description here

At high level, it looks like that this approach should have worked. Does anyone have any idea why it didn't work? How to create the rollback PR in this case?

PS - I'm aware of git revert. However, I'm curious as why the above approach didn't work.

Thanks in advance!

Rahul
  • 637
  • 5
  • 16
  • Some of your wording doesn't make sense: "Add the old changes to rollback branch. Create another commit." What do you mean by "old changes"? "the old changes were not updated in the master branch" ?? In other words, what EXACTY are the commits on the Rollback branch. You should draw a diagram such as you see in `git help merge`. – Inigo Jan 16 '23 at 09:49
  • @Inigo I have updated it for more clarity.. – Rahul Jan 16 '23 at 13:23

1 Answers1

0

At high level, it looks like that this approach should have worked.

My guess is that you think git merge rollback replays the two commits unique to rollback onto the current branch (main). If git did that, the first commit would do nothing since main already has that change. The second commit would restore the properties file to its old state. This is what you expected. But this is not how git merge works.

git merge does not replay commits

What git merge does by default is called a three-way merge. As explained in an answer to another question:

You might be best off looking for a description of a 3-way merge algorithm. A high-level description would go something like this:

  1. Find a suitable merge base B - a version of the file that is an ancestor of both of the new versions (X and Y), and usually the most recent such base (although there are cases where it will have to go back further, which is one of the features of gits default recursive merge)
  2. Perform diffs of X with B and Y with B.
  3. Walk through the change blocks identified in the two diffs. If both sides introduce the same change in the same spot, accept either one; if one introduces a change and the other leaves that region alone, introduce the change in the final; if both introduce changes in a spot, but they don't match, mark a conflict to be resolved manually.

The git merge docs has a less detailed explanation, but explains exactly the behavior that is confusing you:

With the strategies that use 3-way merge (including the default, ort), if a change is made on both branches, but later reverted on one of the branches, that change will be present in the merged result; some people find this behavior confusing. It occurs because only the heads and the merge base are considered when performing a merge, not the individual commits. The merge algorithm therefore considers the reverted change as no change at all, and substitutes the changed version instead.

I believe the reason does this is for a few reasons:

  • 99% of the time, merging is about parallel lines of development, working on different things, and you want to merge both those things. Thus you don't want to, by default, give priority to changes made on one branch over the other. If rollback were a feature branch, you wouldn't want it to just override the changes made by impl.
  • 99.9% when a developer one branch makes a change and subsequently undoes it, the intent is "Pretend I never did that". So the behavior that confuses some people most of the time does what people expect.
  • You can achieve the other behavior by specifying a different merge strategy, e.g. git merge -s ort -X theirs rollaback , or by using other commands like cherry-pick or rebase, depending on what you exactly want.

solutions #1: git revert

Nix the rollback branch and just use git revert:

git checkout main
git revert main..IMPL

This is simplest.

solutions #2: branch rollback from impl

If for some reason you must do it via a PR, or for whatever reason must have a rollback branch, then branch rollback off of impl, not master:

git checkout impl
git checkout -b rollback
git revert main..IMPL

It will look like this:

1 main
 \
  2 impl
   \ 
    3 rollback

merging impl to deploy:

1---4 main
 \ /
  2 impl
   \
    3 rollback

merging rollback to rollback:

1---4-------5 main
 \ /       /
  2 impl  /
   \     /
    3----  rollback

Not only will app.properties be reverted as you expected, the git history is simpler AND a better representation of what happened. Compare it to the result of your current approach, which has two identical, redundant commits (2 and 3), and misleadingly splits into three branches at the beginning.

  2  rollout
 / \
1---5---6 main
 \     /
  3---4  rollback

It looks even better if you allow fast-forward merges.

1 main
 \
  2 impl
   \ 
    3 rollback

Deploy ends up like this:

1---2 main, impl
     \ 
      3 rollback

and rollback like this:

    impl 
1---2---3 main, rollback
Inigo
  • 12,186
  • 5
  • 41
  • 70