Add Lzma library files to the repository#124003
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds the XZ Utils library (version 5.8.2) to the dotnet/runtime repository as preparatory work for issue #1542. The addition includes the complete LZMA compression library source code, build configurations, documentation, and integration scripts.
Changes:
- Adds XZ Utils 5.8.2 library files from the tukaani-project/xz repository
- Includes CMake and Autotools build system configurations
- Adds documentation, examples, and internationalization support files
- Configures the library for static linking with multithreading disabled
Reviewed changes
Copilot reviewed 75 out of 497 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/native/external/xz/* | Complete XZ Utils 5.8.2 library source code including build scripts, documentation, examples, and localization files |
| src/native/external/xz.cmake | CMake configuration to integrate XZ Utils into dotnet/runtime build system with specific feature flags |
| src/native/external/xz.version | Version tracking file indicating XZ Utils v5.8.2 from tukaani-project |
| src/native/external/cgmanifest.json | Component governance manifest entry for XZ Utils dependency |
|
Tagging subscribers to this area: @karelz, @dotnet/area-system-io-compression |
| #include "tuklib_common.h" | ||
| TUKLIB_DECLS_BEGIN | ||
|
|
||
| #define tuklib_open_stdxx TUKLIB_SYMBOL(tuklib_open_stdxxx) |
There was a problem hiding this comment.
The macro name tuklib_open_stdxx looks like a typo/inconsistency with the header/implementation name tuklib_open_stdxxx. Consider renaming the macro to tuklib_open_stdxxx to avoid confusing API consumers and accidental use of the wrong identifier.
| #define tuklib_open_stdxx TUKLIB_SYMBOL(tuklib_open_stdxxx) | |
| #define tuklib_open_stdxxx TUKLIB_SYMBOL(tuklib_open_stdxxx) |
|
I am getting compile errors on linux: Seems like there is something wrong with the exports specification: System.IO.Compression.Native$ nm libSystem.IO.Compression.Native.so | grep " lzma_stream_decoder"
00000000003c51c0 t lzma_stream_decoder
00000000003c4410 t lzma_stream_decoder_initAll the other exported entries have But the exports file seems to be generated correctly: I am not sure what is going on here, maybe there is a problem because the library functions are declared as runtime/src/native/external/xz/src/liblzma/api/lzma/container.h Lines 838 to 840 in d8731e0 I will need to investigate a bit more |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Should be working now |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| <NativeSystemLibrary Include="z" Condition="'$(UseSystemZlib)' == 'true'" /> | ||
| <NativeSystemLibrary Include="brotlienc;brotlidec;brotlicommon" Condition="'$(UseSystemBrotli)' == 'true'" /> | ||
| <NativeSystemLibrary Include="zstd" Condition="'$(UseSystemZstd)' == 'true'" /> | ||
| <NativeSystemLibrary Include="lzma" Condition="'$(UseSystemXz)' == 'true'" /> |
There was a problem hiding this comment.
The condition uses UseSystemXz, but this PR introduces UseSystemLzma (and uses it elsewhere in the same file). This makes the selection logic inconsistent and can cause -llzma to be omitted/added incorrectly; update the condition to reference UseSystemLzma.
| <NativeSystemLibrary Include="lzma" Condition="'$(UseSystemXz)' == 'true'" /> | |
| <NativeSystemLibrary Include="lzma" Condition="'$(UseSystemLzma)' == 'true'" /> |
| <NativeLibrary Include="$(IlcSdkPath)brotlienc$(LibFileExt)" /> | ||
| <NativeLibrary Include="$(IlcSdkPath)brotlidec$(LibFileExt)" /> | ||
| <NativeLibrary Include="$(IlcSdkPath)zstd_static$(LibFileExt)" /> | ||
| <NativeLibrary Include="$(IlcSdkPath)lzma$(LibFileExt)" /> |
There was a problem hiding this comment.
The library name here is lzma$(LibFileExt) (e.g., lzma.lib), but the packaging manifest in this PR adds lzma_static.lib. This mismatch is likely to cause build/package failures on Windows unless an import/static lib named lzma.lib is produced; consider aligning this to lzma_static$(LibFileExt) (or whichever artifact is actually built).
| <NativeLibrary Include="$(IlcSdkPath)lzma$(LibFileExt)" /> | |
| <NativeLibrary Include="$(IlcSdkPath)lzma_static$(LibFileExt)" /> |
|
More to address on 32-bit Windows build |
| list(APPEND CMAKE_REQUIRED_DEFINITIONS | ||
| -D_GNU_SOURCE | ||
| -D_NETBSD_SOURCE | ||
| -D_OPENBSD_SOURCE | ||
| -D__EXTENSIONS__ |
There was a problem hiding this comment.
add_compile_definitions sets _OPENBSD_SOURCE (with a leading underscore) but CMAKE_REQUIRED_DEFINITIONS appends -D_OPENBSD_SOURCE (missing underscore). This mismatch can cause CMake feature checks to run with different feature-test macros than the actual build. Update the required definition to -D_OPENBSD_SOURCE with the leading underscore to match.
| if(HAVE_SYS_PARAM_H) | ||
| set(TUKLIB_CPUCORES_DEFINITIONS | ||
| "HAVE_PARAM_H;TUKLIB_CPUCORES_SYSCTL" | ||
| CACHE INTERNAL "") |
There was a problem hiding this comment.
Same macro inconsistency as in tuklib_physmem.cmake: the checks are gated on HAVE_SYS_PARAM_H but the propagated compile definitions use HAVE_PARAM_H. Align this to HAVE_SYS_PARAM_H so the C sources see the expected macro.
Preparatory PR for #1542