Skip to content

Conversation

elichai
Copy link

@elichai elichai commented Mar 26, 2025

Bzip2 has a feature flag for allowing a rust backend (See trifectatechfoundation/bzip2-rs#40 (comment))
this exposes that feature to users so they can easily compile to wasm32-unknown-unknown

@Pr0methean Pr0methean changed the title Allow choosing bzip2 rust backend feat: Allow choosing bzip2 rust backend Apr 2, 2025
@Pr0methean Pr0methean enabled auto-merge April 2, 2025 13:33
@Pr0methean Pr0methean added the Please fix failing tests Tests are failing with this change; please fix them. label Apr 2, 2025
@Pr0methean
Copy link
Member

@elichai Is this still needed now that #352 is merged?

@elichai
Copy link
Author

elichai commented May 14, 2025

I think so, AFAIU it only touched deflate, not bzip2

@Pr0methean
Copy link
Member

Pr0methean commented May 15, 2025

error: package libbz2-rs-sys v0.1.3 cannot be built because it requires rustc 1.82 or newer, while the currently active rustc version is 1.75.0

Is this increase in the MSRV actually necessary? If so, see the Cargo.toml for instructions on updating it. If not, I prefer to increase it as slowly as possible, even though 1.82 does meet the stable-for-6-months requirement.

@elichai
Copy link
Author

elichai commented May 15, 2025

Is this increase in the MSRV actually necessary? If so, see the Cargo.toml for instructions on updating it. If not, I prefer to increase it as slowly as possible, even though 1.82 does meet the stable-for-6-months requirement.

I can look more deeply into why libbz2-rs-sys requires Rust 1.82 and if I can reduce it, but do note that it only affects users that will activate this feature.

I can try to update the --all-features MSRV job to exclude this feature and test it in a seperate job, or maybe even integrate cargo-hack to properly permute the features combinations

@Pr0methean
Copy link
Member

@elichai After merging from master, I get the following error:

package `zip` depends on `bzip2` with feature `libbz2-rs-sys` but `bzip2` does not have that feature.
 An optional dependency with that name exists, but that dependency uses the "dep:" syntax in the features table, so it does not have an implicit feature with that name.

Is this PR still needed given that bzip2-sys is optional in bzip2 0.6.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Please fix failing tests Tests are failing with this change; please fix them.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants