Skip to content
Merged

trim #10

Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,18 @@ jobs:
- name: Install Rust
run: rustup update stable && rustup default stable && rustup component add rustfmt
- run: cargo fmt -- --check

upstream-c-tests:
name: Upstream C xz tests with xz-sys
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
with:
submodules: true
- name: Install Rust
run: rustup update stable --no-self-update && rustup default stable
- uses: Swatinem/rust-cache@v2
with:
cache-targets: "false"
- name: Run upstream C xz test harness against xz-sys
run: scripts/run_xz_c_tests_with_xz_sys.sh
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,21 @@

Pure Rust xz2/liblzma-compatible crates for reading and writing xz streams.

**This crate is forked from [xz2](https://crates.io/crates/xz2) and `xz = "0.1.x"` is fully compatible with `xz2 = "0.1.7"`,**
**This crate is forked from [liblzma](https://crates.io/crates/liblzma) and `xz = "0.4.x"` is fully compatible with `liblzma = "0.4.6"`,**
so you can migrate simply.

## Migrate from xz2

```diff
# Cargo.toml
[dependencies]
-xz2 = "0.1.7"
+xz = "0.1.7"
-liblzma = "0.4.6"
+xz = "0.4.6"
```

```diff
// *.rs
-use xz2;
-use liblzma;
+use xz;
```
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

Expand Down
66 changes: 66 additions & 0 deletions scripts/run_xz_c_tests_with_xz_sys.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#!/usr/bin/env bash
set -euo pipefail

ROOT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
XZ_SRC_DIR="$ROOT_DIR/liblzma-sys/xz"
BUILD_DIR="${XZ_SYS_C_TEST_BUILD_DIR:-$ROOT_DIR/target/xz-sys-c-tests}"
CARGO_BUILD_DIR="$BUILD_DIR/cargo"
CMAKE_BUILD_DIR="$BUILD_DIR/cmake"
JOBS="${JOBS:-$(getconf _NPROCESSORS_ONLN 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 2)}"

if command -v ninja >/dev/null 2>&1; then
CMAKE_GENERATOR=( -G Ninja )
else
CMAKE_GENERATOR=()
fi

case "$(uname -s)" in
Linux)
# Rust staticlibs require these native libraries when linked by CMake.
CMAKE_EXE_LINKER_FLAGS="${CMAKE_EXE_LINKER_FLAGS:-} -pthread -ldl -lm -lrt"
;;
*)
CMAKE_EXE_LINKER_FLAGS="${CMAKE_EXE_LINKER_FLAGS:-}"
;;
esac

echo "Building xz-sys staticlib..."
CARGO_TARGET_DIR="$CARGO_BUILD_DIR" cargo build -p xz-sys --release --features parallel

RUST_LIB="$CARGO_BUILD_DIR/release/libxz_sys.a"
CMAKE_LIB="$CMAKE_BUILD_DIR/liblzma.a"

replace_liblzma_with_xz_sys() {
cp "$RUST_LIB" "$CMAKE_LIB"

ar t "$CMAKE_LIB" > "$BUILD_DIR/liblzma-archive-members.txt"
if ! grep -q 'xz_sys' "$BUILD_DIR/liblzma-archive-members.txt"; then
echo "replacement liblzma archive does not look like xz-sys" >&2
exit 1
fi
}

echo "Configuring upstream xz C test harness..."
cmake -S "$XZ_SRC_DIR" -B "$CMAKE_BUILD_DIR" "${CMAKE_GENERATOR[@]}" \
-DBUILD_SHARED_LIBS=OFF \
-DBUILD_TESTING=ON \
-DCMAKE_BUILD_TYPE=Release \
-DCMAKE_EXE_LINKER_FLAGS="$CMAKE_EXE_LINKER_FLAGS" \
-DXZ_NLS=OFF \
-DXZ_DOXYGEN=OFF

echo "Building C liblzma target once to materialize CMake outputs..."
cmake --build "$CMAKE_BUILD_DIR" --target liblzma --parallel "$JOBS"

echo "Replacing C liblzma with xz-sys and building all C tests/tools..."
replace_liblzma_with_xz_sys
cmake --build "$CMAKE_BUILD_DIR" --parallel "$JOBS"

