From c40575e4e046904d5dbd28826bf5308915093f15 Mon Sep 17 00:00:00 2001 From: OpenStack Proposal Bot Date: Fri, 4 Nov 2016 14:08:31 +0000 Subject: [PATCH] Add bash scripts style checking for IPA Updated file tox.ini allows to check code style in all ".sh" files included in Ironic Python Agent. Checking can be invoked by calling either "tox" or "tox -e pep8". Change-Id: Ie76605737c7db10a064f2aebfda388372a4c0591 Closes-Bug: #1625215 --- .../common/generate_upper_constraints.sh | 72 +++++++++---------- imagebuild/tinyipa/build_files/bootlocal.sh | 2 +- ironic_python_agent/shell/write_image.sh | 10 +-- test-requirements.txt | 2 +- tools/config/check_uptodate.sh | 9 ++- tools/config/generate_sample.sh | 25 +++---- tools/run_bashate.sh | 23 ++++++ tox.ini | 4 ++ 8 files changed, 85 insertions(+), 62 deletions(-) create mode 100755 tools/run_bashate.sh diff --git a/imagebuild/common/generate_upper_constraints.sh b/imagebuild/common/generate_upper_constraints.sh index cd61b784b..6d36d3298 100755 --- a/imagebuild/common/generate_upper_constraints.sh +++ b/imagebuild/common/generate_upper_constraints.sh @@ -6,74 +6,74 @@ DESTINATION="$1" TOX_INI_UPPER_CONSTRAINT_URL="$(${COMMON_ROOT}/extract_upper_constraints_from_tox_ini.sh ${COMMON_ROOT}/../../tox.ini)" copy() { - local src=$1 - local destination=$2 + local src=$1 + local destination=$2 - if test -z "${src}"; then - return 1 - fi + if test -z "${src}"; then + return 1 + fi - if test -e "${src}"; then - log "File '${src}' exists. Using as upper-constraints." - cp "${src}" "${destination}" - else - log "File '${src}' not found. Skipping local file strategy." - return 1 - fi - return 0 + if test -e "${src}"; then + log "File '${src}' exists. Using as upper-constraints." + cp "${src}" "${destination}" + else + log "File '${src}' not found. Skipping local file strategy." + return 1 + fi + return 0 } download() { - local url=$1 - local destination=$2 + local url=$1 + local destination=$2 - if test -z "${url}"; then - return 1 - else - log "Downloading from '${url}'" - curl ${url} -o "${destination}" - fi - return 0 + if test -z "${url}"; then + return 1 + else + log "Downloading from '${url}'" + curl ${url} -o "${destination}" + fi + return 0 } log() { - echo "${SCRIPT_NAME}: ${@}" + echo "${SCRIPT_NAME}: ${@}" } fail() { - log ${@} - exit 1 + log ${@} + exit 1 } upper_constraints_is_not_null() { - test "${UPPER_CONSTRAINTS_FILE:-""}" != "" + test "${UPPER_CONSTRAINTS_FILE:-""}" != "" } copy_uc() { - copy "${UPPER_CONSTRAINTS_FILE:-""}" "${DESTINATION}" + copy "${UPPER_CONSTRAINTS_FILE:-""}" "${DESTINATION}" } download_uc() { - download "${UPPER_CONSTRAINTS_FILE:-""}" "${DESTINATION}" + download "${UPPER_CONSTRAINTS_FILE:-""}" "${DESTINATION}" } copy_new_requirements_uc() { - copy "/opt/stack/new/requirements/upper-constraints.txt" "${DESTINATION}" + copy "/opt/stack/new/requirements/upper-constraints.txt" "${DESTINATION}" } download_from_tox_ini_url() { - log "tox.ini indicates '${TOX_INI_UPPER_CONSTRAINT_URL}' as fallback." - download "${TOX_INI_UPPER_CONSTRAINT_URL}" "${DESTINATION}" + log "tox.ini indicates '${TOX_INI_UPPER_CONSTRAINT_URL}' as fallback." + download "${TOX_INI_UPPER_CONSTRAINT_URL}" "${DESTINATION}" } log "Generating local constraints file..." if upper_constraints_is_not_null; then - log "UPPER_CONSTRAINTS_FILE is defined as '${UPPER_CONSTRAINTS_FILE:-""}'" - copy_uc || download_uc || fail "Failed to copy or download file indicated in UPPER_CONSTRAINTS_FILE." + log "UPPER_CONSTRAINTS_FILE is defined as '${UPPER_CONSTRAINTS_FILE:-""}'" + copy_uc || download_uc || fail "Failed to copy or download file indicated in UPPER_CONSTRAINTS_FILE." else - log "UPPER_CONSTRAINTS_FILE is not defined. Using fallback strategies." + log "UPPER_CONSTRAINTS_FILE is not defined. Using fallback strategies." - copy_new_requirements_uc || \ - download_from_tox_ini_url || fail "Failed to download upper-constraints.txt from '${TOX_INI_UPPER_CONSTRAINT_URL}'." + copy_new_requirements_uc || \ + download_from_tox_ini_url || fail "Failed to download upper-constraints.txt from '${TOX_INI_UPPER_CONSTRAINT_URL}'." fi diff --git a/imagebuild/tinyipa/build_files/bootlocal.sh b/imagebuild/tinyipa/build_files/bootlocal.sh index 244cc4f9c..dbc03ed0f 100755 --- a/imagebuild/tinyipa/build_files/bootlocal.sh +++ b/imagebuild/tinyipa/build_files/bootlocal.sh @@ -20,7 +20,7 @@ fi # Install IPA and dependecies if ! type "ironic-python-agent" > /dev/null ; then - python /tmp/get-pip.py --no-wheel --no-index --find-links=file:///tmp/wheelhouse ironic_python_agent + python /tmp/get-pip.py --no-wheel --no-index --find-links=file:///tmp/wheelhouse ironic_python_agent fi export PYTHONOPTIMIZE=1 diff --git a/ironic_python_agent/shell/write_image.sh b/ironic_python_agent/shell/write_image.sh index 3f07e07aa..11abb9fc2 100755 --- a/ironic_python_agent/shell/write_image.sh +++ b/ironic_python_agent/shell/write_image.sh @@ -17,14 +17,14 @@ set -e log() { - echo "`basename $0`: $@" + echo "`basename $0`: $@" } usage() { - [[ -z "$1" ]] || echo -e "USAGE ERROR: $@\n" - echo "`basename $0`: IMAGEFILE DEVICE" - echo " - This script images DEVICE with IMAGEFILE" - exit 1 + [[ -z "$1" ]] || echo -e "USAGE ERROR: $@\n" + echo "`basename $0`: IMAGEFILE DEVICE" + echo " - This script images DEVICE with IMAGEFILE" + exit 1 } IMAGEFILE="$1" diff --git a/test-requirements.txt b/test-requirements.txt index eea4ed819..d204b3a80 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -8,7 +8,7 @@ testtools>=1.4.0 # MIT python-subunit>=0.0.18 # Apache-2.0/BSD oslotest>=1.10.0 # Apache-2.0 os-testr>=0.8.0 # Apache-2.0 - +bashate>=0.2 # Apache-2.0 # Doc requirements doc8 # Apache-2.0 sphinx!=1.3b1,<1.4,>=1.2.1 # BSD diff --git a/tools/config/check_uptodate.sh b/tools/config/check_uptodate.sh index deca94045..49e49f0a3 100755 --- a/tools/config/check_uptodate.sh +++ b/tools/config/check_uptodate.sh @@ -17,9 +17,8 @@ trap "rm -rf $TEMPDIR" EXIT tools/config/generate_sample.sh -b ./ -p ${PROJECT_NAME} -o ${TEMPDIR} -if ! diff -u ${TEMPDIR}/${CFGFILE_NAME} ${CFGFILE} -then - echo "${0##*/}: ${PROJECT_NAME}.conf.sample is not up to date." - echo "${0##*/}: Please run ${0%%${0##*/}}generate_sample.sh." - exit 1 +if ! diff -u ${TEMPDIR}/${CFGFILE_NAME} ${CFGFILE} ; then + echo "${0##*/}: ${PROJECT_NAME}.conf.sample is not up to date." + echo "${0##*/}: Please run ${0%%${0##*/}}generate_sample.sh." + exit 1 fi diff --git a/tools/config/generate_sample.sh b/tools/config/generate_sample.sh index 4b7d7f0c5..7058a1299 100755 --- a/tools/config/generate_sample.sh +++ b/tools/config/generate_sample.sh @@ -5,9 +5,12 @@ print_hint() { } PARSED_OPTIONS=$(getopt -n "${0##*/}" -o hb:p:m:l:o: \ - --long help,base-dir:,package-name:,output-dir:,module:,library: -- "$@") + --long help,base-dir:,package-name:,output-dir:,module:,library: -- "$@") -if [ $? != 0 ] ; then print_hint ; exit 1 ; fi +if [ $? != 0 ] ; then + print_hint ; + exit 1 +fi eval set -- "$PARSED_OPTIONS" @@ -57,30 +60,25 @@ while true; do done BASEDIR=${BASEDIR:-`pwd`} -if ! [ -d $BASEDIR ] -then +if ! [ -d $BASEDIR ] ; then echo "${0##*/}: missing project base directory" >&2 ; print_hint ; exit 1 -elif [[ $BASEDIR != /* ]] -then +elif [[ $BASEDIR != /* ]] ; then BASEDIR=$(cd "$BASEDIR" && pwd) fi PACKAGENAME=${PACKAGENAME:-${BASEDIR##*/}} PACKAGENAME=`echo $PACKAGENAME | tr - _` TARGETDIR=$BASEDIR/$PACKAGENAME -if ! [ -d $TARGETDIR ] -then +if ! [ -d $TARGETDIR ] ; then echo "${0##*/}: invalid project package name" >&2 ; print_hint ; exit 1 fi OUTPUTDIR=${OUTPUTDIR:-$BASEDIR/etc} # NOTE(bnemec): Some projects put their sample config in etc/, # some in etc/$PACKAGENAME/ -if [ -d $OUTPUTDIR/$PACKAGENAME ] -then +if [ -d $OUTPUTDIR/$PACKAGENAME ] ; then OUTPUTDIR=$OUTPUTDIR/$PACKAGENAME -elif ! [ -d $OUTPUTDIR ] -then +elif ! [ -d $OUTPUTDIR ] ; then echo "${0##*/}: cannot access \`$OUTPUTDIR': No such file or directory" >&2 exit 1 fi @@ -91,8 +89,7 @@ FILES=$(find $TARGETDIR -type f -name "*.py" ! -path "*/tests/*" ! -path "*/nova -exec grep -l "Opt(" {} + | sed -e "s/^$BASEDIRESC\///g" | sort -u) RC_FILE="`dirname $0`/oslo.config.generator.rc" -if test -r "$RC_FILE" -then +if test -r "$RC_FILE" ; then source "$RC_FILE" fi diff --git a/tools/run_bashate.sh b/tools/run_bashate.sh new file mode 100755 index 000000000..d74db7d9f --- /dev/null +++ b/tools/run_bashate.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# +# Licensed 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. + +# Ignoring E003(Indent not multiple of 4) +# Ignoring E006(Line longer than 79 characters) + +find "$@" -not \( -type d -name .?\* -prune \) \ + -type f \ + \( \ + -name \*.sh \ + \) \ + -print0 | xargs -0 bashate -v -iE003,E006 -eE005,E042 diff --git a/tox.ini b/tox.ini index 25fe8b21d..074e4dfc8 100644 --- a/tox.ini +++ b/tox.ini @@ -26,8 +26,12 @@ setenv = commands = ostestr {posargs} [testenv:pep8] +whitelist_externals = bash commands = flake8 {posargs:ironic_python_agent imagebuild} + # Run bashate during pep8 runs to ensure violations are caught by + # the check and gate queues. + {toxinidir}/tools/run_bashate.sh {toxinidir} doc8 doc/source README.rst [testenv:cover]