XComp commented on code in PR #23195:
URL: https://github.com/apache/flink/pull/23195#discussion_r1294597504
##########
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}
Review Comment:
> \- use mvnw by default
fyi: The log message of ceb63ffa is a bit confusing. You claim that the
Maven Wrapper is used by default in that commit. But this is only set up in
d9fe1649
##########
tools/ci/maven-utils.sh:
##########
@@ -21,12 +21,10 @@ function run_mvn {
MVN_CMD="${M2_HOME}/bin/mvn"
fi
- ARGS=$@
- INVOCATION="$MVN_CMD $MVN_GLOBAL_OPTIONS $ARGS"
if [[ "$MVN_RUN_VERBOSE" != "false" ]]; then
- echo "Invoking mvn with '$INVOCATION'"
+ echo "Invoking mvn with '$MVN_GLOBAL_OPTIONS ${@}'"
Review Comment:
From the commit message:
> This comes with a slight maintenance cost to maven-utils.sh#run_mvn, but
given that we'll likely throw this out in 1.19 anyway (because there are better
ways to implement this on later maven versions), we can bear that cost.
What are the costs? The quotes around `$@`? :thinking: Is there a Jira
issue for the "1.19 work"?
##########
tools/ci/compile_ci.sh:
##########
@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+################################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+################################################################################
+
+#
+# This script is the CI entrypoint for compiling Flink and running QA checks
that don't require tests.
+#
Review Comment:
```suggestion
if [[ ! -z "${AZURE_LOCATION}" ]]; then
echo 'No "$AZURE_LOCATION" environmental found. This script is the CI
entrypoint for compiling Flink and running QA checks that don't require tests.
Please use ./compile.sh for local setups.'
exit 1
fi
```
##########
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}
EXIT_CODE=$?
if [ $EXIT_CODE != 0 ] ; then
echo "ERROR in Scaladocs. Printing full output:"
- cat scaladoc.out ; rm scaladoc.out
+ cat ${scaladoc_output}
exit $EXIT_CODE
fi
echo "============ Checking bundled dependencies marked as optional
============"
-${CI_DIR}/verify_bundled_optional.sh $MVN_CLEAN_COMPILE_OUT "$CI_DIR" "$(pwd)"
|| exit $?
+MVN=$MVN ${CI_DIR}/verify_bundled_optional.sh $MVN_CLEAN_COMPILE_OUT || exit $?
echo "============ Checking scala suffixes ============"
-${CI_DIR}/verify_scala_suffixes.sh "$CI_DIR" "$(pwd)" || exit $?
+MVN=$MVN ${CI_DIR}/verify_scala_suffixes.sh || exit $?
Review Comment:
Same as above: Obsolete variable initialization `MVN`, AFAICS
##########
tools/ci/compile.sh:
##########
@@ -112,7 +127,7 @@ echo "============ Run license check ============"
find $MVN_VALIDATION_DIR
# We use a different Scala version with Java 17
if [[ ${PROFILE} != *"jdk17"* ]]; then
- ${CI_DIR}/license_check.sh $MVN_CLEAN_COMPILE_OUT $CI_DIR $(pwd)
$MVN_VALIDATION_DIR || exit $?
+ MVN=$MVN ${CI_DIR}/license_check.sh $MVN_CLEAN_COMPILE_OUT
$MVN_VALIDATION_DIR || exit $?
Review Comment:
Same as above: Obsolete variable initialization MVN, AFAICS
##########
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}
EXIT_CODE=$?
if [ $EXIT_CODE != 0 ] ; then
echo "ERROR in Scaladocs. Printing full output:"
- cat scaladoc.out ; rm scaladoc.out
+ cat ${scaladoc_output}
exit $EXIT_CODE
fi
echo "============ Checking bundled dependencies marked as optional
============"
-${CI_DIR}/verify_bundled_optional.sh $MVN_CLEAN_COMPILE_OUT "$CI_DIR" "$(pwd)"
|| exit $?
+MVN=$MVN ${CI_DIR}/verify_bundled_optional.sh $MVN_CLEAN_COMPILE_OUT || exit $?
Review Comment:
Isn't `MVN_$MVN` kind of obsolete? Or is it meant for documentation
purposes? :thinking:
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
