mirror of https://github.com/citusdata/citus.git
Make check_enterprise_merge script stricter (#3918)
We've had two issues with merge conflicts to enterprise in the last week, that suddenly happened. Because of this CI check this actually blocks all community PRs from being merged. This PR tries to improve on the previous script we had, by putting tougher constraints on when a merge is allowed. Previously the check would pass in two cases: 1. This PR be merged without conflicts into `enterprise-master` 2. A branch exists with the same name as this PR on enterprise and that can be merged into `enterprise-master`. The first case stays the same, but I've changed the second case to require the following instead: 1. A branch exists on enterprise with the same name as this PR 2. **NEW: This branch contains the the last commit of the community PR branch** 3. This branch can be merged into enterprise-master This makes sure the enterprise branch is actually up to date and not forgotten about. If we still get problems with this change, future improvements could be: 1. Check that the PR on enterprise passes CI 2. Check that the PR on enterprise has been approved 3. Require the enterprise PR branch to be merged before merging community.pull/3829/head
Jelte Fennema
GitHub
parent
3a789352b6
commit
b3ec6fbe7a
|
|
@ -1,33 +1,60 @@
|
|||
|
||||
# check-merge-to-enterprise Job
|
||||
|
||||
When you open a PR on community, if it creates a conflict with enterprise-master, the check-merge-to-enterprise will fail. Say your branch name is `X`, we will refer to `X` on community as `community/X` and on enterprise as `enterprise/X`. If the job already passes, you are done, nothing further required! Otherwise follow the below steps:
|
||||
When you open a PR on community, if it creates a conflict with
|
||||
enterprise-master, the check-merge-to-enterprise will fail. Say your branch name
|
||||
is `$PR_BRANCH`, we will refer to `$PR_BRANCH` on community as
|
||||
`community/$PR_BRANCH` and on enterprise as `enterprise/$PR_BRANCH`. If the
|
||||
job already passes, you are done, nothing further required! Otherwise follow the
|
||||
below steps. First make sure these two things are the case:
|
||||
|
||||
- You first need to get approval from your reviewer for `community/X`. Only follow the next steps after you are about to merge the branch to community master. Otherwise, the `check-merge-to-enterprise` check might falsely pass. (For example when you added new commits to `community/X` but forgot to update `enterprise/X`).
|
||||
- You need to synchronize the same branch, `X` locally on enterprise (ideally don't push yet). For example if `community` is added as a remote in your enterprise repo, you can do the following:
|
||||
|
||||
```bash
|
||||
git fetch --all # this will fetch community/X
|
||||
git checkout community/X
|
||||
git checkout -b X # now you have X in your enterprise repo, which we refer to as enterprise/X
|
||||
```
|
||||
|
||||
- You need to merge `enterprise-master` to `enterprise/X`. Solve any conflicts( and make sure to remove any parts that should not be in enterprise even though it doesn't have a conflict), on enterprise repository:
|
||||
1. Get approval from your reviewer for `community/$PR_BRANCH`. Only follow the
|
||||
next steps after you are about to merge the branch to community master.
|
||||
2. Make sure your commits are in a nice state, since you should not do
|
||||
"squash and merge" on Github later. Otherwise you will certainly get
|
||||
duplicate commits and possibly get merge conflicts with enterprise again.
|
||||
|
||||
Once that's done, you need to create a merged version of your PR branch on the
|
||||
enterprise repo. For example if `community` is added as a remote in
|
||||
your enterprise repo, you can do the following:
|
||||
|
||||
```bash
|
||||
export PR_BRANCH=<YOUR BRANCHNAME OF THE PR HERE>
|
||||
git checkout enterprise-master
|
||||
git pull # make sure enterprise-master in your local is up-to-date
|
||||
git checkout X
|
||||
git merge origin/enterprise-master # assuming origin is the remote of citus-enterprise
|
||||
git pull # Make sure your local enterprise-master is up to date
|
||||
git fetch community # Fetch your up to date branch name
|
||||
git checkout -b "$PR_BRANCH" enterprise-master
|
||||
```
|
||||
Now you have X in your enterprise repo, which we refer to as
|
||||
`enterprise/$PR_BRANCH` (even though in git commands you would reference it as
|
||||
`origin/$PR_BRANCH`). This branch is currently the same as `enterprise-master`.
|
||||
First to make review easier, you should merge community master into it. This
|
||||
should apply without any merge conflicts:
|
||||
|
||||
```bash
|
||||
git merge community/master
|
||||
```
|
||||
Now you need to merge `community/$PR_BRANCH` to `enterprise/$PR_BRANCH`. Solve
|
||||
any conflicts and make sure to remove any parts that should not be in enterprise
|
||||
even though it doesn't have a conflict, on enterprise repository:
|
||||
|
||||
```bash
|
||||
git merge "community/$PR_BRANCH"
|
||||
```
|
||||
|
||||
- You should push this to enterprise and open a PR so that the job on community will see this branch. Also this will trigger CI to verify changes are correct.
|
||||
- You should get approval for the merge conflict changes on `enterprise/X`, preferably from the same reviewer as they are familiar with the change.
|
||||
- You should rerun the `check-merge-to-enterprise` check on `community/X`. You can use re-run from failed option in circle CI.
|
||||
- You can now merge `community/X` to `community/master`.
|
||||
- You can merge `enterprise/X` to `enterprise-master`.
|
||||
- Manually merge `community master` to `enterprise-master`. [https://github.com/citusdata/citus-enterprise/blob/enterprise-master/CONTRIBUTING.md#merging-community-changes-onto-enterprise](https://github.com/citusdata/citus-enterprise/blob/enterprise-master/CONTRIBUTING.md#merging-community-changes-onto-enterprise)
|
||||
- You can use `git checkout --ours <conflicting file names>` for all conflicting files. This will use local/our version to resolve the conflicts.
|
||||
- Now you can push to `enterprise-master`. Now you can rerun the check on `community/master` and it should pass the check.
|
||||
1. You should push this branch to the enterprise repo. This is so that the job
|
||||
on community will see this branch.
|
||||
2. Wait until tests on `enterprise/$PR_BRANCH` pass.
|
||||
3. Create a PR on the enterprise repo for your `enterprise/$PR_BRANCH` branch.
|
||||
4. You should get approval for the merge conflict changes on
|
||||
`enterprise/$PR_BRANCH`, preferably from the same reviewer as they are
|
||||
familiar with the change.
|
||||
5. You should rerun the `check-merge-to-enterprise` check on
|
||||
`community/$PR_BRANCH`. You can use re-run from failed option in circle CI.
|
||||
6. You can now merge the PR on community. Be sure to NOT use "squash and merge",
|
||||
but instead use the regular "merge commit" mode.
|
||||
7. You can now merge the PR on enterprise. Be sure to NOT use "squash and merge",
|
||||
but instead use the regular "merge commit" mode.
|
||||
|
||||
The subsequent PRs on community will be able to pass the `check-merge-to-enterprise` check as long as they don't have a conflict with `enterprise-master`.
|
||||
The subsequent PRs on community will be able to pass the
|
||||
`check-merge-to-enterprise` check as long as they don't have a conflict with
|
||||
`enterprise-master`.
|
||||
|
|
|
|||
|
|
@ -1,55 +1,100 @@
|
|||
#!/bin/bash
|
||||
|
||||
# Testing this script locally requires you to set the following environment
|
||||
# variables:
|
||||
# CIRCLE_BRANCH, GIT_USERNAME and GIT_TOKEN
|
||||
|
||||
# fail if trying to reference a variable that is not set.
|
||||
set -u
|
||||
# exit immediately if a command fails
|
||||
set -e
|
||||
# Fail on pipe failures
|
||||
set -o pipefail
|
||||
|
||||
# try_merge sees if we can merge "from" branch to "to" branch
|
||||
# it will exit with nonzero code if the merge fails because of conflicts.
|
||||
try_merge() {
|
||||
to=$1
|
||||
from=$2
|
||||
git checkout "${to}"
|
||||
# this will exit since -e option is set and it will return non-zero code on conflicts.
|
||||
git merge --no-ff --no-commit "${from}"
|
||||
PR_BRANCH="${CIRCLE_BRANCH}"
|
||||
ENTERPRISE_REMOTE="https://${GIT_USERNAME}:${GIT_TOKEN}@github.com/citusdata/citus-enterprise"
|
||||
|
||||
# For echo commands "set -x" would show the message effectively twice. Once as
|
||||
# part of the echo command shown by "set -x" and once because of the output of
|
||||
# the echo command. We do not want "set -x" to show the echo command. We only
|
||||
# want to see the actual message in the output of echo itself. This function is
|
||||
# a trick to do so. Read the StackOverflow post below to understand why this
|
||||
# works and what this works around.
|
||||
# Source: https://superuser.com/a/1141026/242593
|
||||
shopt -s expand_aliases
|
||||
alias echo='{ save_flags="$-"; set +x;} 2> /dev/null; echo_and_restore'
|
||||
echo_and_restore() {
|
||||
builtin echo "$*"
|
||||
#shellcheck disable=SC2154
|
||||
case "$save_flags" in
|
||||
(*x*) set -x
|
||||
esac
|
||||
}
|
||||
|
||||
cd /tmp
|
||||
if [ ! -d citus-enterprise ]; then
|
||||
git clone https://${GIT_USERNAME}:${GIT_TOKEN}@github.com/citusdata/citus-enterprise
|
||||
fi
|
||||
# Make sure that on a failing exit we show a useful message
|
||||
hint_on_fail() {
|
||||
exit_code=$?
|
||||
if [ $exit_code != 0 ]; then
|
||||
echo HINT: To solve this failure look here: https://github.com/citusdata/citus/blob/master/src/test/scripts/README.md#check-merge-to-enterprise-job
|
||||
fi
|
||||
exit $exit_code
|
||||
}
|
||||
trap hint_on_fail EXIT
|
||||
|
||||
cd citus-enterprise
|
||||
|
||||
# List executed commands. This is done so debugging this script is easier when
|
||||
# it fails. It's explicitly done after git remote add so username and password
|
||||
# are not shown in CI output (even though it's also filtered out by CircleCI)
|
||||
set -x
|
||||
|
||||
# Clone current git repo (which should be community) to a temporary working
|
||||
# directory and go there
|
||||
GIT_DIR_ROOT="$(git rev-parse --show-toplevel)"
|
||||
TMP_GIT_DIR="$(mktemp --directory -t citus-merge-check.XXXXXXXXX)"
|
||||
git clone "$GIT_DIR_ROOT" "$TMP_GIT_DIR"
|
||||
cd "$TMP_GIT_DIR"
|
||||
|
||||
# Fails in CI without this
|
||||
git config user.email "citus-bot@microsoft.com"
|
||||
git config user.name "citus bot"
|
||||
|
||||
# reset repository into usable state if script ran before
|
||||
git fetch origin
|
||||
git reset --hard
|
||||
git checkout enterprise-master
|
||||
git reset --hard origin/enterprise-master
|
||||
# Disable "set -x" temporarily, because $ENTERPRISE_REMOTE contains passwords
|
||||
{ set +x ; } 2> /dev/null
|
||||
git remote add enterprise "$ENTERPRISE_REMOTE"
|
||||
set -x
|
||||
|
||||
branch_name="${CIRCLE_BRANCH}"
|
||||
git remote set-url --push enterprise no-pushing
|
||||
|
||||
# check if the branch on community exists on enterprise
|
||||
# the output will not be empty if it does
|
||||
if [ `git branch -r --list origin/$branch_name` ]
|
||||
then
|
||||
try_merge enterprise-master origin/$branch_name
|
||||
else
|
||||
# add community as a remote if not already added
|
||||
set +e
|
||||
if ! git ls-remote community > /dev/null; then
|
||||
set -e
|
||||
git remote add --no-tags community git@github.com:citusdata/citus.git
|
||||
fi
|
||||
set -e
|
||||
# Fetch enterprise-master
|
||||
git fetch enterprise enterprise-master
|
||||
|
||||
# prevent pushes to community and update branch we care about
|
||||
git remote set-url --push community no-pushing
|
||||
git fetch community $branch_name
|
||||
|
||||
try_merge enterprise-master community/$branch_name
|
||||
git checkout "enterprise/enterprise-master"
|
||||
|
||||
if git merge --no-commit "origin/$PR_BRANCH"; then
|
||||
echo "INFO: community PR branch could be merged into enterprise-master, so everything is good"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# undo partial merge
|
||||
git merge --abort
|
||||
|
||||
if ! git fetch enterprise "$PR_BRANCH" ; then
|
||||
echo "ERROR: enterprise/$PR_BRANCH was not found and community PR branch could not be merged into enterprise-master"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Show the top commit of the enterprise PR branch to make debugging easier
|
||||
git log -n 1 "enterprise/$PR_BRANCH"
|
||||
|
||||
# Check that this branch contains the top commit of the current community PR
|
||||
# branch. If it does not it means it's not up to date with the current PR, so
|
||||
# the enterprise branch should be updated.
|
||||
if ! git merge-base --is-ancestor "origin/$PR_BRANCH" "enterprise/$PR_BRANCH" ; then
|
||||
echo "ERROR: enterprise/$PR_BRANCH is not up to date with community PR branch"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Now check if we can merge the enterprise PR into enterprise-master without
|
||||
# issues.
|
||||
git merge --no-commit "enterprise/$PR_BRANCH"
|
||||
|
|
|
|||
Loading…
Reference in New Issue