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

Generate bindings for functions provided by the cshim using bindgen. #818

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

thomcc
Copy link
Contributor

@thomcc thomcc commented Oct 24, 2022

Depends on #813, so ignore the 3 patches that come from that.

Need to think a little bit about the approach here, since this is kinda hacky. It might be better done after #798, but I'm not 100% sure yet.

include_h.push(format!("pg{}.h", major_version));

let bindgen_output = run_bindgen(&pg_config, &include_h)
let header_src = format!(
Copy link
Contributor

Choose a reason for hiding this comment

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

cute.

I wonder if our c-shim should be last? It probably doesn't matter, but I guess bindgen would start by picking up its #includes first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if our c-shim should be last?

I did it first in order to catch any cases where it is using files without including them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Admittedly we'd probably catch that when building the cshim, so I don't mind putting it second really.

@thomcc
Copy link
Contributor Author

thomcc commented Oct 24, 2022

Not sure why it passes locally but it seems that this needs to have the path to the cshim added to the include when building the cshim (this is more a note for future me than anybody else). That's not hard, I'll do that in a bit (this will need rebasing anyway after #813 lands).

@thomcc thomcc force-pushed the shim-header branch 2 times, most recently from 6343504 to 1bd30b9 Compare October 26, 2022 18:21
@thomcc
Copy link
Contributor Author

thomcc commented Oct 26, 2022

I'm going to say we can't actually do this one until we drop pg10 support, due to the bool differences.

@workingjubilee
Copy link
Member

Blocked on:

@BradyBonnette
Copy link
Contributor

@workingjubilee @thomcc #830 has been merged into develop.

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

Successfully merging this pull request may close these issues.

4 participants