From 462537d0523887b65625512e8b8ae5ac07b03625 Mon Sep 17 00:00:00 2001 From: Nicholas Chammas Date: Sun, 15 Mar 2020 13:09:35 +0900 Subject: [PATCH] [SPARK-31153][BUILD] Cleanup several failures in lint-python This PR cleans up several failures -- most of them silent -- in `dev/lint-python`. I don't understand how we haven't been bitten by these yet. Perhaps we've been lucky? Fixes include: * Fix how we compare versions. All the version checks currently in `master` silently fail with: ``` File "", line 2 print(LooseVersion("""2.3.1""") >= LooseVersion("""2.4.0""")) ^ IndentationError: unexpected indent ``` Another problem is that `distutils.version` is undocumented and unsupported. * Fix some basic bugs. e.g. We have an incorrect reference to `$PYDOCSTYLEBUILD`, which doesn't exist, which was causing the doc style test to silently fail with: ``` ./dev/lint-python: line 193: --version: command not found ``` * Stop suppressing error output! It's hiding problems and serves no purpose here. `lint-python` is part of our CI build and is currently doing any combination of the following: silently failing; incorrectly skipping tests; incorrectly downloading libraries when a suitable library is already available. No. Lots of manual testing with `set -x` enabled. Closes #27910 from nchammas/SPARK-31153-lint-python. Authored-by: Nicholas Chammas Signed-off-by: HyukjinKwon --- dev/lint-python | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/dev/lint-python b/dev/lint-python index e9ed83dec5fc..9d7dfad7c534 100755 --- a/dev/lint-python +++ b/dev/lint-python @@ -27,6 +27,19 @@ MINIMUM_PYCODESTYLE="2.4.0" SPHINX_BUILD="sphinx-build" +PYTHON_EXECUTABLE="python3" + +function satisfies_min_version { + local provided_version="$1" + local expected_version="$2" + echo "$( + "$PYTHON_EXECUTABLE" << EOM +from setuptools.extern.packaging import version +print(version.parse('$provided_version') >= version.parse('$expected_version')) +EOM + )" +} + function compile_python_test { local COMPILE_STATUS= local COMPILE_REPORT= @@ -56,7 +69,7 @@ function pycodestyle_test { local PYCODESTYLE_STATUS= local PYCODESTYLE_REPORT= local RUN_LOCAL_PYCODESTYLE= - local VERSION= + local PYCODESTYLE_VERSION= local EXPECTED_PYCODESTYLE= local PYCODESTYLE_SCRIPT_PATH="$SPARK_ROOT_DIR/dev/pycodestyle-$MINIMUM_PYCODESTYLE.py" local PYCODESTYLE_SCRIPT_REMOTE_PATH="https://raw.githubusercontent.com/PyCQA/pycodestyle/$MINIMUM_PYCODESTYLE/pycodestyle.py" @@ -69,11 +82,8 @@ function pycodestyle_test { # check for locally installed pycodestyle & version RUN_LOCAL_PYCODESTYLE="False" if hash "$PYCODESTYLE_BUILD" 2> /dev/null; then - VERSION=$( $PYCODESTYLE_BUILD --version 2> /dev/null) - EXPECTED_PYCODESTYLE=$( (python3 -c 'from distutils.version import LooseVersion; - print(LooseVersion("""'${VERSION[0]}'""") >= LooseVersion("""'$MINIMUM_PYCODESTYLE'"""))')\ - 2> /dev/null) - + PYCODESTYLE_VERSION="$($PYCODESTYLE_BUILD --version)" + EXPECTED_PYCODESTYLE="$(satisfies_min_version $PYCODESTYLE_VERSION $MINIMUM_PYCODESTYLE)" if [ "$EXPECTED_PYCODESTYLE" == "True" ]; then RUN_LOCAL_PYCODESTYLE="True" fi @@ -117,7 +127,6 @@ function pycodestyle_test { function flake8_test { local FLAKE8_VERSION= - local VERSION= local EXPECTED_FLAKE8= local FLAKE8_REPORT= local FLAKE8_STATUS= @@ -128,11 +137,9 @@ function flake8_test { exit 1 fi - FLAKE8_VERSION="$($FLAKE8_BUILD --version 2> /dev/null)" - VERSION=($FLAKE8_VERSION) - EXPECTED_FLAKE8=$( (python3 -c 'from distutils.version import LooseVersion; - print(LooseVersion("""'${VERSION[0]}'""") >= LooseVersion("""'$MINIMUM_FLAKE8'"""))') \ - 2> /dev/null) + _FLAKE8_VERSION=($($FLAKE8_BUILD --version)) + FLAKE8_VERSION="${_FLAKE8_VERSION[0]}" + EXPECTED_FLAKE8="$(satisfies_min_version $FLAKE8_VERSION $MINIMUM_FLAKE8)" if [[ "$EXPECTED_FLAKE8" == "False" ]]; then echo "\ @@ -174,10 +181,8 @@ function pydocstyle_test { return fi - PYDOCSTYLE_VERSION="$($PYDOCSTYLEBUILD --version 2> /dev/null)" - EXPECTED_PYDOCSTYLE=$(python3 -c 'from distutils.version import LooseVersion; \ - print(LooseVersion("""'$PYDOCSTYLE_VERSION'""") >= LooseVersion("""'$MINIMUM_PYDOCSTYLE'"""))' \ - 2> /dev/null) + PYDOCSTYLE_VERSION="$($PYDOCSTYLE_BUILD --version)" + EXPECTED_PYDOCSTYLE="$(satisfies_min_version $PYDOCSTYLE_VERSION $MINIMUM_PYDOCSTYLE)" if [[ "$EXPECTED_PYDOCSTYLE" == "False" ]]; then echo "\