Skip to content

refactor(cmake): Rename CMake dependency variables to align with the latest dependency directory layout.#1353

Merged
Bill-hbrhbr merged 4 commits into
y-scope:mainfrom
Bill-hbrhbr:rename-deps-core-var
Oct 1, 2025
Merged

refactor(cmake): Rename CMake dependency variables to align with the latest dependency directory layout.#1353
Bill-hbrhbr merged 4 commits into
y-scope:mainfrom
Bill-hbrhbr:rename-deps-core-var

Conversation

@Bill-hbrhbr

@Bill-hbrhbr Bill-hbrhbr commented Sep 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Following:

This PR renames the remaining CMake variables that referenced the old deps directory layout so they accurately reflect the new build/deps/cpp/cmake-settings/all-core.cmake structure.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

  • Variable renaming only.

Summary by CodeRabbit

  • New Features

    • No user-facing changes in this release.

  • Refactor

    • Standardized internal build configuration variable names.
    • Aligned dependency search paths to use a unified convention.

  • Chores

    • Updated build scripts to improve consistency in dependency resolution.
    • Maintained policy handling with no impact on external behaviour.

@Bill-hbrhbr Bill-hbrhbr requested a review from a team as a code owner September 29, 2025 16:39
@coderabbitai

coderabbitai Bot commented Sep 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

Renames CMake variables from CLP_CORE_DEPS_DIR to CLP_DEPS_CPP_DIR and updates related include and conditional references in core build scripts and the FindLibLZMA module. Adjusts PKG_CONFIG_PATH construction accordingly. No changes to exported/public entities or external behaviour.

Changes

Cohort / File(s) Summary
CMake vars/path updates (core build)
components/core/CMakeLists.txt		 Renamed CLP_CORE_DEPS_DIR → CLP_DEPS_CPP_DIR; updated include path; renamed RESULT_VARIABLE to CLP_DEPS_CPP_CORE_SETTINGS_FILE_PATH; updated conditional checks accordingly; CMP0144 handling continues using updated path var.
CMake find-module env path update
components/core/cmake/Modules/FindLibLZMA.cmake		 Replaced CLP_CORE_DEPS_DIR with CLP_DEPS_CPP_DIR in PKG_CONFIG_PATH setup and restoration to influence pkg-config resolution during find.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly summarises the main change of renaming CMake dependency variables to match the updated directory layout, uses a conventional commit prefix, and is concise and specific enough for readers to understand the primary intent without extraneous detail.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Bill-hbrhbr Bill-hbrhbr changed the title chore(cmake): Rename variables related to core cpp dependencies to reflect their attributes more appropriately. chore(cmake): Rename cpp core dependency variables to match new directory structure. Sep 29, 2025
LinZhihao-723
LinZhihao-723 requested changes Sep 29, 2025
View reviewed changes

@LinZhihao-723 LinZhihao-723 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other all lgtm. The only question is: shouldn't CLP_CPP_DEPS_DIR make more sense? I understand CLP_DEPS_CORE_DIR would match the hierarchical structure of the actual directory, but it reads a bit weird though.

@Bill-hbrhbr

Bill-hbrhbr commented Sep 30, 2025

Copy link
Copy Markdown
Contributor Author

Other all lgtm. The only question is: shouldn't CLP_CPP_DEPS_DIR make more sense? I understand CLP_DEPS_CORE_DIR would match the hierarchical structure of the actual directory, but it reads a bit weird though.

I think I want to match how it's done in taskfiles, e.g.

clp/taskfiles/deps/main.yaml

Line 10 in 62bea59

G_DEPS_CPP_DIR: "{{.G_DEPS_DIR}}/cpp"

Before your PR I always wanted to change the original CLP_CORE_DEPS_DIR to CLP_DEPS_CORE_DIR but found that a bit unnecessary. Now that we've renamed the deps structure, I think it's worthwhile again.

In short, want to align CMake variable naming with Task.

LinZhihao-723
LinZhihao-723 approved these changes Sep 30, 2025
View reviewed changes

@LinZhihao-723 LinZhihao-723 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. For the PR title, how about:

refactor(cmake): Rename CMake dependency variables to align with the latest dependency directory layout.

@Bill-hbrhbr Bill-hbrhbr changed the title chore(cmake): Rename cpp core dependency variables to match new directory structure. chore(cmake): Rename CMake dependency variables to align with the latest dependency directory layout. Oct 1, 2025
@Bill-hbrhbr Bill-hbrhbr changed the title chore(cmake): Rename CMake dependency variables to align with the latest dependency directory layout. refactor(cmake): Rename CMake dependency variables to align with the latest dependency directory layout. Oct 1, 2025
@Bill-hbrhbr Bill-hbrhbr merged commit d25f16b into y-scope:main Oct 1, 2025
31 of 32 checks passed
@Bill-hbrhbr Bill-hbrhbr deleted the rename-deps-core-var branch October 1, 2025 07:48
junhaoliao pushed a commit to junhaoliao/clp that referenced this pull request May 17, 2026
@Bill-hbrhbr
refactor(cmake): Rename CMake dependency variables to align with the …
3c7bac9 
…latest dependency directory layout. (y-scope#1353)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sitaowang1998 sitaowang1998 sitaowang1998 approved these changes

@LinZhihao-723 LinZhihao-723 LinZhihao-723 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Bill-hbrhbr @sitaowang1998 @LinZhihao-723