build: rework build script

- remove code duplication
- disable jaegertracing (we don't use it at the moment)
- more advanced logic to deal with git safe directories
- inherit build options from environment and print out the complete
  set of build options set in the script
- fix linter warnings from shellcheck
- make linter run all pre-commit linting actions
- make markdownlint not complain about multiple headings with the same
  name unless they are under the same heading themselves

Signed-off-by: Moritz Röhrich <[email protected]>

.github/workflows/lint.yaml

@@ -1,15 +1,20 @@
+---
 name: Lint
-on: [push, pull_request, workflow_dispatch]
+on:
+  - push
+  - pull_request
+  - workflow_dispatch
+
 jobs:
   lint:
     runs-on: ubuntu-latest
     steps:
+
       - name: Set up Git repository
         uses: actions/checkout@v3
-      - name: Lint
-        uses: DavidAnson/markdownlint-cli2-action@v7
-        with:
-          command: config
-          globs: |
-            .github/workflows/config/custom.markdownlint.jsonc
-            '**/*.md'
+
+      - name: Set up Python3
+        uses: actions/setup-python@v4
+
+      - name: Pre Commit
+        uses: pre-commit/[email protected]

.markdownlint.json

@@ -0,0 +1,3 @@
+{
+  "MD024": { "siblings_only": "true" }
+}

.pre-commit-config.yaml

@@ -1,34 +1,37 @@
+---
 repos:
--   repo: https://github.com/pre-commit/pre-commit-hooks
+  - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.0.1
     hooks:
-    - id: trailing-whitespace
-    - id: check-yaml
-    - id: check-added-large-files
-    - id: end-of-file-fixer
-    - id: debug-statements
-    - id: mixed-line-ending
-    - id: detect-private-key
-    - id: destroyed-symlinks
-    - id: check-shebang-scripts-are-executable
-    - id: check-case-conflict
+      - id: trailing-whitespace
+      - id: check-yaml
+        args:
+          - --allow-multiple-documents
+      - id: check-added-large-files
+      - id: end-of-file-fixer
+      - id: debug-statements
+      - id: mixed-line-ending
+      - id: detect-private-key
+      - id: destroyed-symlinks
+      - id: check-shebang-scripts-are-executable
+      - id: check-case-conflict
 
-- repo: https://github.com/jumanjihouse/pre-commit-hooks
-  rev: 2.1.5
-  hooks:
-    - id: forbid-binary
-      exclude: >-
-        (?x)^(
-          assets/images/.*
-        )$
-    - id: git-check
+  - repo: https://github.com/jumanjihouse/pre-commit-hooks
+    rev: 2.1.5
+    hooks:
+      - id: forbid-binary
+        exclude: >-
+          (?x)^(
+            assets/images/.*
+          )$
+      - id: git-check
 
-- repo: https://github.com/codespell-project/codespell
-  rev: v2.1.0
-  hooks:
-    - id: codespell
+  - repo: https://github.com/codespell-project/codespell
+    rev: v2.1.0
+    hooks:
+      - id: codespell
 
-- repo: https://github.com/DavidAnson/markdownlint-cli2
-  rev: v0.4.0
-  hooks:
-    - id: markdownlint-cli2
+  - repo: https://github.com/DavidAnson/markdownlint-cli2
+    rev: v0.4.0
+    hooks:
+      - id: markdownlint-cli2

CHANGELOG.md

@@ -9,8 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- Added `unittest_rgw_sfs_metadata_compatibility` and `unittest_rgw_sfs_gc` unit tests
-  for testing rgw/sfs.
+- Added `unittest_rgw_sfs_metadata_compatibility` and `unittest_rgw_sfs_gc`
+  unit tests for testing rgw/sfs.
 
 ## [0.8.0] - 2022-11-10
 

README.md

@@ -57,7 +57,5 @@ 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.
 
-=======
-
 [1]: https://github.com/aquarist-labs/s3gw-charts
 [2]: https://s3gw-docs.readthedocs.io/en/latest/

build/build-radosgw.sh

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 set -e
 
@@ -9,118 +9,120 @@ set -e
 # Debug: Usually a classic debug build including debugging information, no optimization etc.
 CMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE:-"Debug"}
 
-CEPH_DIR=$(realpath ${CEPH_DIR:-"/srv/ceph"})
+CEPH_DIR=$(realpath "${CEPH_DIR:-"/srv/ceph"}")
 S3GW_CCACHE_DIR=${S3GW_CCACHE_DIR:-"${CEPH_DIR}/build.ccache"}
-WITH_TESTS=${WITH_TESTS:-"OFF"}
+S3GW_BUILD_DIR="${CEPH_DIR}/build"
+
+WITH_JAEGER=${WITH_JAEGER:-"OFF"}
+WITH_LTTNG=${WITH_LTTNG:-"OFF"}
+WITH_MANPAGE=${WITH_MANPAGE:-"OFF"}
+WITH_OPENLDAP=${WITH_OPENLDAP:-"OFF"}
+WITH_RADOSGW_AMQP_ENDPOINT=${WITH_RADOSGW_AMQP_ENDPOINT:-"OFF"}
 WITH_RADOSGW_DBSTORE=${WITH_RADOSGW_DBSTORE:-"OFF"}
+WITH_RADOSGW_KAFKA_ENDPOINT=${WITH_RADOSGW_KAFKA_ENDPOINT:-"OFF"}
+WITH_RADOSGW_LUA_PACKAGES=${WITH_RADOSGW_LUA_PACKAGES:-"OFF"}
+WITH_RADOSGW_MOTR=${WITH_RADOSGW_MOTR:-"OFF"}
+WITH_RADOSGW_SELECT_PARQUET=${WITH_RADOSGW_SELECT_PARQUET:-"OFF"}
+WITH_RDMA=${WITH_RDMA:-"OFF"}
+WITH_SYSTEM_BOOST=${WITH_SYSTEM_BOOST:-"ON"}
+WITH_TESTS=${WITH_TESTS:-"OFF"}
+
+NPROC=${NPROC:-$(nproc --ignore=2)}
 
 CEPH_CMAKE_ARGS=(
+  "-DWITH_CCACHE=ON"
+  "-DBOOST_J=${NPROC}"
   "-DCMAKE_C_COMPILER=gcc-11"
   "-DCMAKE_CXX_COMPILER=g++-11"
   "-DENABLE_GIT_VERSION=ON"
   "-DWITH_PYTHON3=3"
-  "-DWITH_CCACHE=ON"
-  "-DWITH_TESTS=${WITH_TESTS}"
   "-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}"
-  "-DWITH_RADOSGW_AMQP_ENDPOINT=OFF"
-  "-DWITH_RADOSGW_KAFKA_ENDPOINT=OFF"
-  "-DWITH_RADOSGW_SELECT_PARQUET=OFF"
-  "-DWITH_RADOSGW_MOTR=OFF"
+  "-DWITH_JAEGER=${WITH_JAEGER}"
+  "-DWITH_LTTNG=${WITH_LTTNG}"
+  "-DWITH_MANPAGE=${WITH_MANPAGE}"
+  "-DWITH_OPENLDAP=${WITH_OPENLDAP}"
+  "-DWITH_RADOSGW_AMQP_ENDPOINT=${WITH_RADOSGW_AMQP_ENDPOINT}"
   "-DWITH_RADOSGW_DBSTORE=${WITH_RADOSGW_DBSTORE}"
-  "-DWITH_RADOSGW_LUA_PACKAGES=OFF"
-  "-DWITH_MANPAGE=OFF"
-  "-DWITH_OPENLDAP=OFF"
-  "-DWITH_LTTNG=OFF"
-  "-DWITH_RDMA=OFF"
-  "-DWITH_SYSTEM_BOOST=ON"
-  ${CEPH_CMAKE_ARGS}
+  "-DWITH_RADOSGW_KAFKA_ENDPOINT=${WITH_RADOSGW_KAFKA_ENDPOINT}"
+  "-DWITH_RADOSGW_LUA_PACKAGES=${WITH_RADOSGW_LUA_PACKAGES}"
+  "-DWITH_RADOSGW_MOTR=${WITH_RADOSGW_MOTR}"
+  "-DWITH_RADOSGW_SELECT_PARQUET=${WITH_RADOSGW_SELECT_PARQUET}"
+  "-DWITH_RDMA=${WITH_RDMA}"
+  "-DWITH_SYSTEM_BOOST=${WITH_SYSTEM_BOOST}"
+  "-DWITH_TESTS=${WITH_TESTS}"
+  "${CEPH_CMAKE_ARGS}"
 )
