-
Notifications
You must be signed in to change notification settings - Fork 4
Configure CLI tool for static runtime linking on all platforms #42
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.
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
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request renames the CMake CLI build option from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
@copilot 在 fedora 的工作流里面还有报错, 错误内容: |
... Fixed in a21bb94. The CLI now checks if static libstdc++ is available before attempting to link statically. On Fedora (and other systems without libstdc++-static installed), it will automatically fall back to dynamic linking instead of failing the build. The build will now succeed on Fedora without requiring any additional packages, though static linking can still be enabled by installing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR configures the CLI tool for static runtime linking across all platforms to create standalone executables with minimal dependencies. The implementation uses platform-specific approaches: MSVC /MT flags on Windows, conditional static linking of libstdc++/libgcc on Linux with graceful fallback, and leverages macOS's default static C++ library linking.
Key changes:
- Created reusable CMake module for MSVC static runtime configuration with support for both modern (3.15+) and legacy CMake versions
- Added conditional Linux static linking with availability checks to avoid build failures on systems without static libraries
- Moved BUILD_CCAP_CLI option definition earlier in CMakeLists.txt for proper evaluation order
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cmake/StaticRuntime.cmake | New CMake module providing ccap_configure_static_msvc_runtime() function for MSVC static runtime linking |
| CMakeLists.txt | Moved BUILD_CCAP_CLI option to top-level and added static runtime configuration for ccap library when building CLI with static library |
| cli/ccap-cli.cmake | Added platform-specific static linking configuration for CLI executable (MSVC runtime, Linux libstdc++/libgcc with fallback) |
| docs/content/cli.md | Added "Distribution and Static Linking" section documenting platform-specific behavior and dependency verification commands |
| docs/content/cli.zh.md | Chinese translation of the distribution and static linking documentation |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.vscode/tasks.json (1)
209-236: Duplicate task definition detected.There are two identical "Reload Project" tasks defined at lines 209-222 and 223-236. This appears to be a copy-paste error that should be removed.
🔎 Proposed fix
Remove the duplicate task block (lines 223-236):
{ "label": "Reload Project", "type": "shell", "command": "echo", "args": [ "Done!", ], "group": "build", "dependsOn": [ "Clean Project", "Load Project" ], "dependsOrder": "sequence" }, - { - "label": "Reload Project", - "type": "shell", - "command": "echo", - "args": [ - "Done!", - ], - "group": "build", - "dependsOn": [ - "Clean Project", - "Load Project" - ], - "dependsOrder": "sequence" - },
🧹 Nitpick comments (12)
examples/desktop/glfw/deps/mingw/_mingw_dxhelper.h (1)
1-117: Vendored mingw-w64 runtime header looks correct.This is a well-known, public domain header from the mingw-w64 runtime package providing cross-compiler compatibility macros for anonymous unions/structs and Wine interoperability. The macro guards are properly structured, and the mutual NONAMELESSUNION/NONAMELESSSTRUCT definitions (lines 10-17) ensure consistent behavior.
Consider tracking the source version (e.g., in a
READMEorNOTICEfile withindeps/mingw/) to simplify future updates if the upstream mingw-w64 runtime evolves.examples/desktop/glfw/deps/nuklear_glfw_gl2.h (1)
1-5: Vendored third-party code: verify against upstream.This appears to be a standard Nuklear GLFW/GL2 backend from the Nuklear demo examples. Consider verifying this matches the upstream version and documenting the source/version in a comment or README.
examples/desktop/glfw/deps/tinycthread.c (1)
131-131: Minor: Double semicolon.Harmless, but there's a double semicolon at the end of this line.
- return pthread_mutex_unlock(mtx) == 0 ? thrd_success : thrd_error;; + return pthread_mutex_unlock(mtx) == 0 ? thrd_success : thrd_error;examples/desktop/glfw/deps/tinycthread.h (2)
112-116: Minor: Typo in internal struct name.
_ttherad_timespecshould likely be_tthread_timespec(missing 't'). This is cosmetic since the struct is only accessed via thetimespecmacro.
442-442: Minor: Header guard comment mismatch.The closing comment refers to
_TINYTHREAD_H_but the actual guard defined at line 24 is_TINYCTHREAD_H_(with 'C').-#endif /* _TINYTHREAD_H_ */ +#endif /* _TINYCTHREAD_H_ */examples/desktop/glfw/deps/linmath.h (1)
4-6: Duplicate#include <string.h>.Lines 4 and 6 both include
<string.h>. While harmless due to include guards, one can be removed for cleanliness.Suggested fix
#include <string.h> #include <math.h> -#include <string.h>docs/content/cli.md (1)
70-75: Good documentation of static runtime linking behavior.The section accurately describes:
- Windows: Static MSVC runtime (
/MT) to eliminate VCRUNTIME DLL dependency- Linux: Conditional static linking with fallback to dynamic linking
However, consider adding verification commands to help users confirm their builds are properly statically linked.
🔎 Suggested addition: Verification commands
Add after line 75:
+ +**Verifying Static Linking:** + +On Windows, check for runtime dependencies: +```bash +dumpbin /dependents ccap.exe +# Should not list VCRUNTIME140.dll or similar +``` + +On Linux, check dynamic library dependencies: +```bash +ldd ccap +# Should show only system libraries (libc, libm, etc.) +# Should NOT show libstdc++.so or libgcc_s.so if fully static +```CMakeLists.txt (1)
24-29: Good validation for standalone build configuration.The warning appropriately alerts users when
CCAP_BUILD_CLI_STANDALONEis combined withCCAP_BUILD_SHARED=ON, since this combination may not produce a truly standalone binary.Consider making this a fatal error instead of a warning, since standalone builds fundamentally require static linking.
🔎 Consider making this a fatal error
if (CCAP_BUILD_CLI_STANDALONE AND CCAP_BUILD_SHARED) - message(WARNING "ccap: CCAP_BUILD_CLI_STANDALONE requires static library build. " - "CCAP_BUILD_SHARED=ON may result in runtime dependencies. " - "Consider setting -DCCAP_BUILD_SHARED=OFF for truly standalone CLI.") + message(FATAL_ERROR "ccap: CCAP_BUILD_CLI_STANDALONE requires static library build. " + "Please set -DCCAP_BUILD_SHARED=OFF for standalone CLI.") endif ().github/workflows/release.yml (2)
912-931: Consider adding explicit permissions to thebuild-cli-musljob.The static analysis tool (CodeQL) correctly identifies that this job lacks an explicit permissions block. While it inherits default permissions, explicitly setting minimal permissions improves security posture.
🔎 Proposed fix
build-cli-musl: name: Build CLI Tool (musl/static) needs: test + permissions: + contents: read strategy: fail-fast: false matrix:
962-965: Consider pinning the Alpine image to a specific version.Using
alpine:3will auto-update to any 3.x release. For reproducible builds, consider pinning to a specific version likealpine:3.20or using a digest.cli/ccap_cli.cpp (1)
554-594: Consider extracting BGR to RGB conversion to reduce duplication.The BGR to RGB conversion logic is duplicated for both JPG and PNG code paths. This could be refactored into a helper function.
examples/desktop/glfw/deps/wayland/xdg-shell.xml (1)
1-30: Vendored Wayland protocol file looks correct.This is the standard xdg-shell protocol XML (version 6) from the wayland-protocols project, bundled as a GLFW dependency for Wayland support. The copyright headers, MIT license, and protocol definitions are consistent with the official upstream specification.
Consider adding a brief
READMEor comment in thedeps/wayland/directory noting the source (wayland-protocols) and version for future maintenance/updates.
Add comprehensive container-based validation for both glibc and musl builds: - glibc version: verify CLI runs in debian:stable-slim (minimal glibc environment) without development tools, X11, or GPU libraries - musl version: verify CLI runs in BusyBox, Alpine, and Debian for true portability across different Linux distributions This ensures: 1. glibc builds don't have unexpected dynamic dependencies (no libX11, libGLFW, etc.) 2. musl builds are truly portable and run anywhere 3. No development tools or graphics libraries are accidentally linked The verification runs CLI basic commands (--help, --version, --list-devices) and fails the build immediately if any dependencies are missing, preventing release of broken binaries. Verification steps: - ARM64 glibc: skipped (would require QEMU, tested in musl job) - x86_64 glibc: tested in debian:stable-slim - x86_64 musl: tested in busybox, alpine, and debian - ARM64 musl: tested in busybox, alpine, and debian (multi-arch support) Related to: PR #42 (Configure CLI tool for static runtime linking on all platforms)
- Add helper functions to eliminate code duplication: * print_step() / print_section() - Consistent formatting * extract_header_version() / extract_version_from_file() - Version extraction * check_tag_exists_locally() / check_tag_exists_remotely() - Tag validation * validate_clean_working_directory() - Check uncommitted changes * validate_branch_synced_with_remote() - Check remote sync * validate_official_release_requirements() - Core validation function - Replace ~130 lines of duplicated validation code with single function call - Improve maintainability: validation logic now centralized in one place - Enhance readability: main flow is clearer with details in functions Related to PR #42 quality assurance work. Follows commit 4325d51 (release.yml container verification).
6f82ca8 to
c112d39
Compare
Implement comprehensive static runtime linking configuration for the CLI tool to eliminate external runtime dependencies and improve portability across Windows, Linux, and macOS platforms. Key changes: - Create StaticRuntime.cmake module for reusable MSVC /MT configuration - Add CCAP_BUILD_CLI option (renamed from BUILD_CCAP_CLI) for better naming consistency - Implement conditional static linking for Linux (libstdc++, libgcc) - Add library availability checks before applying static linking flags - Move CLI build option to top of CMakeLists.txt for correct evaluation order - Update documentation with static linking configuration details - Use CheckLibraryExists for robust static library detection Technical details: - MSVC: Configure /MT (MultiThreaded) runtime for static CRT linking - Linux: Statically link libstdc++ and libgcc when available - Proper PARENT_SCOPE handling in CMake functions for flag propagation - Graceful fallback when static libraries are not available BREAKING CHANGE: BUILD_CCAP_CLI option renamed to CCAP_BUILD_CLI Co-authored-by: LeeGoDamn <243561453+LeeGoDamn@users.noreply.github.com>
09ac0de to
6b2fd1f
Compare
…ulti-platform builds This is a comprehensive update that enhances the CLI tool with advanced features, complete GLFW 3.4 integration, multi-platform CI/CD pipelines, and support for both glibc and musl C library variants. Version: - Bump version to 1.4.1 CLI Features: - Add CCAP_BUILD_CLI_STANDALONE option for independent CLI builds - Integrate full GLFW 3.4 source for cross-platform preview functionality - Add automatic GLFW dependency detection on Linux with Wayland/X11 support - Add JPG/PNG image format support via stb_image_write with quality control - Add comprehensive build information display in --version output - Add runtime linking information (static/dynamic) detection and display - Add libc type detection (glibc/musl) in version output - Improve help and version output formatting - Enhance error handling and warning messages Build Optimizations: - Enable LTO (Link Time Optimization) for Release builds - Add automatic symbol stripping in Release builds - Fix MSVC static runtime configuration for CMake < 3.15 - Improve Linux static linking strategy with optional fallback CI/CD Enhancements: - Add complete Wayland and X11 development dependencies for GLFW builds - Implement glibc and musl build variants for Linux - Add ARM64 cross-compilation support with QEMU verification - Add clean container-based verification for Linux CLI builds - Pin Ubuntu versions for package compatibility (ubuntu-20.04 for glibc builds) - Use ubuntu-latest with Docker containers for better isolation - Add Alpine container tests for musl builds with QEMU support - Add comprehensive CLI tool verification in release workflow - Add pkg-config to Docker builds for better dependency detection - Improve static binary verification logic across all variants Platform Support: - Windows: Full GLFW support with static linking - Linux (glibc): Tested on Ubuntu 20.04, 24.04 with Wayland/X11 - Linux (musl): Tested on Alpine with full static linking - ARM64: Cross-compilation support for both glibc and musl Development Experience: - Add dev.cmake.example updates with new CLI options - Update VS Code tasks for CLI tool builds - Improve copilot-instructions with dev.cmake usage guidance - Add CLI build verification scripts - Extract reusable functions in release.sh for better maintainability - Remove unnecessary GLFW files (examples, tests, docs) to reduce size Documentation: - Remove redundant GLFW_STATIC_LINKING_LIMITATION.md (content covered in cli.md) - Add comprehensive static linking documentation - Update CMAKE_OPTIONS.md with new build options Dependencies: - GLFW 3.4: Full source integration for preview functionality - stb_image_write: For JPG/PNG output support - Wayland/X11: Dynamic detection and linking on Linux - pkg-config: For dependency discovery in Docker builds
e6b8631 to
928afca
Compare
The CLI tool required runtime library installation on Windows (VCRUNTIME) and had C++ standard library dependencies on Linux, complicating distribution to end users.
Changes
Windows (MSVC)
/MTflag for static MSVC runtime linking viaCMAKE_MSVC_RUNTIME_LIBRARY(CMake 3.15+) or flag replacement (older versions)ccaplibrary and CLI executable when building CLI with static libraryLinux
check_cxx_source_compilesmacOS
Implementation
cmake/StaticRuntime.cmakemodule withccap_configure_static_msvc_runtime()functionBUILD_CCAP_CLIoption definition to top-level options for correct evaluation orderBUILD_CCAP_CLI=ONandCCAP_BUILD_SHARED=OFFto preserve library flexibilityDocumentation
libstdc++-staticResult
CLI tool distributes as standalone executable with minimal dependencies where static linking is available. On systems without static libraries (like Fedora), the build succeeds with dynamic linking instead of failing.
# Linux with static linking - only system libraries $ ldd ccap linux-vdso.so.1 libm.so.6 libc.so.6 /lib64/ld-linux-x86-64.so.2Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.