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

bring the ffi module to 3.2.x #852

Merged
merged 9 commits into from
May 10, 2024

Conversation

cdisselkoen
Copy link
Contributor

Description of changes

main has a bunch of great FFI changes that are scheduled for release in 4.0, but some users would like a 3.x-compatible version of cedar-wasm. Obviously, changes to the structs in the cedar_policy::frontend module are breaking, so we can’t backport them to 3.x. But, since we are planning to rename frontend to ffi in 4.0 anyway, this PR proposes release the new FFI interface in the ffi module on 3.x, so that 3.2+ will have both the frontend and ffi modules and consumers can use either. Existing users of frontend are not broken as there are no changes in the frontend module, and since cedar-wasm has not been released yet, there’s no harm in having it be based on the ffi interface even on 3.x.

Issue #, if available

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

  • A backwards-compatible change requiring a minor version bump to cedar-policy (e.g., addition of a new API).

I confirm that this PR (choose one, and delete the other options):

  • Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).

I confirm that cedar-spec (choose one, and delete the other options):

  • Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.

Signed-off-by: Craig Disselkoen <[email protected]>
Signed-off-by: Craig Disselkoen <[email protected]>
@cdisselkoen
Copy link
Contributor Author

Apparently I don't have enough #[allow(deprecated)] already, for the frontend's usage of its own types and functions

@cdisselkoen
Copy link
Contributor Author

Also worth noting that this doesn't include the changes in #837 because #837 is not merged yet. If this gets merged before #837, we'll backport #837 separately. If #837 gets merged first, I'll adapt this PR.

Copy link
Contributor

@khieta khieta left a comment

Choose a reason for hiding this comment

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

PR looks fine (pending resolution of CI failures), but it occurs to me that we never actually moved any cedar-wasm/ code back to the 3.2 release branch. Are you planning on doing that next?

cedar-testing/src/cedar_test_impl.rs Outdated Show resolved Hide resolved
cedar-policy/CHANGELOG.md Outdated Show resolved Hide resolved
@cdisselkoen
Copy link
Contributor Author

Yes, I'm planning on bringing cedar-wasm in a separate PR

Signed-off-by: Craig Disselkoen <[email protected]>
@cdisselkoen
Copy link
Contributor Author

It seems these warnings are not silenceable; upstream Rust issue rust-lang/rust#47238. Since we deny warnings, we'll need a workaround here.

Signed-off-by: Craig Disselkoen <[email protected]>
Signed-off-by: Craig Disselkoen <[email protected]>
@cdisselkoen
Copy link
Contributor Author

Alright, the workaround of never-denying-on-deprecated-warnings in CI does seem to work.

@john-h-kastner-aws
Copy link
Contributor

john-h-kastner-aws commented May 9, 2024

FYI: Java CI failures are expected pending cedar-policy/cedar-java#141, but the cedar-drt CI should pass

edit: java fix also needs #855

@cdisselkoen cdisselkoen merged commit f920a26 into release/3.2.x May 10, 2024
15 of 16 checks passed
@cdisselkoen cdisselkoen deleted the cdisselkoen/ffi-module-in-3.2.x branch May 10, 2024 14:43
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.

5 participants