2

Our team uses a release branching strategy. A week before each release, we will create a branch off of master, e.g. "Release_1.2.3". We send this branch to our quality engineers for verification, and use the week prior to the release to merge in critical bug fixes before releasing our software to our users. During this time, feature development continues on master. Once Release 1.2.3 is officially released, we merge Release_1.2.3 back into master.

This approach generally works for us, except for one flaw: When working on changes targeting Release_1.2.3, some devs have incorrectly merged master into their development branch and then manually deleted the unnecessary changes. This causes issues when merging Release_1.2.3 back into master, since git views the deletions made as being a part of the history, and will try to make the same deletions on master.

We've tried to educate devs about this issue, but we have a large team contributing to the codebase (including many devs without much experience in git). Instead, I'm looking for an automated way to prevent these sorts of harmful merges. We use Github's Branch Protection Rules to prevent direct commits to master; can we use a similar approach here?

Alternatively, is there a programmatic way of detecting that a branch contains a harmful merge? We could potentially incorporate that into our existing CI system.

Harry Williams
  • 310
  • 3
  • 11
  • 1
    Don't allow any of the protected branches to be pushed directly to the server by anyone (maybe 1 or 2 seniors). Merges to those branches must be via Pull Request only. You may allow a lot of people to review and approve the PRs, but only give certain people permission to merge them. You have to lock these processes in place to make sure even the more senior people don't accidentally merge something to the wrong branch. – Adrian J. Moreno Feb 17 '23 at 20:32
  • We actually already have these protections in place: non-admins cannot merge directly into master or a Release* branch; all changes must go through pull requests. However, these pull requests might still contain harmful merges. It's not obvious to detect that a harmful merge is part of a PR unless you know to look for it, which is why I'm looking for an automated solution. – Harry Williams Feb 17 '23 at 20:44
  • 1
    In one of our repos where we use a rebase strategy for feature branches, we have a gate that validates there are no new merge commits on the branch. We also prevent more than X commits. These can be overridden for shared branch merges, or in the rare case where a feature branch should have a new merge commit on it, or more than X new commits. This usually catches all of the accidental wrong starting branch scenarios you described. – TTT Feb 17 '23 at 21:04
  • Just thought of this now and don't have time to write it up (perhaps someone else wants to run with it though). What if you look at the commit IDs on `git log Release_XXX..master` and make sure none of them appear in a PR into `Release_XXX`? (Unless you're merging `master` in...) – TTT Feb 17 '23 at 21:18

1 Answers1

2

If you can convince your developer to install a client-side hook in their repo/.git/hooks folder, that allows for a control as close as possible of the action.

With Git 2.24+ (Q4 2019), you can use a pre-merge-commit similar to this script:

#!/bin/sh

# Get the commit hash of the head of the branch being merged
MERGE_HEAD=$(cat .git/MERGE_HEAD)

# Retrieve the branch name using the commit hash
merge_from=$(git branch --contains $MERGE_HEAD | grep -v '^\*' | sed 's/^ *//')

if [ "$merge_from" != "master" ]
then
  exit 0
fi

echo "You cannot merge master branch into the current branch."

exit 1

Or, if .git/MERGE_HEAD is no longer there when the pre-merge-commit is invoked, a possible option would be to use the reflog:

#!/bin/sh

# Get the commit hash of the previous HEAD
PREVIOUS_HEAD=$(git reflog -1 --pretty=%H HEAD@{1})

# Retrieve the branch name using the commit hash
merge_from=$(git branch --contains $PREVIOUS_HEAD | grep -v '^\*' | sed 's/^ *//')

if [ "$merge_from" != "master" ]
then
  exit 0
fi

echo "You cannot merge master branch into the current branch."

exit 1

That is less reliable than a server-side hook, but it can help.

VonC
  • 1,262,500
  • 529
  • 4,410
  • 5,250
  • If you're on `feature`, and merging in `master`, wouldn't "current_branch" be `feature` instead of the incoming branch name? – TTT Feb 20 '23 at 19:31
  • @TTT If you merge *to* `master`, you should *switch to* `master` first, then merge `feature`. Hence current branch would be `master`. – VonC Feb 20 '23 at 19:40
  • My interpretation of the question is that OP needs a way to prevent people from merging `master` into their `feature` branches, specifically for those feature branches that are targeting `release` instead of `master`. (Also, OP didn't say this, but I assume the same problem happens when `feature` branches are created from `master` but then target `release`, in which case the local hook can't detect it.) – TTT Feb 20 '23 at 19:51
  • @TTT Good point. Getting the branch you are merging from is tricky though... – VonC Feb 20 '23 at 20:01
  • Nice edit. I think now it gets pretty close. (I might look for some flavor of containing "master" rather than equal to "master", since other heads might be at the same commit. And maybe handle `origin/master` as well as local `master`, but it's a good general idea.) You're right though- it's "tricky". ;) – TTT Feb 20 '23 at 23:56