-
Notifications
You must be signed in to change notification settings - Fork 501
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
Mac support #1393
Mac support #1393
Conversation
We cannot rename the ios-driver artifact. The best we can do is make it empty and depend on a shared native-driver. |
Okay, that was not brought up on the windows pull request. I'll see what I can do about that requirement |
Any update? Want me to take over? |
Sorry, I've been basically waiting for the kotlin native cocapods plugin to support swiftui desktop before fixing this up. Probably can't work on this til sometime next week. So go ahead if you want to. |
Curious question: why is this blocked on swiftui? |
@saket it's not. Just trying to say that if my swiftUI app would actually build targeting desktop it would encourage me to fix up this pull request faster. Sorry, I can try and make the requested changes soon. |
Okay, I added the ios-driver artifact back in. I also added a InvalidMutabilityException workaround for the LogSqliteDriverTest. Not exactly sure if using AtomicReference is the best way to fix that. And I tried adding a native test driver for coroutines-extensions... but I'm running into more InvalidMutabilityException's there. like: query[macosX64]
Is this a problem with sqldelight or just the testing code? |
the native driver changes have been merged to master, @luca992 would you be able to rebase this pr against master? |
@alecStrong yeah sure, no problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, although the diff is hard to read because its including a bunch of commits that are on master, if you do git rebase -i master
and manually drop
commits that are already on master from your branch, does that fix it?
drivers/ios-driver/build.gradle
Outdated
@@ -4,25 +4,7 @@ kotlin { | |||
sourceSets { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we decided against having an ios-driver artifact and just doing the switch to native-driver, so this module can be removed entirely
fc0c7e8
to
9c1b39c
Compare
Rebased @DanielRBaird pull request onto master's latest changes and add macosX64 as a target for the native driver
Looks like the
transaction logs are correct
test is failing withkotlin.native.concurrent.InvalidMutabilityException: mutation attempt of frozen kotlin.Array@68c20b68
... but as far as I can see that same test is not being run on ios targets? so that might also might be an issue for ios?