X-Git-Url: https://git.arvados.org/arvados-dev.git/blobdiff_plain/2147a538d2b64392a2c86e90a123c47e7efaac24..400c39aabb36fa98b8878806bd5210677495527a:/git/hooks/coding-standards.sh diff --git a/git/hooks/coding-standards.sh b/git/hooks/coding-standards.sh index d1b03dc..d4e4c71 100755 --- a/git/hooks/coding-standards.sh +++ b/git/hooks/coding-standards.sh @@ -13,6 +13,20 @@ $oldrev = ARGV[1] $newrev = ARGV[2] $user = ENV['USER'] +def blacklist bl + all_revs = `git rev-list #{$oldrev}..#{$newrev}`.split("\n") + all_revs.each do |rev| + bl.each do |b| + if rev == b + puts "Revision #{b} is blacklisted, you must remove it from your branch (possibly using git rebase) before you can push." + exit 1 + end + end + end +end + +blacklist ['26d74dc0524c87c5dcc0c76040ce413a4848b57a'] + # Only enforce policy on the master branch exit 0 if $refname != 'refs/heads/master' @@ -24,7 +38,7 @@ $broken_commit_message = /Please enter a commit message to explain why this merg $wrong_way_merge_master = /Merge( remote-tracking)? branch '([^\/]+\/)?master' into/ $merge_master = /Merge branch '[^']+'((?! into)| into master)/ $pull_merge = /Merge branch 'master' of / -$refs_or_closes_or_no_issue = /(refs #|closes #|no issue #)/i +$refs_or_closes_or_no_issue = /(refs #|closes #|fixes #|no issue #)/i # enforced custom commit message format def check_message_format @@ -32,37 +46,42 @@ def check_message_format merge_revs = `git rev-list --first-parent --min-parents=2 #{$oldrev}..#{$newrev}`.split("\n") # single_revs = `git rev-list --first-parent --max-parents=1 #{$oldrev}..#{$newrev}`.split("\n") broken = false + no_ff = false merge_revs.each do |rev| message = `git cat-file commit #{rev} | sed '1,/^$/d'` if $wrong_way_merge_master.match(message) - puts "\n[POLICY] This appears to be a merge from master into a feature\n" - puts "\nbranch. Commits to master must merge from the feature\n" - puts "\nbranch into master.\n\n" + puts "\n[POLICY] Only non-fast-forward merges into master are allowed. Please" + puts "reset your master branch:" + puts " git reset --hard origin/master" + puts "and then merge your branch with the --no-ff option:" + puts " git merge your-branch --no-ff\n" + puts "Remember to add a reference to an issue number in the merge commit!\n" puts "\n******************************************************************\n" - puts "\nOffending commit: #{rev}\n\n" - puts "\nOffending commit message:\n\n" + puts "\nOffending commit: #{rev}\n" + puts "\nOffending commit message:\n" puts message puts "\n******************************************************************\n" puts "\n\n" broken = true + no_ff = true elsif $pull_merge.match(message) - puts "\n[POLICY] This appears to be a git pull merge of remote master into local\n" - puts "\nmaster. In order to maintain a linear first-parent history of master,\n" - puts "\nplease reset your branch and remerge or rebase using the latest master.\n" + puts "\n[POLICY] This appears to be a git pull merge of remote master into local" + puts "master. In order to maintain a linear first-parent history of master," + puts "please reset your branch and remerge or rebase using the latest master.\n" puts "\n******************************************************************\n" - puts "\nOffending commit: #{rev}\n\n" + puts "\nOffending commit: #{rev}\n" puts "\nOffending commit message:\n\n" puts message puts "\n******************************************************************\n" puts "\n\n" broken = true elsif not $merge_master.match(message) and not - puts "\n[POLICY] This does not appear to be a merge of a feature\n" - puts "\nbranch into master. Merges must follow the format\n" - puts "\n\"Merge branch 'feature-branch'\".\n" + puts "\n[POLICY] This does not appear to be a merge of a feature" + puts "branch into master. Merges must follow the format" + puts "\"Merge branch 'feature-branch'\".\n" puts "\n******************************************************************\n" - puts "\nOffending commit: #{rev}\n\n" + puts "\nOffending commit: #{rev}\n" puts "\nOffending commit message:\n\n" puts message puts "\n******************************************************************\n" @@ -74,10 +93,10 @@ def check_message_format all_revs.each do |rev| message = `git cat-file commit #{rev} | sed '1,/^$/d'` if $broken_commit_message.match(message) - puts "\n[POLICY] Rejected broken commit message for including boilerplate\n" - puts "\ninstruction text.\n\n" + puts "\n[POLICY] Rejected broken commit message for including boilerplate" + puts "instruction text.\n" puts "\n******************************************************************\n" - puts "\nOffending commit: #{rev}\n\n" + puts "\nOffending commit: #{rev}\n" puts "\nOffending commit message:\n\n" puts message puts "\n******************************************************************\n" @@ -85,11 +104,14 @@ def check_message_format broken = true end - if not $refs_or_closes_or_no_issue.match(message) - puts "\n[POLICY] All commits to master must include an issue using \"refs #\" or\n" - puts "\n\"closes #\", or specify \"no issue #\"\n\n" + # Do not test when the commit is a no_ff merge (which will be rejected), because + # this test will complain about *every* commit in the merge otherwise, obscuring + # the real reason for the rejection (the no_ff merge) + if not no_ff and not $refs_or_closes_or_no_issue.match(message) + puts "\n[POLICY] All commits to master must include an issue using \"refs #\" or" + puts "\"closes #\", or specify \"no issue #\"\n" puts "\n******************************************************************\n" - puts "\nOffending commit: #{rev}\n\n" + puts "\nOffending commit: #{rev}\n" puts "\nOffending commit message:\n\n" puts message puts "\n******************************************************************\n"