You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "waynexia (via GitHub)" <gi...@apache.org> on 2024/06/01 03:38:35 UTC
[PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
waynexia opened a new pull request, #5829:
URL: https://github.com/apache/arrow-rs/pull/5829
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
I hit this while trying to upgrade arrow for datafusion.
Parquet depends on `zstd-sys` and datafusion depends on `async-compression` which then depends on `zstd-sys` via `zstd-safe`.
Parquet cannot use `zstd-sys` beyond `2.0.9` due to https://github.com/gyscos/zstd-rs/issues/269. And `zstd-safe` has two available versions `7.0.0` which depends on `zstd-sys` `2.0.7` and `7.1.0` on `zstd-sys` `2.10.0`.
So to make parquet be compatible with projects use `async-compression`, the only available version to `zstd-sys` is `2.0.7`
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
Downgrade and pin `zstd-sys` to `2.0.7`
# Are there any user-facing changes?
Yes, the dependences
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
This patch needs to be included in the next release https://github.com/apache/arrow-rs/issues/5688 if possible
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#issuecomment-2143437702
I'm somewhat inclined to think we should just remove the version pin, and document that people using wasm32 will need to manually pin zstd-sys themselves. We're creating pain for all of our users by pinning this crate, to save some pain for a very small subset of our community...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#discussion_r1623215428
##########
parquet/Cargo.toml:
##########
@@ -52,9 +52,9 @@ snap = { version = "1.0", default-features = false, optional = true }
brotli = { version = "6.0", default-features = false, features = ["std"], optional = true }
flate2 = { version = "1.0", default-features = false, features = ["rust_backend"], optional = true }
lz4_flex = { version = "0.11", default-features = false, features = ["std", "frame"], optional = true }
-zstd = { version = "0.13.0", optional = true, default-features = false }
# TODO: temporary to fix parquet wasm build
# upstream issue: https://github.com/gyscos/zstd-rs/issues/269
+zstd = { version = "<0.13.1", optional = true, default-features = false }
Review Comment:
The same, `<0.13.1` here will also pick `0.12`, `0.11`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "waynexia (via GitHub)" <gi...@apache.org>.
waynexia commented on PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#issuecomment-2143444988
Removed them in [d7d5e9b](https://github.com/apache/arrow-rs/pull/5829/commits/d7d5e9b6c8f46980dca809ddba8468c5128dfe4c) based on above discussion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#discussion_r1623220467
##########
parquet/Cargo.toml:
##########
@@ -52,10 +52,8 @@ snap = { version = "1.0", default-features = false, optional = true }
brotli = { version = "6.0", default-features = false, features = ["std"], optional = true }
flate2 = { version = "1.0", default-features = false, features = ["rust_backend"], optional = true }
lz4_flex = { version = "0.11", default-features = false, features = ["std", "frame"], optional = true }
-zstd = { version = "0.13.0", optional = true, default-features = false }
-# TODO: temporary to fix parquet wasm build
-# upstream issue: https://github.com/gyscos/zstd-rs/issues/269
-zstd-sys = { version = "=2.0.9", optional = true, default-features = false }
+zstd = { version = "0.13", optional = true, default-features = false }
+zstd-sys = { version = "2.0", optional = true, default-features = false }
Review Comment:
I suspect zstd-sys can be removed entirely
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "waynexia (via GitHub)" <gi...@apache.org>.
waynexia commented on code in PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#discussion_r1623222195
##########
parquet/Cargo.toml:
##########
@@ -52,10 +52,8 @@ snap = { version = "1.0", default-features = false, optional = true }
brotli = { version = "6.0", default-features = false, features = ["std"], optional = true }
flate2 = { version = "1.0", default-features = false, features = ["rust_backend"], optional = true }
lz4_flex = { version = "0.11", default-features = false, features = ["std", "frame"], optional = true }
-zstd = { version = "0.13.0", optional = true, default-features = false }
-# TODO: temporary to fix parquet wasm build
-# upstream issue: https://github.com/gyscos/zstd-rs/issues/269
-zstd-sys = { version = "=2.0.9", optional = true, default-features = false }
+zstd = { version = "0.13", optional = true, default-features = false }
+zstd-sys = { version = "2.0", optional = true, default-features = false }
Review Comment:
Moved to target-specified deps (for CI)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "waynexia (via GitHub)" <gi...@apache.org>.
waynexia commented on PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#issuecomment-2143442367
> I'm somewhat inclined to think we should just remove the version pin, and document that people using wasm32 will need to manually pin zstd-sys themselves. We're creating pain for all of our users by pinning this crate, to save some pain for a very small subset of our community... Ultimately if zstd-sys broke support for wasm32 it is on them to fix it
That's also an option. Cargo provides [patch mechanism](https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#overriding-dependencies) to override deps
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#issuecomment-2143374944
Maybe <= 2.0.9?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Relax zstd-sys Version Pin [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#issuecomment-2143440933
> I'm somewhat inclined to think we should just remove the version pin, and document that people using wasm32 will need to manually pin zstd-sys themselves.
I agree that libs like parquet should not pin dependencies. This forces users to adopt the same pinned version, increasing the likelihood of conflicts. Instead of pinning versions downstream, it's better for us to collaborate upstream to address the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#issuecomment-2143442742
> Cargo provides [patch mechanism](https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#overriding-dependencies) to override deps
They shouldn't need to go that far, if they have a lockfile they can just use `cargo update --precise`, otherwise they can just declare an explicit dependency on zstd-sys and pin it there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Relax zstd-sys Version Pin [arrow-rs]
Posted by "waynexia (via GitHub)" <gi...@apache.org>.
waynexia commented on PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#issuecomment-2143461061
Thank you all :heart:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "waynexia (via GitHub)" <gi...@apache.org>.
waynexia commented on PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#issuecomment-2143436180
Well, it turns out that cargo does not support "less than" comparison on patch position. The patch version is ignored in `<2.0.10` and leads to a selection on `1.x`.
>package `zstd-sys v1.4.3+zstd.1.3.4`
... which satisfies dependency `zstd-sys = "<2.0.10"` (locked to 1.4.3+zstd.1.3.4) of package `parquet v51.0.0 (/__w/arrow-rs/arrow-rs/parquet)` links to native library `zstd`
I'm afraid we have to use `=2.0.7` instead...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#discussion_r1623215343
##########
parquet/Cargo.toml:
##########
@@ -55,7 +55,7 @@ lz4_flex = { version = "0.11", default-features = false, features = ["std", "fra
zstd = { version = "0.13.0", optional = true, default-features = false }
# TODO: temporary to fix parquet wasm build
# upstream issue: https://github.com/gyscos/zstd-rs/issues/269
-zstd-sys = { version = "=2.0.7", optional = true, default-features = false }
+zstd-sys = { version = "<=2.0.9", optional = true, default-features = false }
Review Comment:
This could pick up `zstd-sys 1.0`, please make sure we also have `>= 2.0.0`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "waynexia (via GitHub)" <gi...@apache.org>.
waynexia commented on PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#issuecomment-2143428425
> Maybe <= 2.0.9?
That looks better 👍 Changed in [fc4c06a](https://github.com/apache/arrow-rs/pull/5829/commits/fc4c06a96c1e6e8d888ca379ea0283f32f3f336b)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] build: pin zstd-sys to 2.0.7 [arrow-rs]
Posted by "waynexia (via GitHub)" <gi...@apache.org>.
waynexia commented on code in PR #5829:
URL: https://github.com/apache/arrow-rs/pull/5829#discussion_r1623217039
##########
parquet/Cargo.toml:
##########
@@ -55,7 +55,7 @@ lz4_flex = { version = "0.11", default-features = false, features = ["std", "fra
zstd = { version = "0.13.0", optional = true, default-features = false }
# TODO: temporary to fix parquet wasm build
# upstream issue: https://github.com/gyscos/zstd-rs/issues/269
-zstd-sys = { version = "=2.0.7", optional = true, default-features = false }
+zstd-sys = { version = "<=2.0.9", optional = true, default-features = false }
Review Comment:
Ahh, that works 💯
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org