Update libmagic to version 5.45 #4673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request has been linked to Shortcut Story #38521: Update libmagic and use the upstream vcpkg port.. |
😞 Thanks for trying though! |
On Linux it found somewhere a liblzma header and so enabled support for it, but could not link to liblzma. Now if the specific compression feature is not enabled, support will be explicitly disabled when configuring. Because we don't use the `MAGIC_COMPRESS` flag anywhere, we don't have to enable these features.
teo-tsirpanis
changed the title
teo-tsirpanis
changed the title
teo-tsirpanis
force-pushed
the
teo/libmagic-upstream
branch
from
6912369
to
9653207
Compare
|
Read the Docs build fails because it does not find |
|
Really guessing here but doesn't state it failed rather than 'command not found'? Could it be an autoconf version issue? It changes slowly but I think there may have been some difference between 2.69 and now 2.71. Can you replicate in a container and getter info on versions used? |
5 tasks
teo-tsirpanis
force-pushed
the
teo/libmagic-upstream
branch
from
885de2a
to
677c0f0
Compare
|
CI is green, this is ready to review. |
KiterLuc
approved these changes
Mar 11, 2024
KiterLuc
added a commit
that referenced
this pull request
Mar 13, 2024
KiterLuc
added a commit
that referenced
this pull request
Mar 13, 2024
teo-tsirpanis
pushed a commit
that referenced
this pull request
Mar 13, 2024
robertbindar
pushed a commit
that referenced
this pull request
Mar 22, 2024
KiterLuc
pushed a commit
that referenced
this pull request
Jun 17, 2024
[SC-25167](https://app.shortcut.com/tiledb-inc/story/25167) [SC-47655](https://app.shortcut.com/tiledb-inc/story/47655) [SC-47656](https://app.shortcut.com/tiledb-inc/story/47656) [SC-47657](https://app.shortcut.com/tiledb-inc/story/47657) [SC-47658](https://app.shortcut.com/tiledb-inc/story/47658) This PR overhauls the facilities to embed and load the `magic.mgc` file that is needed by libmagic: * The most important change is the removal of `magic_mgc_gzipped.bin.tar.bz2`. This file contained a copy of `magic.mgc` that was compressed, converted to escaped C characters, packed and compressed again to take less space, and stored in source control, so that at build time to get unpacked and `#include`d in `mgc_dict.cc`. Because this file was being prepared ahead of time by a manually invoked C++ program, this approach had the disadvantage that it tied the Core to a specific version of libmagic. This was made evident in #4673, where just updating libmagic was not enough; we also had to update `magic_mgc_gzipped.bin.tar.bz2`. What we do now is rely on CMake to find `magic.mgc` and perform its entire preparation at build time. The C++ program was rewritten to be a CMake script, which makes it much simpler and enables it to run on cross-compilation scenarios. The script accepts the uncompressed `magic.mgc` file, compresses it and produces a header file of the following format: ```cpp static const unsigned char magic_mgc_compressed_bytes[] = { 0x28, 0xb5, 0x2f, 0xfd, … }; constexpr size_t magic_mgc_compressed_size = sizeof(magic_mgc_compressed_bytes); // Editorial note: we used to prepend the decompressed size at the start of the // binary blob, but this was non-standard and could not be easily done by CMake. constexpr size_t magic_mgc_decompressed_size = 7041352; ``` * The algorithm to compress `magic.mgc` was changed from gzip to zstd, resulting in a 17.9% reduction of the compressed size (from 333067 το 273500 bytes). * Tests for `mgc_dict` were also updated to use Catch2, and were wired to run along with the other standalone unit tests. * This necessitated to make an object library for `mgc_dict`, which was done as well. Validated by successfully running `unit_mgc_dict` locally. --- TYPE: BUILD DESC: Improve embedding of `magic.mgc` and allow compiling with any libmagic version.
teo-tsirpanis
added a commit
that referenced
this pull request
Oct 2, 2024
[SC-56749](https://app.shortcut.com/tiledb-inc/story/56749/update-the-libmagic-vcpkg-port-overlay-to-version-5-45) This PR updates our custom vcpkg port overlay for libmagic to version 5.45. Unlike the last time we attempted this (#4673), this PR does not replace the port with the upstream autotools-based one, and is not expected to be a build-breaking change. --- TYPE: IMPROVEMENT DESC: Updated libmagic to version 5.45.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SC-38521
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