Build: support including src/ #1152
Conversation
WalkthroughThe changes update include path configurations across the build system and source files. The Makefile now prepends the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Caller
participant Project as Project::init()
participant Include as includeIfExist()
Client->>Project: Call init()
Project->>Include: includeIfExist("src", isSystem)
Include-->>Project: Verify "src" exists and add include path
Project->>Include: includeIfExist("include", isSystem)
Include-->>Project: Verify "include" exists and add include path
Project-->>Client: Return constructed Project object
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (35)
🚧 Files skipped from review as they are similar to previous changes (34)
🧰 Additional context used🪛 Cppcheck (2.10-2)src/Builder/Project.cc[performance] 23-23: Function parameter 'name' should be passed by const reference. (passedByValue) [performance] 23-23: Function parameter 'value' should be passed by const reference. (passedByValue) ⏰ Context from checks skipped due to timeout of 90000ms (12)
🔇 Additional comments (4)
✨ Finishing Touches
🪧 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 (
|
![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 (3)
src/Cmd/Clean.cc (1)
27-27: Note on TODO comment.There's a TODO at line 27 suggesting the shared use of filesystem path logic across sources. Consider refactoring this functionality into a helper utility if it appears in multiple locations.
src/Builder/Project.cc (1)
18-23: Good abstraction for handling include directories.This new helper method simplifies the conditional addition of include directories and makes the code more maintainable.
For better performance, consider passing the path parameter by const reference:
-void Project::includeIfExist(fs::path path, bool isSystem) { +void Project::includeIfExist(const fs::path& path, bool isSystem) {🧰 Tools
🪛 Cppcheck (2.10-2)
[performance] 23-23: Function parameter 'name' should be passed by const reference.
(passedByValue)
[performance] 23-23: Function parameter 'value' should be passed by const reference.
(passedByValue)
src/Cmd/Fmt.cc (1)
3-10: Direct Include Paths Update
The updated include directives now reference header files directly (e.g.,"Algos.hpp") instead of using relative paths (e.g.,"../Algos.hpp"). This change aligns with the PR objective to support including thesrc/directory by assuming common include directories are set up in the build system (e.g., via the-Isrcflag). Please verify that:
- All referenced header files are accessible via the updated include paths.
- The build configuration is updated accordingly to search within the
src/directory.- Consistency is maintained across all modules that previously used relative paths.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
Makefile(1 hunks)src/Builder/Compiler.cc(1 hunks)src/Builder/Compiler.hpp(1 hunks)src/Builder/Project.cc(3 hunks)src/Builder/Project.hpp(2 hunks)src/Cmd/Add.cc(1 hunks)src/Cmd/Add.hpp(1 hunks)src/Cmd/Build.cc(1 hunks)src/Cmd/Build.hpp(1 hunks)src/Cmd/Clean.cc(1 hunks)src/Cmd/Clean.hpp(1 hunks)src/Cmd/Common.hpp(1 hunks)src/Cmd/Fmt.cc(1 hunks)src/Cmd/Fmt.hpp(1 hunks)src/Cmd/Help.cc(1 hunks)src/Cmd/Help.hpp(1 hunks)src/Cmd/Init.cc(1 hunks)src/Cmd/Init.hpp(1 hunks)src/Cmd/Lint.cc(1 hunks)src/Cmd/Lint.hpp(1 hunks)src/Cmd/New.cc(1 hunks)src/Cmd/New.hpp(1 hunks)src/Cmd/Remove.cc(1 hunks)src/Cmd/Remove.hpp(1 hunks)src/Cmd/Run.cc(1 hunks)src/Cmd/Run.hpp(1 hunks)src/Cmd/Search.cc(2 hunks)src/Cmd/Search.hpp(1 hunks)src/Cmd/Test.cc(1 hunks)src/Cmd/Test.hpp(1 hunks)src/Cmd/Tidy.cc(1 hunks)src/Cmd/Tidy.hpp(1 hunks)src/Cmd/Version.cc(1 hunks)src/Cmd/Version.hpp(1 hunks)src/Rustify/Result.hpp(1 hunks)
🧰 Additional context used
🪛 Cppcheck (2.10-2)
src/Builder/Project.cc
[performance] 23-23: Function parameter 'name' should be passed by const reference.
(passedByValue)
[performance] 23-23: Function parameter 'value' should be passed by const reference.
(passedByValue)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: build & test (GCC 12)
- GitHub Check: build & test (GCC 14)
- GitHub Check: build & test (Apple Clang - macOS 13)
- GitHub Check: build & test (GCC 13)
- GitHub Check: build & test (Clang 18)
- GitHub Check: build & test (Clang 16)
- GitHub Check: build & test (Apple Clang - macOS 15)
- GitHub Check: build & test (Clang 17)
- GitHub Check: build & test (Clang 19)
- GitHub Check: analyze (c-cpp)
- GitHub Check: create
🔇 Additional comments (37)
src/Cmd/New.hpp (1)
3-3: Direct Include Path Usage.
Changing the include from a relative path to"Cli.hpp"aligns with the build system update (i.e. the-Isrcflag) and promotes clarity in header resolution across the project.src/Cmd/Clean.hpp (1)
3-3: Consistent Header Inclusion.
The update to use a direct"Cli.hpp"reference instead of a relative path helps standardize header file lookups and supports the new build configuration.src/Cmd/Tidy.hpp (1)
3-3: Updated Include Directive.
Switching to a direct include of"Cli.hpp"is a good move as it leverages the updated-Isrcflag, fostering consistency and easier maintainability in the project's include paths.src/Cmd/Help.hpp (1)
3-3: Refined Include Reference.
Updating the include directive to"Cli.hpp"is consistent with the overall refactoring efforts. This change simplifies the header inclusion mechanism and aligns perfectly with the updated build system.src/Cmd/Lint.hpp (1)
3-3: Direct Path Inclusion Enhancement.
The alteration to use"Cli.hpp"directly reinforces the move to a standardized include path supported by the-Isrcflag, thereby improving the clarity and robustness of header resolution.src/Cmd/Search.hpp (1)
3-3: Direct include update for "Cli.hpp" is correctly applied.
The change replaces a relative include with a direct include, which aligns with the new build configuration using the-Isrcflag. This makes the header resolution more straightforward and consistent across the project.src/Cmd/Fmt.hpp (1)
3-3: Direct include update for "Cli.hpp" in Fmt.hpp is consistent.
Updating the include path to"Cli.hpp"ensures that the header locations are unambiguous and works hand-in-hand with the Makefile change.src/Cmd/Run.hpp (1)
3-3: Include directive updated to direct path for "Cli.hpp".
This modification simplifies header lookup, making use of the new-Isrcconfiguration. It is consistent with other similar updates in the project.src/Cmd/Remove.hpp (1)
3-3: Revised include path for "Cli.hpp" is correctly implemented.
Switching from a relative path to"Cli.hpp"supports a cleaner include structure, facilitated by the updated build settings.src/Cmd/Add.hpp (1)
3-3: Direct include for "Cli.hpp" in Add.hpp is updated appropriately.
This change adheres to the standardized include paths across the project and ensures correct header resolution thanks to the added-Isrcflag in the Makefile.src/Cmd/Version.cc (1)
3-7: Direct Include Update for Header FilesThe include directives for several headers (e.g., "Cli.hpp", "CurlVersion.hpp", "Diag.hpp", "Git2/Version.hpp", and "Rustify/Result.hpp") have been updated to use direct paths rather than relative ones. This change aligns with the new build configuration (e.g., using the
-Isrcflag) and the overall objective to support including headers from thesrc/directory. Please verify that the build system’s include paths are appropriately configured so these headers are located correctly.src/Cmd/Version.hpp (1)
3-3: Updated Include Directive in Version.hppThe include directive for
Cli.hppnow uses a direct include ("Cli.hpp") rather than a relative path. This change is consistent with the restructuring to improve clarity and maintainability. Ensure thatCli.hppresides in the expected location relative to the include search paths.src/Cmd/Init.hpp (1)
3-3: Consistent Include Path for Cli.hppThe change in this file updates the include directive for
Cli.hppfrom a relative path to a direct one. This is aligned with the new project structure where headers are expected to be found in thesrc/directory. Please make sure that any dependent modules are updated accordingly.src/Cmd/Test.hpp (1)
3-3: Direct Inclusion of Cli.hppThe update replaces the relative path with a direct include for
Cli.hpp, ensuring consistency with the updated include path strategy. This change simplifies header resolution under the new build configuration. Confirm thatCli.hppis accessible via the provided include paths.src/Cmd/Help.cc (1)
3-3: Revised Include Directive in Help.ccThe include for
Cli.hpphas been changed from a relative path to a direct path. This modification helps maintain a consistent include structure in line with the updated build settings (i.e., supportingsrc/includes). Ensure that this change does not adversely affect any downstream dependencies expecting the old relative path.src/Cmd/Test.cc (1)
3-12: Direct Include Paths Updated.
The include directives have been updated from relative paths to direct paths (e.g.,"Algos.hpp","BuildConfig.hpp", etc.), which aligns with the new build configuration using the-Isrcflag. This change improves clarity and consistency in header file resolution.src/Cmd/Lint.cc (1)
3-8: Updated Include Directives for Lint Command.
The changes update the header includes to direct paths (e.g.,"Algos.hpp","Cli.hpp","Command.hpp","Diag.hpp","Manifest.hpp", and"Rustify/Result.hpp"). This is consistent with the new project structure and the updated makefile configuration.src/Builder/Compiler.hpp (1)
3-5: Consistent Header Inclusion.
The include directives have been modified from relative to direct paths (e.g.,#include "Command.hpp"instead of using../). This change works in tandem with the-Isrcflag introduced in the build system, ensuring that header files are found as expected.src/Rustify/Result.hpp (1)
3-3: Refined Include for TermColor.hpp.
The include for"TermColor.hpp"has been updated from a relative path to a direct path. This update reflects the new directory structure and the enhanced include configuration.src/Cmd/Remove.cc (1)
3-6: Unified Header Reference Style.
The include directives for"Cli.hpp","Diag.hpp","Manifest.hpp", and"Rustify/Result.hpp"have been changed to direct paths. This update is consistent with the overall project migration to direct include paths supported by the-Isrcflag.src/Builder/Project.hpp (2)
5-6: Include path changes look good.The change from relative includes to direct paths is consistent with the PR objective to support including from the src/ directory.
19-19:❓ Verification inconclusive
Verify if this method should be private.
This new method is defined in the private section of the class. According to the AI summary, it's used in the
initstatic method. Make sure thatinitproperly creates a Project instance before calling this private method.
🏁 Script executed:
#!/bin/bash # Check how init uses includeIfExist grep -A 20 "Result<Project>\s*Project::init" src/Builder/Project.ccLength of output: 69
Action Required: Verify the Instance Initialization in
Project::initThe private method
includeIfExistis defined as intended in the header. However, the automated grep search for its usage inProject::initdid not return any output, leaving us uncertain about whetherProject::initcreates a valid instance before calling this method. Please manually verify or re-run the search (for example, with a simpler grep/rg pattern) insrc/Builder/Project.ccto confirm that the instance is properly constructed prior to invokingincludeIfExist.
- Confirm that
Project::initconstructs aProjectinstance before any calls toincludeIfExist.- Check if the method call appears within a context that guarantees a valid object state.
src/Cmd/Init.cc (1)
3-8: Include path updates look good.The changes simplify the include statements by replacing relative paths (
../HeaderName.hpp) with direct paths (HeaderName.hpp). This works well with the-Isrcflag added to the build system.src/Cmd/New.cc (1)
3-9: Include path changes are consistent.The changes to move from relative paths to direct includes are consistent with the rest of the PR and will work correctly with the
-Isrcinclude path added to the build system.src/Cmd/Common.hpp (1)
3-4: Include path changes are aligned with the PR objective.The updates to direct include paths are consistent with the other changes in this PR and support the goal of including headers directly from the src/ directory.
src/Cmd/Build.hpp (1)
3-6: Updated include directives to match the new project structure.The changes update header includes from relative paths to direct paths (e.g.,
"Builder/BuildProfile.hpp"instead of"../Builder/BuildProfile.hpp"). This enhances clarity and maintains consistency across the codebase, especially with the new-Isrcflag in the build configuration.src/Cmd/Clean.cc (1)
3-6: Standardized include paths in Clean.cc.The header files (
"Cli.hpp","Diag.hpp","Manifest.hpp","Rustify/Result.hpp") are now included using direct paths. This change is in line with the updated project directory restructuring and the use of the-Isrcflag.src/Cmd/Build.cc (1)
3-12: Refactored include directives in Build.cc.The updates replace relative paths with direct includes (e.g.,
"Algos.hpp","BuildConfig.hpp","Builder/BuildProfile.hpp", etc.), which aligns with the overall directory restructuring and the updated Makefile settings.src/Cmd/Run.cc (1)
3-12: Consistent update of include paths in Run.cc.All header includes (such as
"Algos.hpp","Builder/BuildProfile.hpp","Cli.hpp","Command.hpp","Diag.hpp","Manifest.hpp","Parallelism.hpp", and"Rustify/Result.hpp") now use direct paths. This change supports the new build configuration and simplifies header resolution.Makefile (1)
38-38: Added-Isrcflag to the INCLUDES variable.Including
-Isrcensures that the compiler prioritizes header files located in thesrcdirectory. This change complements the updated include directives in the source files and streamlines build configuration.src/Cmd/Search.cc (2)
3-5: Improved include paths with direct references.The change from relative paths to direct paths enhances readability and maintainability by making includes less dependent on file location.
52-52: Consistent path update for GraphQL include.Changed from relative path to direct path for consistency with other includes.
src/Cmd/Tidy.cc (1)
3-11: Standardized include paths.Updated include directives from relative paths to direct paths, which makes the code more maintainable and less dependent on file location hierarchy.
src/Builder/Compiler.cc (1)
3-5: Consistent include path standardization.Converted relative include paths to direct paths, aligning with the project-wide standardization of includes.
src/Cmd/Add.cc (1)
3-6: Standardized include paths.Updated include directives from relative paths to direct paths, consistent with changes across the codebase.
src/Builder/Project.cc (2)
3-7: Standardized include paths.Updated include directives from relative paths to direct paths, consistent with changes across the codebase.
36-39: Cleaner initialization with the new includeIfExist method.The initialization process is now more consistent and readable, with clear handling of both "src" and "include" directories.
Summary by CodeRabbit
New Features
Refactor
Chores