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

Extract cbindgen'd interface into its own crate #8013

Merged
merged 2 commits into from Jul 10, 2019

Conversation

@illicitonion
Copy link
Contributor

commented Jul 4, 2019

This means that we compile engine before we try running cbindgen over
it, so we get useful parse errors out of rustc, rather than errors
manifesting themselves as cbindgen going "There was a parse error
somewhere!"

I think ideally I would've moved the engine crate out into a new
subcrate, but this way around better preserves git history and avoids
merge conflicts.

@blorente
Copy link
Contributor

left a comment

Cool, thanks!

@jsirois

jsirois approved these changes Jul 6, 2019

@stuhood

stuhood approved these changes Jul 9, 2019

Copy link
Member

left a comment

Awesome.

# We need to parse the engine crate to codegen PyResult (because externs are both something we use
# from Python in engine, and something we expose to python in engine_cffi).
#
# We need to parse the logging crate to codegen PythonLogLevel and Destination.

This comment has been minimized.

Copy link
@stuhood

stuhood Jul 9, 2019

Member

This should mention the list in engine_cffi/build.rs, and vice-versa.

@cosmicexplorer
Copy link
Contributor

left a comment

I don't have as much time as I'd want to right now to go over this, but I'm extremely excited about this change, and I believe it can close #8009! Thank you so much!

I think ideally I would've moved the engine crate out into a new
subcrate, but this way around better preserves git history and avoids
merge conflicts.

I think I agree with this, but I also feel like separating the ffi interface specifically into a subcrate makes a lot of sense by itself as an incremental step. I'd love to have an interface to other languages at some point.

Extract cbindgen'd interface into its own crate
This means that we compile engine before we try running cbindgen over
it, so we get useful parse errors out of rustc, rather than errors
manifesting themselves as cbindgen going "There was a parse error
somewhere!"

I think ideally I would've moved the engine crate out into a new
subcrate, but this way around better preserves git history and avoids
merge conflicts.

@illicitonion illicitonion force-pushed the twitter:dwagnerhall/cbindgen branch from ed24aa2 to 4720f06 Jul 10, 2019

@illicitonion illicitonion merged commit bb97011 into pantsbuild:master Jul 10, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@illicitonion illicitonion deleted the twitter:dwagnerhall/cbindgen branch Jul 10, 2019

illicitonion added a commit that referenced this pull request Jul 10, 2019

Extract cbindgen'd interface into its own crate (#8013)
This means that we compile engine before we try running cbindgen over
it, so we get useful parse errors out of rustc, rather than errors
manifesting themselves as cbindgen going "There was a parse error
somewhere!"

I think ideally I would've moved the engine crate out into a new
subcrate, but this way around better preserves git history and avoids
merge conflicts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.