Skip to content

Fix tidy-check when VCPKG is used#310

Open
Noorts wants to merge 1 commit intoduckdb:mainfrom
Noorts:fix-tidy-check-vcpkg
Open

Fix tidy-check when VCPKG is used#310
Noorts wants to merge 1 commit intoduckdb:mainfrom
Noorts:fix-tidy-check-vcpkg

Conversation

@Noorts
Copy link

@Noorts Noorts commented Jan 28, 2026

Problem

Hey team!

The tidy-check job's extension build can fail because the required dependencies are not retrieved by VCPKG. This problem is described by #223.

Changes

I've copied the Setup vcpkg step from the _extension_distribution.yml workflow into the _extension_code_quality.yml workflow. Thus, VCPKG is now installed prior to the extension build.

Furthermore, I've updated the tidy-check Makefile target to include VCPKG in the build process. For that I've taken inspiration from the Makefile's debug target.

Result

Using these changes my tidy-check now successfully builds the extension (including its dependencies).

Before: https://github.com/Noorts/PDXearch/actions/runs/21448678175/job/61771370390

# Before
CMake Warning at CMakeLists.txt:1341 (message):
  Extension 'pdxearch' has a vcpkg.json, but build was not run with VCPKG.
  If build fails, check out VCPKG build instructions in
  'duckdb/extension/README.md' or try manually installing the dependencies in
  /home/runner/work/PDXearch/PDXearch/vcpkg.json


-- Found OpenMP_C: -fopenmp (found version "4.5")
-- Found OpenMP_CXX: -fopenmp (found version "4.5")
-- Found OpenMP: TRUE (found version "4.5")
CMake Error at /home/runner/work/PDXearch/PDXearch/CMakeLists.txt:23 (find_package):
  By not providing "FindEigen3.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "Eigen3", but
  CMake did not find one.

After: https://github.com/Noorts/PDXearch/actions/runs/21448728200/job/61771541738.

Remaining Concerns

I've included USE_MERGED_VCPKG_MANIFEST, but I'm not familiar with the feature and haven't tested it. It would be nice to confirm that the approach is correct, or to change it as necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant