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

Android: Bump ndk/ndk-glue version #2047

Merged
merged 2 commits into from
Nov 2, 2021

Conversation

msiglreith
Copy link
Member

@msiglreith msiglreith commented Nov 1, 2021

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

Tested with a small sample application that it still runs with the updated ndk version.
This is breaking as users need to update their version of ndk-glue as well as it internally has a static global variable which should be unique across crates!

#1995

@msiglreith msiglreith mentioned this pull request Nov 1, 2021
9 tasks
@@ -125,6 +126,6 @@ To ensure compatibility with older MacOS systems, winit links to
CGDisplayCreateUUIDFromDisplayID through the CoreGraphics framework.
However, under certain setups this function is only available to be linked
through the newer ColorSync framework. So, winit provides the
`WINIT_LINK_COLORSYNC` environment variable which can be set to `1` or `true`
`WINIT_LINK_COLORSYNC` environment variable which can be set to `1` or `true`
Copy link
Member

@maroider maroider Nov 2, 2021

Choose a reason for hiding this comment

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

This isn't strictly speaking related to the ndk in any way, but trailing whitespace also annoys me...

Copy link
Member

@maroider maroider left a comment

Choose a reason for hiding this comment

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

This change seems simple enough, and CI passes, so that's nice.
If there isn't anything wrong with ndk = "0.4.0", then I don't think there's much else to check.

Is the omission of a re-export (as requested in #1993/#1995) intentional?

@msiglreith
Copy link
Member Author

Is the omission of a re-export (as requested in #1993/#1995) intentional?

Right I missed that part. I'm experimenting with it a bit and not sure yet - adding features (e.g logger) for transitive depenencies is not supported, requiring us to re-export it somehow (not sure how tho with target specific features) or the user needs respecify the ndk-glue dependency with the requested feature themself. Another part is requiring to override the path to the ndk-glue to make the macro work (see the ndk_glue = ".." part).

#[cfg_attr(target_os = "android", ndk_glue::main(backtrace = "on", ndk_glue = "winit::platform::android::ndk_glue"))]
fn main() {
   ...
}

Overall I would probably keep it out for now. Maybe somehow has a suggestion how properly export it. The above attempts doesn't look so user friendly.

@msiglreith msiglreith merged commit 5f4df54 into rust-windowing:master Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants