Skip to content

[vcpkg baseline][qca] Fix pkgconfig dependency properly#30702

Merged
vicroms merged 2 commits intomicrosoft:masterfrom
dg0yt:qca2
Apr 10, 2023
Merged

[vcpkg baseline][qca] Fix pkgconfig dependency properly#30702
vicroms merged 2 commits intomicrosoft:masterfrom
dg0yt:qca2

Conversation

@dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Apr 6, 2023

Amends #30686.
Now using proper vcpkg_find_aquire_prorgram, same pattern as in curl.

  • 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.

@Cheney-W Cheney-W added the category:port-bug The issue is with a library, which is something the port should already support label Apr 7, 2023
JackBoosY
JackBoosY previously approved these changes Apr 7, 2023
@Cheney-W Cheney-W added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Apr 7, 2023
@dg0yt
Copy link
Contributor Author

dg0yt commented Apr 8, 2023

@ras0219-msft @Neumann-A We should reflect the proper provisioning and usage of pkgconf(ig). There are multiple bad patterns now.

  • vcpkg_find_acquire_program is a bad because it is covering too many programs, rebuilding more ports than needed for individual changes. (Related: discussion on adding 7z for osx, [intel-mkl] Overhaul, install osx #30483 (comment)).
  • Port pkgconf is bad because it doesn't systematically interact with FindPkgConfig.cmake. Adding it wrongly to one port's dependencies can lead to errors elsewhere. I understand now that the qca error wasn't fixed by my initial attempt ([vcpkg baseline][qca] Control plugin dependencies #30686) because of this combination circumstances:
    • qca didn't explicitly pass the host pkgconf path to CMake. (Now part of this PR.)
    • vcpkg.cmake adds the target toolchain tools directories to CMAKE_PROGRAM_PATH because vcpkg_cmake_configure doesn't pass VCPKG_HOST_TRIPLET. (Related for user projects and with comments on side effects: Extend triplet detection in CMake toolchain #25529)
    • The recent update to libigl added another pkgconf dependency without host property.
    • libigl is installed before qca in vcpkg CI.
  • FindPkgConfig.cmake would als consider the standard environment variables. But all vcpkg methods just ignore that when looking for system pkg-config, or when passing a fixed path via OPTIONS to cmake. This variable is useful when dealing with sysroots - traditionally, it is common practice to use specific wrapper scripts for pkg-config when doing cross builds.

Personally I would prefer to revise this PR to use pkgconf instead of vcpkg_find_acquire_program. And the answer would also apply for necessary fixes to other ports such as libigl.

PS: vcpkg_find_acquire_program runs in script mode and knows HOST_TRIPLET and CURRENT_HOST_INSTALLED_DIR, but doesn't generally direct find_program to look into CURRENT_HOST_INSTALLED_DIR. (It does so explicitly for gn.)

@vicroms vicroms merged commit f668713 into microsoft:master Apr 10, 2023
@dg0yt dg0yt deleted the qca2 branch April 10, 2023 05:24
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:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants