kennknowles commented on a change in pull request #13342:
URL: https://github.com/apache/beam/pull/13342#discussion_r523262786
##########
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)
Review comment:
This is so it can be run from a different branch than either of the
changes. For example I used this to test this branch. More generally, this is
helpful to test any changes to the script. We can run them against known other
refs with changes.
##########
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
+
+echo "Comparing linkage of artifact lists $ARTIFACT_LISTS using baseline
$BASELINE_REF and proposal $PROPOSED_REF"
+
+OUTPUT_DIR=build/linkagecheck
+mkdir -p $OUTPUT_DIR
+
ACCUMULATED_RESULT=0
function runLinkageCheck () {
- COMMIT=$1
- BRANCH=$2
- MODE=$3 # "baseline" or "validate"
- for ARTIFACT in $ARTIFACTS; do
- echo "`date`:" "Running linkage check (${MODE}) for ${ARTIFACT} in
${BRANCH}"
+ MODE=$1 # "baseline" or "validate"
- BASELINE_FILE=${OUTPUT_DIR}/baseline-${ARTIFACT}.xml
+ for ARTIFACT_LIST in $ARTIFACT_LISTS; do
Review comment:
Have a space-separated list of comma-separated lists seems weird. I kept
it, but maybe we can simplify?
##########
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:
I guess these should actually get `rev-parse` to check that they are
different hashes.
##########
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."
Review comment:
We could also possibly do a smarter check using `git status` or
`./gradlew model`
##########
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
+
+echo "Comparing linkage of artifact lists $ARTIFACT_LISTS using baseline
$BASELINE_REF and proposal $PROPOSED_REF"
+
+OUTPUT_DIR=build/linkagecheck
+mkdir -p $OUTPUT_DIR
+
ACCUMULATED_RESULT=0
function runLinkageCheck () {
- COMMIT=$1
- BRANCH=$2
- MODE=$3 # "baseline" or "validate"
- for ARTIFACT in $ARTIFACTS; do
- echo "`date`:" "Running linkage check (${MODE}) for ${ARTIFACT} in
${BRANCH}"
+ MODE=$1 # "baseline" or "validate"
- BASELINE_FILE=${OUTPUT_DIR}/baseline-${ARTIFACT}.xml
+ for ARTIFACT_LIST in $ARTIFACT_LISTS; do
+ echo "`date`:" "Running linkage check (${MODE}) for ${ARTIFACT_LISTS}"
+
+ BASELINE_FILE=${OUTPUT_DIR}/baseline-${ARTIFACT_LIST}.xml
Review comment:
This would be cleaner if the `checkLinkage` task just put something in
`build/reports/linkageChecker` and this script copied it out to a temp dir for
the purpose of comparison.
##########
File path: sdks/java/build-tools/beam-linkage-check.sh
##########
@@ -88,29 +102,25 @@ function runLinkageCheck () {
fi
set +e
- ./gradlew -Ppublishing -PjavaLinkageArtifactIds=$ARTIFACT
${BASELINE_OPTION} :checkJavaLinkage
+ set -x
+ ./gradlew -Ppublishing -PskipCheckerFramework
-PjavaLinkageArtifactIds=$ARTIFACT_LIST ${BASELINE_OPTION} :checkJavaLinkage
RESULT=$?
set -e
+ set +x
if [ "$MODE" = "validate" ]; then
echo "`date`:" "Done: ${RESULT}"
ACCUMULATED_RESULT=$((ACCUMULATED_RESULT | RESULT))
fi
done
}
+echo "Establishing baseline linkage for $(git rev-parse --abbrev-ref
$BASELINE_REF)"
+git -c advice.detachedHead=false checkout $BASELINE_REF
+runLinkageCheck baseline
Review comment:
The only nontrivial shared code between `runLinkageCheck baseline` and
`runLinkageCheck validate` is the loop over the artifact lists. So if we didn't
need the loop, we may as well just inline the functions.
----------------------------------------------------------------
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:
[email protected]
