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

Revert non-working fix aiming to support docs.rs #104

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vectronic
Copy link
Contributor

I originally provided a PR #82 (merged) for this issue: #72

I have since discovered more about Rust compilation and procedural macros and realised my "fix" doesn't work as per this: rust-lang/docs.rs#1823

The two issues with attempting to use OUT_DIR:

  • OUT_DIR is not defined when running any type of cargo build except when a build.rs exists.
  • if OUT_DIR is defined (because a build.rs exists), the deno_bindgen_macro will output to ${OUT_DIR}/bindings.json as expected, but the cli.ts does not have OUT_DIR defined and will default to looking for bindings.json in the current working folder.

I also see that my "fix" introduced another issue which was fixed here: #89

After learning more and understanding more... I humbly submit a PR which:

On my journey, I have also discovered that the issue of procedural macros having side-effects has been discussed elsewhere: rust-lang/cargo#9084

It would seem this issue is a general issue with using procedural macros for outputting auxiliary data and out of my league in terms of resolving.

I am not sure what the solution is, but for now I will have to put up with the fact that when my project makes use of deno_bindgen I can't refer to the generated Rust docs on docs.rs...

@zifeo
Copy link
Contributor

zifeo commented Sep 20, 2022

@vectronic Would not it be better to keep the environment variables but having it freed from cargo system?

@vectronic
Copy link
Contributor Author

@zifeo thanks for the response. The issue I have is that there doesn't seem to be anyway to specify env vars when a build (for docs) is performed on docs.rs.

The only way possibly to go down that route would be to use env vars that OPT IN to the bindings.json being produced. This would make the bindgen_macro a 'no-op' by default and things would work on docs.rs.

Then the deno_bindgen CLI could potentially spawn the cargo build with the env var set when it is desired to output bindings.json In this scenario the env-var could just be used as an 'emit' flag and output to the current project root as it does now.

This therefore creates a breaking change from the point of view of the Rust code, but a non-breaking change from the point of view of the Deno code....

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