Skip to content

build!: Install ystdlib as a package to avoid duplicate ystdlib files. #1038

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

Closed
sitaowang1998 wants to merge 51 commits into from
Closed

build!: Install ystdlib as a package to avoid duplicate ystdlib files. #1038

Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
1cc7e76
Install ystdlib as package
sitaowang1998 Jun 26, 2025
8e0e4d5
Bump cmake min version to 3.23
sitaowang1998 Jun 26, 2025
aba106f
Install cmake 3.23.5 for ubuntu jammy
sitaowang1998 Jun 26, 2025
917fd86
Merge branch 'main' into ystdlib-package
sitaowang1998 Jun 26, 2025
9a2c19e
Merge branch 'main' into ystdlib-package
sitaowang1998 Jun 26, 2025
71e96ac
Remove unnecessary cmake variable
sitaowang1998 Jun 26, 2025
430f91c
Use homebrew llvm in macos GH runner
sitaowang1998 Jun 26, 2025
2f1b08d
Revert "Use homebrew llvm in macos GH runner"
sitaowang1998 Jun 26, 2025
f324686
Temporarily remove other workflows
sitaowang1998 Jun 26, 2025
a51b899
Add setting of environment variables
sitaowang1998 Jun 26, 2025
04cc82a
Revert "Temporarily remove other workflows"
sitaowang1998 Jun 26, 2025
0e29499
Bug fix
sitaowang1998 Jun 26, 2025
cfad209
Temporarily remove other GH workflows
sitaowang1998 Jun 26, 2025
753e555
Add check on PATH variable
sitaowang1998 Jun 26, 2025
15f3097
Revert "Add check on PATH variable"
sitaowang1998 Jun 27, 2025
55516f5
Use llvm
sitaowang1998 Jun 27, 2025
2f7963d
Bug fix
sitaowang1998 Jun 27, 2025
0679303
Add llvm library
sitaowang1998 Jun 27, 2025
71c0a05
Remove ld flags
sitaowang1998 Jun 27, 2025
7d534bf
Revert "Temporarily remove other GH workflows"
sitaowang1998 Jun 27, 2025
072482f
Merge branch 'main' into macos-llvm
sitaowang1998 Jun 27, 2025
cf2c37b
Experiment with extra clang-tidy flags
sitaowang1998 Jun 27, 2025
fa5dfc2
Temporarily remove other GH workflows
sitaowang1998 Jun 27, 2025
e2434cb
Revert "Experiment with extra clang-tidy flags"
sitaowang1998 Jun 27, 2025
39961b4
Add more environment flags
sitaowang1998 Jun 27, 2025
5e9d8c6
Experiment with environment variables
sitaowang1998 Jun 27, 2025
9478ae8
Add more experiment with environment variables AR and RANLIB
sitaowang1998 Jun 27, 2025
fca4857
Remove environment flags for flags
sitaowang1998 Jun 27, 2025
67d2363
Experiment with clang-tidy argument
sitaowang1998 Jun 27, 2025
2d82296
Try add llvm to clang-tidy sysroot
sitaowang1998 Jun 28, 2025
260b69d
Revert "Try add llvm to clang-tidy sysroot"
sitaowang1998 Jun 28, 2025
c91dc84
Not use llvm toolchain in clp
sitaowang1998 Jun 28, 2025
693e2d0
Revert "Not use llvm toolchain in clp"
sitaowang1998 Jun 28, 2025
234a1f3
Experiment with extra args
sitaowang1998 Jun 28, 2025
99cdacc
Remove errno_t=int from clang-tidy flags
sitaowang1998 Jun 28, 2025
0f1034a
Bug fix
sitaowang1998 Jun 28, 2025
db7cd48
Revert "Temporarily remove other GH workflows"
sitaowang1998 Jun 28, 2025
9e2c951
Add rsize clang-tidy flag in all clang-tidy tasks
sitaowang1998 Jun 28, 2025
7e528b4
Merge branch 'main' into macos-llvm
sitaowang1998 Jun 28, 2025
aadd28c
Merge branch 'main' into ystdlib-package
sitaowang1998 Jun 30, 2025
d80898d
Merge branch 'main' into macos-llvm
sitaowang1998 Jun 30, 2025
5fd31da
Merge branch 'macos-llvm' into ystdlib-package
sitaowang1998 Jun 30, 2025
ea5813a
Remove unnecessary if guard in cmake
sitaowang1998 Jul 7, 2025
453a9c7
Improve comments in install script
sitaowang1998 Jul 7, 2025
c923e72
Merge branch 'main' into ystdlib-package
sitaowang1998 Jul 7, 2025
18ddf6c
Remove fix for macOS
sitaowang1998 Jul 9, 2025
be467e8
Disable ci on macOS 13 and 14
sitaowang1998 Jul 9, 2025
c2501df
Improve sequence and comment in install script
sitaowang1998 Jul 10, 2025
dcb3e28
Remove PIC flag
sitaowang1998 Jul 10, 2025
06527be
Merge branch 'main' into ystdlib-package
sitaowang1998 Jul 10, 2025
eed0d81
Add default flags to ystdlib build
sitaowang1998 Jul 10, 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
7 changes: 5 additions & 2 deletions .github/workflows/clp-core-build-macos.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ jobs:
strategy:
matrix:
os:
- "macos-13"
- "macos-14"
# NOTE: We temporarily disable macOS 13 and 14 since older versions of `AppleClang` do
# not support C++ 20 required by `ystdlib`.
# See https://github.com/y-scope/clp/issues/1090 for more details.
# - "macos-13"
# - "macos-14"
- "macos-15"
Comment on lines +48 to 53

