You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/11/16 15:47:37 UTC

[GitHub] [beam] suztomo commented on a change in pull request #13342: [BEAM-11260][BEAM-11261] Remove inappropriate assumptions about repo from linkage check script

suztomo commented on a change in pull request #13342:
URL: https://github.com/apache/beam/pull/13342#discussion_r524361509



##########
File path: sdks/java/build-tools/beam-linkage-check.sh
##########
@@ -36,46 +36,60 @@ set -o pipefail
 set -e
 
 # These default artifacts are common causes of linkage errors.
-ARTIFACTS="beam-sdks-java-core
-  beam-sdks-java-io-google-cloud-platform
-  beam-runners-google-cloud-dataflow-java
-  beam-sdks-java-io-hadoop-format"
-
-if [ ! -z "$1" ]; then
-  ARTIFACTS=$1
+DEFAULT_ARTIFACT_LISTS=" \
+  beam-sdks-java-core \
+  beam-sdks-java-io-google-cloud-platform \
+  beam-runners-google-cloud-dataflow-java \
+  beam-sdks-java-io-hadoop-format \
+"
+
+BASELINE_REF=$1
+PROPOSED_REF=$2
+ARTIFACT_LISTS=$3
+
+if [ -z "$ARTIFACT_LISTS" ]; then
+  ARTIFACT_LISTS=$DEFAULT_ARTIFACT_LISTS
 fi
 
-BRANCH_NAME=$(git symbolic-ref --short HEAD)
+if [ -z "$BASELINE_REF" ] || [ -z "$PROPOSED_REF" ] || [ -z "$ARTIFACT_LISTS" ] ; then
+  echo "Usage: $0 <baseline ref> <proposed ref> [artifact lists]"
+  exit 1
+fi
 
 if [ ! -d buildSrc ]; then
-  echo "Please run this script in the Beam project root:"
-  echo "  /bin/bash sdks/java/build-tools/beam-linkage-check.sh"
-  exit 1
+  echo "Directory 'buildSrc' not found. Please run this script from the root directory of a clone of the Beam git repo."
 fi
 
-if [ "$BRANCH_NAME" = "master" ]; then
-  echo "Please run this script on a branch other than master"
+if [ "$BASELINE_REF" = "$PROPOSED_REF" ]; then

Review comment:
       Nice enhancement.

##########
File path: sdks/java/build-tools/beam-linkage-check.sh
##########
@@ -36,46 +36,60 @@ set -o pipefail
 set -e
 
 # These default artifacts are common causes of linkage errors.
-ARTIFACTS="beam-sdks-java-core
-  beam-sdks-java-io-google-cloud-platform
-  beam-runners-google-cloud-dataflow-java
-  beam-sdks-java-io-hadoop-format"
-
-if [ ! -z "$1" ]; then
-  ARTIFACTS=$1
+DEFAULT_ARTIFACT_LISTS=" \

Review comment:
       Comman-separated list: Linkage Checker analyzes a dependency graph generated from the list of artifact, as if a project depends on the list of artifacts. For example, running Linkage Checker for "io.grpc:grpc-context:1.23.0,com.google.guava:guava:28.0-jre", checks a dependency graph of a pseudo project that depends on the two artifacts.
   
   Space-separated list: different invocation of Linkage Checker.
   
   I will improve documentation / comments regarding this.

##########
File path: sdks/java/build-tools/beam-linkage-check.sh
##########
@@ -36,46 +36,60 @@ set -o pipefail
 set -e
 
 # These default artifacts are common causes of linkage errors.
-ARTIFACTS="beam-sdks-java-core
-  beam-sdks-java-io-google-cloud-platform
-  beam-runners-google-cloud-dataflow-java
-  beam-sdks-java-io-hadoop-format"
-
-if [ ! -z "$1" ]; then
-  ARTIFACTS=$1
+DEFAULT_ARTIFACT_LISTS=" \
+  beam-sdks-java-core \
+  beam-sdks-java-io-google-cloud-platform \
+  beam-runners-google-cloud-dataflow-java \
+  beam-sdks-java-io-hadoop-format \
+"
+
+BASELINE_REF=$1
+PROPOSED_REF=$2
+ARTIFACT_LISTS=$3
+
+if [ -z "$ARTIFACT_LISTS" ]; then
+  ARTIFACT_LISTS=$DEFAULT_ARTIFACT_LISTS
 fi
 
-BRANCH_NAME=$(git symbolic-ref --short HEAD)
+if [ -z "$BASELINE_REF" ] || [ -z "$PROPOSED_REF" ] || [ -z "$ARTIFACT_LISTS" ] ; then
+  echo "Usage: $0 <baseline ref> <proposed ref> [artifact lists]"
+  exit 1
+fi
 
 if [ ! -d buildSrc ]; then
-  echo "Please run this script in the Beam project root:"
-  echo "  /bin/bash sdks/java/build-tools/beam-linkage-check.sh"
-  exit 1
+  echo "Directory 'buildSrc' not found. Please run this script from the root directory of a clone of the Beam git repo."
 fi
 
-if [ "$BRANCH_NAME" = "master" ]; then
-  echo "Please run this script on a branch other than master"
+if [ "$BASELINE_REF" = "$PROPOSED_REF" ]; then
+  echo "Baseline and proposed refs are identical; cannot compare their linkage errors!"
   exit 1
 fi
 
-OUTPUT_DIR=build/linkagecheck
-mkdir -p $OUTPUT_DIR
-
 if [ ! -z "$(git diff)" ]; then
   echo "Uncommited change detected. Please commit changes and ensure 'git diff' is empty."
   exit 1
 fi
 
+STARTING_REF=$(git rev-parse --abbrev-ref HEAD)
+function cleanup() {
+  git checkout $STARTING_REF
+}
+trap cleanup EXIT

Review comment:
       Nice!




----------------------------------------------------------------
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.

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