mcvsubbu commented on a change in pull request #7149:
URL: https://github.com/apache/incubator-pinot/pull/7149#discussion_r668214495



##########
File path: compatibility-verifier/checkoutAndBuild.sh
##########
@@ -86,67 +100,145 @@ while [ "$1" != "" ]; do
   shift
 done
 
-if [ -z "$olderCommit" -a ! -z "$newerCommit" ]; then
-   echo "If old commit hash is not given, then new commit has should not be 
given either"
-   usage
-   exit 1
-fi
-
-if [ -z "$olderCommit" ]; then
-  # Newer commit must also be null, so take the previous commit as the older 
one.
-  olderCommit=`git log  --pretty=format:'%h' -n 2|tail -1`
-fi
 
+# Validate and create working directory; set up build path names
 if [ -z $workingDir ]; then
   echo "Working directory must be specified"
   usage
   exit 1
 fi
-
 if [ -d $workingDir ]; then
   echo "Directory ${workingDir} already exists. Use a new directory."
   usage
   exit 1
 fi
-
 mkdir -p ${workingDir}
 if [ $? -ne 0 ]; then
   echo "Could not create ${workingDir}"
   exit 1
 fi
 workingDir=$(absPath "$workingDir")
-
+cmdDir=$(absPath "$cmdDir")
+oldBuildOutFile="${workingDir}/oldBuild.out"
+oldTargetDir="$workingDir"/oldTargetDir
+newBuildOutFile="${workingDir}/newBuild.out"
 newTargetDir="$workingDir"/newTargetDir
+curBuildOutFile="${workingDir}/currentBuild.out"
+
+# Find value of olderCommit hash
+if [ -z "$olderCommit" -a ! -z "$newerCommit" ]; then
+   echo "If old commit hash is not given, then new commit should not be given 
either"
+   usage
+   exit 1
+fi
+if [ -z "$olderCommit" ]; then
+  # Newer commit must also be null, so take the previous commit as the older 
one.
+  olderCommit=`git log  --pretty=format:'%h' -n 2|tail -1`
+fi
+
+buildNewTarget=0
+
 if [ -z "$newerCommit" ]; then
-  echo "Compiling current tree as newer version"
-  (cd $cmdDir/.. && mvn install package -DskipTests -Pbin-dist -D 
jdk.version=8 ${PINOT_MAVEN_OPTS} && mvn -pl pinot-tools package -DskipTests 
-Djdk.version=8 ${PINOT_MAVEN_OPTS} && mvn -pl pinot-integration-tests package 
-DskipTests -Djdk.version=8 ${PINOT_MAVEN_OPTS})
-  if [ $? -ne 0 ]; then
-    echo Compile failed.
-    exit 1
-  fi
-  ln -s $(absPath "${cmdDir}/..") "${newTargetDir}"
+  echo "Assuming current tree as newer version"
+  ln -s "${cmdDir}/.." "${newTargetDir}"
 else
   if ! mkdir -p "$newTargetDir"; then
     echo "Failed to create target directory ${newTargetDir}"
     exit 1
   fi
-  echo "Building the new version ..."
-  checkoutAndBuild "$newerCommit" "$newTargetDir"
-  if [ $? -ne 0 ];then
-    echo Could not build new version at "${newerCommit}"
-    exit 1
-  fi
+  echo "Checking out new version at commit \"${newerCommit}\""
+  checkOut "$newerCommit" "$newTargetDir"
+  buildNewTarget=1
 fi
 
-oldTargetDir="$workingDir"/oldTargetDir
 if ! mkdir -p "$oldTargetDir"; then
   echo "Failed to create target directory ${oldTargetDir}"
   exit 1
 fi
-echo "Building the old version ... "
-checkoutAndBuild "$olderCommit" "$oldTargetDir"
-if [ $? -ne 0 ]; then
-  echo Could not build older version at "${olderCommit}"
-  exit 1
+
+echo "Checking out old version at commit \"${olderCommit}\""
+checkOut "$olderCommit" "$oldTargetDir"
+
+# Start builds in parallel.
+# We need to build current directory and old commit for sure.
+# In addition, if the newer version is not the current one,
+# we will need to build that as well.
+echo Starting build for compat checker at ${cmdDir}
+(cd ${cmdDir}/..; build ${curBuildOutFile} 1) &
+curBuildPid=$!
+echo Starting build for old version at ${oldTargetDir}
+(cd ${oldTargetDir}; build ${oldBuildOutFile} 0) &
+oldBuildPid=$!
+if [ ${buildNewTarget} -eq 1 ]; then
+  echo Starting build for new version at ${newTargetDir}
+  (cd ${newTargetDir}; build ${newBuildOutFile} 0) &
+  newBuildPid=$!
 fi
+
+echo Awaiting build complete for old commit
+while true ; do
+  printf "."

