Conversation
|
|
||
| - `maplibre-native-core` or `maplibre-native-renderer` for the core functionality. | ||
| - SDKs: | ||
| - `maplibre-native-sdk-apple` (iOS and macOS, potentially merged with `maplibre-navigation-ios`) |
There was a problem hiding this comment.
The package managers on Apple platforms are oriented around Git(Hub) repositories. Merging the map SDK and navigation SDK repositories effectively means eliminating the map SDK in favor of a monolithic navigation SDK unless significant workarounds are put in place.
There was a problem hiding this comment.
OK, will remove this.
|
|
||
| 3. The proposal is implemented for the core. | ||
|
|
||
| 4. SDK repositories are created on by one as needed and they migrate to use `maplibre-native-core`. |
There was a problem hiding this comment.
Will any new repository keep the commit history or start over from scratch? There are a number of tools such as git-filter-repo for creating a new repository that contains just the history relevant to a given path.
There was a problem hiding this comment.
Splitting out a separate repository for platform-specific code has several benefits that only apply if commit history outside the platform/*/ directory gets filtered out:
- It’ll be a lot easier to find platform-specific changes without noise from other platforms or core.
- Contributors to core would be less likely to make unnecessary changes to platform code, such as in Replace
#pragma - markwith comments #736.1 - Contributors who primarily work on platform-specific code can get away with only making a shallow clone of the main gl-native repository, avoiding a lot of redundant disk space usage.
- The maplibre-gl-native-distribution repository can be folded back into a maplibre-gl-native-ios repository, simplifying the release process, improving discovery by downstream developers, and simplifying the installation steps.
- Source-based distribution would be infeasible in a monorepo or even a separate repository containing the monorepo’s history, because downstream developers would have to endure a ton of unnecessary cloning. This would be especially problematic with SPM, which tends to reclone the repository quite often and disables Xcode functionality until it’s done cloning.
- One platform’s SDK could more easily pull in dependencies using the platform’s usual package managers instead of relying on the lowest common denominator of package managers, Git submodules.
Unfortunately, it looks like history has already been broken more than once for a large number of platform-specific files. Git can only track history across file moves, but not manual copy-pasting across separate working copies. This breakage is only a minor annoyance if the repository remains a monorepo forever, but I think it would be prudent not to make that assumption. If platform-specific code is ever split back out into a separate repository at some point in the future, the filtering I’m proposing here would actually cause history to be lost.
Footnotes
-
No one uses GCC to compile AppKit and UIKit code these days. It’s completely unnecessary to make the Objective-C coding style conform to GCC’s C/C++ warnings. This case of repository-wide find-and-replace was harmless, but there may be other gotchas in the future. ↩
There was a problem hiding this comment.
Would you be open to the idea of starting over, so to speak?
The platform/ios/ directory was copied over from a separate mapbox-gl-native-ios repository, presumably at the last commit before the license changed. (It would be good to confirm that.) Not too many commits have touched this directory since, only a hundred or so, and many were only necessary in order to make the code build in a monorepo again.
Consider a workflow along these lines:
- Create a new maplibre-gl-native-ios repository (or whatever you’d like to call it). Move aside the main branch.
- Add mapbox-gl-native-ios as a remote.
- Check out the last commit of mapbox-gl-native-ios that was permissively licensed, as well as any ios-* and macos-* release tags up to that point.
- Create a new main branch from that commit.
- Filter the repository to only include platform/{darwin,ios,macos}/ and certain files in the root directory, like .gitmodules, Cartfile, and LICENSE.md.
- Add maplibre-gl-native as a remote.
- Cherry-pick relevant commits from platform/ios/ in maplibre-gl-native into the new branch. Git should be able to automatically resolve discrepancies like the redundant platform/ios/ in maplibre-gl-native.
- If cherry-picking would be too time-consuming, rebasing would work too, but you’ll have to undo any monorepo-specific changes that would now be counterproductive.
- Push the tags and copy over any ios-* and macos-* releases you care about.
- Delete platform/ios/ and any ios-* and macos-* tags and releases from maplibre-gl-native.
You could clean up the submodules afterwards. It’d be a lot easier than cleaning up the history of .gitmodules in a massive interactive rebase.
There was a problem hiding this comment.
Picking up on this. Should this be added to the proposal or is it out of scope here?
There was a problem hiding this comment.
Thank you for this design proposal! Quite ambitious, but doable and of course neccesary in the long term. I think the devil is in the details with these things. Perhaps instead of agreeing on a code re-organisation design proposal and going for it, we instead could launch a code re-organisation experiment, which we make final if it succeeds (you mentioned @birkskyum tried some things already).
Whatever we decide, we should maintain operational readiness while we complete the process of code re-organisation. For example, we should still be able to push out releases with critical bug fixes during that time.
| - `platform/qt` | ||
| - `test`: common (headless) tests for all platforms; disabled only in exceptional cases if some features are not applicable due to the nature of a specific platform | ||
|
|
||
| The core itself ideally should have no binaries unless they are really platform-independent (e.g. headless rendering). |
There was a problem hiding this comment.
tests for the core at test, and the platform test at platform test like platform/apple/ios/test, platform/apple/macOS/test?
There was a problem hiding this comment.
Tests can be binary, of course, but they will not be distributed with the library.
Ideally there should be no platform-specific tests.
There was a problem hiding this comment.
Ideally there should be no platform-specific tests.
As long as platform-specific is being distributed, that’s surface area that should be tested by platform-specific tests. That said, there doesn’t need to be much if any platform-specific testing of core code.
There was a problem hiding this comment.
Could we introduce a little specificity here? As in how will we organize the unit tests?
There was a problem hiding this comment.
I added a few points:
- platform specific tests should go under platform
- sdk specific tests should not be in the core
| - `maplibre-native-sdk-qt` (Qt bindings, should probably be merged with `qt-geoservices-maplibre-gl`) | ||
| - `maplibre-native-sdk-js` | ||
|
|
||
| Alternatively SDKs could be without the explicit `sdk` in the name. |
There was a problem hiding this comment.
I am definitely in favor of dropping the sdk part.
| - Node.js Binaries and SDK | ||
| - GLFW SDK | ||
|
|
||
| Navigation SDKs could then be merged with respective platform SDK. |
There was a problem hiding this comment.
In favor of keeping them separate.
|
|
||
| ### Release cycle | ||
|
|
||
| The core should have an independent release cycle. At this point we imagine SDKs to be much more actively developed. This also allows bigger refactoring to take place without affecting SDKs (e.g. Metal migration). |
There was a problem hiding this comment.
Correct me if I am wrong, but if I understand you correctly, what you are pointing out here is that we can make a big changes to the core and then each individual platform can adopt those changes at their own pace at a later time. I agree there is a benefit to this, but at the same time I think that core changes have no benefit on their own if the supported platforms are not modified alongside those changes. The big benefit right now is that if you make a change that breaks a platform in the core, we know it immediately because CI will fail (in theory). Updating a platform that is pinned to a specific core version will potentially be much harder with a indepdenent release cycle, because all changes would need to be traced down.
An independent release cycle without separate repos would not have my preference. That would just be very confusing in my eyes.
I think it is possible to make bigger refactorings without affecting the SDKs with a monorepo by using feature flags and the likes.
There was a problem hiding this comment.
If the public-facing API of the core stays the same, there is no need to explicitly release SDKs. I guess from practical point of view this will still happen, but may not happen as often.
A clear separation would also help clients reverting to older versions of core temporarily if bugs are found.
There was a problem hiding this comment.
Interesting conversation. If the public facing core API does not change, the platforms need not to be changed. However, can we guarantee for platforms like Android static linking won't be required with core?
As in should the build artifact from core also contain source?
There was a problem hiding this comment.
In principle none of the platforms would require static linking, I currently do use dynamic libraries on Android, and static on iOS. Usually static linking reduces the size a bit though.
|
|
||
| When a feature is added to the core it may not be added to all SDKs at the same time. | ||
|
|
||
| ## Repository split - yes or no? |
There was a problem hiding this comment.
I know that this design propsal is intended to kick off a discussion, but when we merge this design proposal we should have resolved open discussion points like this.
There was a problem hiding this comment.
I think the consensus was not to interfere with the metal migration too much to start cleaning-up core in parallel and gradually split out SDKs if needed.
There was a problem hiding this comment.
A clean repository split becomes more difficult the more significant changes occur to this repository, especially if there’s an intention to preserve history. That said, for iOS, the cleanest solution for now remains starting over from the fork point of mapbox-gl-native-ios and cherry-picking relevant changes; as long as that possibility remains despite the Metal migration, then the split can happen later.
| MapLibre core could be provided as a submodule, but it is not necessary. For mobile platforms this may or may not be the case, depending on the level of the integration, for example: | ||
|
|
||
| - on Android if Kotlin/Java is mostly used, then distributing the core separately as a dynamic library is completely reasonable | ||
| - on iOS where static building may be much more beneficial due to size the core will probably be built together with the SDK |
There was a problem hiding this comment.
@1ec5 mentioned on Slack that a source-only distribution is the prefered way of distribution for Apple platforms now. For MapLibre I don't know if would apply to just the SDK (probably written in Swift in the long term) or also the core written in C++.
There was a problem hiding this comment.
This was in the context of the discussion on package managers. It’s a lot easier to set up, for instance, SPM based on source distribution. MapLibre already has a workaround for SPM based on a binary distribution in a dedicated repository, but it isn’t ideal in terms of requiring additional release steps and less obvious installation steps.
There are some hurdles to source-only distribution: the CMake-based build system is not straightforward enough for tight integration with package managers. mbgl takes a relatively long time to build. The whole gl-native repository and its submodules are too large to comfortably make every application developer clone it anew every time they resolve dependencies.
These hurdles mostly go away if the platform-specific code (other than a few low-level odds and ends) wind up in a separate repository like mapbox-gl-native-ios. That repository couldn’t achieve source-only distribution partly because it carried the entire commit and tag history of gl-native along with it: #789 (comment). But mbgl might still need to be distributed as a formal dependency package.
To be clear, source distribution is not mutually exclusive of binary distribution. It would remain possible to attach a prebuilt binary to a GitHub release, just as the gl-native-distribution repository currently does.
There was a problem hiding this comment.
Onboard with the package manager point.
There was a problem hiding this comment.
By the way, if core is packaged up as a build artifact for iOS or macOS, note that a framework can’t expose any Objective-C++ because the C++ won’t translate across ABI boundaries. This means any framework/library vended by core for iOS/macOS would need to stick to pure C or C++ in its public interface. This should already be the case, but it would influence where we draw the line between core and SDK codebases.
| This proposal does not touch much on repository splitting. If SDKs continue to live in the main repository, they should be put under `sdk/<platform>`. The following primary SDKs should be created and supported based on the currently available code: | ||
|
|
||
| - Android SDK | ||
| - Apple SDK (common between all Apple platforms) |
There was a problem hiding this comment.
It seems like it's more complicated than this in the sense that there is code for iOS only and code for Mac only. Should there just be Mac/ and iOS/ directories instead? The problem there is that there are commonalities between them, which is why you have nested platforms
There was a problem hiding this comment.
iOS and macOS share a lot in common. The somewhat misleadingly named “Darwin” folders are entirely shared between Apple platforms (potentially also tvOS and watchOS). There’s “Darwin” code at a low level in mbgl for networking and such, and then there’s “Darwin” code at the SDK level for geometry model objects and various developer-facing utilities. The nested directory structure is due to how MapLibre folded mapbox-gl-native-ios back into a monorepo. (Unfortunately, 41f0be3 broke file histories in the platform directories.) When the iOS and macOS SDKs were originally part of the mapbox-gl-native monorepo, there was no nesting, but there were ../darwin/ references in various parts of the build configuration.
In principle, if macOS had been within scope for gl-native when it was first created, there could’ve been only a single SDK for the Apple platforms. However, there would’ve been a fair amount of conditional compilation to smooth out the differences between UIKit and AppKit. Case in point: the coordinate system in AppKit is vertically flipped compared to UIKit. The biggest area of difference between the two SDKs is currently MGLMapView, but breaking up that monolithic class could yield additional opportunities for code sharing. That kind of refactoring would probably represent scope-creep for the effort here, but it’s worth trying to bring the Apple platforms together over time rather than allowing them to diverge further.
| - `platform/qt` | ||
| - `test`: common (headless) tests for all platforms; disabled only in exceptional cases if some features are not applicable due to the nature of a specific platform | ||
|
|
||
| The core itself ideally should have no binaries unless they are really platform-independent (e.g. headless rendering). |
There was a problem hiding this comment.
tests for the core at test, and the platform test at platform test like platform/apple/ios/test, platform/apple/macOS/test?
|
|
||
| ### Build infrastructure | ||
|
|
||
| The core should be exclusively built by `CMake`. All currently supported platforms are supported by it. No wrapper script or `Makefile` should be used. Can export `Xcode` projects if needed. |
There was a problem hiding this comment.
Does the demo and SDK Framework project https://github.com/alanchenboy/maplibre-gl-native/tree/main/platform/ios/platform/ios/ios.xcodeproj still exist?
There was a problem hiding this comment.
We compile iOS/macOS under the directory platform/iOS and use the existing Xcode project and workspace, so if we fully migrate it to CMake how can we do this just under the command line? Add more compile flags like MBGL_WITH_QT?
There was a problem hiding this comment.
I suspect we keep a Qt and Node.js flag. If you build without flags for a platform it is assumed that you want to build directly for that native platform.
There was a problem hiding this comment.
I mean add MBGL_WITH_IOS and MBGL_WITH_MACOS to let cmake know which platform should take?
There was a problem hiding this comment.
And do these platform-depend projects that need to implement by CMake, I think it is not necessary, if we not how let CMake compile it.
There was a problem hiding this comment.
Not sure I understand the second question.
To answer first, I think it will not be needed as one could just set iOS as the platform (basically cross-compling).
Hi @louwers, this is one of the things that still needs to be defined. There are multiple options:
Note that moving files effectively breaks history for those files, but I guess it's enough that commit history is preserved. |
#789 (comment) proposes a workflow for creating a platform-specific repository (using iOS/macOS as an example) that has only the relevant history, including relevant history from the Mapbox days. This should work whether or not the existing maplibre-gl-native repository continues to function as the source for core code. A separate core repository would require rewriting history for .gitmodules, which I’ve never done before. |
| - "*renderer*" or "*core*" ("*core*" will be used throughout the document) usually refers to the core part of MapLibre Native that is platform-independent to the extent that it builds and runs | ||
| - "*platform*" refers to the operating system and/or a platform that MapLibre runs on, examples are Linux, Windows, macOS, iOS, Android but also Node.js and Qt which provide an universal platform layer that we can use |
There was a problem hiding this comment.
Nit: Missing periods in the end.
thehoneymad
left a comment
There was a problem hiding this comment.
On a summary, I like the change proposed.
I am very pro of breaking the platform code out of the monorepo. And I would like to settle that discussion regardless of the outcome in my arguments favour or not.
- Breaking the platforms out will allow platform engineers to work on platforms. Breakage introduced in core that requires new platform change will be very apparent the moment new core version is pulled in and build fails. It works as a forcing function to update the platforms.
- A separate platform repo can organize tests required only for a single platform. Introduce build, test kits only for a single platform.
- The core can do a headless rendering test that guarantess the behaviour of core for happy path actions from supported platforms. Platform specific tests are implemented in platform side and the core need not know about it.
- Platform plugins and compatibility with said plugins can be managed by the platform repository. The core's responsibility is to ensure platform contract is not broken. A platform's responsibility is to ensure it works with current core along with all plugins.
Point of interests / tangents
- I think Android NDK would require a static linking with the core. It is not a dealbreaker.
- Versioning is better to be defined with config over git tags. Git tags usually require a long clone in build system. Increases the build time.
|
|
||
| - "*renderer*" or "*core*" ("*core*" will be used throughout the document) usually refers to the core part of MapLibre Native that is platform-independent to the extent that it builds and runs | ||
| - "*platform*" refers to the operating system and/or a platform that MapLibre runs on, examples are Linux, Windows, macOS, iOS, Android but also Node.js and Qt which provide an universal platform layer that we can use | ||
| - "*SDK*", "*language bindings*" or "*platform bindings*" refer to the interface that is provided to integrate MapLibre Native core to the client applications or projects. |
There was a problem hiding this comment.
I would suggest dropping the SDK from here. The term has become overloaded.
For example:
- Navigation SDK stands for a Navigation Development Kit on a platform using MapLibre Map Client.
- MapLibre Android SDK is the development kit for the Map Client for Android.
I would rather stick to Platform Bindings here.
There was a problem hiding this comment.
We should drop the term completely from the whole design proposal in that case.
| - `platform/darwin` or `platform/apple` | ||
| - `platform/windows` | ||
| - `platform/qt` |
There was a problem hiding this comment.
Nice. What did we mean by language bindings here?
There was a problem hiding this comment.
I think language bindings are out of scope of the core repo. Core is C++ and that's it. We add stuff so we can build for a platform, but we do not add additional languages that just wrap C++ core.
| - `platform/qt` | ||
| - `test`: common (headless) tests for all platforms; disabled only in exceptional cases if some features are not applicable due to the nature of a specific platform | ||
|
|
||
| The core itself ideally should have no binaries unless they are really platform-independent (e.g. headless rendering). |
There was a problem hiding this comment.
Could we introduce a little specificity here? As in how will we organize the unit tests?
|
|
||
| ### Release cycle | ||
|
|
||
| The core should have an independent release cycle. At this point we imagine SDKs to be much more actively developed. This also allows bigger refactoring to take place without affecting SDKs (e.g. Metal migration). |
There was a problem hiding this comment.
Interesting conversation. If the public facing core API does not change, the platforms need not to be changed. However, can we guarantee for platforms like Android static linking won't be required with core?
As in should the build artifact from core also contain source?
| MapLibre core could be provided as a submodule, but it is not necessary. For mobile platforms this may or may not be the case, depending on the level of the integration, for example: | ||
|
|
||
| - on Android if Kotlin/Java is mostly used, then distributing the core separately as a dynamic library is completely reasonable | ||
| - on iOS where static building may be much more beneficial due to size the core will probably be built together with the SDK |
There was a problem hiding this comment.
Onboard with the package manager point.
| - Node.js Binaries and SDK | ||
| - GLFW SDK | ||
|
|
||
| Navigation SDKs could then be merged with respective platform SDK. |
| - `maplibre-native-sdk-qt` (Qt bindings, should probably be merged with `qt-geoservices-maplibre-gl`) | ||
| - `maplibre-native-sdk-js` | ||
|
|
||
| Alternatively SDKs could be without the explicit `sdk` in the name. |
|
|
||
| ### Repository naming | ||
|
|
||
| - `maplibre-native-core` or `maplibre-native-renderer` for the core functionality. |
There was a problem hiding this comment.
I would name it maplibre-native-core. Rendering is the major part of it. But there's a bit more to that.
|
|
||
| 3. The proposal is implemented for the core. | ||
|
|
||
| 4. SDK repositories are created on by one as needed and they migrate to use `maplibre-native-core`. |
By config, do you mean a package manager or build script? Note that the previous arrangement relied on Git submodules, which also require cloning but can be tied to a specific commit rather than a tag per se. |
|
I think we should converge on the proposal. As far as I see there are no big issues. I tried to downscope the repository split a bit (especially as Metal migration is in full swing). |
|
|
||
| If needed for specific platforms (iOS comes to mind), a dedicated test app could be made to run the necessary rendering tests. This app should implement the minimal functionality that does not include the actual full-fledged integration of MapLibre (e.g. gestures, widgets, ...). | ||
|
|
||
| Testing should be done by `CTest` if the platform allows (may not be possible for all integration tests on mobile devices). |
There was a problem hiding this comment.
There is no reason why we should require platforms to wrap their own testing framework with CTest.
There was a problem hiding this comment.
This is for core only.
| - gestures support on toucscreen devices | ||
| - location services (GPS, location from the mobile network) | ||
|
|
||
| While SDKs continue to live in the main repository, they should be put under `sdk/<platform>`. |
There was a problem hiding this comment.
I suggest we keep the platforms in the platform folder, as it is more intuitive, and touches much less files.
There was a problem hiding this comment.
In my opinion this biggest issue with this design proposal in its current form is that it is not actionable. It describes a desired end-state but no concrete details on the current situation and how to achieve the desired state. It is very ambitious but too all-encompassing and vague.
@ntadej If you want to continue to push for the code re-organisation, I suggest that you focus on one specific part (e.g. an independent release cycle for the core) and one platform (e.g. Qt) so we get a more detailed look at what a code organisation design with a decoupled core looks like. I also would suggest that we make explicit in the design proposal if we stick to a monorepo or split out the platform bindings.
Here is another idea. Design proposals are quite 'waterfall', but they are a good tool design and think about things together sometimes. What if we take a more agile approach? We create an issue to split off the Qt platform in another repository. We make a list of changes needed in terms of testing, releases and code structure to get it done. We can call it an experiment. After the transfer is complete we should be in a much better shape to do the same for the other platforms.
|
Closing this because it is quite old at this point. New proposals in a similar vein that reflect the current situation of course welcome. Let's reflect on the Qt split off in a separate discussion thread. |
This is the initial code organisation design proposal. Comments are welcome and encouraged!
It is not very verbose, I'm not sure in what detail I should have gone (and I also did not really have time). We can discuss at the technical meeting or in the PR here.