Skip to content

[libmagic] Add CMake config.#35274

Merged
vicroms merged 6 commits intomicrosoft:masterfrom
teo-tsirpanis:libmagic-config
Dec 1, 2023
Merged

[libmagic] Add CMake config.#35274
vicroms merged 6 commits intomicrosoft:masterfrom
teo-tsirpanis:libmagic-config

Conversation

@teo-tsirpanis
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
  • 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.

Copy link
Copy Markdown
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

The config also needs to append to IMPORTED_CONFIGURATIONS.

Comment thread ports/libmagic/unofficial-libmagic-config.cmake.in Outdated
Comment thread ports/libmagic/unofficial-libmagic-config.cmake.in Outdated
Comment thread ports/libmagic/unofficial-libmagic-config.cmake.in Outdated
Comment thread ports/libmagic/unofficial-libmagic-config.cmake.in Outdated
Comment thread ports/libmagic/unofficial-libmagic-config.cmake.in Outdated
@teo-tsirpanis
Copy link
Copy Markdown
Contributor Author

Feedback addressed. I also set IMPORTED_LINK_INTERFACE_LANGUAGES_${config} when we have a static library.

@teo-tsirpanis teo-tsirpanis requested a review from dg0yt November 22, 2023 21:10
Comment thread ports/libmagic/unofficial-libmagic-config.cmake.in Outdated
@teo-tsirpanis teo-tsirpanis requested a review from dg0yt November 22, 2023 23:15
@JonLiu1993 JonLiu1993 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Nov 23, 2023
Comment on lines +69 to +74
include(CMakePackageConfigHelpers)
configure_package_config_file(
"${CMAKE_CURRENT_LIST_DIR}/unofficial-${PORT}-config.cmake.in"
"${CURRENT_PACKAGES_DIR}/share/unofficial-${PORT}/unofficial-${PORT}-config.cmake"
INSTALL_DESTINATION "share/unofficial-${PORT}"
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FTR I believe that configure_package_config_file is intented for CMake project mode, but here it is used in script mode. Let's hope there are no future changes which break this use case.

@JonLiu1993
Copy link
Copy Markdown
Contributor

Tested usage successfully by libmagic:x64-windows triplet:

Total install time: 12 min
libmagic provides CMake targets:

    find_package(unofficial-libmagic REQUIRED)
    target_link_libraries(main PRIVATE unofficial::libmagic::libmagic)

The magic.mgc file can be accessed from the unofficial-libmagic_DICTIONARY variable.

@JonLiu1993 JonLiu1993 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Nov 24, 2023
@teo-tsirpanis teo-tsirpanis requested a review from dg0yt November 28, 2023 13:04
@teo-tsirpanis
Copy link
Copy Markdown
Contributor Author

Is there anything left to do here?

@vicroms vicroms added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Nov 29, 2023
Comment thread ports/libmagic/unofficial-libmagic-config.cmake.in
@vicroms vicroms merged commit 16ee2ec into microsoft:master Dec 1, 2023
@teo-tsirpanis teo-tsirpanis deleted the libmagic-config branch December 1, 2023 10:22
@JonLiu1993 JonLiu1993 removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Dec 4, 2023
KiterLuc pushed a commit to TileDB-Inc/TileDB that referenced this pull request Mar 12, 2024
[SC-38521](https://app.shortcut.com/tiledb-inc/story/38521/update-libmagic-and-use-the-upstream-vcpkg-port)

Split from #4553.

This PR updates libmagic to version 5.45 and switches to using a vcpkg
port closer to the upstream one, which we can easily consume with
find_package(unofficial-libmagic) since microsoft/vcpkg#35274.

One complication is that the upstream port builds libmagic with its
official autotools-based build system, which is significantly slower on
Windows (on Linux it builds pretty fast). I tried to upstream the
CMake-based port I had added in #4119, but the PR was rejected.

Apart from binary caching, there is unfortunately nothing we can do
about the build performance regression. We could maintain the
CMake-based port for our own use, but it will split what we build and
what a future user of TileDB from vcpkg will build, and that port is not
without its problems (it failed for example when I tried cross-compiling
to arm64-windows, because it tried to execute the arm64 file.exe on my
x64 machine).

---
TYPE: BUILD
DESC: Update libmagic to version 5.45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist 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.

4 participants