Skip to content

[enchantum] fix cmake config path - #53257

Merged
Billy O'Neal (BillyONeal) merged 3 commits into
microsoft:masterfrom
toge:enchantum-fix-cmake-config
Aug 7, 2026
Merged

[enchantum] fix cmake config path#53257
Billy O'Neal (BillyONeal) merged 3 commits into
microsoft:masterfrom
toge:enchantum-fix-cmake-config

Conversation

@toge

Copy link
Copy Markdown
Contributor
  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version, or no changes were necessary.
  • Any fixed CI baseline and CI feature baseline entries are removed from that file, or no entries needed to be changed.
  • All patch files in the port are applied and succeed.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Exactly one version is added in each modified versions file.

Problem

Upstream installs the CMake package files into two different locations:

  • enchantumConfig.cmake / enchantumConfigVersion.cmake<prefix>/cmake/
  • enchantumTargets.cmake<prefix>/share/enchantum/cmake/

The portfile called vcpkg_cmake_config_fixup(CONFIG_PATH "cmake"), which moves
only the config files into share/enchantum/, leaving the targets file behind in
share/enchantum/cmake/. Since enchantumConfig.cmake does
include("${CMAKE_CURRENT_LIST_DIR}/enchantumTargets.cmake"), any consumer using
find_package(enchantum CONFIG REQUIRED) fails with:

include could not find requested file:
  .../share/enchantum/enchantumTargets.cmake

Fix

Before running vcpkg_cmake_config_fixup, move the config files into
share/enchantum/cmake/ next to the targets file, then run the fixup with
CONFIG_PATH "share/${PORT}/cmake". The fixup then relocates all three files
together into share/enchantum/ and corrects the PACKAGE_PREFIX_DIR
computation.

@BillyONeal Billy O'Neal (BillyONeal) 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.

GPT 5.6 Sol says:

The diagnosis in the PR description is correct, but the proposed relocation does not produce the claimed final layout.

At ports/enchantum/portfile.cmake lines 17-23, the two config files are first moved into share/enchantum/cmake/, then vcpkg_cmake_config_fixup(CONFIG_PATH "share/${PORT}/cmake") is called. A clean local install completed successfully, but installed:

share/enchantum/enchantumConfig.cmake
share/enchantum/enchantumConfigVersion.cmake
share/enchantum/cmake/enchantumTargets.cmake

The installed config still contains:

include("${CMAKE_CURRENT_LIST_DIR}/enchantumTargets.cmake")

Therefore both Release and Debug example configurations fail with:

include could not find requested file:
  .../share/enchantum/enchantumTargets.cmake

which is plausible; should we add a test port since the lab is clearly not exercising this?

Correct the advertised language standard and ensure CI compiles a consumer through the installed enchantum CMake package and target.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@BillyONeal

Copy link
Copy Markdown
Member

Shock and horror the above review comment was wrong but I also asked it to add a test port, would you consider toge#34 ?

@BillyONeal
Billy O'Neal (BillyONeal) marked this pull request as draft August 7, 2026 01:15
[enchantum] add CMake integration regression test
@BillyONeal
Billy O'Neal (BillyONeal) marked this pull request as ready for review August 7, 2026 22:07

@BillyONeal Billy O'Neal (BillyONeal) 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.

Thanks!

@BillyONeal
Billy O'Neal (BillyONeal) enabled auto-merge (squash) August 7, 2026 22:08
@BillyONeal
Billy O'Neal (BillyONeal) merged commit c4d9956 into microsoft:master Aug 7, 2026
16 checks passed
@dg0yt

Copy link
Copy Markdown
Contributor

Why not a patch and proper upstreaming... at least opening an issue?

@BillyONeal

Copy link
Copy Markdown
Member

Why not a patch and proper upstreaming... at least opening an issue?

I thought it was specific to us due to where we required the CMake configs to go so I didn't ask to involve upstream.

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.

3 participants