Skip to content
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

Use bindgen on docs.rs #426

Merged

Conversation

yutannihilation
Copy link
Contributor

Currently, the build on docs.rs fails with the following error (ref: build log). This is because the precomputed bindings for the version of R that docs.rs uses, R 3.6, doesn't contain the graphics-related objects because the file was generated long ago.

We have several options to avoid this error (see #415 (comment)), but I believe this is the best; we cannot expect what version of R docs.rs will use, but using bindgen should make the build success with arbitrary version.

[INFO] [stderr]  Documenting extendr-api v0.3.1 (/opt/rustwide/workdir)
[INFO] [stderr] error[E0432]: unresolved imports `libR_sys::DevDesc`, `libR_sys::R_GE_gcontext`
[INFO] [stderr]   --> src/graphics/mod.rs:83:20
[INFO] [stderr]    |
[INFO] [stderr] 83 | pub use libR_sys::{DevDesc, R_GE_gcontext};
[INFO] [stderr]    |                    ^^^^^^^  ^^^^^^^^^^^^^ no `R_GE_gcontext` in the root
[INFO] [stderr]    |                    |
[INFO] [stderr]    |                    no `DevDesc` in the root
[INFO] [stderr] 
[INFO] [stderr] error[E0412]: cannot find type `DevDesc` in this scope
[INFO] [stderr]   --> src/graphics/device_driver.rs:82:32
[INFO] [stderr]    |
[INFO] [stderr] 82 |     fn activate(&mut self, dd: DevDesc) {}
[INFO] [stderr]    |                                ^^^^^^^ not found in this scope
[INFO] [stderr] 
[INFO] [stderr] error[E0412]: cannot find type `R_GE_gcontext` in this scope
...snip...

@Ilia-Kosenkov
Copy link
Member

Is there anything else here that needs to be addressed? Or should we review and merge already?

@yutannihilation
Copy link
Contributor Author

First of all, can we agree on this direction? As commented on #415, there are many possible solutions.

Copy link
Member

@Ilia-Kosenkov Ilia-Kosenkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we seemed to agree to use bindgen, let's merge this PR.

@yutannihilation
Copy link
Contributor Author

Thanks!

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.

None yet

2 participants