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

Upgrade bindgen. #53

Merged
merged 3 commits into from
Jan 5, 2021
Merged

Upgrade bindgen. #53

merged 3 commits into from
Jan 5, 2021

Conversation

frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Dec 9, 2020

This was needed to get things compiling on Apple Silicon.

https://github.com/rust-lang/rust-bindgen/blob/master/CHANGELOG.md

This was needed to get things compiling on Apple Silicon.
src/proj.rs Outdated
err = proj_errno(self.c_proj);
},
Transformation::Projection => unsafe {
proj_errno_reset(self.c_proj);
trans = proj_trans_array(self.c_proj, inv, pj.len(), pj.as_mut_ptr());
trans = proj_trans_array(self.c_proj, inv, pj_len, pj.as_mut_ptr());
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@michaelkirk michaelkirk Dec 10, 2020

Choose a reason for hiding this comment

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

Won't this break against our prebuilt bindings which expect a usize?

I admit - I'm kind of surprised by the changes made to bindgen.

Copy link
Member

Choose a reason for hiding this comment

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

I suppose we can just re-generate all the prebuilt bindings with the new bindgen.

Copy link
Member

Choose a reason for hiding this comment

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

🤦 Oh yeah, I never did #44, so we don't have any prebuilt bindings. 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

I admit - I'm kind of surprised by the changes made to bindgen.

there appears to be an ongoing discussions about that change rust-lang/rust-bindgen#1901

@urschrei
Copy link
Member

urschrei commented Dec 9, 2020

Bors try

bors bot added a commit that referenced this pull request Dec 9, 2020
@frewsxcv
Copy link
Member Author

frewsxcv commented Dec 10, 2020

Yay tests pass on CI (still not locally)

@michaelkirk
Copy link
Member

michaelkirk commented Dec 10, 2020

Yay tests pass on CI (still not locally)

Hrmm... could you paste the bindings.rs that are being generated and the error you're seeing? I'm super invested in the future of size_t vs. bindgen vs M1 now 😅

(reminder to self: we'll probably want to update bindings_docs-rs.rs whenever we update bindgen in case the output changes other semantics. I think this is just a manual process of copying bindings.rs from the target/*/proj-sys*/ directory)

@urschrei
Copy link
Member

I think this is just a manual process of copying bindings.rs from the target//proj-sys/ directory)

Yep, it was a very sophisticated workaround…

@frewsxcv
Copy link
Member Author

Just double checking, are there any outstanding questions here? I opted in to .size_t_is_usize(true), so there's minimal changes in the pull request

@michaelkirk
Copy link
Member

Hrmm... could you paste the bindings.rs that are being generated and the error you're seeing? I'm super invested in the future of size_t vs. bindgen vs M1 now 😅

LGTM!

I'm still curious as to what specific failure you were seeing on your M1 and would love to peep the bindings if you still somehow have them.

@frewsxcv
Copy link
Member Author

bors r=michaelkirk

@frewsxcv
Copy link
Member Author

Hrmm... could you paste the bindings.rs that are being generated and the error you're seeing? I'm super invested in the future of size_t vs. bindgen vs M1 now 😅

LGTM!

I'm still curious as to what specific failure you were seeing on your M1 and would love to peep the bindings if you still somehow have them.

I don't unfortunately :-/

bors bot added a commit that referenced this pull request Dec 14, 2020
53: Upgrade bindgen. r=michaelkirk a=frewsxcv

This was needed to get things compiling on Apple Silicon.

https://github.com/rust-lang/rust-bindgen/blob/master/CHANGELOG.md

Co-authored-by: Corey Farwell <[email protected]>
@bors
Copy link
Contributor

bors bot commented Dec 14, 2020

try

Timed out.

@frewsxcv
Copy link
Member Author

bors retry

@bors
Copy link
Contributor

bors bot commented Dec 14, 2020

Already running a review

@bors
Copy link
Contributor

bors bot commented Dec 14, 2020

Build failed:

  • ci result

@michaelkirk
Copy link
Member

huh. I reran the action and CI passed. I fear for our flapping CI.

bors retry

@bors
Copy link
Contributor

bors bot commented Jan 5, 2021

Build succeeded:

@bors bors bot merged commit 172aac2 into master Jan 5, 2021
@frewsxcv frewsxcv deleted the bumpbindgen branch January 16, 2021 23:29
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.

3 participants