Skip to content

Modify tensorflow-sys/src/lib.rs to be bindgen generated #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 11, 2017

Conversation

nbigaouette-eai
Copy link
Contributor

@nbigaouette-eai nbigaouette-eai commented Mar 8, 2017

This MR gets rid of the hand-written tensorflow-sys/src/lib.rs file, and replaces it with two files:

  1. tensorflow-sys/src/bindgen.rs is the result of bindgen --blacklist-type max_align_t /usr/include/tensorflow/c_api.h > tensorflow-sys/src/bindgen.rs;
  2. tensorflow-sys/src/lib.rs simply include!() the generated wrapper, adding some #![allow(...)] and re-exporting the enum values.

I've added the option --blacklist-type max_align_t to bindgen so that the automatically generated unit tests don't fail:

     Running `/home/nbigaouette/projects/agents/tensorflow_rust.git/target/debug/deps/tensorflow_sys-b18d6c19e08d67bd`

running 5 tests
test bindgen_test_layout_TF_AttrMetadata ... ok
test bindgen_test_layout_TF_Buffer ... ok
test bindgen_test_layout_TF_Output ... ok
test bindgen_test_layout_TF_Input ... ok
test bindgen_test_layout_max_align_t ... FAILED

failures:

---- bindgen_test_layout_max_align_t stdout ----
        thread 'bindgen_test_layout_max_align_t' panicked at 'assertion failed: `(left == right)` (left: `24`, right: `32`): Size of: max_align_t', src/bindgen.rs:82
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    bindgen_test_layout_max_align_t

test result: FAILED. 4 passed; 1 failed; 0 ignored; 0 measured

error: test failed

See rust-lang/rust-bindgen#550 for the bindgen issue about this unit test.

Should close issue #57.

Version of bindgen used: 0.22.1

Command used:
bindgen --blacklist-type max_align_t /usr/include/tensorflow/c_api.h > src/bindgen.rs
Note that the enums values are rexported (`pub use TF_[...]::*`)
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@nbigaouette-eai
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@nbigaouette
Copy link

nbigaouette commented Mar 9, 2017

The CI failure is due to tensorflow failing to build and not this MR.

Server finished RPC without an explicit exit code
thread 'main' panicked at 'failed to execute "bazel" "build" "--jobs=2" "--compilation_mode=opt" "--copt=-march=native" "tensorflow:libtensorflow.so"', tensorflow-sys/build.rs:101
note: Run with `RUST_BACKTRACE=1` for a backtrace.

See similar issues in the main tensorflow repository:

MR #65 could aleviate this problem by using a pre-built binary for tensoflow. This would help a lot!

@adamcrume
Copy link
Contributor

Could you add a script for running bindgen?

@adamcrume adamcrume merged commit 022f4d0 into tensorflow:master Mar 11, 2017
@adamcrume
Copy link
Contributor

Thanks!

@nbigaouette-eai nbigaouette-eai deleted the bindgen branch March 13, 2017 00:18
@daschl
Copy link
Contributor

daschl commented Mar 15, 2017

I think this is a great middle ground between hand-generating the API and forcing couple of compile time dependencies on the user :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants