Skip to content

build!: Install ystdlib as a CMake package to avoid issues when projects use both Spider and ystdlib through add_subdirectory. #160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sitaowang1998 merged 13 commits into from
Aug 6, 2025
Merged

build!: Install ystdlib as a CMake package to avoid issues when projects use both Spider and ystdlib through add_subdirectory. #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a63aaa9
Install ystdlib as package
sitaowang1998 Jun 26, 2025
a0dbb9b
Update ci to install later cmake; Update cmake min version
sitaowang1998 Jun 26, 2025
649b03f
Bug fix for ci script
sitaowang1998 Jun 26, 2025
c77c662
Fix file permission for install-cmake script
sitaowang1998 Jun 26, 2025
87b973e
Improve cmake install script based on code rabbit suggestions
sitaowang1998 Jun 26, 2025
c99c31e
Remove unnecessary check in cmake
sitaowang1998 Jul 7, 2025
61cb255
Merge branch 'main' into ystdlib_package
sitaowang1998 Aug 4, 2025
75a28fc
Remove cmake 0144 policy
sitaowang1998 Aug 4, 2025
d2ceaf5
Merge branch 'main' into ystdlib_package
sitaowang1998 Aug 5, 2025
0bb4e39
Update ystdlib to use latest yscope-dev-utils
sitaowang1998 Aug 5, 2025
49e8759
Merge branch 'main' into ystdlib_package
sitaowang1998 Aug 5, 2025
a1f0a0c
Merge branch 'main' into ystdlib_package
sitaowang1998 Aug 5, 2025
3995742
Merge branch 'main' into ystdlib_package
sitaowang1998 Aug 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# CMake 3.22.1 is the default on Ubuntu 22.04
cmake_minimum_required(VERSION 3.22.1)
cmake_minimum_required(VERSION 3.23)

