build: Add outcome and quickcpplib dependency.
#39
Conversation
WalkthroughThis pull request updates the build configuration and dependency management. In the CMake configuration, the dependency inclusion order is reorganized, the path for dependency settings is updated, and a new check for the Outcome library is introduced using Changes
Sequence Diagram(s)Outcome Library Check in CMakesequenceDiagram
autonumber
participant CMake as CMake Build System
participant OutcomeLib as Outcome Library
CMake->>OutcomeLib: find_package(outcome REQUIRED)
alt Outcome Found
OutcomeLib-->>CMake: Return success (print status)
else Outcome Not Found
OutcomeLib-->>CMake: Trigger fatal error
end
Dependency Installation WorkflowsequenceDiagram
autonumber
participant User as User
participant Engine as Task Engine
participant Init as install-all-init
participant Outcome as install-outcome
participant QuickCpp as install-quickcpplib
participant Finish as install-all-finish
participant Settings as add-package-root-to-cmake-settings
User->>Engine: Trigger install-all task
Engine->>Init: Run install-all-init (create deps dir, remove old settings)
Engine->>Outcome: Run install-outcome (download Outcome, verify checksum)
Outcome->>Settings: Set package root for Outcome
Engine->>QuickCpp: Run install-quickcpplib (download, verify checksum)
QuickCpp->>Settings: Set package root for QuickCppLib
Engine->>Finish: Run install-all-finish (append include statements)
Engine-->>User: Installation completed
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Bill-hbrhbr
changed the title
outcome and quickcpplib dependency.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
CMakeLists.txt (1)
84-86: Module Path Inclusion Reordering
Moving thelist(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/CMake)statement so that it comes after the outcome dependency check is logically sound. However, adding a brief comment explaining why this reordering benefits the build (e.g. to avoid potential module conflicts) would help future maintainers.taskfiles/deps.yaml (4)
6-9: Definition of New Variables
The new variables (G_OUTCOME_LIB_NAME,G_OUTCOME_WORK_DIR,G_QUICKCPPLIB_LIB_NAME, andG_QUICKCPPLIB_WORK_DIR) are clearly defined and follow an intuitive naming convention. Suggestion: Consider pinning thequickcpplibdependency to a specific version rather than using themasterbranch to enhance reproducibility and stability in builds.
18-27: Initial Dependency Setup Task
Theinstall-all-inittask correctly creates the required dependencies directory and removes any existing CMake settings file. The inclusion of a command to set CMP0074 is a good detail. Please verify that appending to the settings file does not lead to format issues if the file is empty or missing a trailing newline.
35-47: Centralised Package Root Setting
Theadd-package-root-to-settingstask is implemented cleanly, using variables to dynamically insert the package root into the CMake settings file. The inclusion of comments about the atomicity of writes is appreciated. A slight enhancement could be to add a brief note on why this atomicity is critical (i.e. preventing race conditions during file writes).
84-100: QuickCppLib Installation Task
The task for installing QuickCppLib is well defined and resembles the structure of the other dependency tasks. Important Note: The URL uses themasterbranch (archive/master.tar.gz), which could lead to non-reproducible builds. It is recommended to pin this dependency to a stable release tag if one is available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CMakeLists.txt(2 hunks)taskfiles/deps.yaml(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: unit-tests
CMakeLists.txt
[warning] 1-1: CMake Deprecation Warning: Compatibility with CMake < 3.10 will be removed from a future version of CMake.
[warning] 24-24: CMake Deprecation Warning: Compatibility with CMake < 3.10 will be removed from a future version of CMake.
[warning] 51-51: CMake Warning (dev): Policy CMP0148 is not set: The FindPythonInterp and FindPythonLibs modules are removed.
[warning] 92-92: WARNING: git submodule update failed with code 0, trying a direct git clone.
[warning] 24-24: CMake Deprecation Warning: Compatibility with CMake < 3.10 will be removed from a future version of CMake.
[warning] 24-24: CMake Deprecation Warning: Compatibility with CMake < 3.10 will be removed from a future version of CMake.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (5)
CMakeLists.txt (1)
53-58: Dependency Check for Outcome
The addition offind_package(outcome REQUIRED)along with the clear conditional messaging is well implemented. This ensures that a fatal error is raised if the Outcome library is not available. Be sure that the outcome package is installed consistently across target environments.taskfiles/deps.yaml (4)
14-17: Splitting the Installation Workflow
Dividing theinstall-alltask intoinstall-all-initandinstall-all-runimproves modularity and clarity of the dependency installation process. This separation makes the flow easier to manage and debug.
28-34: Sequential Execution in Installation Run
Explicitly listing sub-tasks (install-Catch2,install-outcome, andinstall-quickcpplib) in theinstall-all-runtask guarantees that dependencies are installed in the correct order. Confirm that the declared dependency of Outcome on QuickCppLib (viadeps: "install-quickcpplib") is functioning as expected.
48-62: Catch2 Installation Task
Theinstall-Catch2task is structured well, integrating the remote tarball download and then setting the package root via the helper task. The commands and variables are consistent with the overall dependency management approach.
63-83: Outcome Installation with Dependent QuickCppLib
Theinstall-outcometask specifies a dependency oninstall-quickcpplib, which aligns with Outcome’s requirements. The task sets detailed parameters (including SHA256, URL, GEN_ARGS, and JOBS) correctly. Please ensure that the provided SHA256 checksum is current and that the GEN_ARGS meet your build configuration needs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
taskfiles/deps.yaml (2)
4-9: New Dependency Variables for Outcome and QuickCppLib
The addition ofG_OUTCOME_LIB_NAME,G_OUTCOME_WORK_DIR,G_QUICKCPPLIB_LIB_NAME, andG_QUICKCPPLIB_WORK_DIRis clear and consistent with the existing naming conventions. Note that the PR objectives mention the lack of versioning for quickcpplib; you might consider pinning a specific version or commit in the future to protect against compatibility issues.
84-100: QuickCppLib Installation Task & Versioning Consideration
Theinstall-quickcpplibtask is implemented cleanly using the remote tarball approach. However, note that downloading from the master branch may risk instability due to the lack of explicit versioning. It would be prudent to consider pinning to a specific release or commit hash in the future to ensure build reproducibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CMakeLists.txt(3 hunks)taskfile.yaml(1 hunks)taskfiles/deps.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-ystdlib-cpp (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (7)
taskfile.yaml (1)
14-17: Directory & Settings File Variables Updated
The new variableG_DEPS_CMAKE_SETTINGS_DIRis introduced and the definition ofG_DEPS_CMAKE_SETTINGS_FILEhas been updated accordingly. This change improves modularity by centralising the CMake settings into its own subdirectory.taskfiles/deps.yaml (6)
12-17: Splitting the Install-All Task
Dividing theinstall-alltask intoinstall-all-initandinstall-all-runincreases clarity and control over the dependency installation sequence. This modular structure aids in maintaining inter-dependencies effectively during the build process.
18-23: Initialization Task – install-all-init
Theinstall-all-inittask neatly ensures that the CMake settings directory exists and cleans up any pre-existing settings file. This is a good safeguard. Just confirm that removing the settings file does not conflict with any parallel processes.
24-30: Installation Run Task – install-all-run
Defining theinstall-all-runtask with dependencies oninstall-Catch2,install-outcome, andinstall-quickcpplibprovides a logical flow to the installation process. It correctly enforces that outcome and quickcpplib are installed in the intended order.
31-47: Task to Add Package Root Settings
The new taskadd-package-root-to-settingsis well-architected. It encapsulates the commands to write package-specific root settings into their own file, then includes that file in the overall CMake settings. The in-line comments regarding atomic writes add useful context. Ensure that your directory variable (G_DEPS_CMAKE_SETTINGS_DIR) is consistently defined across all tasks.
48-62: Catch2 Installation Task Enhancements
Theinstall-Catch2task now includes a call toadd-package-root-to-settingsto set its installation prefix. This integration is smooth and consistent with the overall dependency management approach.
63-83: Outcome Library Installation Task
The newinstall-outcometask is clearly structured. It correctly depends oninstall-quickcpplibto enforce the necessary installation order. The use of additional generator arguments referencing quickcpplib’s settings is a smart solution, though please double-check that the referenced file path is exactly as intended.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
taskfiles/deps.yaml (1)
89-105:⚠️ Potential issueQuickCppLib Installation Task and Version Pinning Concern
The
install-quickcpplibtask is implemented correctly. However, the URL points to the master branch tarball, which may result in unstable builds due to changes not being version-pinned. It is recommended to specify a version or commit hash to ensure reproducibility and stability over time.
🧹 Nitpick comments (2)
taskfiles/deps.yaml (2)
32-41: Finalisation Step: Appending CMake SettingsThe
install-all-finishtask appends the include statements for each dependency’s CMake settings file. Although the multi-line echo command works here, please verify that the formatting—for example, newline characters and any extra whitespace—does not affect CMake’s parsing of the settings file.
42-52: Configuration of Package Root via CMake SettingsThe
add-package-root-to-cmake-settingstask provides a generic approach to setting the installation prefix for each package. One suggestion is to ensure that, if newline escapes are intended in the echo command, you might consider usingecho -efor consistency with other sections; otherwise, double-check that the output format meets CMake’s expectations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
taskfiles/deps.yaml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build-ystdlib-cpp (ubuntu-24.04)
- GitHub Check: build-ystdlib-cpp (macos-15)
- GitHub Check: build-ystdlib-cpp (ubuntu-22.04)
- GitHub Check: lint-check (macos-15)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (6)
taskfiles/deps.yaml (6)
6-9: New Dependency Variables for Outcome and QuickCppLib AddedThe new variables
G_OUTCOME_LIB_NAME,G_OUTCOME_WORK_DIR,G_QUICKCPPLIB_LIB_NAME, andG_QUICKCPPLIB_WORK_DIRare introduced in a manner consistent with the existing Catch2 variables. This maintains clarity and consistency in naming conventions.
14-17: Modularisedinstall-allTaskSegmenting the
install-alltask into sub-tasks—install-all-init,install-all-run, andinstall-all-finish—improves clarity and maintainability. Please ensure that any related documentation is updated to reflect this new structure.
19-24: Initialization Task for Dependency SetupThe
install-all-inittask appropriately creates the required directory and removes the existing CMake settings file. Verify that the removal of the settings file does not inadvertently discard other important configurations.
25-31: Run Task with Dependency HandlingThe
install-all-runtask now invokes the sub-tasks for Catch2, Outcome, and QuickCppLib. This update aligns with the PR objectives. Ensure that any inter-dependencies (such as Outcome relying on QuickCppLib) are properly enforced by the defined dependencies, possibly double-checking the execution order.
53-67: Catch2 Installation Task EnhancementThe modifications to the
install-Catch2task now include an invocation ofadd-package-root-to-cmake-settings, which integrates well with the new dependency system. Ensure that theINSTALL_PREFIXis correctly computed for subsequent use during the build process.
68-88: Outcome Installation Task with QuickCppLib DependencyWithin the
install-outcometask, a dependency on QuickCppLib is clearly defined via thedepsfield, and GEN_ARGS reference QuickCppLib’s CMake settings appropriately. Please verify that this dependency ordering guarantees QuickCppLib is installed prior to Outcome.
|
32-41: Finalisation Step: Appending CMake Settings The install-all-finish task appends the include statements for each dependency’s CMake settings file. Although the multi-line echo command works here, please verify that the formatting—for example, newline characters and any extra whitespace—does not affect CMake’s parsing of the settings file. 42-52: Configuration of Package Root via CMake Settings The add-package-root-to-cmake-settings task provides a generic approach to setting the installation prefix for each package. One suggestion is to ensure that, if newline escapes are intended in the echo command, you might consider using echo -e for consistency with other sections; otherwise, double-check that the output format meets CMake’s expectations. @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@Bill-hbrhbr Thanks for the clarification! You're absolutely right - the multi-line echo in lines 32-41 doesn't affect CMake parsing, and since there are no newline escapes in the echo command in lines 42-52, the 🏁 Script executed: Length of output: 728 @Bill-hbrhbr After reviewing the code in
Your implementation looks solid as-is. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
taskfiles/deps.yaml (1)
19-24: Directory Refresh in install-all-init
The commands to remove and recreate the CMake settings directory ensure a clean slate for each dependency installation. However, verify that the variable{{.G_DEPS_CMAKE_SETTINGS_DIR}}is correctly defined so that no unintended directories are affected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
taskfiles/deps.yaml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (ubuntu-24.04)
🔇 Additional comments (8)
taskfiles/deps.yaml (8)
6-9: Addition of New Dependency Variables
The new variables for Outcome and QuickCppLib are clearly added and follow the existing naming conventions. This improves clarity when referring to these dependencies throughout the tasks. Ensure that downstream tasks consistently use these variables.
14-17: Segmentation of Installation Tasks
Dividing theinstall-alltask intoinstall-all-init,install-all-run, andinstall-all-finishenhances modularity and readability. This clear separation helps manage concurrency and dependency inter-relations.
25-30: Ensuring Proper Dependency Order in install-all-run
Including the dependenciesinstall-Catch2andinstall-outcomeis appropriate. Notably, sinceinstall-outcomedeclares a dependency oninstall-quickcpplib, the overall order should maintain correctness. It is advisable to double-check that the concurrency framework respects these inter-dependencies.
31-40: Appending Include Statements in install-all-finish
The loop appendinginclude($file)statements to the CMake settings file is an effective solution. As discussed previously, please verify that the formatting (newlines, spacing) does not interfere with CMake parsing.
41-55: Modular Package Root Configuration
Theadd-package-root-to-cmake-settingstask introduces a flexible way to add package root paths into the CMake settings. The multi-line echo command appears appropriately formatted; however, ensure that its layout meets CMake's parsing requirements.
56-70: Consistent Handling for Catch2 Installation
The Catch2 installation task leverages the shared utility to install the remote tar file and then correctly invokes the package-root addition task. This consistency supports maintainability.
71-93: Outcome Installation with Dependency Enforcement
Theinstall-outcometask correctly specifies a dependency oninstall-quickcpplibto ensure proper build order. The use ofGEN_ARGSto pass additional configuration options is clear—just confirm that referencing the QuickCppLib settings file via the-Cflag is intended and effective.
94-112:❓ Verification inconclusive
QuickCppLib Installation and Version Stability Consideration
The QuickCppLib installation follows the established pattern, including the configuration via the package root settings task. However, since the URL points to the master branch, consider whether version stability is a concern. Using a tagged release or commit hash may help prevent issues from future changes on master.
Ensure stable QuickCppLib versioning for consistent builds
The current installation task intaskfiles/deps.yaml(lines 94–112) is implemented correctly according to our established pattern. However, the URL currently points to the master branch—which risks incorporating unexpected changes over time. It is recommended to pin QuickCppLib to a specific tagged release or commit hash for improved version stability.
- Verify that the selected version meets the project's stability requirements.
- If proceeding with a stable release, update the URL and corresponding
FILE_SHA256accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CMake/ystdlib-cpp-helpers.cmake(3 hunks)CMakeLists.txt(3 hunks)taskfiles/deps.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (9)
CMake/ystdlib-cpp-helpers.cmake (4)
17-52: Well-structured interface target function for third-party librariesThis new function is a clean way to expose include paths for third-party libraries. It properly checks for required variables and follows CMake best practices.
105-110: Good defensive coding with interface library validationAdding this validation prevents a common CMake error where developers might try to specify private dependencies for interface (header-only) libraries.
117-117: Improved linking clarity for interface librariesThis explicit linking of public libraries for interface targets makes the intent clearer.
134-140: Better organization of library linkingConsolidating public and private links into a single call improves readability and maintainability.
taskfiles/deps.yaml (5)
6-9: Variables follow project conventionsThe new variables for Outcome and QuickCppLib follow the established naming conventions in the project.
14-18: Good task decompositionBreaking the installation task into initialization, execution, and finalization improves maintainability and makes the process more understandable.
32-41: Automated settings inclusion improves maintainabilityThis task cleverly automates the inclusion of dependency CMake settings files, making it easier to add new dependencies in the future without modifying this code.
42-55: DRY approach for package root settingsCreating a dedicated task for adding package roots to settings files reduces code duplication and centralizes this functionality.
88-89: Good use of CMake policiesSetting these policies is important for ensuring proper functionality with modern CMake features while maintaining compatibility with older code.
|
By passing |
|
We pass |
|
We pass Changed in version 3.27: This module is available only if policy CMP0148 is not set to NEW. This means for cmake version:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
taskfiles/deps.yaml (1)
95-113: QuickCppLib installation is pinned to a specific commit.
Theinstall-quickcpplibtask now pins QuickCppLib to a specific commit (f3e452e) using its tarball URL and associated SHA256 checksum. This change enhances reproducibility by avoiding potential drift from using the master branch. It would be beneficial to add an inline comment or documentation note indicating the rationale for pinning to this commit for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
taskfiles/deps.yaml(2 hunks)
🔇 Additional comments (8)
taskfiles/deps.yaml (8)
6-9: New dependency variables are defined properly.
The new variables for Outcome and QuickCppLib (G_OUTCOME_LIB_NAME,G_OUTCOME_WORK_DIR,G_QUICKCPPLIB_LIB_NAME, andG_QUICKCPPLIB_WORK_DIR) follow the existing naming convention used for Catch2, which helps maintain clarity and consistency in dependency management.
14-18: Clear segmentation of installation phases.
Defining theinstall-alltask to sequentially invokeinstall-all-init,install-all-run, andinstall-all-finishestablishes a well-structured build process that handles preparation, concurrent installations, and final configuration. This approach aligns well with the PR objectives.
20-25: Initialisation task setup is appropriate.
Theinstall-all-inittask effectively cleans up any pre-existing CMake settings by removing the designated directory and subsequently recreating it. This ensures that the subsequent dependency configuration begins from a clean state.
26-31: Sequential dependency execution in the run task.
In theinstall-all-runtask, the inclusion of dependencies for both Catch2 and Outcome (with Outcome itself depending on QuickCppLib) ensures that the installation order respects inter-dependencies. This sequencing is vital for a reliable build process.
32-41: Appending CMake settings is implemented correctly.
Theinstall-all-finishtask loops through the CMake settings files and appends include statements to the main settings file. This aggregation method is neat and should work as intended—just verify that the formatting remains acceptable for CMake parsing.
42-56: Reusable task for adding package roots.
Theadd-package-root-to-cmake-settingstask is well abstracted: by requiring theNAMEandINSTALL_PREFIXvariables, it provides a generic mechanism to append the necessary configuration to the CMake settings file. This promotes reusability and consistency across different dependencies.
67-70: Consistent integration for Catch2.
In theinstall-Catch2task, the subsequent call toadd-package-root-to-cmake-settingsensures that the Catch2 installation is correctly referenced in the CMake settings file. The defined paths for Catch2 are consistent and maintainable.
72-94: Outcome installation task is correctly configured.
Theinstall-outcometask uses the correct URL for Outcome v2.2.11 and provides essential build arguments (such as disabling testing, enforcing a Release build type, and setting the appropriate CMake policies). Its dependency oninstall-quickcpplibguarantees that the required QuickCppLib is installed first. Please ensure that the provided SHA256 checksum is verified against the downloaded archive.
As discussed offline, we are removing this policy since its benefits are not very obvious. |
|
When building
For loading initial cache file /home/bingranhu/fork/fork2/ystdlib-cpp/build/deps/CMake/quickcpplib.cmake
CMake Deprecation Warning at cmake/QuickCppLibBootstrap.cmake:23 (cmake_minimum_required):
The OLD behavior for policy CMP0074 will be removed from a future version
of CMake.
The cmake-policies(7) manual explains that the OLD behaviors of all
policies are deprecated and that a policy should be set to OLD only under
specific short-term circumstances. Projects should be ported to the NEW
behavior and not rely on setting a policy to OLD.
Call Stack (most recent call first):
CMakeLists.txt:38 (include)
-- quickcpplib not found, cloning git repository and installing into /home/bingranhu/fork/fork2/ystdlib-cpp/build/deps/outcome/outcome-build/quickcpplib ...For omitting the policy: loading initial cache file /home/bingranhu/fork/fork2/ystdlib-cpp/build/deps/CMake/quickcpplib.cmake
CMake Warning (dev) at cmake/QuickCppLibBootstrap.cmake:32 (find_package):
Policy CMP0074 is not set: find_package uses <PackageName>_ROOT variables.
Run "cmake --help-policy CMP0074" for policy details.
Use the cmake_policy command to set the policy and suppress this warning.
CMake variable quickcpplib_ROOT is set to:
/home/bingranhu/fork/fork2/ystdlib-cpp/build/deps/quickcpplib/quickcpplib-install
For compatibility, CMake is ignoring the variable.
Call Stack (most recent call first):
CMakeLists.txt:38 (include)
This warning is for project developers. Use -Wno-dev to suppress it.
-- quickcpplib not found, cloning git repository and installing into: /home/bingranhu/fork/fork2/ystdlib-cpp/build/deps/outcome/outcome-build/quickcpplibAccording to the policy doc: This policy was introduced in CMake version 3.12. It may be set by cmake_policy() or cmake_minimum_required(). If it is not set, CMake warns, and uses OLD behavior.
|
|
By including this policy, we ensure that |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
taskfiles/deps.yaml (4)
20-25: Initialization Task: Safe Directory Handling
In the “install-all-init” task, the commands to remove and recreate the CMake settings directory (lines 23–24) are standard. However, ensure that the variable{{.G_DEPS_CMAKE_SETTINGS_DIR}}is properly defined to avoid accidental deletion of unintended directories. Consider adding a comment or safeguard if there’s any risk of the variable being empty.
32-41: Iterative Inclusion in “install-all-finish”
The Bash loop (lines 36–40) appropriately iterates over all CMake settings files and conditionally appends them to the main settings file. For extra robustness, consider ensuring that filenames with spaces (if ever possible) are handled correctly by quoting the variable$file. Additionally, a brief inline comment could help future maintainers understand why the comparison with{{.G_DEPS_CMAKE_SETTINGS_FILE}}is required.
42-56: Generate CMake Package Root Settings
The “add-package-root-to-cmake-settings” task (lines 42–56) is a neat way to centralise the installation prefix for each dependency. Verify that the multi-line echo command produces a valid CMake snippet and consider whether multiple invocations could lead to duplicate entries in the settings file. A note or safeguard for idempotency might be beneficial.
94-111: Pin QuickCppLib to a Specific Commit for Reproducibility
The “install-quickcpplib” task (lines 94–111) now uses a fixed URL with a specific commit hash (f3e452e) for QuickCppLib. This ensures reproducible builds and aligns with the recent decision to pin to a specific version. It may be beneficial to include a comment indicating that this commit corresponds to the one used during Outcome v2.2.11’s release, so future maintainers understand the rationale behind this version choice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/.clang-format(1 hunks)taskfiles/deps.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/.clang-format
🔇 Additional comments (5)
taskfiles/deps.yaml (5)
4-9: Define New Dependency Variables Consistently
The new variables for Outcome and QuickCppLib (lines 6–9) are clearly defined and follow the existing naming convention. This improves the clarity of dependency paths.
12-19: Group “install-all” Task Subdivisions
The “install-all” task now explicitly runs with the “once” flag and chains the sub-tasks (init, run, finish). This explicit grouping clarifies the sequential execution flow for installing all dependencies.
26-31: Define Dependency Order in “install-all-run”
The “install-all-run” task (lines 26–31) correctly includes dependencies ("install-Catch2" and "install-outcome"). The structure clarifies the required order. Just verify that these dependencies execute in the intended sequential order, particularly given that “install-outcome” depends on QuickCppLib.
57-71: Install Catch2 with Proper Settings
The “install-Catch2” task (lines 57–71) is well-configured, pinning to version v3.8.0 with an appropriate SHA256 checksum. The subsequent setting of its package root also helps with CMake integration. No further changes are necessary here, provided the version remains up to date.
72-93: Outcome Installation and Dependency Resolution
The “install-outcome” task (lines 72–93) correctly lists “install-quickcpplib” as a dependency, ensuring the proper build order. The GEN_ARGS include:
- A reference to the QuickCppLib CMake settings file,
- Disabling testing,
- Setting the build type to Release, and
- Enforcing the CMake policy (
-DCMAKE_POLICY_DEFAULT_CMP0074=NEW).These settings align with the integration requirements. Verify that these GEN_ARGS reliably configure Outcome against the pinned QuickCppLib version.
Description
Summary
Adds a new dep
outcome, which requiresquickcpplib.Before
C++23std::expected<T, E>becomes fully supported on all compilers and platforms, we useoutcomefor functions that wish to return either a value type or an error code type (preferably derived fromystdlib::error_handling::ErrorCode).In CLP, the current way to use
outcomeis to clone the repo and use its single header file. However, this results in slower compilation time and execution time versus directly using a built and installedoutcomelibrary.Implementation details:
deps:install-allis split into multiple tasks:deps:install-all-init: Does all the preparation work before installing deps.deps:install-all-run: Runs deps install in parallel, but respect the inter-dependencies between different packages.deps:install-all-finish: Writes to a cmake settings fileG_DEPS_CMAKE_SETTINGS_FILEthat allowsystdlib-cppto find all its required components.Potential issues:
quickcpplibdoesn't have any versioning or tag refs, andoutcomealways uses themaster/mainbranch ofquickcpplibbecause they are from the same author. This could result in our file256 hash becoming stale if the master is updated, butTaskmay fail to recognize that the deps need to be reinstalled.quickcpplibto a version that's compatible with ouroutcomeversion to ensure build robustness.:utils:cmake:generateis unnecessarily triggered every time
deps:install-allruns due to anmd5summismatch of<packageName>-src.Since some dep packages clone their own dependencies during the CMake configure stage, this significantly increases build time.
Checklist
breaking change.
Validation performed
outcomeandquickcpplibsuccessfully found while buildingystdlib-cpp.Summary by CodeRabbit
catch2andoutcomefor library header matching.