From ab6bedd85dc29906ac2f175f603ae3b43ab03535 Mon Sep 17 00:00:00 2001
From: Josh Rosen <joshrosen@databricks.com>
Date: Mon, 28 Dec 2015 10:40:03 -0800
Subject: [SPARK-12508][PROJECT-INFRA] Fix minor bugs in
 dev/tests/pr_public_classes.sh script

This patch fixes a handful of minor bugs in the `dev/tests/pr_public_classes.sh` script, which is used by the `run_tests_jenkins` script to detect the addition of new public classes:

- Account for differences between BSD and GNU `sed` in order to allow the script to run on OS X.
- Diff `$ghprbActualCommit^...$ghprbActualCommit ` instead of `master...$ghprbActualCommit`: since `ghprbActualCommit` is a merge commit which results from merging the PR into the target branch, this will give us the desired diff and will avoid certain race-conditions which could lead to false-positives.
- Use `echo -e` instead of `echo` so that newline characters are handled correctly in output. This should fix a formatting glitch which caused the output to appear on a single line in the GitHub comment (see [the SC2028 page](https://github.com/koalaman/shellcheck/wiki/SC2028) on the Shellcheck wiki for more details).

Author: Josh Rosen <joshrosen@databricks.com>

Closes #10455 from JoshRosen/fix-pr-public-classes-test.
---
 dev/tests/pr_public_classes.sh | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

(limited to 'dev')

diff --git a/dev/tests/pr_public_classes.sh b/dev/tests/pr_public_classes.sh
index 927295b88c..41c5d3ee8c 100755
--- a/dev/tests/pr_public_classes.sh
+++ b/dev/tests/pr_public_classes.sh
@@ -24,36 +24,44 @@
 #
 # Arg1: The Github Pull Request Actual Commit
 #+ known as `ghprbActualCommit` in `run-tests-jenkins`
-# Arg2: The SHA1 hash
-#+ known as `sha1` in `run-tests-jenkins`
-#
-
-# We diff master...$ghprbActualCommit because that gets us changes introduced in the PR
-#+ and not anything else added to master since the PR was branched.
 
 ghprbActualCommit="$1"
-sha1="$2"
+
+# $ghprbActualCommit is an automatic merge commit generated by GitHub; its parents are some Spark
+# master commit and the tip of the pull request branch.
+
+# By diffing$ghprbActualCommit^...$ghprbActualCommit and filtering to examine the diffs of only
+# non-test files, we can gets us changes introduced in the PR and not anything else added to master
+# since the PR was branched.
+
+# Handle differences between GNU and BSD sed
+if [[ $(uname) == "Darwin" ]]; then
+    SED='sed -E'
+else
+    SED='sed -r'
+fi
 
 source_files=$(
-  git diff master...$ghprbActualCommit --name-only  `# diff patch against master from branch point` \
+  git diff $ghprbActualCommit^...$ghprbActualCommit --name-only  `# diff patch against master from branch point` \
     | grep -v -e "\/test"                               `# ignore files in test directories` \
     | grep -e "\.py$" -e "\.java$" -e "\.scala$"        `# include only code files` \
     | tr "\n" " "
 )
+
 new_public_classes=$(
-  git diff master...$ghprbActualCommit ${source_files}      `# diff patch against master from branch point` \
+  git diff $ghprbActualCommit^...$ghprbActualCommit ${source_files}      `# diff patch against master from branch point` \
     | grep "^\+"                              `# filter in only added lines` \
-    | sed -r -e "s/^\+//g"                    `# remove the leading +` \
+    | $SED -e "s/^\+//g"                      `# remove the leading +` \
     | grep -e "trait " -e "class "            `# filter in lines with these key words` \
     | grep -e "{" -e "("                      `# filter in lines with these key words, too` \
     | grep -v -e "\@\@" -e "private"          `# exclude lines with these words` \
     | grep -v -e "^// " -e "^/\*" -e "^ \* "  `# exclude comment lines` \
-    | sed -r -e "s/\{.*//g"                   `# remove from the { onwards` \
-    | sed -r -e "s/\}//g"                     `# just in case, remove }; they mess the JSON` \
-    | sed -r -e "s/\"/\\\\\"/g"               `# escape double quotes; they mess the JSON` \
-    | sed -r -e "s/^(.*)$/\`\1\`/g"           `# surround with backticks for style` \
-    | sed -r -e "s/^/  \* /g"                 `# prepend '  *' to start of line` \
-    | sed -r -e "s/$/\\\n/g"                  `# append newline to end of line` \
+    | $SED -e "s/\{.*//g"                     `# remove from the { onwards` \
+    | $SED -e "s/\}//g"                       `# just in case, remove }; they mess the JSON` \
+    | $SED -e "s/\"/\\\\\"/g"                 `# escape double quotes; they mess the JSON` \
+    | $SED -e "s/^(.*)$/\`\1\`/g"             `# surround with backticks for style` \
+    | $SED -e "s/^/  \* /g"                   `# prepend '  *' to start of line` \
+    | $SED -e "s/$/\\\n/g"                    `# append newline to end of line` \
     | tr -d "\n"                              `# remove actual LF characters`
 )
 
@@ -61,5 +69,5 @@ if [ -z "$new_public_classes" ]; then
   echo " * This patch adds no public classes."
 else
   public_classes_note=" * This patch adds the following public classes _(experimental)_:"
-  echo "${public_classes_note}\n${new_public_classes}"
+  echo -e "${public_classes_note}\n${new_public_classes}"
 fi
-- 
cgit v1.2.3