From 5176dd8a0ee5e30f03cf531cebb63c78bf9ba67c Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Thu, 9 Jan 2025 12:07:10 +0100 Subject: [PATCH] refactor(linux): address code review comments Also some cleanup. --- resources/docker-images/android/Dockerfile | 3 +++ resources/docker-images/base/Dockerfile | 1 + resources/docker-images/build.sh | 12 ++++++++---- resources/docker-images/linux/run-tests.sh | 5 +++++ resources/docker-images/run.sh | 22 +++++++++++----------- resources/docker-images/web/run-tests.sh | 5 +++++ 6 files changed, 33 insertions(+), 15 deletions(-) diff --git a/resources/docker-images/android/Dockerfile b/resources/docker-images/android/Dockerfile index e86d496ab8..91cd095a69 100644 --- a/resources/docker-images/android/Dockerfile +++ b/resources/docker-images/android/Dockerfile @@ -47,6 +47,9 @@ VOLUME /home/build/build WORKDIR /home/build/build # Pre-install gradle. This will put files in ~/.gradle which will speed up builds. +# Note it would be safer to copy these files directly from our repo rather than +# getting it over the Internet, but Docker doesn't allow us to copy files +# from outside the current directory when building the image. RUN mkdir -p $HOME/tmp/gradle/wrapper && \ # KMEA uses gradle-7.6.4-bin curl --location --output $HOME/tmp/gradle/wrapper/gradle-wrapper.jar https://raw.githubusercontent.com/keymanapp/keyman/master/android/KMEA/gradle/wrapper/gradle-wrapper.jar && \ diff --git a/resources/docker-images/base/Dockerfile b/resources/docker-images/base/Dockerfile index 4c24e89bb0..87516b3f96 100644 --- a/resources/docker-images/base/Dockerfile +++ b/resources/docker-images/base/Dockerfile @@ -32,6 +32,7 @@ RUN echo "build ALL=(ALL) NOPASSWD: ALL" >> /etc/sudoers RUN < /usr/bin/bashwrapper #!/bin/bash export KEYMAN_USE_NVM=1 +export DOCKER_RUNNING=true EOF # Install NVM diff --git a/resources/docker-images/build.sh b/resources/docker-images/build.sh index 01af657bcd..8100459d47 100755 --- a/resources/docker-images/build.sh +++ b/resources/docker-images/build.sh @@ -49,7 +49,9 @@ _add_build_args() { _convert_parameters_to_build_args() { build_args=() build_version= - local required_node_version="$(_print_expected_node_version)" + local required_node_version + # shellcheck disable=SC2034 + required_node_version="$(_print_expected_node_version)" _add_build_args UBUNTU_VERSION KEYMAN_DEFAULT_VERSION_UBUNTU_CONTAINER "" _add_build_args JAVA_VERSION KEYMAN_VERSION_JAVA java @@ -82,8 +84,9 @@ build_action() { OPTION_NO_CACHE="--no-cache" fi + # shellcheck disable=SC2164 cd "${platform}" - # shellcheck disable=SC2248 + # shellcheck disable=SC2248,SC2086 docker build ${OPTION_NO_CACHE:-} --platform amd64 -t "keymanapp/keyman-${platform}-ci:${build_version}" "${build_args[@]}" . # If the user didn't specify particular versions we will additionaly create an image # with the tag 'default'. @@ -91,7 +94,8 @@ build_action() { builder_echo debug "Setting default tag for ${platform}" docker build --platform amd64 -t "keymanapp/keyman-${platform}-ci:default" "${build_args[@]}" . fi - cd - || true + # shellcheck disable=SC2164,SC2103 + cd - builder_echo success "Docker image 'keymanapp/keyman-${platform}-ci:${build_version}' built" } @@ -99,7 +103,7 @@ test_action() { local platform=$1 builder_echo debug "Testing image for ${platform}" - ./run.sh ${platform} -- ./build.sh configure,build,test:${platform} + ./run.sh "${platform}" -- ./build.sh configure,build,test:"${platform}" } if builder_has_action build; then diff --git a/resources/docker-images/linux/run-tests.sh b/resources/docker-images/linux/run-tests.sh index 962418467e..fbc5fb3b67 100755 --- a/resources/docker-images/linux/run-tests.sh +++ b/resources/docker-images/linux/run-tests.sh @@ -1,6 +1,11 @@ #!/usr/bin/env bash set -e +if [[ -z "${DOCKER_RUNNING:-}" ]]; then + echo "This script is intended to be run inside a docker container." + exit 0 +fi + # Start system dbus sudo dbus-daemon --system --fork diff --git a/resources/docker-images/run.sh b/resources/docker-images/run.sh index c85c15e873..c42d6c2a02 100755 --- a/resources/docker-images/run.sh +++ b/resources/docker-images/run.sh @@ -21,37 +21,37 @@ builder_describe \ builder_parse "$@" run_android() { - docker run -it --rm -v ${KEYMAN_ROOT}:/home/build/build \ - -v ${KEYMAN_ROOT}/core/build/docker-core:/home/build/build/core/build \ + docker run -it --rm -v "${KEYMAN_ROOT}":/home/build/build \ + -v "${KEYMAN_ROOT}/core/build/docker-core":/home/build/build/core/build \ keymanapp/keyman-android-ci:default \ "${builder_extra_params[@]}" } run_core() { - docker run -it --rm -v ${KEYMAN_ROOT}:/home/build/build \ - -v ${KEYMAN_ROOT}/core/build/docker-core:/home/build/build/core/build \ + docker run -it --rm -v "${KEYMAN_ROOT}":/home/build/build \ + -v "${KEYMAN_ROOT}/core/build/docker-core":/home/build/build/core/build \ keymanapp/keyman-core-ci:default \ "${builder_extra_params[@]}" } run_linux() { - mkdir -p ${KEYMAN_ROOT}/linux/build/docker-linux - docker run -it --privileged --rm -v ${KEYMAN_ROOT}:/home/build/build \ - -v ${KEYMAN_ROOT}/core/build/docker-core:/home/build/build/core/build \ - -v ${KEYMAN_ROOT}/linux/build/docker-linux:/home/build/build/linux/build \ + mkdir -p "${KEYMAN_ROOT}/linux/build/docker-linux" + docker run -it --privileged --rm -v "${KEYMAN_ROOT}":/home/build/build \ + -v "${KEYMAN_ROOT}/core/build/docker-core":/home/build/build/core/build \ + -v "${KEYMAN_ROOT}/linux/build/docker-linux":/home/build/build/linux/build \ -e DESTDIR=/tmp \ keymanapp/keyman-linux-ci:default \ "${builder_extra_params[@]}" } run_web() { - docker run -it --privileged --rm -v ${KEYMAN_ROOT}:/home/build/build \ - -v ${KEYMAN_ROOT}/core/build/docker-core:/home/build/build/core/build \ + docker run -it --privileged --rm -v "${KEYMAN_ROOT}":/home/build/build \ + -v "${KEYMAN_ROOT}/core/build/docker-core":/home/build/build/core/build \ keymanapp/keyman-web-ci:default \ "${builder_extra_params[@]}" } -mkdir -p ${KEYMAN_ROOT}/core/build/docker-core +mkdir -p "${KEYMAN_ROOT}/core/build/docker-core" builder_run_action android run_android builder_run_action core run_core diff --git a/resources/docker-images/web/run-tests.sh b/resources/docker-images/web/run-tests.sh index 1a3100159f..84871d8b19 100755 --- a/resources/docker-images/web/run-tests.sh +++ b/resources/docker-images/web/run-tests.sh @@ -1,4 +1,9 @@ #!/usr/bin/env bash +if [[ -z "${DOCKER_RUNNING:-}" ]]; then + echo "This script is intended to be run inside a docker container." + exit 0 +fi + set -e echo "Starting Xvfb..." Xvfb -screen 0 1024x768x24 :33 &> /dev/null &