From 89bc5fc945891bec4322fe14b6d11d0cdc1ca267 Mon Sep 17 00:00:00 2001 From: Ward Vandewege Date: Wed, 23 Jan 2019 10:12:59 -0500 Subject: [PATCH] 9945: address a bunch of review comments. Arvados-DCO-1.1-Signed-off-by: Ward Vandewege --- .../test-package-arvados-node-manager.sh | 11 +- ...kage-python27-python-arvados-cwl-runner.sh | 14 +- ...st-package-python27-python-arvados-fuse.sh | 12 +- ...e-python27-python-arvados-python-client.sh | 11 +- build/python-package-scripts/after-install.sh | 50 ------- build/python-package-scripts/before-remove.sh | 50 ------- build/run-build-packages.sh | 2 +- build/run-library.sh | 122 +++++++----------- sdk/cwl/fpm-info.sh | 2 +- .../arvados-docker-cleaner.service | 2 +- services/fuse/fpm-info.sh | 2 +- services/nodemanager/fpm-info.sh | 2 +- 12 files changed, 61 insertions(+), 219 deletions(-) delete mode 100755 build/python-package-scripts/after-install.sh delete mode 100755 build/python-package-scripts/before-remove.sh diff --git a/build/package-testing/test-package-arvados-node-manager.sh b/build/package-testing/test-package-arvados-node-manager.sh index e2ada8ed62..9300f4cc69 100755 --- a/build/package-testing/test-package-arvados-node-manager.sh +++ b/build/package-testing/test-package-arvados-node-manager.sh @@ -4,19 +4,12 @@ # SPDX-License-Identifier: AGPL-3.0 set -e -arvados-node-manager --version -set +e -PYTHON=`ls /usr/share/python2.7/dist/arvados-node-manager/bin/python2.7` +arvados-node-manager --version -if [ "$PYTHON" != "" ]; then - set -e - exec $PYTHON </dev/null 2>&1; then - # Red Hat ("%{...}" is interpolated at package build time) - pkg="%{name}" - pkgtype=rpm - prefix="${RPM_INSTALL_PREFIX}" -else - # Debian - script="$(basename "${0}")" - pkg="${script%.postinst}" - pkgtype=deb - prefix=/usr -fi - -# populated from the build script -# dash only supports one array, $@ -if [ "%FPM_BINARIES" != "" ]; then - set %FPM_BINARIES -fi - -# Install symlinks to the binary/binaries -if [ "$pkg" != "" ]; then - - if [ "%FPM_BINARIES" != "" ]; then - # read from $@ - for binary do - if [ -e /usr/bin/$binary ]; then - rm -f /usr/bin/$binary - fi - ln -s /usr/share/%PYTHON/dist/$pkg/bin/$binary /usr/bin/$binary - done - fi - - # special case for arvados-cwl-runner - if [ "${pkg#python-}" = "arvados-cwl-runner" ]; then - if [ -e /usr/bin/cwl-runner ]; then - rm -f /usr/bin/cwl-runner - fi - ln -s /usr/share/%PYTHON/dist/$pkg/bin/$binary /usr/bin/cwl-runner - fi -fi - diff --git a/build/python-package-scripts/before-remove.sh b/build/python-package-scripts/before-remove.sh deleted file mode 100755 index 2ea15ee705..0000000000 --- a/build/python-package-scripts/before-remove.sh +++ /dev/null @@ -1,50 +0,0 @@ -#!/bin/sh -# Copyright (C) The Arvados Authors. All rights reserved. -# -# SPDX-License-Identifier: AGPL-3.0 - -set -e - -# Detect rpm-based systems: the exit code of the following command is zero -# on rpm-based systems -if /usr/bin/rpm -q -f /usr/bin/rpm >/dev/null 2>&1; then - # Red Hat ("%{...}" is interpolated at package build time) - pkg="%{name}" - pkgtype=rpm - prefix="${RPM_INSTALL_PREFIX}" -else - # Debian - script="$(basename "${0}")" - pkg="${script%.prerm}" - pkgtype=deb - prefix=/usr -fi - -# populated from the build script -# dash only supports one array, $@ -if [ "%FPM_BINARIES" != "" ]; then - set %FPM_BINARIES -fi - -if [ "$pkg" != "" ]; then - # Remove the binary python files so the package manager doesn't throw warnings - # on removing the package. - find /usr/share/%PYTHON/dist/$pkg -iname *.pyc -exec rm {} \; || true - find /usr/share/%PYTHON/dist/$pkg -iname *.pyo -exec rm {} \; || true - - if [ "%FPM_BINARIES" != "" ]; then - # read from $@ - for binary do - if [ -L /usr/bin/$binary ]; then - # Remove the symlinks we installed - rm -f /usr/bin/$binary - fi - done - fi - - if [ "${pkg#python-}" = "arvados-cwl-runner" ]; then - if [ -L /usr/bin/cwl-runner ]; then - rm -f /usr/bin/cwl-runner - fi - fi -fi diff --git a/build/run-build-packages.sh b/build/run-build-packages.sh index 04b4a7956a..f1cc6965cd 100755 --- a/build/run-build-packages.sh +++ b/build/run-build-packages.sh @@ -331,7 +331,7 @@ package_go_binary tools/keep-exercise keep-exercise \ fpm_build_virtualenv "arvados-python-client" "sdk/python" fpm_build_virtualenv "arvados-python-client" "sdk/python" "python3" -# Arvadow cwl runner +# Arvados cwl runner fpm_build_virtualenv "arvados-cwl-runner" "sdk/cwl" # The PAM module diff --git a/build/run-library.sh b/build/run-library.sh index 16cb0bc1ef..79d05071b1 100755 --- a/build/run-library.sh +++ b/build/run-library.sh @@ -232,11 +232,6 @@ test_package_presence() { rpm_architecture="x86_64" deb_architecture="amd64" - if [[ "$pkgtype" =~ ^(python|python3)$ ]]; then - rpm_architecture="noarch" - deb_architecture="all" - fi - if [[ "$pkgtype" =~ ^(src)$ ]]; then rpm_architecture="noarch" deb_architecture="all" @@ -298,6 +293,9 @@ test_package_presence() { echo "Package $complete_pkgname exists, not rebuilding!" curl -o ./${complete_pkgname} ${centos_repo}${complete_pkgname} return 1 + elif test -f "$WORKSPACE/packages/$TARGET/processed/${complete_pkgname}" ; then + echo "Package $complete_pkgname exists, not rebuilding!" + return 1 else echo "Package $complete_pkgname not found, building" return 0 @@ -408,51 +406,30 @@ fpm_build_virtualenv () { return 0 fi - echo "Building $FORMAT package for $PKG from $PKG_DIR" cd $WORKSPACE/$PKG_DIR - # Create an sdist - echo "Creating sdist..." - rm -rf dist/* - $python setup.py $DASHQ_UNLESS_DEBUG sdist - if [[ "$?" != "0" ]]; then + if ! $python setup.py $DASHQ_UNLESS_DEBUG sdist; then echo "Error, unable to run python setup.py sdist for $PKG" exit 1 fi - # Determine the package version from the generated sdist archive PACKAGE_PATH=`(cd dist; ls *tar.gz)` - PYTHON_VERSION=${PACKAGE_PATH#$PKG-} - - # For historical reasons, arvados-fuse is called arvados_fuse in python land - # Same with crunchstat-summary. - # We should fix this, but for now let's just make this script dtrt. - PKG_ALTERNATE=${PKG//-/_} - PYTHON_VERSION=${PYTHON_VERSION#$PKG_ALTERNATE-} - - # An even more complicated exception - if [[ "$PKG" == "libpam-arvados" ]]; then - PYTHON_VERSION=${PYTHON_VERSION#arvados-pam-} - fi - if [[ "$PACKAGE_PATH" == "$PYTHON_VERSION" ]]; then - echo "Error, unable to determine python package version for $PKG from $PACKAGE_PATH" - exit 1 - fi - PYTHON_VERSION=${PYTHON_VERSION%.tar.gz} + # Determine the package version from the generated sdist archive + PYTHON_VERSION=${ARVADOS_BUILDING_VERSION:-$(awk '($1 == "Version:"){print $2}' *.egg-info/PKG-INFO)} # See if we actually need to build this package; does it exist already? # We can't do this earlier than here, because we need PYTHON_VERSION... # This isn't so bad; the sdist call above is pretty quick compared to # the invocation of virtualenv and fpm, below. - test_package_presence "$PYTHON_PKG" $PYTHON_VERSION $PACKAGE_TYPE $ARVADOS_BUILDING_ITERATION amd64 - if [[ "$?" != "0" ]]; then - echo "exists" + if ! test_package_presence "$PYTHON_PKG" $PYTHON_VERSION $PACKAGE_TYPE $ARVADOS_BUILDING_ITERATION; then return 0 fi + echo "Building $FORMAT package for $PKG from $PKG_DIR" + # Package the sdist in a virtualenv echo "Creating virtualenv..." @@ -463,16 +440,22 @@ fpm_build_virtualenv () { virtualenv_command="virtualenv --python `which $python` $DASHQ_UNLESS_DEBUG build/usr/share/$python/dist/$PYTHON_PKG" - $virtualenv_command - - if [[ "$?" != "0" ]]; then + if ! $virtualenv_command; then echo "Error, unable to run" echo " $virtualenv_command" exit 1 fi - build/usr/share/$python/dist/$PYTHON_PKG/bin/pip install $DASHQ_UNLESS_DEBUG $CACHE_FLAG -U pip - build/usr/share/$python/dist/$PYTHON_PKG/bin/pip install $DASHQ_UNLESS_DEBUG $CACHE_FLAG -U wheel + if ! build/usr/share/$python/dist/$PYTHON_PKG/bin/pip install $DASHQ_UNLESS_DEBUG $CACHE_FLAG -U pip; then + echo "Error, unable to upgrade pip with" + echo " build/usr/share/$python/dist/$PYTHON_PKG/bin/pip install $DASHQ_UNLESS_DEBUG $CACHE_FLAG -U pip" + exit 1 + fi + if ! build/usr/share/$python/dist/$PYTHON_PKG/bin/pip install $DASHQ_UNLESS_DEBUG $CACHE_FLAG -U wheel; then + echo "Error, unable to upgrade wheel with" + echo " build/usr/share/$python/dist/$PYTHON_PKG/bin/pip install $DASHQ_UNLESS_DEBUG $CACHE_FLAG -U wheel" + exit 1 + fi if [[ "$TARGET" != "centos7" ]] || [[ "$PYTHON_PKG" != "python-arvados-fuse" ]]; then build/usr/share/$python/dist/$PYTHON_PKG/bin/pip install $DASHQ_UNLESS_DEBUG $CACHE_FLAG $PACKAGE_PATH @@ -495,16 +478,14 @@ fpm_build_virtualenv () { # virtualenv_tools.py script that doesn't work in python3 without serious # patching, minus the parts we don't need (modifying pyc files, etc). for binfile in `ls bin/`; do - file --mime bin/$binfile |grep -q binary - if [[ "$?" != "0" ]]; then + if ! file --mime bin/$binfile |grep -q binary; then # Not a binary file if [[ "$binfile" =~ ^activate(.csh|.fish|)$ ]]; then # these 'activate' scripts need special treatment sed -i "s/VIRTUAL_ENV=\".*\"/VIRTUAL_ENV=\"\/usr\/share\/$python\/dist\/$PYTHON_PKG\"/" bin/$binfile sed -i "s/VIRTUAL_ENV \".*\"/VIRTUAL_ENV \"\/usr\/share\/$python\/dist\/$PYTHON_PKG\"/" bin/$binfile else - grep -q -E '^#!.*/bin/python\d?' bin/$binfile - if [[ "$?" == "0" ]]; then + if grep -q -E '^#!.*/bin/python\d?' bin/$binfile; then # Replace shebang line sed -i "1 s/^.*$/#!\/usr\/share\/$python\/dist\/$PYTHON_PKG\/bin\/python/" bin/$binfile fi @@ -514,8 +495,8 @@ fpm_build_virtualenv () { cd - >$STDOUT_IF_DEBUG - find build -iname *.pyc -exec rm {} \; - find build -iname *.pyo -exec rm {} \; + find build -iname '*.pyc' -exec rm {} \; + find build -iname '*.pyo' -exec rm {} \; # Finally, generate the package echo "Creating package..." @@ -559,17 +540,20 @@ fpm_build_virtualenv () { fi fi - if [[ "${DEBUG:-0}" != "0" ]]; then + if [[ "$DEBUG" != "0" ]]; then COMMAND_ARR+=('--verbose' '--log' 'info') fi COMMAND_ARR+=('-v' "$PYTHON_VERSION") COMMAND_ARR+=('--iteration' "$ARVADOS_BUILDING_ITERATION") COMMAND_ARR+=('-n' "$PYTHON_PKG") - COMMAND_ARR+=('--before-remove' "$WORKSPACE/build/python-package-scripts/before-remove-$PKG.tmp.sh") - COMMAND_ARR+=('--after-install' "$WORKSPACE/build/python-package-scripts/after-install-$PKG.tmp.sh") COMMAND_ARR+=('-C' "build") + if [[ -e "$WORKSPACE/$PKG_DIR/$PKG.service" ]]; then + COMMAND_ARR+=('--after-install' "${WORKSPACE}/build/go-python-package-scripts/postinst") + COMMAND_ARR+=('--before-remove' "${WORKSPACE}/build/go-python-package-scripts/prerm") + fi + if [[ "$python" == "python2.7" ]]; then COMMAND_ARR+=('--depends' "$PYTHON2_PACKAGE") else @@ -588,46 +572,38 @@ fpm_build_virtualenv () { fpminfo="$WORKSPACE/$PKG_DIR/fpm-info.sh" if [[ -e "$fpminfo" ]]; then echo "Loading fpm overrides from $fpminfo" - source "$fpminfo" - if [[ "$?" != "0" ]]; then + if ! source "$fpminfo"; then echo "Error, unable to source $WORKSPACE/$PKG_DIR/fpm-info.sh for $PKG" exit 1 fi fi - if [[ -e "$WORKSPACE/$PKG_DIR/bin" ]]; then - FPM_BINARIES="" - for binary in `ls $WORKSPACE/$PKG_DIR/bin`; do - FPM_BINARIES+="'$binary' " - done - fi - - # create a custom version of the package scripts, with fpm_binaries populated - cp -f $WORKSPACE/build/python-package-scripts/before-remove.sh $WORKSPACE/build/python-package-scripts/before-remove-$PKG.tmp.sh - cp -f $WORKSPACE/build/python-package-scripts/after-install.sh $WORKSPACE/build/python-package-scripts/after-install-$PKG.tmp.sh - sed -i s/%FPM_BINARIES/"$FPM_BINARIES"/g $WORKSPACE/build/python-package-scripts/before-remove-$PKG.tmp.sh - sed -i s/%FPM_BINARIES/"$FPM_BINARIES"/g $WORKSPACE/build/python-package-scripts/after-install-$PKG.tmp.sh - - # Make sure the package scripts know the correct path where its files are installed - sed -i s/%PYTHON/$python/g $WORKSPACE/build/python-package-scripts/before-remove-$PKG.tmp.sh - sed -i s/%PYTHON/$python/g $WORKSPACE/build/python-package-scripts/after-install-$PKG.tmp.sh - for i in "${fpm_depends[@]}"; do COMMAND_ARR+=('--depends' "$i") done COMMAND_ARR+=("${fpm_args[@]}") + # Make sure to install all our package binaries in /usr/local/bin. + # We have to walk $WORKSPACE/$PKG_DIR/bin rather than + # $WORKSPACE/build/usr/share/$python/dist/$PYTHON_PKG/bin/ to get the list + # because the latter also includes all the python binaries for the virtualenv. + # We have to take the copies of our binaries from the latter directory, though, + # because those are the ones we rewrote the shebang line of, above. + if [[ -e "$WORKSPACE/$PKG_DIR/bin" ]]; then + for binary in `ls $WORKSPACE/$PKG_DIR/bin`; do + COMMAND_ARR+=("usr/share/$python/dist/$PYTHON_PKG/bin/$binary=/usr/local/bin/") + done + fi + COMMAND_ARR+=(".") FPM_RESULTS=$("${COMMAND_ARR[@]}") FPM_EXIT_CODE=$? - fpm_verify $FPM_EXIT_CODE $FPM_RESULTS - # if something went wrong and debug is off, print out the fpm command that errored - if [[ 0 -ne $? ]] && [[ "$STDOUT_IF_DEBUG" == "/dev/null" ]]; then - echo "fpm returned an error execurin the command:" + if ! fpm_verify $FPM_EXIT_CODE $FPM_RESULTS && [[ "$STDOUT_IF_DEBUG" == "/dev/null" ]]; then + echo "fpm returned an error executing the command:" echo echo -e "\n${COMMAND_ARR[@]}\n" else @@ -635,10 +611,6 @@ fpm_build_virtualenv () { mv $WORKSPACE/$PKG_DIR/dist/*$FORMAT $WORKSPACE/packages/$TARGET/ fi echo - - # clean up - rm -f $WORKSPACE/build/python-package-scripts/before-remove-$PKG.tmp.sh - rm -f $WORKSPACE/build/python-package-scripts/after-install-$PKG.tmp.sh } # Build packages for everything @@ -712,7 +684,7 @@ fpm_build () { # 12271 - As FPM-generated packages don't include scripts by default, the # packages cleanup on upgrade depends on files being listed on the %files # section in the generated SPEC files. To remove DIRECTORIES, they need to - # be listed in that sectiontoo, so we need to add this parameter to properly + # be listed in that section too, so we need to add this parameter to properly # remove lingering dirs. But this only works for python2: if used on # python33, it includes dirs like /opt/rh/python33 that belong to # other packages. @@ -720,7 +692,7 @@ fpm_build () { COMMAND_ARR+=('--rpm-auto-add-directories') fi - if [[ "${DEBUG:-0}" != "0" ]]; then + if [[ "$DEBUG" != "0" ]]; then COMMAND_ARR+=('--verbose' '--log' 'info') fi diff --git a/sdk/cwl/fpm-info.sh b/sdk/cwl/fpm-info.sh index 5bfee91af9..fa8f43e6e0 100644 --- a/sdk/cwl/fpm-info.sh +++ b/sdk/cwl/fpm-info.sh @@ -7,6 +7,6 @@ case "$TARGET" in fpm_depends+=(libgnutls-deb0-28 libcurl3-gnutls) ;; debian* | ubuntu*) - fpm_depends+=(libcurl3-gnutls) + fpm_depends+=(libcurl3-gnutls libpython2.7) ;; esac diff --git a/services/dockercleaner/arvados-docker-cleaner.service b/services/dockercleaner/arvados-docker-cleaner.service index 0221707cf4..29697e440a 100644 --- a/services/dockercleaner/arvados-docker-cleaner.service +++ b/services/dockercleaner/arvados-docker-cleaner.service @@ -23,7 +23,7 @@ RestartPreventExitStatus=2 # This unwieldy ExecStart command detects at runtime whether # arvados-docker-cleaner is installed with the Python 3.3 Software # Collection, and if so, invokes it with the "scl" wrapper. -ExecStart=/bin/sh -c 'if [ -e /opt/rh/python33/root/bin/arvados-docker-cleaner ]; then exec scl enable python33 arvados-docker-cleaner; else exec arvados-docker-cleaner; fi' +ExecStart=/bin/sh -c 'if [ -e /opt/rh/rh-python35/root/bin/arvados-docker-cleaner ]; then exec scl enable rh-python35 arvados-docker-cleaner; else exec arvados-docker-cleaner; fi' # systemd<=219 (centos:7, debian:8, ubuntu:trusty) obeys StartLimitInterval in the [Service] section StartLimitInterval=0 diff --git a/services/fuse/fpm-info.sh b/services/fuse/fpm-info.sh index f789abe692..fd94ef7afa 100644 --- a/services/fuse/fpm-info.sh +++ b/services/fuse/fpm-info.sh @@ -9,6 +9,6 @@ case "$TARGET" in fpm_depends+=(fuse-libs) ;; debian* | ubuntu*) - fpm_depends+=(libcurl3-gnutls) + fpm_depends+=(libcurl3-gnutls libpython2.7) ;; esac diff --git a/services/nodemanager/fpm-info.sh b/services/nodemanager/fpm-info.sh index 0abc6a08ea..c4a9dbb47b 100644 --- a/services/nodemanager/fpm-info.sh +++ b/services/nodemanager/fpm-info.sh @@ -4,6 +4,6 @@ case "$TARGET" in debian* | ubuntu*) - fpm_depends+=(libcurl3-gnutls) + fpm_depends+=(libcurl3-gnutls libpython2.7) ;; esac -- 2.30.2