Review comment:
       This prints a "." character until the build is completed. Since builds 
are happening in background, one dot is printed every 5 seconds, showing (sort 
of like) a progress bar. Printing an entire message will just cause a lot of 
output here, and lose the significance of the messages printed before and after 
it.

##########
File path: compatibility-verifier/checkoutAndBuild.sh
##########
@@ -86,67 +100,145 @@ while [ "$1" != "" ]; do
   shift
 done
 
-if [ -z "$olderCommit" -a ! -z "$newerCommit" ]; then
-   echo "If old commit hash is not given, then new commit has should not be 
given either"
-   usage
-   exit 1
-fi
-
-if [ -z "$olderCommit" ]; then
-  # Newer commit must also be null, so take the previous commit as the older 
one.
-  olderCommit=`git log  --pretty=format:'%h' -n 2|tail -1`
-fi
 
+# Validate and create working directory; set up build path names
 if [ -z $workingDir ]; then
   echo "Working directory must be specified"
   usage
   exit 1
 fi
-
 if [ -d $workingDir ]; then
   echo "Directory ${workingDir} already exists. Use a new directory."
   usage
   exit 1
 fi
-
 mkdir -p ${workingDir}
 if [ $? -ne 0 ]; then
   echo "Could not create ${workingDir}"
   exit 1
 fi
 workingDir=$(absPath "$workingDir")
-
+cmdDir=$(absPath "$cmdDir")
+oldBuildOutFile="${workingDir}/oldBuild.out"
+oldTargetDir="$workingDir"/oldTargetDir
+newBuildOutFile="${workingDir}/newBuild.out"
 newTargetDir="$workingDir"/newTargetDir
+curBuildOutFile="${workingDir}/currentBuild.out"
+
+# Find value of olderCommit hash
+if [ -z "$olderCommit" -a ! -z "$newerCommit" ]; then
+   echo "If old commit hash is not given, then new commit should not be given 
either"
+   usage
+   exit 1
+fi
+if [ -z "$olderCommit" ]; then
+  # Newer commit must also be null, so take the previous commit as the older 
one.
+  olderCommit=`git log  --pretty=format:'%h' -n 2|tail -1`
+fi
+
+buildNewTarget=0
+
 if [ -z "$newerCommit" ]; then
-  echo "Compiling current tree as newer version"
-  (cd $cmdDir/.. && mvn install package -DskipTests -Pbin-dist -D 
jdk.version=8 ${PINOT_MAVEN_OPTS} && mvn -pl pinot-tools package -DskipTests 
-Djdk.version=8 ${PINOT_MAVEN_OPTS} && mvn -pl pinot-integration-tests package 
-DskipTests -Djdk.version=8 ${PINOT_MAVEN_OPTS})
-  if [ $? -ne 0 ]; then
-    echo Compile failed.
-    exit 1
-  fi
-  ln -s $(absPath "${cmdDir}/..") "${newTargetDir}"
+  echo "Assuming current tree as newer version"
+  ln -s "${cmdDir}/.." "${newTargetDir}"
 else
   if ! mkdir -p "$newTargetDir"; then
     echo "Failed to create target directory ${newTargetDir}"
     exit 1
   fi
-  echo "Building the new version ..."
-  checkoutAndBuild "$newerCommit" "$newTargetDir"
-  if [ $? -ne 0 ];then
-    echo Could not build new version at "${newerCommit}"
-    exit 1
-  fi
+  echo "Checking out new version at commit \"${newerCommit}\""
+  checkOut "$newerCommit" "$newTargetDir"
+  buildNewTarget=1
 fi
 
-oldTargetDir="$workingDir"/oldTargetDir
 if ! mkdir -p "$oldTargetDir"; then
   echo "Failed to create target directory ${oldTargetDir}"
   exit 1
 fi
-echo "Building the old version ... "
-checkoutAndBuild "$olderCommit" "$oldTargetDir"
-if [ $? -ne 0 ]; then
-  echo Could not build older version at "${olderCommit}"
-  exit 1
+
+echo "Checking out old version at commit \"${olderCommit}\""
+checkOut "$olderCommit" "$oldTargetDir"
+
+# Start builds in parallel.
+# We need to build current directory and old commit for sure.
+# In addition, if the newer version is not the current one,
+# we will need to build that as well.
+echo Starting build for compat checker at ${cmdDir}
+(cd ${cmdDir}/..; build ${curBuildOutFile} 1) &
+curBuildPid=$!
+echo Starting build for old version at ${oldTargetDir}
+(cd ${oldTargetDir}; build ${oldBuildOutFile} 0) &
+oldBuildPid=$!
+if [ ${buildNewTarget} -eq 1 ]; then
+  echo Starting build for new version at ${newTargetDir}
+  (cd ${newTargetDir}; build ${newBuildOutFile} 0) &
+  newBuildPid=$!
 fi
