From 42f898bf839356a8e3aebad2bb0ec68bbba040d9 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Mon, 23 Sep 2019 09:05:18 -0700 Subject: [PATCH] Fix format-diff.sh detecting changes vs. upstream (#5831) Summary: format-diff.sh, a.k.a. 'make format', would use 'master' to decide which commits are probably unpublished. Much better to use facebook remote master since local master may not be caught up and may have its own unpublished commits. Script now tries to compare against facebook remote master branch (branch pointer is updated with any fetch or pull), because those differences are what would be considered the differences for a pull request. Also, script would compare against *parent* of merge-base with that reference point, which is just wrong since that includes the last published commit. In case of problems, you can now customize the reference point, by setting the FORMAT_UPSTREAM variable. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5831 Test Plan: manual Differential Revision: D17528462 Pulled By: pdillinger fbshipit-source-id: 50fdb8795d683bf3c14d449669c1a5299e0dfa8b --- build_tools/format-diff.sh | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/build_tools/format-diff.sh b/build_tools/format-diff.sh index f8fbb64fb..4c0f0e5d5 100755 --- a/build_tools/format-diff.sh +++ b/build_tools/format-diff.sh @@ -54,15 +54,25 @@ fi set -e uncommitted_code=`git diff HEAD` -LAST_MASTER=`git merge-base master HEAD` # If there's no uncommitted changes, we assume user are doing post-commit -# format check, in which case we'll check the modified lines since last commit -# from master. Otherwise, we'll check format of the uncommitted code only. +# format check, in which case we'll try to check the modified lines vs. the +# facebook/rocksdb.git master branch. Otherwise, we'll check format of the +# uncommitted code only. if [ -z "$uncommitted_code" ] then - # Check the format of last commit - diffs=$(git diff -U0 $LAST_MASTER^ | $CLANG_FORMAT_DIFF -p 1) + # Attempt to get name of facebook/rocksdb.git remote. + [ "$FORMAT_REMOTE" ] || FORMAT_REMOTE="$(git remote -v | grep 'facebook/rocksdb.git' | head -n 1 | cut -f 1)" + # Fall back on 'origin' if that fails + [ "$FORMAT_REMOTE" ] || FORMAT_REMOTE=origin + # Use master branch from that remote + [ "$FORMAT_UPSTREAM" ] || FORMAT_UPSTREAM="$FORMAT_REMOTE/master" + # Get the common ancestor with that remote branch. Everything after that + # common ancestor would be considered the contents of a pull request, so + # should be relevant for formatting fixes. + FORMAT_UPSTREAM_MERGE_BASE="$(git merge-base "$FORMAT_UPSTREAM" HEAD)" + # Get the differences + diffs=$(git diff -U0 "$FORMAT_UPSTREAM_MERGE_BASE" | $CLANG_FORMAT_DIFF -p 1) else # Check the format of uncommitted lines, diffs=$(git diff -U0 HEAD | $CLANG_FORMAT_DIFF -p 1) @@ -76,12 +86,12 @@ fi # Highlight the insertion/deletion from the clang-format-diff.py's output COLOR_END="\033[0m" -COLOR_RED="\033[0;31m" -COLOR_GREEN="\033[0;32m" +COLOR_RED="\033[0;31m" +COLOR_GREEN="\033[0;32m" echo -e "Detect lines that doesn't follow the format rules:\r" # Add the color to the diff. lines added will be green; lines removed will be red. -echo "$diffs" | +echo "$diffs" | sed -e "s/\(^-.*$\)/`echo -e \"$COLOR_RED\1$COLOR_END\"`/" | sed -e "s/\(^+.*$\)/`echo -e \"$COLOR_GREEN\1$COLOR_END\"`/" @@ -104,7 +114,7 @@ fi # Do in-place format adjustment. if [ -z "$uncommitted_code" ] then - git diff -U0 $LAST_MASTER^ | $CLANG_FORMAT_DIFF -i -p 1 + git diff -U0 "$FORMAT_UPSTREAM_MERGE_BASE" | $CLANG_FORMAT_DIFF -i -p 1 else git diff -U0 HEAD^ | $CLANG_FORMAT_DIFF -i -p 1 fi