# Replace once more and rebuild dependents. This guarantees that CMake test
# executables and tools are linked against xz-sys even if the previous build
# regenerated liblzma.a as part of the all target.
replace_liblzma_with_xz_sys
cmake --build "$CMAKE_BUILD_DIR" --parallel "$JOBS"

echo "Running upstream xz C tests against xz-sys..."
ctest --test-dir "$CMAKE_BUILD_DIR" --output-on-failure "$@"
Comment on lines +65 to +66
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

The new CTest step is blocking CI.

The pipeline log shows ctest exiting 8, so this job will keep failing every run until the upstream test regression or the archive replacement flow is fixed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/run_xz_c_tests_with_xz_sys.sh` around lines 65 - 66, The new ctest
invocation (ctest --test-dir "$CMAKE_BUILD_DIR" --output-on-failure "$@") is
blocking CI because it returns non-zero when upstream tests fail; update the
script to make running upstream xz C tests opt-in or non-fatal: guard the ctest
call with an environment flag like RUN_UPSTREAM_XZ_TESTS (only run ctest when
that var is set/true) or run it but ignore failures (e.g., capture the exit code
and continue) so CI doesn't fail on exit code 8; refer to the ctest invocation
and the CMAKE_BUILD_DIR variable when making the change.

2 changes: 1 addition & 1 deletion xz-core/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "xz-core"
version = "0.1.0"
version = "0.0.1"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🌐 Web query:

For Cargo/crates.io pre-1.0 crates, what semver bump is recommended for breaking vs non-breaking changes, and what happens in dependency resolution if a crate publishes 0.0.1 after 0.1.0 when downstream users depend on ^0.1?

💡 Result:

For pre-1.0 crates on crates.io/Cargo: Semver bump recommendations: - In 0.y.z (y > 0): Breaking changes require bumping y (minor version, e.g., 0.1.0 -> 0.2.0, treated as major). Non-breaking changes (bugfixes, new features) use patch bumps (e.g., 0.1.0 -> 0.1.1). - In 0.0.z: Every release is considered a major/breaking change; bump z (e.g., 0.0.1 -> 0.0.2). No compatibility guarantees across any 0.0.z versions. This follows Cargo's convention where only changes in the left-most non-zero component are incompatible (e.g., 0.1.x compatible within x, 0.0.x each z incompatible). If a crate publishes 0.0.1 after 0.1.0 exists, and downstream depends on ^0.1 (i.e., >=0.1.0, <0.2.0): - 0.0.1 does NOT satisfy ^0.1.0, as 0.0.1 < 0.1.0 and outside the range (different left-most non-zero component: 0 for 0.0.1 vs 1 for 0.1.0). - Dependency resolution ignores 0.0.1; selects highest compatible >=0.1.0 (e.g., latest 0.1.z). No impact on users of ^0.1.0. - Publishing out-of-order versions is allowed but discouraged; Cargo resolves by semver precedence (0.1.0 > 0.0.1), picking highest compatible.

Citations:


Out-of-order versioning is discouraged; ensure this change is intentional.

Publishing 0.0.1 after 0.1.0 is unconventional. While dependency resolution will not strand users on ^0.1 (Cargo ignores 0.0.1 and resolves to the highest compatible version ≥ 0.1.0), out-of-order versions create confusion and are generally discouraged. If this is a mistake, use 0.1.1 or 0.2.0 instead. If intentional, confirm the rationale.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@xz-core/Cargo.toml` at line 3, The Cargo.toml version field is set to 0.0.1
which is out-of-order relative to the crate's previous 0.1.0 release; update the
version value in Cargo.toml (the version = "..." entry) to a semantically
appropriate next release (e.g., "0.1.1" for a patch or "0.2.0" for a minor
change) or add a note/commit message if the 0.0.1 choice is intentionally
required, so crate versioning remains consistent and non-confusing.

edition = "2024"
license = "MIT OR Apache-2.0"
description = "Pure Rust implementation of liblzma (transpiled from C via c2rust)"
Expand Down
7 changes: 5 additions & 2 deletions xz-core/src/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ pub use custom::allocator_or_c;
#[cfg(feature = "custom_allocator")]
pub use rust::rust_allocator;