+
+echo Awaiting build complete for old commit
+while true ; do
+  printf "."
+  ps -p ${oldBuildPid} 1>/dev/null 2>&1
+  if [ $? -ne 0 ]; then
+    printf "\n"
+    wait ${oldBuildPid}
+    oldBuildStatus=$?
+    break
+  fi
+  sleep 5
+done
+
+echo Awaiting build complete for compat checker
+while true ; do
+  printf "."

Review comment:
       Same

##########
File path: compatibility-verifier/checkoutAndBuild.sh
##########
@@ -86,67 +100,145 @@ while [ "$1" != "" ]; do
   shift
 done
 
-if [ -z "$olderCommit" -a ! -z "$newerCommit" ]; then
-   echo "If old commit hash is not given, then new commit has should not be 
given either"
-   usage
-   exit 1
-fi
-
-if [ -z "$olderCommit" ]; then
-  # Newer commit must also be null, so take the previous commit as the older 
one.
-  olderCommit=`git log  --pretty=format:'%h' -n 2|tail -1`
-fi
 
+# Validate and create working directory; set up build path names
 if [ -z $workingDir ]; then
   echo "Working directory must be specified"
   usage
   exit 1
 fi
-
 if [ -d $workingDir ]; then
   echo "Directory ${workingDir} already exists. Use a new directory."
   usage
   exit 1
 fi
-
 mkdir -p ${workingDir}
 if [ $? -ne 0 ]; then
   echo "Could not create ${workingDir}"
   exit 1
 fi
 workingDir=$(absPath "$workingDir")
-
+cmdDir=$(absPath "$cmdDir")
+oldBuildOutFile="${workingDir}/oldBuild.out"
+oldTargetDir="$workingDir"/oldTargetDir
+newBuildOutFile="${workingDir}/newBuild.out"
 newTargetDir="$workingDir"/newTargetDir
+curBuildOutFile="${workingDir}/currentBuild.out"
+
+# Find value of olderCommit hash
+if [ -z "$olderCommit" -a ! -z "$newerCommit" ]; then
+   echo "If old commit hash is not given, then new commit should not be given 
either"
+   usage
+   exit 1
+fi
+if [ -z "$olderCommit" ]; then
+  # Newer commit must also be null, so take the previous commit as the older 
one.
+  olderCommit=`git log  --pretty=format:'%h' -n 2|tail -1`
+fi
+
+buildNewTarget=0
+
 if [ -z "$newerCommit" ]; then
-  echo "Compiling current tree as newer version"
-  (cd $cmdDir/.. && mvn install package -DskipTests -Pbin-dist -D 
jdk.version=8 ${PINOT_MAVEN_OPTS} && mvn -pl pinot-tools package -DskipTests 
-Djdk.version=8 ${PINOT_MAVEN_OPTS} && mvn -pl pinot-integration-tests package 
-DskipTests -Djdk.version=8 ${PINOT_MAVEN_OPTS})
-  if [ $? -ne 0 ]; then
-    echo Compile failed.
-    exit 1
-  fi
-  ln -s $(absPath "${cmdDir}/..") "${newTargetDir}"
+  echo "Assuming current tree as newer version"
+  ln -s "${cmdDir}/.." "${newTargetDir}"
 else
   if ! mkdir -p "$newTargetDir"; then
     echo "Failed to create target directory ${newTargetDir}"
     exit 1
   fi
-  echo "Building the new version ..."
-  checkoutAndBuild "$newerCommit" "$newTargetDir"
-  if [ $? -ne 0 ];then
-    echo Could not build new version at "${newerCommit}"
-    exit 1
-  fi
+  echo "Checking out new version at commit \"${newerCommit}\""
+  checkOut "$newerCommit" "$newTargetDir"
+  buildNewTarget=1
 fi
 
-oldTargetDir="$workingDir"/oldTargetDir
 if ! mkdir -p "$oldTargetDir"; then
   echo "Failed to create target directory ${oldTargetDir}"
   exit 1
 fi
-echo "Building the old version ... "
-checkoutAndBuild "$olderCommit" "$oldTargetDir"
-if [ $? -ne 0 ]; then
-  echo Could not build older version at "${olderCommit}"
-  exit 1
+
+echo "Checking out old version at commit \"${olderCommit}\""
+checkOut "$olderCommit" "$oldTargetDir"
+
+# Start builds in parallel.
+# We need to build current directory and old commit for sure.
+# In addition, if the newer version is not the current one,
+# we will need to build that as well.
+echo Starting build for compat checker at ${cmdDir}
+(cd ${cmdDir}/..; build ${curBuildOutFile} 1) &

Review comment:
       Comments are in line 162 through 165. If they are not clear enough, 
please suggest alternate wording




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to