Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vcpkg-cmake-config] Adapt get_filename_component package prefix for CMake 3.29.1 #38017

Conversation

WangWeiLin-MV
Copy link
Contributor

@WangWeiLin-MV WangWeiLin-MV commented Apr 7, 2024

Since CMake version 3.29.2 reverted these changes in Kitware - CMake - 9420, closed this issue.

See Kitware - CMake - 25873 and Kitware - CMake - 25827


Fix #37968.

Function vcpkg_cmake_config_fixup in vcpkg-cmake-config cannot adjust the prefix for CMake version 3.29.1 because the module CMakePackageConfigHelpers altered package config PACKAGE_PREFIX_DIR to "package name and a global counter" at Kitware - CMake - !9390.

Add replacement for new formats to fix it.

Checklist

  • 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.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

Test

Port tinyobjloader usage tests pass for CMake version 3.29.0 and 3.29.1

  • The config file in lib/tinyobjloader/cmake was moved to share/tinyobjloader.
  • The get_filename_component prefix was modified to the correct deepness.
    • For 3.29.0, get_filename_component(PACKAGE_PREFIX_DIR "${CMAKE_CURRENT_LIST_DIR}/../../" ABSOLUTE)
    • For 3.29.1, get_filename_component(PACKAGE_${CMAKE_FIND_PACKAGE_NAME}_COUNTER_1 "${CMAKE_CURRENT_LIST_DIR}/../../" ABSOLUTE)

@WangWeiLin-MV WangWeiLin-MV added the category:port-bug The issue is with a library, which is something the port should already support label Apr 7, 2024
@WangWeiLin-MV WangWeiLin-MV added the info:internal This PR or Issue was filed by the vcpkg team. label Apr 7, 2024
@WangWeiLin-MV WangWeiLin-MV marked this pull request as ready for review April 9, 2024 01:26
@Neumann-A
Copy link
Contributor

The script version also needs this otherwise old versions will be broken with newer cmake

KiterLuc pushed a commit to TileDB-Inc/TileDB that referenced this pull request Apr 9, 2024
CMake 3.29.1 had a change that broke vcpkg's `vcpkg_cmake_config_fixup`
command. This PR updates the port containing the command to adapt to the
new behavior. The change will be upstreamed with microsoft/vcpkg#38017.

Validated locally. For ease of review, the first commit copies the
existing port as-is, and the second commit updates it.

Fixes #4857
Fixes TileDB-Inc/TileDB-CSharp#405
Fixes the root cause of TileDB-Inc/conda-forge-nightly-controller#83

---
TYPE: NO_HISTORY
@WangWeiLin-MV
Copy link
Contributor Author

The script version also needs this otherwise old versions will be broken with newer cmake

Yes, this PR only fixes the CMakePackageConfigHelpers compatibility of vcpkg-cmake-config.

For any port sources that uses the CMake internal variable PACKAGE_PREFIX_DIR will be broken with the newer version, his requires the upstream fix. For some vcpkg ports with PACKAGE_PREFIX_DIR variable, we will fix it in subsequent PRs.

@jimwang118 jimwang118 added the info:reviewed Pull Request changes follow basic guidelines label Apr 10, 2024
github-actions bot pushed a commit to TileDB-Inc/TileDB that referenced this pull request Apr 10, 2024
CMake 3.29.1 had a change that broke vcpkg's `vcpkg_cmake_config_fixup`
command. This PR updates the port containing the command to adapt to the
new behavior. The change will be upstreamed with microsoft/vcpkg#38017.

Validated locally. For ease of review, the first commit copies the
existing port as-is, and the second commit updates it.

Fixes #4857
Fixes TileDB-Inc/TileDB-CSharp#405
Fixes the root cause of TileDB-Inc/conda-forge-nightly-controller#83

---
TYPE: NO_HISTORY

(cherry picked from commit 690479f)
KiterLuc pushed a commit to TileDB-Inc/TileDB that referenced this pull request Apr 10, 2024
CMake 3.29.1 had a change that broke vcpkg's `vcpkg_cmake_config_fixup`
command. This PR updates the port containing the command to adapt to the
new behavior. The change will be upstreamed with microsoft/vcpkg#38017.

Validated locally. For ease of review, the first commit copies the
existing port as-is, and the second commit updates it.

Fixes #4857
Fixes TileDB-Inc/TileDB-CSharp#405
Fixes the root cause of TileDB-Inc/conda-forge-nightly-controller#83

---
TYPE: NO_HISTORY

(cherry picked from commit 690479f)
teo-tsirpanis added a commit to TileDB-Inc/TileDB that referenced this pull request Apr 10, 2024
CMake 3.29.1 had a change that broke vcpkg's `vcpkg_cmake_config_fixup`
command. This PR updates the port containing the command to adapt to the
new behavior. The change will be upstreamed with microsoft/vcpkg#38017.

Validated locally. For ease of review, the first commit copies the
existing port as-is, and the second commit updates it.

Fixes #4857
Fixes TileDB-Inc/TileDB-CSharp#405
Fixes the root cause of TileDB-Inc/conda-forge-nightly-controller#83

---
TYPE: NO_HISTORY
@JavierMatosD JavierMatosD added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Apr 10, 2024
@jimwang118 jimwang118 mentioned this pull request Apr 11, 2024
@bradking
Copy link

CMake 3.29.2 reverts the change.

@JavierMatosD
Copy link
Contributor

@ras0219, @BillyONeal, @data-queue, @vicroms - Since CMake is going to pursue a different solution for this problem, we shouldn't do this and we should support the new solution. We also need to update to scripts/vcpkgTools.xml to point to CMake 3.29.2

@WangWeiLin-MV WangWeiLin-MV deleted the port/vcpkg-cmake-config/adapt-package-prefix-cmake-3.29.1 branch April 12, 2024 01:57
dudoslav pushed a commit to TileDB-Inc/TileDB that referenced this pull request Apr 17, 2024
CMake 3.29.1 had a change that broke vcpkg's `vcpkg_cmake_config_fixup`
command. This PR updates the port containing the command to adapt to the
new behavior. The change will be upstreamed with microsoft/vcpkg#38017.

Validated locally. For ease of review, the first commit copies the
existing port as-is, and the second commit updates it.

Fixes #4857
Fixes TileDB-Inc/TileDB-CSharp#405
Fixes the root cause of TileDB-Inc/conda-forge-nightly-controller#83

---
TYPE: NO_HISTORY
JavierMatosD pushed a commit that referenced this pull request Apr 22, 2024
Since `CMake` version `3.29.2` reverted [Kitware -CMake -
9390](https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9390) in
[Kitware - CMake -
9420](https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9420),
update it.

Fix #37968. Take the place of #38017.

See [Kitware - CMake -
25873](https://gitlab.kitware.com/cmake/cmake/-/issues/25873) and
[Kitware - CMake -
25827](https://gitlab.kitware.com/cmake/cmake/-/issues/25827)

Co-authored-by: FrankXie <v-frankxie@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vcpkg-cmake-config] Wrong *Config.cmake files for CMake 3.29.1
6 participants