XComp commented on code in PR #23195:
URL: https://github.com/apache/flink/pull/23195#discussion_r1297104015
##########
tools/ci/compile.sh:
##########
@@ -69,34 +80,38 @@ fi
echo "============ Checking Javadocs ============"
+javadoc_output=/tmp/javadoc.out
+
# use the same invocation as .github/workflows/docs.sh
-run_mvn javadoc:aggregate -DadditionalJOption='-Xdoclint:none' \
+$MVN javadoc:aggregate -DadditionalJOption='-Xdoclint:none' \
Review Comment:
```suggestion
$MVN javadoc:aggregate -T1 -DadditionalJOption='-Xdoclint:none' \
```
according to your statement "every call (apart from mvn exec has to run with
a parallelism of 1.". But tbh, I don't see value here because we're not parsing
the output. In this sense, it would be up for the user to decide. :shrug:
##########
tools/ci/compile.sh:
##########
@@ -69,34 +80,38 @@ fi
echo "============ Checking Javadocs ============"
+javadoc_output=/tmp/javadoc.out
+
# use the same invocation as .github/workflows/docs.sh
-run_mvn javadoc:aggregate -DadditionalJOption='-Xdoclint:none' \
+$MVN javadoc:aggregate -DadditionalJOption='-Xdoclint:none' \
-Dmaven.javadoc.failOnError=false -Dcheckstyle.skip=true
-Denforcer.skip=true -Dspotless.skip=true -Drat.skip=true \
- -Dheader=someTestHeader > javadoc.out
+ -Dheader=someTestHeader > ${javadoc_output}
EXIT_CODE=$?
if [ $EXIT_CODE != 0 ] ; then
echo "ERROR in Javadocs. Printing full output:"
- cat javadoc.out ; rm javadoc.out
+ cat ${javadoc_output}
exit $EXIT_CODE
fi
echo "============ Checking Scaladocs ============"
-run_mvn scala:doc -Dcheckstyle.skip=true -Denforcer.skip=true
-Dspotless.skip=true -pl flink-scala 2> scaladoc.out
+scaladoc_output=/tmp/scaladoc.out
+
+$MVN scala:doc -Dcheckstyle.skip=true -Denforcer.skip=true
-Dspotless.skip=true -pl flink-scala 2> ${scaladoc_output}
Review Comment:
```suggestion
$MVN scala:doc -T1 -Dcheckstyle.skip=true -Denforcer.skip=true
-Dspotless.skip=true -pl flink-scala 2> ${scaladoc_output}
```
according to your statement "every call (apart from mvn exec has to run with
a parallelism of 1.". But here as well: We're not parsing the output. So it
would be up to the user how to deal with it.
##########
tools/ci/verify_bundled_optional.sh:
##########
@@ -17,31 +17,58 @@
# limitations under the License.
#
+usage() {
+ echo "Usage: $0 <maven-build-output>"
+ echo " <maven-build-output> A file containing the output
of the Maven build."
+ echo ""
+ echo "mvnw clean package > <maven-build-output>"
+ echo ""
+ echo "The environment variable MVN is used to specify the Maven binaries;
defaults to 'mvnw'."
+ echo "See further details in the JavaDoc of ShadeOptionalChecker."
+}
+
+while getopts 'h' o; do
+ case "${o}" in
+ h)
+ usage
+ exit 0
+ ;;
+ esac
+done
+
+if [[ "$#" != "1" ]]; then
+ usage
+ exit 1
+fi
+
## Checks that all bundled dependencies are marked as optional in the poms
MVN_CLEAN_COMPILE_OUT=$1
-CI_DIR=$2
-FLINK_ROOT=$3
-source "${CI_DIR}/maven-utils.sh"
+MVN=${MVN:-./mvnw}
-cd "$FLINK_ROOT" || exit
+dependency_plugin_output=/tmp/dependency_tree_optional.txt
-dependency_plugin_output=${CI_DIR}/optional_dep.txt
+$MVN dependency:tree -B -T1 > "${dependency_plugin_output}"
Review Comment:
```suggestion
# dependency:tree needs to run without multi-threading support to ensure
that the output stays parseable (the lines appear in a sequential order)
$MVN dependency:tree -B -T1 > "${dependency_plugin_output}"
```
Should we add a comment to ensure that there is no desire to "optimize" the
maven call in the future?
##########
tools/ci/verify_scala_suffixes.sh:
##########
@@ -37,41 +37,52 @@
#
# The script uses 'mvn dependency:tree -Dincludes=org.scala-lang' to list Scala
# dependent modules.
-CI_DIR=$1
-FLINK_ROOT=$2
+
+
+usage() {
+ echo "Usage: $0"
+ echo ""
+ echo "The environment variable MVN is used to specify the Maven binaries;
defaults to 'mvnw'."
+ echo "See further details in the JavaDoc of ScalaSuffixChecker."
+}
+
+while getopts 'h' o; do
+ case "${o}" in
+ h)
+ usage
+ exit 0
+ ;;
+ esac
+done
+
+MVN=${MVN:-./mvnw}
echo "--- Flink Scala Dependency Analyzer ---"
echo "Analyzing modules for Scala dependencies using 'mvn dependency:tree'."
echo "If you haven't built the project, please do so first by running \"mvn
clean install -DskipTests\""
-source "${CI_DIR}/maven-utils.sh"
-
-cd "$FLINK_ROOT" || exit
+dependency_plugin_output=/tmp/dependency_tree_scala.txt
-dependency_plugin_output=${CI_DIR}/dep.txt
-
-run_mvn dependency:tree -Dincludes=org.scala-lang,:*_2.1*:: ${MAVEN_ARGUMENTS}
>> "${dependency_plugin_output}"
+$MVN dependency:tree -Dincludes=org.scala-lang,:*_2.1*:: ${MAVEN_ARGUMENTS}
-T1 > "${dependency_plugin_output}"
Review Comment:
```suggestion
# dependency:tree needs to run without multi-threading support to ensure
that the output stays parseable (the lines appear in a sequential order)
$MVN dependency:tree -Dincludes=org.scala-lang,:*_2.1*:: ${MAVEN_ARGUMENTS}
-T1 > "${dependency_plugin_output}"
```
Should we add a comment to ensure that there is no desire to "optimize" the
maven call in the future?
##########
tools/ci/compile.sh:
##########
@@ -18,37 +18,48 @@
################################################################################
#
-# This file contains tooling for compiling Flink
+# This script compiles Flink and runs all QA checks apart from tests.
+#
+# This script should not contain any CI-specific logic; put these into
compile_ci.sh instead.
+#
+# Usage: [MVN=/path/to/maven] tools/ci/compile.sh [additional maven args]
+# - Use the MVN environment variable to point the script to another maven
installation.
+# - Any script argument is forwarded to the Flink maven build. Use it to
skip/modify parts of the build process.
+#
+# Tips:
+# - '-Pskip-webui-build' skips the WebUI build.
+# - '-Dfast' skips Maven QA checks.
+# - '-Dmaven.clean.skip' skips recompilation of classes.
+# Example: tools/ci/compile.sh -Dmaven.clean.skip -Dfast -Pskip-webui-build,
use -Dmaven.clean.skip to avoid recompiling classes.
+#
+# Warnings:
+# - Skipping modules via '-pl [!]<module>' is not recommended because checks
may assume/require a full build.
#
-HERE="`dirname \"$0\"`" # relative
-HERE="`( cd \"$HERE\" && pwd )`" # absolutized and normalized
-if [ -z "$HERE" ] ; then
- exit 1 # fail
-fi
-CI_DIR="$HERE/../ci"
+CI_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" &>/dev/null && pwd)
MVN_CLEAN_COMPILE_OUT="/tmp/clean_compile.out"
+MVN=${MVN:-./mvnw}
# Deploy into this directory, to run license checks on all jars staged for
deployment.
# This helps us ensure that ALL artifacts we deploy to maven central adhere to
our license conditions.
MVN_VALIDATION_DIR="/tmp/flink-validation-deployment"
+rm -rf ${MVN_VALIDATION_DIR}
# source required ci scripts
source "${CI_DIR}/stage.sh"
source "${CI_DIR}/shade.sh"
-source "${CI_DIR}/maven-utils.sh"
echo "Maven version:"
-run_mvn -version
+$MVN -version
echo
"=============================================================================="
echo "Compiling Flink"
echo
"=============================================================================="
EXIT_CODE=0
-run_mvn clean deploy
-DaltDeploymentRepository=validation_repository::default::file:$MVN_VALIDATION_DIR
-Dflink.convergence.phase=install -Pcheck-convergence \
- -Dmaven.javadoc.skip=true -U -DskipTests | tee $MVN_CLEAN_COMPILE_OUT
+$MVN clean deploy
-DaltDeploymentRepository=validation_repository::default::file:$MVN_VALIDATION_DIR
-Dflink.convergence.phase=install -Pcheck-convergence \
Review Comment:
```suggestion
# deploy needs to run without multi-threading support to ensure that the
output stays parseable (the lines appear in a sequential order)
$MVN clean deploy
-DaltDeploymentRepository=validation_repository::default::file:$MVN_VALIDATION_DIR
-Dflink.convergence.phase=install -Pcheck-convergence \
```
just as a proposal here as well.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
