Null safety (cherry-pick from upstream)

[m0nac0]

Fixes #16

Switch to null safe (NNBD) mode, mostly cherry-picked and adapted from upstream (tobrun/flutter-mapbox-gl). Includes two more recent bug fixes from upstream that seem to be related.

[m0nac0]

I haven't found any regressions, feedback and results of your manual testing are very welcome!

[kuhnroyal]

This is huge, I had a couple things laying around but didn't find the time to push it further. I will see if i can test this tomorrow.

[kuhnroyal]

I am running into fatal error: module 'Mapbox' not found in https://github.com/m0nac0/maplibre-annotation-extension/blob/master/MapboxAnnotationExtension/Annotations/MGLStyleAnnotation.h
Any idea what I can change in my setup?

[m0nac0]

Are you compiling the example app?

[m0nac0]

And have you run pod update, as the CI does? If I remember correctly, that was how @mindthefish solved this issue in #9.

[kuhnroyal]

Will, I am trying to use it in my app which is now possible due to the NNBD migration.
I am trying to mirror the example and added both pods to my Podfile, like in the example.
I also ran pod update which downloaded the git repos.

[m0nac0]

Are you also running pod update in the same folder as the CI? I unfortunately don't know that much about iOS development, but the error is exactly the same I was seeing before #9 was merged.

[kuhnroyal]

Hmm somehow the Mapbox framework is still in there, don't think this is something that we want.

[kuhnroyal]

Ok, I am starting to understand this a little better. Managed to get it to work on this branch. Looking good so far.

[m0nac0]

May I ask what you did to solve this?

A wild guess: could this file https://github.com/m0nac0/maplibre-annotation-extension/blob/master/Podfile.lock be the reason the Mapbox SDK is/was wrongfully loaded?

[kuhnroyal]

There are 2 things that were confusing to me.

1. The xcframework in MapLibre-native is still called Mapbox in 5.12.1 (I didn't see this before) - that is why the name change was reverted in m0nac0/maplibre-annotation-extension@0a24c3a
2. Due to the Git dependency in the Podfile you always need to run pod update AND afterwards pod install or it won't work

And yes I am wondering the same, not sure if the Podfile.lock should even be committed there.

[mindthefish]

There are 2 things that were confusing to me.

1. The xcframework in MapLibre-native is still called Mapbox in 5.12.1 (I didn't see this before) - that is why the name change was reverted in m0nac0/maplibre-annotation-extension@0a24c3a
2. Due to the Git dependency in the Podfile you always need to run pod update AND afterwards pod install or it won't work

And yes I am wondering the same, not sure if the Podfile.lock should even be committed there.

Yes, pod install has to be executed after pod update, although the output says it is running pod install after update automatically.
Regarding the Podfile.lock: Is it best practice to leave it out of the repository?

[kuhnroyal]

Hmm I don't think it matters, the Podfile should only be used for the example app in that repo. We are only interest in the .podspec when using it as dependency.

[mindthefish]

I was able to run the examples of that branch on an iPad Pro running iOS 14.7.1 and will test on 15.0.2 on Monday. Not done with the review though.

[mindthefish]

Able to compile and run on iPadOS 15.0.2. Crashes when I want to download offline maps.

Region download progress 0.0
libc++abi: terminating with uncaught exception of type std::__1::regex_error
* thread #27, name = 'com.mapbox.mbgl.DatabaseFileSource', stop reason = signal SIGABRT
    frame #0: 0x00000001bb8e79c4 libsystem_kernel.dylib`__pthread_kill + 8
libsystem_kernel.dylib`__pthread_kill:
->  0x1bb8e79c4 <+8>:  b.lo   0x1bb8e79e4               ; <+40>
    0x1bb8e79c8 <+12>: pacibsp
    0x1bb8e79cc <+16>: stp    x29, x30, [sp, #-0x10]!
    0x1bb8e79d0 <+20>: mov    x29, sp
Target 0: (Runner) stopped.
Lost connection to device.
Exited (sigterm)

[m0nac0]

@mindthefish Have you checked whether that crash also happens before this PR?

[mindthefish]

Just tested, yes, it also happens on main. It is not caused by this PR.

[m0nac0]

Thank you for testing, would you mind opening a separate issue for the crash then?

[m0nac0]

Would anyone like to give more feedback on this? Otherwise I'd merge this.