-NPROC=${NPROC:-$(nproc --ignore=2)}
 
-build_radosgw() {
-  echo "Building radosgw ..."
+_configure() {
+  echo "Configuring Buildenv ..."
   echo "CEPH_DIR=${CEPH_DIR}"
   echo "NPROC=${NPROC}"
-  echo "CMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}"
   echo "CCACHE_DIR=${S3GW_CCACHE_DIR}"
-  echo "WITH_TESTS=${WITH_TESTS}"
-  echo "WITH_RADOSGW_DBSTORE=${WITH_RADOSGW_DBSTORE}"
+  # shellcheck disable=SC2068
+  for e in ${CEPH_CMAKE_ARGS[@]} ; do
+    echo "${e}"
+  done
+
+  cd "${CEPH_DIR}"
 
-  export CCACHE_DIR=${S3GW_CCACHE_DIR}
+  export CCACHE_DIR="${S3GW_CCACHE_DIR}"
   if [ ! -d "${CCACHE_DIR}" ]; then
     echo "ccache dir not found, create."
     mkdir "${CCACHE_DIR}"
     echo "Created by aquarist-labs/s3gw-tools build-radosgw container" > \
       "${CCACHE_DIR}/README"
   fi
 
-  cd ${CEPH_DIR}
-
   # This is necessary since git v2.35.2 because of CVE-2022-24765
   # but we have to continue in case CEPH_DIR is not a git repo
   # Since git 2.36 the the wildcard '*' is also accepted
-  git config --global --add safe.directory "*" || true
-
-  if [ -d "build" ]; then
-      cd build/
-      cmake -DBOOST_J=${NPROC} ${CEPH_CMAKE_ARGS[@]} ..
-  else
-      ./do_cmake.sh ${CEPH_CMAKE_ARGS[@]}
-      cd build/
+  if ! git config --global safe.directory > /dev/null ; then
+    git config --global --add safe.directory "*" || true
   fi
 
-  ninja -j${NPROC} bin/radosgw
-}
-
-strip_radosgw() {
-  [ "${CMAKE_BUILD_TYPE}" == "Debug" -o "${CMAKE_BUILD_TYPE}" == "RelWithDebInfo" ] && return 0
+  if [ ! -d "${S3GW_BUILD_DIR}" ] ; then
+    echo "build dir not found, create."
+    mkdir "${S3GW_BUILD_DIR}"
+  fi
 
-  echo "Stripping files ..."
-  strip --strip-debug --strip-unneeded \
-    --remove-section=.comment --remove-section=.note.* \
-    ${CEPH_DIR}/build/bin/radosgw \
-    ${CEPH_DIR}/build/lib/*.so
+  cd "${S3GW_BUILD_DIR}"
+  # shellcheck disable=SC2068
+  cmake ${CEPH_CMAKE_ARGS[@]} ..
 }
 
-build_radosgw_test() {
-  echo "Building radosgw test..."
-
-  export CCACHE_DIR=${S3GW_CCACHE_DIR}
-  if [ ! -d "${CCACHE_DIR}" ]; then
-    echo "ccache dir not found, create."
-    mkdir "${CCACHE_DIR}"
-    echo "Created by aquarist-labs/s3gw-tools build-radosgw container" > \
-      "${CCACHE_DIR}/README"
-  fi
+_build() {
+  cd "${S3GW_BUILD_DIR}"
 
-  cd ${CEPH_DIR}
+  make -j"${NPROC}" radosgw
 
-  # This is necessary since git v2.35.2 because of CVE-2022-24765
-  # but we have to continue in case CEPH_DIR is not a git repo
-  git config --global --add safe.directory "*" || true
-
-  if [ -d "build" ]; then
-      cd build/
-      cmake -DBOOST_J=${NPROC} ${CEPH_CMAKE_ARGS[@]} ..
-  else
-      ./do_cmake.sh ${CEPH_CMAKE_ARGS[@]}
-      cd build/
+  if [ "${WITH_TESTS}" == "ON" ] ; then
+    make -j"${NPROC}" \
+      unittest_rgw_sfs_sqlite_users \
+      unittest_rgw_sfs_sqlite_buckets \
+      unittest_rgw_sfs_sqlite_objects \
+      unittest_rgw_sfs_sqlite_versioned_objects \
+      unittest_rgw_sfs_sfs_bucket \
+      unittest_rgw_sfs_metadata_compatibility \
+      unittest_rgw_sfs_gc
   fi
-
-  ninja -j${NPROC} bin/unittest_rgw_sfs_sqlite_users
-  ninja -j${NPROC} bin/unittest_rgw_sfs_sqlite_buckets
-  ninja -j${NPROC} bin/unittest_rgw_sfs_sqlite_objects
-  ninja -j${NPROC} bin/unittest_rgw_sfs_sqlite_versioned_objects
-  ninja -j${NPROC} bin/unittest_rgw_sfs_sfs_bucket
-  ninja -j${NPROC} bin/unittest_rgw_sfs_metadata_compatibility
-  ninja -j${NPROC} bin/unittest_rgw_sfs_gc
 }
 
-build_radosgw
-strip_radosgw
+_strip() {
+  # don't strip debug builds
+  [ "${CMAKE_BUILD_TYPE}" == "Debug" ] \
+    || [ "${CMAKE_BUILD_TYPE}" == "RelWithDebInfo" ] \
+    && return 0
 
-if [ "${WITH_TESTS}" == "ON" ]; then
-  build_radosgw_test
-fi
+  echo "Stripping files ..."
+  strip \
+    --strip-debug \
+    --strip-unneeded \
+    --remove-section=.comment \
+    --remove-section=.note.* \
+    "${CEPH_DIR}"/build/bin/radosgw \
+    "${CEPH_DIR}"/build/lib/*.so
+}
 
-exit 0
+_configure
+_build
+_strip

tests/s3gw-smoke-test.sh

@@ -71,7 +71,7 @@ down_md5=$(md5sum -b obj1.bin.local | cut -f1 -d' ')
 
 [[ "${orig_md5}" == "${down_md5}" ]] || exit 1
 
-s3 get s3://${bucket}/dne.bin && exit 1
+s3 get s3://${bucket}/does-not-exist.bin && exit 1
 
 must_have=("obj1.bin" "obj1.bin.2" "my/obj1.bin")
 ifs_old=$IFS

tests/s3gw-test.sh

@@ -124,4 +124,3 @@ case ${cmd} in
     exit 1
     ;;
 esac
-

tests/test-s3gw-versioning-smoke.py

@@ -198,7 +198,7 @@ def test_put_objects_versioning_enabled(self):
             response = self.s3_client.download_file(bucket_name, object_name, check_deleted_file)
         self.assertTrue('404' in str(context.exception))
 
-        # download the previous version, it should still be reacheable
+        # download the previous version, it should still be reachable
         check_version_file_2 = os.path.join(self.test_dir.name, 'check_version2.bin')
         bucket.download_file(
             object_name,