sunchao / parquet-rs

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

Depending on parquet 0.4.0 from crates errors with git command failure #170

Closed xrl closed 5 years ago

xrl commented 5 years ago

Looks like the published 0.4.0 version of parquet is erroring during build, doesn't matter if it's release or debug.

The build script was added in https://github.com/sunchao/parquet-rs/pull/128

Here's the failing Cargo.toml dependency and then the error which happens during build:

[dependencies]
parquet = "0.4.0"

error:

error: failed to run custom build command for `parquet v0.4.0`
process didn't exit successfully: `/Users/xlange/code/rust/dracula/target/debug/build/parquet-05fa2decd80b738e/build-script-build` (exit code: 101)
--- stdout
Running: `"git" "rev-parse" "HEAD"`

--- stderr
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "Failed: `\"git\" \"rev-parse\" \"HEAD\"` (exit code: 128)"', libcore/result.rs:1009:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: <std::panicking::begin_panic::PanicPayload<A> as core::panic::BoxMeUp>::get
             at libstd/panicking.rs:476
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:390
   6: std::panicking::try::do_call
             at libstd/panicking.rs:325
   7: core::ptr::drop_in_place
             at libcore/panicking.rs:77
   8: core::result::unwrap_failed
             at libcore/macros.rs:26
   9: <core::result::Result<T, E>>::unwrap
             at libcore/result.rs:808
  10: build_script_build::main
             at ./build.rs:23
  11: std::rt::lang_start::{{closure}}
             at libstd/rt.rs:74
  12: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:310
  13: macho_symbol_search
             at libpanic_unwind/lib.rs:102
  14: alloc::alloc::box_free
             at libstd/panicking.rs:289
             at libstd/panic.rs:392
             at libstd/rt.rs:58
  15: std::rt::lang_start
             at libstd/rt.rs:74
  16: build_script_build::run

this is fixed by changing parquet to a git source, where the git command will return a value.

[dependencies]
parquet = { git = "https://github.com/sunchao/parquet-rs.git", branch = "master" }

looks like the build.rs (https://github.com/sunchao/parquet-rs/blob/master/build.rs#L20-L28) needs to be changed so it doesn't unwrap and just fails gracefully?

sunchao commented 5 years ago

Oops. Thanks for reporting @xrl ! I agree that we should fail gracefully in this case. I've created a PR to fix this. Once it gets merged, we'll release 0.4.1.

sadikovi commented 5 years ago

Yes, it was added so we can propagate version for parquet. Unfortunately, I did not account for dependency install being non-repository. I suggest we patch the code ASAP and I will work on a proper fix.