@coderabbitai coderabbitai Bot Jul 9, 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.

🧹 Nitpick (assertive)

Flag a follow-up for re-enabling older macOS runners

Commenting the entries works, but experience shows they tend to be forgotten.
Consider adding a dedicated TODO (or # FIXME) with a target AppleClang version / date, or drive it via a GitHub issue label so CI coverage is restored automatically once the runners catch up.

This keeps platform coverage front-of-mind and avoids silent test gaps.

🤖 Prompt for AI Agents 
In .github/workflows/clp-core-build-macos.yaml around lines 48 to 53, the older
macOS runners (macos-13 and macos-14) are commented out but lack a clear TODO or
FIXME note for re-enabling them. Add a dedicated TODO or FIXME comment
specifying the target AppleClang version or a date to revisit this, or reference
a GitHub issue to track re-enabling these runners, ensuring the platform
coverage is not forgotten and test gaps are avoided.

use_shared_libs:
- true
Expand Down
8 changes: 3 additions & 5 deletions components/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.21)
cmake_minimum_required(VERSION 3.23)

# Toolchain setup must come before the first project() call in the entire CMake buildsystem.
# If CLP is not the top-level project, the following setup has no effect.
Expand Down Expand Up @@ -283,10 +283,8 @@ if(CLP_NEED_YAMLCPP)
endif()

# Add ystdlib
if(CLP_NEED_YSTDLIB)
set(YSTDLIB_CPP_BUILD_TESTING OFF)
add_subdirectory("${CLP_YSTDLIB_SOURCE_DIRECTORY}" "${CMAKE_BINARY_DIR}/ystdlib" EXCLUDE_FROM_ALL)
endif()
find_package(ystdlib REQUIRED)
message(STATUS "Found ystdlib ${ystdlib_VERSION}")

# Find and setup ZStd Library
if(CLP_NEED_ZSTD)
Expand Down
4 changes: 3 additions & 1 deletion components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ set -u
script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
lib_install_scripts_dir=$script_dir/..

# NOTE: boost must be installed first since the remaining packages depend on it
# NOTE: cmake must be at least version 3.23
"$lib_install_scripts_dir"/install-cmake.sh 3.23.5
# NOTE: boost must be installed since the remaining packages depend on it

@coderabbitai coderabbitai Bot Jul 7, 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

Ensure the updated CMake is actually picked up by downstream installs

install-cmake.sh likely drops the new binary in ${HOME}/.local/bin (or similar).
If that directory isn’t already on PATH for the current shell, subsequent invocations of the install-*-from-source.sh helpers will still resolve the distro-provided CMake (< 3.23), defeating the version bump.

Consider sourcing the profile fragment produced by install-cmake.sh (or exporting PATH) before calling any further CMake-driven installers.

🤖 Prompt for AI Agents 
In
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
around lines 12 to 14, after calling install-cmake.sh to install CMake 3.23.5,
ensure the updated CMake binary is used by the current shell. To fix this,
source the profile fragment created by install-cmake.sh or explicitly export the
PATH to include the directory where the new CMake binary is installed (e.g.,
${HOME}/.local/bin) before running any further CMake-dependent install scripts.

"$lib_install_scripts_dir"/install-boost.sh 1.87.0

Comment thread
sitaowang1998 marked this conversation as resolved.
Outdated
"$lib_install_scripts_dir"/libarchive.sh 3.5.1
Expand Down
1 change: 0 additions & 1 deletion components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
ca-certificates \
checkinstall \
cmake \
curl \
build-essential \
git \
Expand Down
23 changes: 9 additions & 14 deletions taskfiles/deps/main.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,20 +374,15 @@ tasks:

ystdlib:
internal: true
vars:
LIB_NAME: "ystdlib"
YSTDLIB_OUTPUT_DIR: "{{.G_DEPS_CORE_DIR}}/{{.LIB_NAME}}-src"
run: "once"
deps:
- task: "utils:init"
cmds:
- task: "yscope-dev-utils:remote:download-and-extract-tar"
- task: "utils:install-remote-cmake-lib"
vars:
CHECKSUM_FILE: "{{.G_DEPS_CORE_CHECKSUMS_DIR}}/{{.LIB_NAME}}.md5"
FILE_SHA256: "36fa0e9d96b7307ca92482343d6ba1091c5576370676e6d423cce32c20e34a3d"
OUTPUT_DIR: "{{.YSTDLIB_OUTPUT_DIR}}"
URL: "https://github.com/y-scope/ystdlib-cpp/archive/d80cf86.tar.gz"
- >-
echo "set(
CLP_YSTDLIB_SOURCE_DIRECTORY \"{{.YSTDLIB_OUTPUT_DIR}}\"
)" > "{{.G_DEPS_CORE_CMAKE_SETTINGS_DIR}}/{{.LIB_NAME}}.cmake"
CMAKE_GEN_ARGS:
- "-DCMAKE_POSITION_INDEPENDENT_CODE=ON"
- "-Dystdlib_BUILD_TESTING=OFF"
- "-DCMAKE_POLICY_DEFAULT_CMP0144=NEW"
Comment thread
sitaowang1998 marked this conversation as resolved.
LIB_NAME: "ystdlib"
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
WORK_DIR: "{{.G_DEPS_DIR}}/ystdlib"
TARBALL_SHA256: "4c027c884506e1775070192f9d1d58238a6d5b078608211fa442477393676738"
TARBALL_URL: "https://github.com/y-scope/ystdlib-cpp/archive/0ae886c.tar.gz"