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 objc2-core-foundation #1461

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Jan 26, 2025

Use objc2-core-foundation, which is a (mostly) auto-generated version of core-foundation-sys.

This avoids error-prone use of CFReleaser::new (called CFRetained::from_raw in objc2-core-foundation) in many cases, since objc2-core-foundation handles that correctly according to the memory management rules. Additionally, we use the CFString conversions in objc2-core-foundation to avoid re-implementing them in this crate.

@madsmtm madsmtm force-pushed the objc2-core-foundation branch 3 times, most recently from 7ebc292 to 5615c75 Compare January 27, 2025 00:12
Cargo.toml Outdated Show resolved Hide resolved
@GuillaumeGomez
Copy link
Owner

Ah also CI failed because of:

error: unused import: `std::ptr::NonNull`
  --> src/unix/apple/macos/ffi.rs:27:5
   |
27 | use std::ptr::NonNull;
   |     ^^^^^^^^^^^^^^^^^

I'm surprised by the changes in the FFI definitions but if it works, then all good for me, thanks!

@madsmtm madsmtm force-pushed the objc2-core-foundation branch from 5615c75 to 0370948 Compare January 27, 2025 14:44
@madsmtm
Copy link
Contributor Author

madsmtm commented Jan 27, 2025

I think I've fix the CI error now, tested each feature locally.

@GuillaumeGomez
Copy link
Owner

Well I guess we'll see soon enough. 😉

Thanks a lot! I'll merge once CI is done.

@GuillaumeGomez GuillaumeGomez merged commit 64f2157 into GuillaumeGomez:master Jan 27, 2025
66 of 67 checks passed
@madsmtm madsmtm deleted the objc2-core-foundation branch January 27, 2025 15:32
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.

2 participants