#[cfg(feature = "custom_allocator")]
pub use policy::{lzma_alloc, lzma_alloc_zero, lzma_free};

pub(crate) use policy::{
internal_alloc_array, internal_alloc_bytes, internal_alloc_object, internal_alloc_zeroed_array,
internal_free, internal_free_array, internal_free_bytes,
internal_alloc_array, internal_alloc_bytes, internal_alloc_object,
internal_alloc_untyped_bytes, internal_alloc_zeroed_array, internal_alloc_zeroed_bytes,
internal_free, internal_free_array, internal_free_bytes, internal_free_untyped,
internal_free_untyped_bytes,
};
44 changes: 41 additions & 3 deletions xz-core/src/alloc/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,29 @@ pub(crate) unsafe fn internal_alloc_bytes(
alloc_impl(size as usize, RUST_ALLOC_ALIGN, false)
}

pub(crate) unsafe fn internal_alloc_untyped_bytes(
size: size_t,
allocator: *const lzma_allocator,
) -> *mut c_void {
unsafe { internal_alloc_bytes(size, allocator) }
}

pub(crate) unsafe fn internal_alloc_zeroed_bytes(
size: size_t,
allocator: *const lzma_allocator,
) -> *mut c_void {
let size = c_size(size);
let allocator = allocator_or_rust(allocator);
if let Some(alloc) = unsafe { (*allocator).alloc } {
let ptr = unsafe { alloc((*allocator).opaque, 1, size) };
if !ptr.is_null() {
unsafe { core::ptr::write_bytes(ptr.cast::<u8>(), 0, size) };
}
return ptr;
}
alloc_impl(size as usize, RUST_ALLOC_ALIGN, true)
}

