Skip to content

Conversation

@petesmc
Copy link
Contributor

@petesmc petesmc commented Jul 13, 2025

It seems overly restrictive to not allow the library to compile on Windows MSVC. I have removed the target_env = gnu restrictions and added a CI build for the MSVC target.

@codecov
Copy link

codecov bot commented Jul 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag Coverage Δ
fuzz-compress_then_decompress ?
fuzz-decompress_chunked ?
test-aarch64-apple-darwin 89.49% <ø> (ø)
test-x86_64-apple-darwin 89.49% <ø> (ø)
test-x86_64-unknown-linux-gnu 89.49% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
bzip2.rs 82.77% <ø> (ø)
libbz2-rs-sys/src/high_level.rs 80.73% <ø> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

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

Neat, thanks!

Do you have cause to use these binaries on msvc?

@folkertdev folkertdev merged commit 55d99da into trifectatechfoundation:main Jul 13, 2025
24 checks passed
#[cfg(all(target_os = "windows", target_env = "gnu"))]
#[cfg(target_os = "windows")]
extern "C" {
fn __acrt_iob_func(idx: libc::c_uint) -> *mut FILE;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this also the correct implementation when using the ucrt.dll and not just msvcrt.dll?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know but now it'll at least build. We don't currently really intend to support the binaries anyway, and for windows the high-level interface is also just nice if it works (bzip2 only uses the low-level interface, where we do really care that it works on windows of course).

Is there a simple way to test for the scenario you describe in CI (I'm really not familiar with msvc)

Copy link
Collaborator

Choose a reason for hiding this comment

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

The x86_64-pc-windows-gnullvm target uses the ucrt.

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