Update rand, proc-macro and zstd dependencies #488

alamb · 2021-06-22T11:08:11Z

Rationale for this change

Keep arrow up to date with rust cargo ecosystem

What changes are included in this PR?

Are there any user-facing changes?

Dependencies change

codecov-commenter · 2021-06-22T11:22:06Z

Codecov Report

Merging #488 (6798e41) into master (04779e0) will increase coverage by 0.00%.
The diff coverage is 82.10%.

❗ Current head 6798e41 differs from pull request most recent head 6ccb25a. Consider uploading reports for the commit 6ccb25a to get more accurate results

@@            Coverage Diff            @@
##           master     #488     +/-   ##
=========================================
  Coverage   82.53%   82.54%             
=========================================
  Files         162      167      +5     
  Lines       43786    45957   +2171     
=========================================
+ Hits        36140    37934   +1794     
- Misses       7646     8023    +377

Impacted Files	Coverage Δ
arrow-flight/src/utils.rs	`0.00% <0.00%> (ø)`
arrow-pyarrow-integration-testing/src/lib.rs	`0.00% <0.00%> (ø)`
arrow/src/array/array_list.rs	`94.88% <ø> (+2.06%)`	⬆️
arrow/src/array/array_string.rs	`97.76% <ø> (+1.71%)`	⬆️
arrow/src/array/raw_pointer.rs	`100.00% <ø> (ø)`
arrow/src/buffer/ops.rs	`96.96% <ø> (ø)`
arrow/src/compute/kernels/aggregate.rs	`73.40% <ø> (ø)`
arrow/src/compute/kernels/cast_utils.rs	`92.50% <ø> (ø)`
arrow/src/compute/kernels/length.rs	`100.00% <ø> (ø)`
arrow/src/error.rs	`8.88% <ø> (-2.23%)`	⬇️
... and 126 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1aace8...6ccb25a. Read the comment docs.

alamb · 2021-06-23T10:49:57Z

arrow/Cargo.toml

@@ -60,6 +63,8 @@ csv = ["csv_crate"]
 ipc = ["flatbuffers"]
 simd = ["packed_simd"]
 prettyprint = ["prettytable-rs"]
+# enable features that rely on std
+std = ["rand/std", "rand/std_rng"]


I am not sure about this one -- I needed to conditionally pull in the rand crate features that rely on std as it is needed for the arrow-pyarrow-integration-testing -- something about how maturin builds arrow requires the std features of rand (seems to build the tests as well)

However, if we use the default features of rand then we can't build for wasm anymore 🤔

@jorgecarleitao I am not sure if you have any thoughts

alamb · 2021-06-23T11:01:31Z

I think technically this is an API change so marking it as such (and meaning we probably should not include it in the arrow 4.x line, but wait for the 5.0 release)

alamb · 2021-06-24T14:49:36Z

I actually think the way cargo versions work, this is not a api change (as cargo will use an older version of the 4.x line if it can't pick a compatible version of these crates

alamb · 2021-06-30T21:01:33Z

@jorgecarleitao I would like to merge this PR in for the 5.0 release -- could you possibly review the changes needed for arrow-pyarrow-integration-testing (I am not sure what about the python build was requiring std).

alamb · 2021-06-30T21:03:00Z

arrow-pyarrow-integration-testing/Cargo.toml

@@ -31,7 +31,7 @@ name = "arrow_pyarrow_integration_testing"
 crate-type = ["cdylib"]

 [dependencies]
-arrow = { path = "../arrow", version = "5.0.0-SNAPSHOT" }
+arrow = { path = "../arrow", version = "5.0.0-SNAPSHOT", features=["std"] }


this was required for some reason otherwise the python integration build failed with errors about not being able to find the rng that requires std

jorgecarleitao

It seems that the rand release complicated a bit our side. I would also be fine with keeping the 0.8 (i.e. not bump up for now).

Regardless, all great stuff 👍

jorgecarleitao · 2021-07-01T03:35:42Z

arrow/Cargo.toml

+rand = { version = "0.8", default-features = false }
+# getrandom is a dependency of rand, not (directly) of arrow
+# need to specify `js` feature to build on wasm
+getrandom = { version = "0.2", features = ["js"] }


we may be able to achieve the same using rand with feature getrandom/js?

I will give it a try

I tried this and sadly I am told:

alamb@ip-10-0-0-124:~/Software/arrow-rs/arrow$ cargo build --target wasm32-unknown-unknown error: failed to parse manifest at `/Users/alamb/Software/arrow-rs/arrow/Cargo.toml` Caused by: feature `getrandom/js` in dependency `rand` is not allowed to contain slashes If you want to enable features of a transitive dependency, the direct dependency needs to re-export those features from the `[features]` table.

alamb@ip-10-0-0-124:~/Software/arrow-rs/arrow$ git diff Cargo.toml diff --git a/arrow/Cargo.toml b/arrow/Cargo.toml index 478f33678..f6255a93d 100644 --- a/arrow/Cargo.toml +++ b/arrow/Cargo.toml @@ -40,10 +40,7 @@ serde = { version = "1.0", features = ["rc"] } serde_derive = "1.0" serde_json = { version = "1.0", features = ["preserve_order"] } indexmap = "1.6" -rand = { version = "0.8", default-features = false } -# getrandom is a dependency of rand, not (directly) of arrow -# need to specify `js` feature to build on wasm -getrandom = { version = "0.2", features = ["js"] } +rand = { version = "0.8", default-features = false, features = ["getrandom/js"] } num = "0.4" csv_crate = { version = "1.1", optional = true, package="csv" } regex = "1.3" alamb@ip-10-0-0-124:~/Software/arrow-rs/arrow$

Sadly, it appears this workaround is the recommended way from rand rate: https://crates.io/crates/rand

WASM support

The WASM target wasm32-unknown-unknown is not automatically supported by rand or getrandom. To solve this, either use a different target such as wasm32-wasi or add a direct dependancy on getrandom with the js feature (if the target supports JavaScript). See getrandom#WebAssembly support.

alamb · 2021-07-06T18:33:10Z

arrow/Cargo.toml

@@ -40,7 +40,10 @@ serde = { version = "1.0", features = ["rc"] }
 serde_derive = "1.0"
 serde_json = { version = "1.0", features = ["preserve_order"] }
 indexmap = "1.6"
-rand = "0.7"


ok, I removed the wacky std feature and found a different work around for arrow-pyarrow-integration-testing

If this passes CI I think it is good enough to merge

github-actions bot added arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate parquet Changes to the parquet crate labels Jun 22, 2021

alamb commented Jun 23, 2021

View reviewed changes

alamb added the api-change Changes to the arrow API label Jun 23, 2021

alamb removed the api-change Changes to the arrow API label Jun 24, 2021

alamb force-pushed the alamb/update_moar_deps branch from d4eb088 to facd201 Compare June 30, 2021 21:01

alamb commented Jun 30, 2021

View reviewed changes

jorgecarleitao approved these changes Jul 1, 2021

View reviewed changes

alamb force-pushed the alamb/update_moar_deps branch from facd201 to a02580a Compare July 6, 2021 18:32

alamb commented Jul 6, 2021

View reviewed changes

alamb added 9 commits July 6, 2021 14:41

update rand

cd5bb8b

Update code for new rand interface

578f770

update zstd

4dd8090

update proc-macro2

24801d7

another

efec71c

Specify js feature of getrandom

1d228cc

enable rng std stuff

e07ca05

checkpoint

e349b96

cleanup

6ccb25a

alamb force-pushed the alamb/update_moar_deps branch from a02580a to 6ccb25a Compare July 6, 2021 18:41

alamb merged commit 80a57be into apache:master Jul 6, 2021

alamb deleted the alamb/update_moar_deps branch July 6, 2021 20:09

This was referenced Jul 7, 2021

master fails to compile with default-features=false #529

Closed

Fix default arrow build #533

Merged

Error building on master - error: cyclic package dependency: package ahash v0.7.4 depends on itself. Cycle #544

Closed

alamb mentioned this pull request Jul 13, 2021

Fix build, Make the js package a feature that can be enabled for wasm, rather than always on #545

Merged

roee88 mentioned this pull request Aug 6, 2021

Make rand an optional dependency #671

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update rand, proc-macro and zstd dependencies #488

Update rand, proc-macro and zstd dependencies #488

alamb commented Jun 22, 2021

codecov-commenter commented Jun 22, 2021 •

edited

Loading

alamb Jun 23, 2021 •

edited

Loading

alamb commented Jun 23, 2021

alamb commented Jun 24, 2021

alamb commented Jun 30, 2021

alamb Jun 30, 2021

jorgecarleitao left a comment

jorgecarleitao Jul 1, 2021

alamb Jul 1, 2021

alamb Jul 6, 2021

alamb Jul 6, 2021

alamb Jul 6, 2021

Update rand, proc-macro and zstd dependencies #488

Update rand, proc-macro and zstd dependencies #488

Conversation

alamb commented Jun 22, 2021

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

codecov-commenter commented Jun 22, 2021 • edited Loading

Codecov Report

alamb Jun 23, 2021 • edited Loading

Choose a reason for hiding this comment

alamb commented Jun 23, 2021

alamb commented Jun 24, 2021

alamb commented Jun 30, 2021

alamb Jun 30, 2021

Choose a reason for hiding this comment

jorgecarleitao left a comment

Choose a reason for hiding this comment

jorgecarleitao Jul 1, 2021

Choose a reason for hiding this comment

alamb Jul 1, 2021

Choose a reason for hiding this comment

alamb Jul 6, 2021

Choose a reason for hiding this comment

alamb Jul 6, 2021

Choose a reason for hiding this comment

alamb Jul 6, 2021

Choose a reason for hiding this comment

codecov-commenter commented Jun 22, 2021 •

edited

Loading

alamb Jun 23, 2021 •

edited

Loading