tazz4843 / whisper-rs

Rust bindings to https://github.com/ggerganov/whisper.cpp
The Unlicense
659 stars 108 forks source link

don't remove bindings/javascript/package.json during build #64

Closed chrisrude closed 1 year ago

chrisrude commented 1 year ago

I've built whisper-rs on both OSX and Linux, and after the build it shows that the file bindings/javascript/package.json has been removed the whisper.cpp directory. It seems like this is intentional, but it causes issues because package.json is a committed file that's part of the whisper.cpp repo. See: https://github.com/ggerganov/whisper.cpp/blob/master/bindings/javascript/package.json

With this change, I do not get any cargo warnings when building on either system, so perhaps whatever was causing the warning earlier has been addressed by a later change.

Feel free to ignore this PR if your experience is different, just wanted to share!

tazz4843 commented 1 year ago

I believe this triggers a warning with “cargo publish”, as it complains files outside the build directory are modified. I’m not able to test at the moment, but will check.

tazz4843 commented 1 year ago

It appears since upstream added it, cargo does not check the contents of the file have not been modified, and cargo publish no longer complains.

tazz4843 commented 1 year ago

Unfortunately, it turns out cargo publish does actually still complain: verification is not performed until a full publish is done.

error: failed to verify package tarball

Caused by:
  Source directory was modified by build.rs during cargo publish. Build scripts should not modify anything outside of OUT_DIR.
  Added: /home/zero/CLionProjects/whisper-rs/target/package/whisper-rs-sys-0.6.1/whisper.cpp/bindings/javascript/package.json

  To proceed despite this, pass the `--no-verify` flag.

Reverting this commit fixes the issue.