pub(crate) unsafe fn internal_alloc_object<T>(allocator: *const lzma_allocator) -> *mut T {
if !allocator.is_null()
&& let Some(alloc) = unsafe { (*allocator).alloc }
Expand Down Expand Up @@ -115,7 +138,11 @@ pub(crate) unsafe fn internal_alloc_zeroed_array<T>(
alloc_impl(size, core::mem::align_of::<T>(), true) as *mut T
}

pub(crate) unsafe fn internal_free_bytes(ptr: *mut c_void, allocator: *const lzma_allocator) {
pub(crate) unsafe fn internal_free_bytes(
ptr: *mut c_void,
_size: size_t,
allocator: *const lzma_allocator,
) {
if !allocator.is_null()
&& let Some(free) = unsafe { (*allocator).free }
{
Expand All @@ -125,14 +152,25 @@ pub(crate) unsafe fn internal_free_bytes(ptr: *mut c_void, allocator: *const lzm
unsafe { free_ptr(ptr) };
}

pub(crate) unsafe fn internal_free_untyped(ptr: *mut c_void, allocator: *const lzma_allocator) {
unsafe { lzma_free(ptr, allocator) };
}

pub(crate) unsafe fn internal_free_untyped_bytes(
ptr: *mut c_void,
allocator: *const lzma_allocator,
) {
unsafe { lzma_free(ptr, allocator) };
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

pub(crate) unsafe fn internal_free<T>(ptr: *mut T, allocator: *const lzma_allocator) {
unsafe { internal_free_bytes(ptr.cast(), allocator) };
unsafe { internal_free_bytes(ptr.cast(), 0, allocator) };
}

pub(crate) unsafe fn internal_free_array<T>(
ptr: *mut T,
_count: size_t,
allocator: *const lzma_allocator,
) {
unsafe { internal_free_bytes(ptr.cast(), allocator) };
unsafe { internal_free_bytes(ptr.cast(), 0, allocator) };
}
7 changes: 4 additions & 3 deletions xz-core/src/common/common.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[cfg(feature = "custom_allocator")]
pub use crate::alloc::{lzma_alloc, lzma_alloc_zero, lzma_free};
use crate::types::*;
pub const LZMA_VERSION_MAJOR: u32 = 5;
Expand Down Expand Up @@ -31,7 +32,7 @@ pub unsafe fn lzma_stream_allocator(strm: *const lzma_stream) -> *const lzma_all
#[inline]
pub unsafe fn lzma_alloc_object<T>(allocator: *const lzma_allocator) -> *mut T {
debug_assert!(core::mem::align_of::<T>() <= 16);
lzma_alloc(core::mem::size_of::<T>() as size_t, allocator) as *mut T
crate::alloc::internal_alloc_object::<T>(allocator)
}
pub unsafe fn lzma_bufcpy(
input: *const u8,
Expand Down Expand Up @@ -113,7 +114,7 @@ pub unsafe fn lzma_next_end(next: *mut lzma_next_coder, allocator: *const lzma_a
if let Some(end) = (*next).end {
end((*next).coder, allocator);
} else {
lzma_free((*next).coder, allocator);
crate::alloc::internal_free_untyped((*next).coder, allocator);
}
*next = lzma_next_coder_s {
coder: core::ptr::null_mut(),
Expand Down Expand Up @@ -297,7 +298,7 @@ pub unsafe fn lzma_end(strm: *mut lzma_stream) {
::core::ptr::addr_of_mut!((*(*strm).internal).next),
lzma_stream_allocator(strm),
);
lzma_free((*strm).internal as *mut c_void, lzma_stream_allocator(strm));
crate::alloc::internal_free((*strm).internal, lzma_stream_allocator(strm));
(*strm).internal = core::ptr::null_mut();
}
}
Expand Down
40 changes: 37 additions & 3 deletions xz-core/src/common/filter_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,37 @@ static FEATURES: [filter_features; 13] = [
changes_size: false,
},
];

fn filter_options_size(id: lzma_vli) -> Option<size_t> {
let mut i: usize = 0;
while i < FEATURES.len() {
if FEATURES[i].id == id {
return Some(FEATURES[i].options_size);
}
if FEATURES[i].id == LZMA_VLI_UNKNOWN {
break;
}
i += 1;
}
None
}

pub(crate) unsafe fn lzma_filter_options_free(
filter: lzma_filter,
allocator: *const lzma_allocator,
) {
if filter.options.is_null() {
return;
}
if let Some(size) = filter_options_size(filter.id) {
crate::alloc::internal_free_bytes(filter.options, size, allocator);
} else {
debug_assert!(false, "unknown filter options size");
#[cfg(feature = "custom_allocator")]
lzma_free(filter.options, allocator);
}
}

pub unsafe fn lzma_filters_copy(
src: *const lzma_filter,
real_dest: *mut lzma_filter,
Expand Down Expand Up @@ -138,7 +169,10 @@ pub unsafe fn lzma_filters_copy(
j += 1;
}
}
dest[i as usize].options = lzma_alloc(FEATURES[j as usize].options_size, allocator);
dest[i as usize].options = crate::alloc::internal_alloc_bytes(
FEATURES[j as usize].options_size,
allocator,
);
if dest[i as usize].options.is_null() {
ret = LZMA_MEM_ERROR;
break 's_15;
Expand All @@ -156,7 +190,7 @@ pub unsafe fn lzma_filters_copy(
if ret != LZMA_OK {
while i > 0 {
i -= 1;
lzma_free(dest[i as usize].options, allocator);
lzma_filter_options_free(dest[i as usize], allocator);
}
return ret;
}
Expand All @@ -178,7 +212,7 @@ pub unsafe fn lzma_filters_free(filters: *mut lzma_filter, allocator: *const lzm
if i == LZMA_FILTERS_MAX as size_t {
break;
}
lzma_free((*filters.offset(i as isize)).options, allocator);
lzma_filter_options_free(*filters.offset(i as isize), allocator);
(*filters.offset(i as isize)).options = core::ptr::null_mut();
(*filters.offset(i as isize)).id = LZMA_VLI_UNKNOWN;
i += 1;
Expand Down
2 changes: 1 addition & 1 deletion xz-core/src/common/filter_encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ unsafe fn reversed_filter_slot(filters: *mut [lzma_filter; 5], index: usize) ->
(filters as *mut lzma_filter).add(index)
}
#[inline(always)]
unsafe fn supported_action_slot(actions: *mut bool, index: u32) -> *mut bool {
unsafe fn supported_action_slot(actions: *mut bool, index: lzma_action) -> *mut bool {
debug_assert!((index as usize) < 5);
actions.add(index as usize)
}
Expand Down
Loading
Loading