The Universe of Disco

Don't let the man page write checks that the programmer can't cash

My big work project is called “Greenlight”. It's a Git branch merging service. After you've pushed a remote branch, say mjd.fix-bugs, you use a very thin client program to ask the Greenlight server to land your branch on master and publish it for you:

    greenlight submit mjd.fix-bugs

Greenlight analyzes the branch to see if it touches any sensitive code that requires signoffs. If so it contacts the correct people on Slack, and asks them to review it. Once they have approved it, Greenlight rebases the branch onto the current master and pushes the result back to master. If the push fails, it retries silently. Throughout, it communicates via Slack what is going on.

A user, Locksher, complained last week that it didn't do what he had expected. He had a Git pre-push hook he had written. Whenever he ran git push, his pre-push hook would look to see if he was pushing to master. If so, it would look at the messages of the commits he was trying to push. If any of them contained WIP or !fixup or !squash, it would abort the push.

With Greenlight, this check wasn't done, because Locksher never pushed to master himself. Instead he pushed to some topic branch, and then asked Greenlight to publish it to master, which it did, including his WIP commits. Oops!

Locksher asked if it was possible to have Greenlight “respect local hooks”. Once I understood what he wanted, my first suggestion was that he wrap the greenlight client in a shell script that did the check he wanted. My second suggestion, less work for him but also less immediate, was that the Greenlight client could look in .git/hooks for a greenlight-pre-submit hook, and run that before communicating with the server, aborting the request if the hook failed. I think this would adequately solve the problem, especially if the calling convention for the new hook was identical to that of pre-push. Then you would just:

        ln -s pre-push .git/hooks/greenlight-pre-submit

and get exactly the desired behavior. I said that if Locksher wanted to implement this, I would include it in the standard client, or alternatively I would open a ticket to implement it myself, eventually.

Locksher suggested instead that the greenlight client configuration should support this:

    [git]
    respect-git-hooks = true

I didn't have time then to answer in detail, so I just said:

I consider that very unlikely.

Here's what I said to him once I did have time to answer in detail:

1. There are currently 23 documented Git hooks, and it's not immediately clear what it would mean to “respect” many of them. I'd have to go over the man page and decide, for each one, what the behavior should be, then possibly implement it, and then document it. Just to pick one example, should Greenlight “respect” your prepare-commit-message hook? If so, how?

2. Even for the hooks where the correct behavior seemed clear to me, it might seem clearly something else to someone else. So the feature is severely under-specified and seems likely to cause confusion. I foresee a future of inquiries like “I set respect-git-hooks but Greenlight didn't run my pre-auto-gc hook.”

3. It is an open-ended promise. The way the option is phrased, it guarantees to “respect” every hook. So it commits me to keep track of what new hooks are introduced in every future version of Git, and to decide what to do about each of them.

4. Since greenlight runs on your local machine, the local version of Git may vary. What if the behavior of Git's pre-cake-slicing hook changes between Git 1.24 and Git 1.26? Now Greenlight will have to implement two behaviors, and look at your local Git version to decide what to do.

Oh, and 5, it is a YAGNI.

In contrast, the functionality provided by greenlight-pre-submit is something someone has actually asked for. It is small, sharply bounded in scope and its definition is completely under my control.

I will elaborate a little on the main items 1–2, that different people might have different ideas about what it means to “respect” a local hook. Consider Locksher's specific request, for greenlight to “respect” his pre-push hook. Another user, say Zubi, could object, quite reasonably, that greenlight submit is not the same as git push, and that the correct way for it to “respect” her pre-push hook is to ignore it. “I want my pre-push hook run when I push a branch,” she might say, “not when I do greenlight submit.” Who could argue with that? (Other than Locksher, of course.)

So then I would have to add an escape hatch for Zubi, so that everyone who didn't want Locksher's feature would have to affirmatively opt out of it.

Nah.

[Other articles in category /prog] permanent link