Add CMake packaging and CI/release pipeline#1
Merged
Conversation
Documents the chosen approach: pre-built release tarballs with CMake config-mode packages, matching OpenMS's existing find_package() pattern. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes: SameMinorVersion versioning, lib/cmake/ conventional path, Windows lib filename docs, cargo test + caching steps, smoke test exit code semantics, cdylib exclusion note. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4 tasks across 4 chunks: CMake config templates, packaging script, GitHub Actions workflow, and local validation. Also updates spec to add advapi32 to Windows system deps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds Config.cmake.in with imported static target and platform-specific system link deps, plus ConfigVersion.cmake.in with SameMinorVersion compatibility semantics. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Assembles release tarballs from cargo build output with header, static library, and configured CMake config files. Handles all 4 target platform/arch combos. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Builds and tests on linux-x86_64, linux-aarch64, macos-arm64, and windows-x86_64. Smoke tests link the C++ example against the static library on each platform. Tag pushes create GitHub Releases with pre-built artifacts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Git Bash's MSYS layer converts /EHsc to C:/Program Files/Git/EHsc and /I to I:/ — breaking cl.exe flag parsing. Using shell: cmd avoids this. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds multi-platform CI/CD via a GitHub Actions workflow, two CMake package/configuration templates for consumer discovery and version semantics, and a packaging script that assembles platform-specific release archives from built artifacts. Changes
Sequence Diagram(s)mermaid Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
docs/superpowers/specs/2026-03-11-cmake-packaging-ci-release-design.md (1)
20-25: Label the plain fenced blocks.markdownlint's MD040 warning will keep firing on these unlabeled fences.
textorplaintextis enough for the archive-name and tree-layout snippets.Also applies to: 29-39, 150-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/specs/2026-03-11-cmake-packaging-ci-release-design.md` around lines 20 - 25, The fenced code blocks containing the archive file names (the timsrust_cpp_bridge-v<VERSION>-*.tar.gz/.zip list), the tree/layout snippets, and the other unlabeled blocks mentioned should be given a language tag (e.g., text or plaintext) immediately after the opening ``` so markdownlint MD040 stops flagging them; find the fences that contain the archive-name list and the tree-layout examples and change their openings from ``` to ```text (or ```plaintext) for each occurrence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 39-45: The workflow uses moving refs for actions (e.g.,
actions/checkout@v4, dtolnay/rust-toolchain@stable, Swatinem/rust-cache@v2 and
the other uses: entries around lines 80, 102, 116, 121), which can introduce
unreviewed changes; replace each `uses: owner/repo@tag` or `@stable`/major-tag
with the corresponding full commit SHA for that exact release (obtain the SHA
from the action repo’s releases/tags), update every `uses:` entry accordingly,
and verify the SHAs are correct and documented in the PR so the release run is
pinned and reproducible.
- Around line 57-60: The Unix smoke-test g++ invocation currently uses the
linker flag -Ltarget/release -ltimsrust_cpp_bridge which lets the linker pick
the shared library; change it to explicitly link the static archive produced by
Cargo (the libtimsrust_cpp_bridge.a in target/release) so the staticlib is
validated and system deps are exercised; update the g++ invocation referenced in
the workflow (the lines that build examples/cpp_client.cpp) to pass the explicit
path to the .a archive instead of -ltimsrust_cpp_bridge and ensure the
platform-specific system libraries (e.g., pthread, dl, m on Linux or Security,
SystemConfiguration, resolv on macOS) are still present in the link command.
- Around line 98-108: Remove the tag-only gating from the "Package" step so
packaging runs for every matrix job (i.e., drop the if: startsWith(github.ref,
'refs/tags/v') on the "Package" step that calls scripts/package.sh), then add a
new step after packaging that unpacks the produced archive and runs a CMake
consumer build to validate find_package() and INTERFACE_LINK_LIBRARIES (use the
same artifact name template timsrust_cpp_bridge-v${{
steps.version.outputs.version }}-${{ matrix.platform }} and matrix archive_ext
to locate the package), and keep the existing "Upload artifact" step tag-gated
as-is so only tagged runs publish artifacts.
In `@cmake/timsrust_cpp_bridgeConfigVersion.cmake.in`:
- Around line 17-18: The version comparison currently uses string equality
(if(PACKAGE_FIND_VERSION STREQUAL PACKAGE_VERSION)) which mis-identifies
numeric-equivalent versions; change the condition to use CMake's semantic
version comparison (if(PACKAGE_FIND_VERSION VERSION_EQUAL PACKAGE_VERSION)) so
PACKAGE_FIND_VERSION and PACKAGE_VERSION are compared component-wise and
PACKAGE_VERSION_EXACT is set correctly when versions are numerically equal.
In `@scripts/package.sh`:
- Around line 72-80: The Windows branch of the packaging step uses "7z a" which
appends to existing zips; modify the branch that runs 7z (the block using
PLATFORM == windows-* and the invocation "7z a -tzip
\"$PROJECT_ROOT/target/package/${ARCHIVE_NAME}.zip\" ...") to remove any
existing "$PROJECT_ROOT/target/package/${ARCHIVE_NAME}.zip" before calling 7z
(e.g., check and rm -f the file) so the archive is recreated deterministically;
leave the non-Windows tar branch unchanged.
- Around line 13-14: Validate the VERSION variable right after parsing the args
in package.sh (where VERSION and TARGET are set) by checking it matches a strict
semantic version pattern (^[0-9]+\\.[0-9]+\\.[0-9]+$); if it does not, print a
usage/error and exit non‑zero. After validation, safely split VERSION into
MAJOR, MINOR, PATCH (e.g., IFS=. read -r MAJOR MINOR PATCH <<<"$VERSION") and
use those numeric parts when building archive names and when writing
timsrust_cpp_bridgeConfigVersion.cmake placeholders instead of trusting the raw
VERSION string; this prevents invalid archive directory names and malformed
CMake version output. Ensure any place that constructs paths or files from
VERSION uses the validated components.
---
Nitpick comments:
In `@docs/superpowers/specs/2026-03-11-cmake-packaging-ci-release-design.md`:
- Around line 20-25: The fenced code blocks containing the archive file names
(the timsrust_cpp_bridge-v<VERSION>-*.tar.gz/.zip list), the tree/layout
snippets, and the other unlabeled blocks mentioned should be given a language
tag (e.g., text or plaintext) immediately after the opening ``` so markdownlint
MD040 stops flagging them; find the fences that contain the archive-name list
and the tree-layout examples and change their openings from ``` to ```text (or
```plaintext) for each occurrence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b8a56e2-abb2-4060-8a35-357f5f1713be
📒 Files selected for processing (6)
.github/workflows/release.ymlcmake/timsrust_cpp_bridgeConfig.cmake.incmake/timsrust_cpp_bridgeConfigVersion.cmake.indocs/superpowers/plans/2026-03-11-cmake-packaging-ci-release.mddocs/superpowers/specs/2026-03-11-cmake-packaging-ci-release-design.mdscripts/package.sh
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/release.yml (3)
39-45:⚠️ Potential issue | 🟠 MajorPin every action to a full commit SHA before publishing releases.
These
uses:entries still point at moving refs (@v4,@v2,@stable). On a workflow that uploads release assets, that leaves the release path open to unreviewed upstream changes. Please replace each ref with the exact commit SHA for the intended action version, especially Line 42.Also applies to: 80-80, 102-102, 116-121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 39 - 45, Replace moving refs with pinned commit SHAs for each GitHub Action used in the workflow: change the uses entries actions/checkout@v4, dtolnay/rust-toolchain@stable, and Swatinem/rust-cache@v2 to their exact commit SHA pins (e.g., actions/checkout@<sha>, dtolnay/rust-toolchain@<sha>, Swatinem/rust-cache@<sha>), and do the same for the other occurrences noted (lines referenced in the review); ensure each uses: value references the full commit SHA for the intended released tag instead of `@v4/`@stable/@v2.
57-60:⚠️ Potential issue | 🟠 MajorLink the Unix smoke tests against the static archive explicitly.
Line 59 and Line 71 still let the linker choose the shared library when both artifacts exist, so CI is not actually validating the archive that
scripts/package.shships. Point both smoke tests attarget/release/libtimsrust_cpp_bridge.adirectly and keep the platform system libs/frameworks on the command line.Suggested change
- g++ -std=c++17 examples/cpp_client.cpp \ - -Iinclude \ - -Ltarget/release -ltimsrust_cpp_bridge \ + g++ -std=c++17 examples/cpp_client.cpp \ + -Iinclude \ + target/release/libtimsrust_cpp_bridge.a \ -lpthread -ldl -lm \ -o target/smoke_test ... - clang++ -std=c++17 examples/cpp_client.cpp \ - -Iinclude \ - -Ltarget/release -ltimsrust_cpp_bridge \ + clang++ -std=c++17 examples/cpp_client.cpp \ + -Iinclude \ + target/release/libtimsrust_cpp_bridge.a \ -framework Security -framework SystemConfiguration \ -lresolv -lpthread \ -o target/smoke_testAlso applies to: 69-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 57 - 60, The g++ commands that build the Unix smoke tests currently use "-Ltarget/release -ltimsrust_cpp_bridge" which allows the dynamic .so/.dylib to be chosen; change those invocations to link the static archive explicitly by replacing the "-L... -l..." usage with the full static archive path "target/release/libtimsrust_cpp_bridge.a" in the g++ command that compiles the examples/cpp_client.cpp (the two smoke-test invocations in the workflow), and leave the platform system libs/frameworks (e.g., -lpthread -ldl -lm or macOS frameworks) on the command line unchanged.
89-105:⚠️ Potential issue | 🟠 MajorRun packaging and a packaged
find_package()smoke test on every matrix job.Line 90 and Line 96 keep the packaging path entirely out of PR and branch CI, so the first time the generated CMake package is exercised is on a tag push. That leaves archive layout, relative-path, and
INTERFACE_LINK_LIBRARIESregressions invisible until release time. Generate a version for non-tag runs as well, runscripts/package.shon every matrix job, and add a small consumer build against the unpacked artifact; only the upload/release steps need to stay tag-gated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 89 - 105, Remove the tag-only gating around the version extraction and Package steps so packaging runs on every matrix job: make the "Extract version from tag" step (or add a fallback) produce a version for non-tag runs (e.g., fallback to GITHUB_SHA or git describe) and call scripts/package.sh with that version in the "Package" step for every matrix job, then add a short consumer build/smoke-test step that unpacks the generated artifact and runs a small CMake consumer against the unpacked package to validate find_package()/INTERFACE_LINK_LIBRARIES; keep only the "Upload artifact" step tag-gated so actual artifact upload still happens only on tag pushes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 82-87: The Windows smoke test step ("Smoke test (Windows)")
currently appends "|| ver>nul" which masks non-zero exits; replace that with
running target\smoke_test.exe directly and then inspect %ERRORLEVEL%: if
%ERRORLEVEL% == 1 return success (exit 0) but for any other non-zero code
(including large codes produced by crashes/access violations) exit with that
code so the job fails; remove the "|| ver>nul" hack and add a small CMD
conditional that checks %ERRORLEVEL% after running target\smoke_test.exe and
only swallow the expected 1 exit, propagating all other errors.
---
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 39-45: Replace moving refs with pinned commit SHAs for each GitHub
Action used in the workflow: change the uses entries actions/checkout@v4,
dtolnay/rust-toolchain@stable, and Swatinem/rust-cache@v2 to their exact commit
SHA pins (e.g., actions/checkout@<sha>, dtolnay/rust-toolchain@<sha>,
Swatinem/rust-cache@<sha>), and do the same for the other occurrences noted
(lines referenced in the review); ensure each uses: value references the full
commit SHA for the intended released tag instead of `@v4/`@stable/@v2.
- Around line 57-60: The g++ commands that build the Unix smoke tests currently
use "-Ltarget/release -ltimsrust_cpp_bridge" which allows the dynamic .so/.dylib
to be chosen; change those invocations to link the static archive explicitly by
replacing the "-L... -l..." usage with the full static archive path
"target/release/libtimsrust_cpp_bridge.a" in the g++ command that compiles the
examples/cpp_client.cpp (the two smoke-test invocations in the workflow), and
leave the platform system libs/frameworks (e.g., -lpthread -ldl -lm or macOS
frameworks) on the command line unchanged.
- Around line 89-105: Remove the tag-only gating around the version extraction
and Package steps so packaging runs on every matrix job: make the "Extract
version from tag" step (or add a fallback) produce a version for non-tag runs
(e.g., fallback to GITHUB_SHA or git describe) and call scripts/package.sh with
that version in the "Package" step for every matrix job, then add a short
consumer build/smoke-test step that unpacks the generated artifact and runs a
small CMake consumer against the unpacked package to validate
find_package()/INTERFACE_LINK_LIBRARIES; keep only the "Upload artifact" step
tag-gated so actual artifact upload still happens only on tag pushes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e69a61ac-ce2e-4115-98e9-638f3117055b
📒 Files selected for processing (1)
.github/workflows/release.yml
- Pin all GitHub Actions to commit SHAs for reproducibility - Link static archive explicitly in smoke tests (not -l which prefers .so) - Run packaging + CMake find_package() validation on every build, not just tags - Use VERSION_EQUAL instead of STREQUAL in ConfigVersion.cmake.in - Validate semver format and rm stale zip before 7z in package.sh - Remove design/plan docs from PR Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The iana_time_zone crate (transitive dep via chrono) requires CoreFoundation symbols. This was exposed by switching to explicit static archive linking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/release.yml (2)
88-93:⚠️ Potential issue | 🔴 CriticalStop masking Windows smoke-test failures.
Line 93 still turns every non-zero exit into success. That hides crashes, access violations, and any unexpected runtime failure in the smoke binary.
Suggested fix
- target\smoke_test.exe || ver>nul + target\smoke_test.exe + if %ERRORLEVEL% EQU 0 exit /b 0 + if %ERRORLEVEL% EQU 1 exit /b 0 + exit /b %ERRORLEVEL%🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 88 - 93, The "Smoke test (Windows)" workflow step is masking failures by appending "|| ver>nul" to the test command; remove that fallback so the step fails on any non-zero exit (i.e., delete the "target\smoke_test.exe || ver>nul" pattern) or replace it with a proper explicit exit propagation (run the smoke binary and let its exit code propagate). Locate the step named "Smoke test (Windows)" and edit the line containing "target\smoke_test.exe || ver>nul" so the smoke test's actual exit status is preserved.
109-128:⚠️ Potential issue | 🟠 MajorValidate the actual package, and force the linker to consume it.
This step still points
CMAKE_PREFIX_PATHat the pre-archive staging tree, not the generated.tar.gz/.zip, so archive-layout bugs can slip through. It also builds a consumer that never references any exported symbol, so the linker may never pull objects from the static archive andINTERFACE_LINK_LIBRARIESregressions can stay green.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 109 - 128, The validation currently points CMAKE_PREFIX_PATH at the pre-archive staging tree (ARCHIVE_DIR) and builds a consumer that never references any exported symbol, so archive-layout and INTERFACE_LINK_LIBRARIES regressions can slip by; change the job to locate the generated archive (from ARCHIVE_DIR), unpack the actual .tar.gz/.zip and point CMAKE_PREFIX_PATH at the extracted package root instead of the staging tree, and modify the test consumer (test_consumer / test.cpp) to reference an exported symbol from timsrust_cpp_bridge (e.g., call or extern a function/type declared in timsrust_cpp_bridge.h) so the linker must pull objects from the static archive while still using find_package(...) and target_link_libraries(tests_consumer timsrust_cpp_bridge::timsrust_cpp_bridge).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmake/timsrust_cpp_bridgeConfigVersion.cmake.in`:
- Around line 6-18: The template currently ignores requested version ranges; add
explicit checks for PACKAGE_FIND_VERSION_RANGE and its boundary variables (e.g.
PACKAGE_FIND_VERSION_LOWER, PACKAGE_FIND_VERSION_UPPER and any
inclusive/exclusive flags) and reject packages outside the requested range by
setting PACKAGE_VERSION_COMPATIBLE FALSE when PACKAGE_VERSION is less than the
lower bound or greater than (or equal-to when exclusive) the upper bound;
integrate these checks into the existing logic around
PACKAGE_FIND_VERSION/PACKAGE_VERSION and ensure PACKAGE_VERSION_EXACT/COMPATIBLE
are set appropriately when the range is satisfied.
---
Duplicate comments:
In @.github/workflows/release.yml:
- Around line 88-93: The "Smoke test (Windows)" workflow step is masking
failures by appending "|| ver>nul" to the test command; remove that fallback so
the step fails on any non-zero exit (i.e., delete the "target\smoke_test.exe ||
ver>nul" pattern) or replace it with a proper explicit exit propagation (run the
smoke binary and let its exit code propagate). Locate the step named "Smoke test
(Windows)" and edit the line containing "target\smoke_test.exe || ver>nul" so
the smoke test's actual exit status is preserved.
- Around line 109-128: The validation currently points CMAKE_PREFIX_PATH at the
pre-archive staging tree (ARCHIVE_DIR) and builds a consumer that never
references any exported symbol, so archive-layout and INTERFACE_LINK_LIBRARIES
regressions can slip by; change the job to locate the generated archive (from
ARCHIVE_DIR), unpack the actual .tar.gz/.zip and point CMAKE_PREFIX_PATH at the
extracted package root instead of the staging tree, and modify the test consumer
(test_consumer / test.cpp) to reference an exported symbol from
timsrust_cpp_bridge (e.g., call or extern a function/type declared in
timsrust_cpp_bridge.h) so the linker must pull objects from the static archive
while still using find_package(...) and target_link_libraries(tests_consumer
timsrust_cpp_bridge::timsrust_cpp_bridge).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56a3d8dd-d8a3-4840-af89-75b98aac988f
📒 Files selected for processing (3)
.github/workflows/release.ymlcmake/timsrust_cpp_bridgeConfigVersion.cmake.inscripts/package.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/package.sh
- Windows smoke test: replace `|| ver>nul` with errorlevel check that only allows exit code 1 (usage) and fails on crashes (>=2) - CMake version config: explicitly reject unsupported version ranges Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Rust static library's C dependencies (sqlite3, zstd) are compiled with the dynamic CRT (/MD). The consumer must match to avoid unresolved __imp_ symbol errors. The previous || ver>/dev/null was masking this failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CMD retains the errorlevel from target\smoke_test.exe (1 = usage). The GEQ 2 check correctly skips, but the script exits with 1. Add explicit exit /b 0 to succeed when errorlevel is 0 or 1. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- actions/checkout v4.3.1 → v6.0.2 - actions/upload-artifact v4.6.2 → v7.0.0 - actions/download-artifact v4.3.0 → v8.0.1 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Ensures the static library is built with the same deployment target as OpenMS/pyOpenMS, avoiding mismatch warnings at link time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmake/timsrust_cpp_bridgeConfig.cmake.in (1)
21-23: Consider usingThreads::Threadsfor portable thread library linking.Direct use of
pthreadworks on Linux but is less portable. CMake'sThreads::Threadsimported target (fromfind_package(Threads)) handles platform differences automatically. However, since this is a static library config and the consumer is expected to handle their own threading setup, the current approach is acceptable for this use case.Alternative using Threads (optional)
If you want maximum portability, you could document that consumers should use
find_package(Threads REQUIRED)and linkThreads::Threadsthemselves, or modify the config to use:if(UNIX AND NOT APPLE) find_package(Threads REQUIRED) set_property(TARGET timsrust_cpp_bridge::timsrust_cpp_bridge APPEND PROPERTY INTERFACE_LINK_LIBRARIES Threads::Threads dl m) endif()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmake/timsrust_cpp_bridgeConfig.cmake.in` around lines 21 - 23, Replace the hard-coded pthread usage with CMake's portable Threads target: inside the UNIX AND NOT APPLE block call find_package(Threads REQUIRED) and update the INTERFACE_LINK_LIBRARIES append on target timsrust_cpp_bridge::timsrust_cpp_bridge to use Threads::Threads (retain dl and m); alternatively, if you prefer to keep threading choice to consumers, add a comment documenting that consumers should call find_package(Threads) and link Threads::Threads themselves instead of relying on pthread.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 73-74: The exit-code check after running target/smoke_test is
incorrect because `$?` is evaluated too late; change the pattern to capture the
command's exit code immediately (e.g. run `target/smoke_test; rc=$?`), then test
`rc` (`if [ $rc -gt 128 ]; then exit 1; fi`), and apply the same change to the
macOS smoke test invocation (the second occurrence referenced in the comment) so
the script reliably inspects the smoke_test process exit status.
---
Nitpick comments:
In `@cmake/timsrust_cpp_bridgeConfig.cmake.in`:
- Around line 21-23: Replace the hard-coded pthread usage with CMake's portable
Threads target: inside the UNIX AND NOT APPLE block call find_package(Threads
REQUIRED) and update the INTERFACE_LINK_LIBRARIES append on target
timsrust_cpp_bridge::timsrust_cpp_bridge to use Threads::Threads (retain dl and
m); alternatively, if you prefer to keep threading choice to consumers, add a
comment documenting that consumers should call find_package(Threads) and link
Threads::Threads themselves instead of relying on pthread.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 354d4a65-3346-4242-8825-574700f98388
📒 Files selected for processing (3)
.github/workflows/release.ymlcmake/timsrust_cpp_bridgeConfig.cmake.incmake/timsrust_cpp_bridgeConfigVersion.cmake.in
singjc
approved these changes
Mar 11, 2026
Contributor
singjc
left a comment
singjc
left a comment
There was a problem hiding this comment.
Nice, thanks! This is a good start to see how easy it will be to integrate within OpenMS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
find_package(timsrust_cpp_bridge REQUIRED)What's included
CMake config templates (
cmake/):timsrust_cpp_bridgeConfig.cmake.in— imported static target with platform-specific system link deps (Linux: pthread/dl/m, macOS: Security/SystemConfiguration/resolv, Windows: ws2_32/userenv/bcrypt/ntdll/advapi32)timsrust_cpp_bridgeConfigVersion.cmake.in— SameMinorVersion compatibilityPackaging script (
scripts/package.sh):CI workflow (
.github/workflows/release.yml):cargo test+cargo build --release+ platform-specific smoke test (compile & link C++ example)v*) trigger packaging + GitHub Release with 4 platform artifactsDesign docs (
docs/superpowers/):OpenMS consumption
Point
CMAKE_PREFIX_PATHat the extracted release tarball.Release artifact layout
Test plan
v0.1.0and verify GitHub Release is created with 4 artifactsfind_package()works with a minimal CMake consumer🤖 Generated with Claude Code
Summary by CodeRabbit