Don't rollback rollbacks

In a version control system, a rollback is a type of change that undoes the effects of a previous commit. In essence, a rollback is a commit that applies the inverse diff of another commit.

At Google, our tools make it trivial to create rollbacks for a given changelist or CL. (A CL is similar to a commit but can be either pending—in review—or submitted.) Making it trivial to create rollback CLs is important in a culture where the standard upon encountering a problem is “rollback first, ask questions later” because it removes friction from the process of backing out problematic changes.

In our specific case, we can create a rollback CL by running a command of the form g4 rollback -c <offending-cl-number> or by clicking a button in the code review system’s page for the offending CL. When doing any of these actions, the interfaces prompt us for the reason behind the rollback and then create a new pending CL, which is linked to the original one via special metadata.

Enough about Google though. While I’m not aware of similar (trivial) automation in other version control systems, these general practices can be applied to and exist in any other system you can imagine.

As an example of a rollback, let’s peek into the Bazel Git repository’s history. There we can find commit d18d3e2:

commit d18d3e2f83f9d582858a3edab7a450c60044028c
Author: Googler <noreply@google.com>
Date:   Fri Feb 16 15:48:49 2018 -0800

    Automated rollback of commit f672a31b8b19baab95373e4f2f6d110aa8b8f0fb.

    *** Reason for rollback ***

    Unclassified general breakages in tests. Rolling back for further
    investigation.

    *** Original change description ***

    Normalized the serialization proto to save space and allow greater
    versatility in storage.

    RELNOTES: None
    PiperOrigin-RevId: 186057879

You can see here that the commit message is derived from a template due to the Automated rollback of commit header and the two visually-separate sections denoted by ***. There is nothing else special about it though. You can also notice that the first line of the commit message is a pretty reasonable subject line: it denotes that this commit is a reversal of a previous change and thus is an indicator that the previous change was problematic.

All good, right? So far, yes… but…

Unfortunately, it’s too easy to abuse rollbacks to resubmit a previous change that was rolled back. I.e. it is too easy to create a rollback of a rollback. This is the same as a double negative, which as we all know are hard to understand and can be misleading depending on your linguistic background. Furthermore, the visual structure of the commit message obscures what happened to the software when reading through history.

From the same repository, take a look at commit e9c885a:

commit e9c885a88137fa10ea0ac95ff33dcfdc79c6cf0a
Author: Googler <noreply@google.com>
Date:   Thu Feb 15 01:18:23 2018 -0800

    Automated rollback of commit fa0fac2a4e8a2e5c01b8390878289d00dcc17dba.

    *** Reason for rollback ***

    Remove example changes; those need to build with the last Bazel release.

    *** Original change description ***

    Automated rollback of commit 0f9c6ea574918dda094cf5423fa3822112846c30.

    *** Reason for rollback ***

    Breaks Kokoro and I accidentally submitted the change without presubmit
    checks.

    *** Original change description ***

    Make __init__.py files creation optional

    Introduce a new attribute to py_binary and py_test to control whether to
    create `__init__.py` or not.

    Fixes https://github.com/bazelbuild/rules_python/issues/55

    Closes #4470.

    PiperOrigin-RevId: 185806241

What is going on here? Who knows! In a summarized view of the repository’s history (where we only see subject lines), this looks like a rollback of a problematic change. When looking at the full text, this still looks like a rollback because of the header. A closer look, however, reveals that this is actually a retry of a submission of a feature that failed to get in on the first try. Not. Confusing. At. All.

But oh well, this isn’t so bad: this is “just” a rollback of a rollback (twice, thus positive meaning). Unfortunately, I’ve encountered rollbacks of rollbacks of rollbacks (thrice, so negative meaning), rollbacks of rollbacks of rollbacks of rollbacks (four times, so positive meaning again)… and… worse.

So don’t do that. Instead, if you have to retry submitting a change that was rolled back, explicitly say so in the change’s description and clearly explain what happened in the previous attempt(s).

For example, the above could have read:

commit e9c885a88137fa10ea0ac95ff33dcfdc79c6cf0a
Author: Googler <noreply@google.com>
Date:   Thu Feb 15 01:18:23 2018 -0800

    Make __init__.py files creation optional

    Introduce a new attribute to py_binary and py_test to control whether to
    create `__init__.py` or not.

    This is a retry of 0f9c6ea574918dda094cf5423fa3822112846c30, which was
    rolled back in fa0fac2a4e8a2e5c01b8390878289d00dcc17dba, because it was
    submitted without properly running presubmit checks and broke Kokoro.
    This retry differs from the original change in that it removes the example
    changes, which need to be built with the last Bazel release.

    Fixes https://github.com/bazelbuild/rules_python/issues/55

    Closes #4470.

    PiperOrigin-RevId: 185806241

Better, right? First, this commit’s message subject line identifies what the change does, which is good for summarized commit lists. Next, the commit message explains more in detail what the change is about, which is the important thing to know when skimming through version control history. And last, the commit message details what happened in the previous attempt at submitting this feature, which is interesting but to a lesser extent.