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

Support cross-compiling in the libmagic vcpkg port overlay, attempt 2. #5338

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

teo-tsirpanis
Copy link
Member

@teo-tsirpanis teo-tsirpanis commented Oct 8, 2024

TYPE: NO_HISTORY

@teo-tsirpanis teo-tsirpanis force-pushed the revert-5332-teo/libmagic-update branch from 3d5554f to 07579db Compare October 8, 2024 00:38
@teo-tsirpanis teo-tsirpanis changed the title Revert "Update the libmagic vcpkg port overlay to version 5.45." [WIP] Try fixing libmagic failures. Oct 10, 2024
teo-tsirpanis added a commit that referenced this pull request Oct 10, 2024
…5333)" (#5343)

Since #5333 the Release workflow has been failing. Let's revert the PR
until we fix the failures (work is underway in #5338), in order to
reduce the noise from the failed runs.

Fixes #5335

---
TYPE: NO_HISTORY
@teo-tsirpanis teo-tsirpanis force-pushed the revert-5332-teo/libmagic-update branch 2 times, most recently from 566a9ec to 0edab59 Compare October 18, 2024 21:38
@teo-tsirpanis teo-tsirpanis changed the title [WIP] Try fixing libmagic failures. Support cross-compiling in the libmagic vcpkg port overlay, attempt 2. Oct 19, 2024
Comment on lines +34 to +38
if(VCPKG_CROSSCOMPILING)
vcpkg_add_to_path(PREPEND "${CURRENT_HOST_INSTALLED_DIR}/tools/libmagic")
elseif(VCPKG_TARGET_IS_WINDOWS AND VCPKG_LIBRARY_LINKAGE STREQUAL dynamic)
set(EXTRA_ARGS "ADD_BIN_TO_PATH")
endif()
Copy link
Member Author

Choose a reason for hiding this comment

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

We move this block before the configure and install steps, so that libmagic's build system can find the file command in the path we built it for the host.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, not quite sure what this does, but have you tested this with cross compilation? (If it works for x86_64 and crosscompilation x86_64->aarch64)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's tested as outlined in #5338 (comment).

As for what this does, in libmagic's CMakeLists.txt we have this command:

add_custom_command(OUTPUT magic.mgc
  COMMAND file -C -m magic
  COMMENT "Compiling magic file"
)

When not cross-compiling, CMake resolves file to the file executable we built, but when cross-compiling file will be passed to the shell as-is and resolved from PATH. The vcpkg_add_to_path command ensures the host's file we built gets found.

The elseif part makes sure libmagic.dll gets found on Windows if we are using dynamic linkage for the dependencies (which we don't currently do).

This block exists in the upstream port BTW.

https://github.com/microsoft/vcpkg/blob/b8d688b979bd54f0269ea1c6101e6a977613175f/ports/libmagic/portfile.cmake#L61-L67

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks a lot for the explanation!

@teo-tsirpanis
Copy link
Member Author

Conda nightlies and Release workflow succeed. This is ready to review.

I tried locally to cross-compile to arm64-windows, but it failed due to reasons related to the Windows command line1. Let's not try to fix that too, the upstream port works and we don't officially support this platform.

Footnotes

  1. If you have an incompatible foo.exe in your working directory and a compatible foo.exe in PATH, running foo will use the former executable in cmd but the latter in Powershell. At this point this is an issue for CMake.

@teo-tsirpanis teo-tsirpanis marked this pull request as ready for review October 19, 2024 19:00
Copy link
Collaborator

@dudoslav dudoslav left a comment

Choose a reason for hiding this comment

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

LGTM

@teo-tsirpanis teo-tsirpanis merged commit 55c5d95 into dev Oct 22, 2024
75 checks passed
@teo-tsirpanis teo-tsirpanis deleted the revert-5332-teo/libmagic-update branch October 22, 2024 07:32
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