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

Support iOS #41

Merged
merged 1 commit into from
Jul 23, 2022
Merged

Support iOS #41

merged 1 commit into from
Jul 23, 2022

Conversation

Kijewski
Copy link
Collaborator

@Kijewski
Copy link
Collaborator Author

I haven't tested this PR, not having an iOS device. @appaquet, can you please have a look if that fixes the problem for you?

@appaquet
Copy link

Your branch now compiles when targeting iOS. Could be a good idea to add MacOS and iOS checks in the GitHub actions flow! I didn't test the functionally though, but since it's using core foundation for both iOS and MacOS, it should be fine in both sets of targets.

@Kijewski
Copy link
Collaborator Author

Kijewski commented Jul 22, 2022

Would you be able to test if it actually works? I don't have any iOS devices, so I can't. I guess we could simply assume it works, if we could actually test it, well, that would be better. :)

No iOS target is supported by cross: https://github.com/strawlab/iana-time-zone/runs/7472202395?check_suite_focus=true, so I don't know how I could add a test. If you do, please tell.

@appaquet
Copy link

I'll try to test this later in my project.

Yeah, you can't really cross compile to iOS with cross since it needs the toolchains to do so, which are available on MacOS with Xcode, but you can't redistribute them. You can actually compile for iOS on GitHub actions though (they have macos runners), but you won't be able to run your tests since you need to wrap it into an application to run it in a simulator.

@Kijewski
Copy link
Collaborator Author

Ah, thanks for the information. It does cross compile fine on a macOS-12 runner.

@astraw
Copy link
Member

astraw commented Jul 22, 2022

The tests run on macOS, including automatically by Github Actions. The code is the same for iOS and macOS, so I guess it works on iOS as long as it compiles but I haven't tested it.

This PR adds a new variant to the GetTimezoneError enum, which will be a breaking change. Is it worth it? I'm not convinced. Also I think I prefer the compile-time error rather than the run-time error when my platform is not supported. Perhaps a middle ground is that the relevant function is simply not defined, which will result in a compile time error but only if the function is called.

@Kijewski
Copy link
Collaborator Author

Kijewski commented Jul 22, 2022

You're right. I removed the new variant.

Perhaps a middle ground is that the relevant function is simply not defined, which will result in a compile time error but only if the function is called.

How can you do that?

@astraw
Copy link
Member

astraw commented Jul 22, 2022

How can you do that?

Ahh, I mis-remembered the old behavior and just played with it now. It doesn't behave like I thought, so please ignore this part of my comment.

@Kijewski Kijewski force-pushed the pr-ios branch 3 times, most recently from 9968952 to 211c4d0 Compare July 22, 2022 17:25
@Kijewski Kijewski changed the title Support iOS, don't compile error if unsupported Support iOS Jul 22, 2022
@Kijewski
Copy link
Collaborator Author

I restored the old behavior, so it still compile_errors.

@appaquet
Copy link

@Kijewski As expected, just tested in an iOS sim on aarch64-apple-ios-sim and it works.

@Kijewski
Copy link
Collaborator Author

Thank you for testing it, and reporting the issue!

@astraw astraw merged commit 49d67f6 into strawlab:main Jul 23, 2022
@astraw
Copy link
Member

astraw commented Jul 23, 2022

published in release 0.1.37 . Thanks all.

@Kijewski Kijewski deleted the pr-ios branch July 23, 2022 20:21
@lopopolo lopopolo added the Tier-2 Rust Tier-2 platform label Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tier-2 Rust Tier-2 platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants