sunchao / parquet-rs

Apache Parquet implementation in Rust
Apache License 2.0
149 stars 20 forks source link

Investigate on publish the project in Cargo crate #32

Closed sunchao closed 6 years ago

sunchao commented 6 years ago

At some point we should investigate on the possibility to publish parquet-rs as a crate so that it can be more easily used by other projects. So far I think one main issue is that parquet.rs needs to be generated before compile time and there is no easy way to do this (something similar was done for protobuf rust which we may borrow). Otherwise, we have to put the generated parquet.rs in the repo, which is not very desirable.

sadikovi commented 6 years ago

We could add something like build.rs, which would build the parquet.rs file. I have seen other projects relying on it, e.g. https://github.com/cmr/openblas-src. For example, Cargo.toml references build.rs (https://github.com/cmr/openblas-src/blob/master/Cargo.toml#L17) and build.rs itself (https://github.com/cmr/openblas-src/blob/master/build.rs).

So technically we do not even need Makefile, we could just always run cargo build, etc. - it will invoke build.rs whenever necessary.

@sunchao Let me know what you think - I can experiment with it.

sunchao commented 6 years ago

@sadikovi Yes build.rs could potentially help. However, generating parquet.rs will require Thrift binary installed on the target machine which I'm not sure if Rust crate allows.

sadikovi commented 6 years ago

Yes, you are right. However, I was naively thinking that we just check if thrift is available and we can generate parquet.rs, otherwise fail with instructions on installing correct version of thrift - just an idea, not sure how user friendly this is.

sadikovi commented 6 years ago

We could also try installing thrift in build.rs, similar to how openblas buids underlying libraries, will that be feasible?

sunchao commented 6 years ago

Yes we can try that. We just need to make sure that Crates.io will allow crates be published in this way. On the user side we can provide some useful message in case Thrift is not installed (or even a script to install the right version of Thrift).

sadikovi commented 6 years ago

Sounds great! I will try playing with it, see if it could be done.