[Tarantool-patches] [PATCH] tools: add a script that checks submodules' commits

[Tarantool-patches] [PATCH] tools: add a script that checks submodules' commits

[Aleksandr Lyapunov via Tarantool-patches]

Sometimes to fix an issue we need to patch our submodule, update it
in tarantool and then patch tarantool using changes in submodule.

During development and review stages we have to create a branch (S)
in submodule and a branch (T) in tarantool that references head of
(S) branch.

When the fix is merged to mater it's very simple to make a mistake:
merge-and-push submodule's branch S and then tarantool's branch T.
It sounds obvious, but that's wrong: tarantool's master should
reference a commit from submodule's master, not a commit from
temporary developer's branch.

It's not hard to fix it but still a maintainer must always remember
that problem. In order to simplify their life it was decided to
create a script that is designed to check such thing before pushing
tarantool to origin/master.

Here is it, with simple usage:
./tools/check_push_master.sh && git push origin master
---
 tools/check_push_master.sh | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100755 tools/check_push_master.sh

diff --git a/tools/check_push_master.sh b/tools/check_push_master.sh
new file mode 100755
index 0000000..5bc2762
--- /dev/null
+++ b/tools/check_push_master.sh
@@ -0,0 +1,29 @@
+#!/bin/bash
+
+# This script is recommended to run before pushing master to origin.
+# It fails (returns 1) if a problem detected, see output.
+exit_status=0
+
+# Check that submodules' commits are from their master branches.
+# The script checks all submodules, but fails only for the following list:
+our_submodules="src/lib/msgpuck:src/lib/small:test-run"
+
+function check_submodule() {
+    local submodule_path=$1
+    local commit=$2
+    git "--git-dir=$(git rev-parse --show-toplevel)/$submodule_path/.git" branch -r --contains $commit | egrep " origin/master$" > /dev/null
+    local commit_is_from_master=$?
+    [[ ":$our_submodules:" =~ ":$submodule_path:" ]]
+    local it_is_our_submodule=$?
+    if [[ $it_is_our_submodule -eq 0 ]] && [[ $commit_is_from_master -eq 1 ]]; then
+        echo "Submodule $submodule_path is set to commit $commit that is not in master branch!"
+        exit_status=1
+    fi
+}
+
+while read -r line; do
+    check_submodule $line
+done <<<"`git "--git-dir=$(git rev-parse --show-toplevel)/.git" ls-tree -r HEAD | egrep "^[0-9]+ commit" | awk '{print $4 " " $3}'`"
+
+# Done
+exit $exit_status
\ No newline at end of file
-- 
2.7.4

Re: [Tarantool-patches] [PATCH] tools: add a script that checks submodules' commits

[Alexander Turenko via Tarantool-patches]

Nice idea :)

I don't mind any scripts that ease developer / maintainer life.

The patch is okay for push (from my point of view), however I would
share some thoughts around. Feel free to pass them over if you don't
want to pay more time here.

I would extend the script for all long-term branches (1.10, 2.*, master)
with a submodule specific rules:

1. test-run's commit should be reachable from test-run's master for all
   long-term branches.
2. I guess it is the same for small, but from what I heard, we possibly
   will fork a stable branch. So the rule will be like (not sure about X
   value):
   - if tarantool's branch is 1.10 / 2.X (X < 8) => small's commit should
     be reachable from small's tarantool-1.10 branch.
   - 2.X (X >= 8), master => reachable from small's master.
3. There is master and tarantool-1.10 in msgpuck, similar to above.

I would also explicitly skip the checks for pushing to any other branch.
So the script may be called before any push and may be integrated into
`.git/hooks/pre-push`. This means, a maintainer will place it here once
and will not need to run it manually.

Maybe it worth to look at the `.git/hooks/pre-push` format from
beginning. At least it accepts a branch to push as the second parameter
that may be useful.

Anyway, the script is useful without those enhancements.

WBR, Alexander Turenko.

> When the fix is merged to mater it's very simple to make a mistake:

Just if'll spin around again: mater -> master.