project(
spider
Expand Down Expand Up @@ -180,12 +179,8 @@ else()
endif()

# Add ystdlib-cpp
set(YSTDLIB_CPP_BUILD_TESTING OFF)
add_subdirectory(
"${SPIDER_YSTDLIB_SOURCE_DIRECTORY}"
"${CMAKE_BINARY_DIR}/ystdlib"
EXCLUDE_FROM_ALL
)
find_package(ystdlib REQUIRED)
message(STATUS "Found ystdlib ${ystdlib_VERSION}.")

find_package(Threads REQUIRED)

Expand Down
27 changes: 15 additions & 12 deletions dep-tasks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ tasks:
- task: "install-mariadb-connector-cpp"
- task: "install-msgpack"
- task: "install-spdlog"
- task: "download-ystdlib"
- task: "install-ystdlib"

install-abseil:
internal: true
Expand Down Expand Up @@ -56,21 +56,24 @@ tasks:
- "-DCATCH_BUILD_TESTING=OFF"
JOBS: "{{.G_DEPS_MAX_PARALLELISM_PER_TASK}}"

download-ystdlib:
install-ystdlib:
internal: true
run: "once"
deps:
- "install-boost"
cmds:
- task: ":utils:remote:download-and-extract-tar"
- task: ":utils:cmake:install-remote-tar"
vars:
FILE_SHA256: "36fa0e9d96b7307ca92482343d6ba1091c5576370676e6d423cce32c20e34a3d"
OUTPUT_DIR: "{{.G_DEPS_DIR}}/ystdlib/ystdlib-src"
URL: "https://github.com/y-scope/ystdlib-cpp/archive/d80cf86.tar.gz"
- |
cat <<EOF >> "{{.G_DEPS_CMAKE_SETTINGS_DIR}}/ystdlib.cmake"
set(
SPIDER_YSTDLIB_SOURCE_DIRECTORY "{{.G_DEPS_DIR}}/ystdlib/ystdlib-src"
)
EOF
CMAKE_PACKAGE_NAME: "ystdlib"
WORK_DIR: "{{.G_DEPS_DIR}}/ystdlib"
TAR_SHA256: "4c027c884506e1775070192f9d1d58238a6d5b078608211fa442477393676738"
TAR_URL: "https://github.com/y-scope/ystdlib-cpp/archive/0ae886c.tar.gz"
CMAKE_SETTINGS_DIR: "{{.G_DEPS_CMAKE_SETTINGS_DIR}}"
CMAKE_GEN_ARGS:
- "-DCMAKE_POSITION_INDEPENDENT_CODE=ON"
- "-Dystdlib_BUILD_TESTING=OFF"
- "-C {{.G_DEPS_CMAKE_SETTINGS_DIR}}/Boost.cmake"
JOBS: "{{.G_DEPS_MAX_PARALLELISM_PER_TASK}}"

install-fmtlib:
internal: true
Expand Down
47 changes: 47 additions & 0 deletions tools/scripts/lib_install/install-cmake.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env bash

# Exit on error
set -euo pipefail

cUsage="Usage: ${BASH_SOURCE[0]} <version>"
if [ "$#" -lt 1 ] ; then
echo $cUsage
exit 1
fi
version=$1

echo "Checking for elevated privileges..."
if [ ${EUID:-$(id -u)} -ne 0 ] ; then
sudo echo "Script can elevate privileges."
fi

# Get number of cpu cores
num_cpus=$(nproc 2>/dev/null || grep -c ^processor /proc/cpuinfo)

package_name=cmake

# Create temp dir for installation
temp_dir=/tmp/${package_name}-installation
mkdir -p $temp_dir
Comment on lines +24 to +25

@coderabbitai coderabbitai Bot Jun 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Race-safe temporary directory

A fixed /tmp/cmake-installation can collide with parallel runs. Leverage mktemp:

-temp_dir=/tmp/${package_name}-installation
-mkdir -p $temp_dir
+temp_dir=$(mktemp -d /tmp/${package_name}-installation-XXXXXX)
🤖 Prompt for AI Agents 
In tools/scripts/lib_install/install-cmake.sh around lines 24 to 25, the
temporary directory is set to a fixed path which can cause collisions in
parallel runs. Replace the fixed directory assignment with a call to mktemp to
create a unique, race-safe temporary directory. Use mktemp with appropriate
options to generate a unique directory under /tmp and assign it to temp_dir.


# Clean up
trap 'rm -rf "$temp_dir"' EXIT

cd $temp_dir

# Download source
tar_filename=cmake-${version}.tar.gz
curl -fsSL https://github.com/Kitware/CMake/releases/download/v${version}/${tar_filename} -o ${tar_filename}
tar xzf ${tar_filename}
cd cmake-${version}
Comment on lines +33 to +36

@coderabbitai coderabbitai Bot Jun 26, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

No integrity check on downloaded tarball

Blindly piping an archive into the build chain is a supply-chain risk. Fetch the official SHA256 from the same release and validate:

curl -fsSL -O https://github.com/Kitware/CMake/releases/download/v${version}/${tar_filename}.sha256
sha256sum -c ${tar_filename}.sha256
🤖 Prompt for AI Agents 
In tools/scripts/lib_install/install-cmake.sh around lines 30 to 33, the script
downloads and extracts the CMake tarball without verifying its integrity. To fix
this, add steps to download the corresponding SHA256 checksum file from the same
release URL and verify the downloaded tarball using sha256sum -c before
extracting it. This ensures the tarball is authentic and prevents supply-chain
risks.


# Build
./bootstrap
make -j${num_cpus}

# Install
if [ ${EUID:-$(id -u)} -ne 0 ] ; then
sudo make install
else
make install
fi
2 changes: 1 addition & 1 deletion tools/scripts/lib_install/linux/install-dev.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ ${privileged_command_prefix} apt-get update
DEBIAN_FRONTEND=noninteractive ${privileged_command_prefix} apt-get install --no-install-recommends -y \
ca-certificates \
checkinstall \
cmake \
curl \
g++ \
gcc \
Expand All @@ -34,6 +33,7 @@ DEBIAN_FRONTEND=noninteractive ${privileged_command_prefix} apt-get install --no

script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
lib_install_scripts_dir="$script_dir/.."
${privileged_command_prefix} "$lib_install_scripts_dir"/install-cmake.sh 3.23.5
# TODO https://github.com/y-scope/spider/issues/86
"$lib_install_scripts_dir"/check-cmake-version.sh

Expand Down