feat: Add CMake FetchContent support to updater #104
Merged
Conversation
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
Implements support for updating CMake FetchContent_Declare() statements in addition to existing submodules, properties files, and scripts. Key features: - Support for path.cmake#DepName and auto-detection syntax - Hash vs tag detection with hash format preservation - Hash-to-tag resolution for version comparison - GitHub Actions output integration - Comprehensive test coverage (23 tests) Resolves: #91 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
Critical fixes and improvements: - Fix GitHub Actions workflow validation to allow # character in paths - Update documentation with CMake examples and usage - Improve comment handling in hash updates - Implement proper ancestry validation for hash updates - Test with real console SDK CMake files 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
- sentry-native.cmake now uses auto-detection (single dependency) - dependencies.cmake now shows explicit dependency name syntax - Better reflects real-world usage patterns
- Split long bullet point into structured sublist - Clear separation of different path format types - Better readability for CMake file options
- Replace $env:TEMP with [System.IO.Path]::GetTempPath() - Use [System.Guid]::NewGuid() for unique directory names - More robust cross-platform compatibility
- Return false instead of true when ancestry validation fails - Change warning to error message for clarity - Prevents potentially incorrect updates when validation is uncertain - Follows fail-safe principle for security-critical operations
- Always replace entire line content after GIT_TAG - Removes potentially outdated version-specific comments - Simplifies regex pattern (no separate hash/tag logic needed) - Cleaner and more predictable behavior
- Check $LASTEXITCODE after git ls-remote calls - Prevent parsing error messages as commit hashes - Fixes potential corruption of CMake files with 'fatal:' etc. - Applies to both Update-CMakeFile and Find-TagForHash functions Fixes critical bug where network failures could corrupt dependency files.
- Tests updating from one git hash to a newer tag's hash - Covers important scenario of hash-to-hash updates - Verifies hash format preservation and comment replacement - Ensures old hash and comments are properly removed
- Move CMake test data from external files to inline here-strings - Group related test scenarios into single test cases for better readability - Reduce test count from 16 to 6 while maintaining same coverage - Remove external testdata/cmake/ directory (no longer needed) - Improve test maintainability - all test input/output visible in one place Test groupings: - Parse scenarios: basic, auto-detect, hash, complex formatting - Multiple deps: auto-detection errors, explicit selection - Error scenarios: missing deps, missing repo/tag - Hash resolution: null results, network failures - Update scenarios: tag-to-tag, hash-to-hash, complex formatting - Update errors: missing dependency updates
- Move test data creation to Context BeforeAll level - Restore individual test cases (16 total) for focused testing - Eliminate data duplication while keeping inline visibility - Best of both worlds: shared setup + granular test cases Structure: - Context BeforeAll: Creates shared test files with inline data - Individual It blocks: Reference shared files for specific scenarios - Clear test names and focused assertions per test case
- Promote function names to Describe level (Parse-CMakeFetchContent, Find-TagForHash, Update-CMakeFile)
- Group tests by CMake file type at Context level
- Each Context has its own test data (no duplication)
- Clear logical organization: function -> file type -> specific tests
Structure:
├── Describe 'Parse-CMakeFetchContent'
│ ├── Context 'Basic single dependency file' (3 tests)
│ ├── Context 'Hash-based dependency file' (1 test)
│ ├── Context 'Complex formatting file' (1 test)
│ ├── Context 'Multiple dependencies file' (2 tests)
│ └── Context 'Malformed files' (2 tests)
├── Describe 'Find-TagForHash'
│ └── Context 'Hash resolution scenarios' (2 tests)
└── Describe 'Update-CMakeFile'
├── Context 'Basic tag updates' (3 tests)
├── Context 'Hash updates' (1 test)
└── Context 'Complex formatting' (1 test)
- Replace generic pattern [a-f0-9]{40} with actual 0.11.0 hash
- More precise assertion: 3bd091313ae97be90be62696a2babe591a988eb8
- Consistent with integration test data expectations
- Eliminates ambiguity in test validation
- Replace generic pattern [a-f0-9]{40} # \d+\.\d+\.\d+ with exact values
- More precise assertion: 3bd091313ae97be90be62696a2babe591a988eb8 # 0\.11\.0
- Matches unit test precision and validates exact expected output
- Eliminates ambiguity in hash-to-tag update validation
- Replace generic \d+\.\d+\.\d+ patterns with exact 0\.11\.0 - More precise assertions for explicit dependency and auto-detection tests - Completes migration from generic patterns to exact expected values - Ensures deterministic test validation across all CMake tests
…straints
- Revert exact version assertions where UpdateDependency gets latest version
- Keep generic patterns \d+\.\d+\.\d+ and [a-f0-9]{40} for future-proof tests
- Integration tests call UpdateDependency without pattern constraints
- Latest version will change over time (0.11.0 → 0.12.0, etc.)
- Unit tests can keep exact values since they specify exact versions
- Document new CMake FetchContent functionality in CHANGELOG.md - References PR #104 for automated dependency updates - Follows existing changelog format and conventions
|
@sentry review |
Added robust parameter validation with type constraints to all CMake helper functions: - Parse-CMakeFetchContent: Validates file path exists and dependency name format - Find-TagForHash: Validates repository URL and 40-char hash format - Test-HashAncestry: Validates repository URL and hash formats - Update-CMakeFile: Validates file path, dependency name, and new value This prevents misuse, improves error handling, and addresses security concerns around parameter injection attacks. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Added validation to ensure CMake dependency names follow proper naming conventions and prevent potential regex injection attacks. Dependency names must start with a letter and contain only alphanumeric characters, underscores, dots, or hyphens. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Implements support for updating CMake
FetchContent_Declare()statements in the updater system, addressing issue #91.This allows automatic dependency updates for console SDK repositories that use CMake FetchContent for dependency management.
Key Features
path/to/file.cmake#DepNameand auto-detection for single dependenciesabc123... # v1.2.3)Implementation Details
New Files
updater/scripts/cmake-functions.ps1- CMake helper functions (extracted for testability)updater/tests/update-dependency-cmake.Tests.ps1- Unit tests for CMake functionsupdater/tests/testdata/cmake/- Test data files covering various CMake patternsModified Files
updater/scripts/update-dependency.ps1- Main integration pointupdater/tests/update-dependency.Tests.ps1- Integration testsUsage Examples
Test Coverage
Testing
Backward Compatibility
This change is fully backward compatible. All existing functionality for submodules, properties files, and scripts remains unchanged.
Related
Resolves #91
🤖 Generated with Claude Code