Skip to content

Fix build errors when WITH_CARDINAL=true#1624

Merged
sre-ci-robot merged 1 commit into
mainfrom
with_cardinal
May 15, 2026
Merged

Fix build errors when WITH_CARDINAL=true#1624
sre-ci-robot merged 1 commit into
mainfrom
with_cardinal

Conversation

@yhmo

@yhmo yhmo commented May 9, 2026

Copy link
Copy Markdown
Collaborator

issue: #1580

We are currently using the recipe branch of the Cardinal repository because the pull request has not yet been merged into the Cardinal repository: https://github.com/zilliztech/cardinal/pull/834. The Cardinal repository fetches Knowhere to execute the CI workflow, creating a coupling between the two repositories. To decouple them:

  1. Use the Cardinal recipe branch in Knowhere to pass the CI check.
  2. In Cardinal's PR #834, use the Knowhere branch to run CI.
  3. Merge the Knowhere PR.
  4. In Cardinal's PR #834, switch to using the Knowhere master branch to run CI, and merge the Cardinal PR once CI has passed.

Expose a variable CARDINAL_VERSION_FORCE_CHECKOUT, so local/CI builds can force thirdparty/cardinalv2 to the configured branch/tag.
After this fix, both "make" command and "make WITH_CARDINAL=true" can succeed.

@mergify

mergify Bot commented May 9, 2026

Copy link
Copy Markdown

@yhmo 🔍 Important: PR Classification Needed!

For efficient project management and a seamless review process, it's essential to classify your PR correctly. Here's how:

  1. If you're fixing a bug, label it as kind/bug.
  2. For small tweaks (less than 20 lines without altering any functionality), please use kind/improvement.
  3. Significant changes that don't modify existing functionalities should be tagged as kind/enhancement.
  4. Adjusting APIs or changing functionality? Go with kind/feature.

For any PR outside the kind/improvement category, ensure you link to the associated issue using the format: “issue: #”.

Thanks for your efforts and contribution to the community!.

Comment thread cmake/libs/cardinal/v2/CMakeLists.txt
@mergify mergify Bot added the ci-passed label May 9, 2026
@yhmo yhmo force-pushed the with_cardinal branch from b7e89ec to f1bf531 Compare May 11, 2026 06:37
@mergify mergify Bot added ci-passed and removed ci-passed labels May 11, 2026
@yhmo yhmo force-pushed the with_cardinal branch from f1bf531 to ccc2a5d Compare May 11, 2026 11:12
@mergify mergify Bot removed the ci-passed label May 11, 2026
@alexanderguzhva

Copy link
Copy Markdown
Collaborator

/hold
@yhmo why is this change needed here at all? Shouldn't the change be introduced in cardinal v1 instead?

@mergify mergify Bot added the ci-passed label May 11, 2026
@yhmo yhmo changed the title Fix build errors when WIRH_CARDINAL=true Fix build errors when WITH_CARDINAL=true May 13, 2026
@mergify mergify Bot added ci-passed and removed ci-passed labels May 13, 2026
@yhmo yhmo force-pushed the with_cardinal branch from ccc2a5d to a9d08b4 Compare May 13, 2026 11:26
@mergify mergify Bot removed the ci-passed label May 13, 2026
@yhmo yhmo force-pushed the with_cardinal branch 2 times, most recently from e18b26e to c83c248 Compare May 14, 2026 04:12
@mergify mergify Bot added the ci-passed label May 14, 2026
@yhmo yhmo force-pushed the with_cardinal branch from c83c248 to ae50f1b Compare May 14, 2026 10:26
@mergify mergify Bot removed the ci-passed label May 14, 2026
Comment thread cmake/libs/cardinal/v1/CMakeLists.txt Outdated
Comment thread cmake/libs/cardinal/v2/CMakeLists.txt Outdated
Comment thread cmake/libs/cardinal/v1/CMakeLists.txt
Comment thread CMakeLists.txt Outdated
@mergify

mergify Bot commented May 15, 2026

Copy link
Copy Markdown

@yhmo e2e jenkins job failed, comment /run-e2e can trigger the job again.

@yhmo yhmo force-pushed the with_cardinal branch 2 times, most recently from 586e142 to fef4147 Compare May 15, 2026 04:15
Comment thread cmake/libs/cardinal/v1/CMakeLists.txt
Comment thread conanfile.py
Comment thread conanfile.py
Signed-off-by: yhmo <yihua.mo@zilliz.com>

@foxspy foxspy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

/lgtm
Going to merge first, then revert the nits later.

@foxspy foxspy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

/lgtm
Going to merge first, then revert the nits later.

@sre-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: foxspy, yhmo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@foxspy

foxspy commented May 15, 2026

Copy link
Copy Markdown
Collaborator

/kind improvement

@sre-ci-robot sre-ci-robot merged commit ce87b59 into main May 15, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants