Attempting to run the macOS target fails with errors

[wtimme]

Steps to reproduce

1. Open OpenStreetMap.xcodeproj
2. Attempt to run by pressing CMD + R

Expected behaviour

The project builds and the app is launched for debugging.

Observed behaviour

The app fails to build with 11 errors.

[wtimme]

So, I spent a good couple of hours on getting the macOS target to build. My intention was to create a framework for the shared code (cp. #155), and to check that the framework properly integrates with both the iOS as well as the macOS project, I need them both to at least successfully build.

The more I went after the build errors and attempted to cut loose bits of code here and there, I was wondering whether this is worth the effort. Sure, a macOS application would be awesome to have as well, but I don't see an actual need for it. If I was editing OSM data on my laptop, I'd fire up JOSM. That editor is way more advanced than what I need, and is still being maintained and extended.

To cut to the point, I suggest we remove the macOS project from the repository and concentrate our energy on bringing the best OSM editing experience to iOS.

This might sound a bit drastic, but I just wanted to put it out there to discuss. @bryceco, what's your take on this? What are your plans for the macOS application?

[tordans]

I agree.
For desktop there is iD and JOSM.
For mobile – iPhone and iPad – there is nothing really. Even browser based solutions like iD don't really work on iPad (some Issues).
So, IMO, focussing on those is the best investment.

[bryceco]

I agree there is no great need for a Mac editor. In addition Apple indicated that next year it will be providing support to simplify porting iOS apps to Mac (see this year's WWDC), and when that support ships it should be pretty easy to tweak the iOS code for desktop. However, until Apple does that and a desktop editor is a reality I don't see any reason to delete the Mac port. It really has no impact on day to day development for iOS, and if Apple changes course it may be useful to someone somewhere.

[wtimme]

Thanks for your input, guys!

We might not need to implement new features on both platforms, that's correct. And we might even be able to make the project build again (which, as I said, already took me a couple of hours, to no avail 🤷‍♂️).

With each API change in the Shared code that is not backwards-compatible, we need to always maintain the macOS app to some extent, even if it's just making sure it at least builds.
We further need to make sure to communicate values in formats that are platform-independent (or, we use something like iosapi.h). Besides being a hurdle for developers who are new to the project, it can get quite time-consuming and slows down development.

Let's assume we keep the code for the macOS target in this repository. What do you guys think should be done about the failing build of the target?

[bryceco]

The Mac port hasn’t compiled in at least 5 years. If someone really wants it to build then the onus on fixing it and maintaining it will fall on them, though I’m happy to help out and actually spent quite a bit of time trying to make it compile last month.

[wtimme]

I've pushed the branch with my attempt to get the target to build. Take a look if you have the time, maybe it helps.

How about we remove the dependency on UIKit and AppKit for the Shared code, and exclusively use Foundation? This way, we can share business logic without having to rely on those #defines in the iosapi.h, making it easier to later use the code with the macOS target. (And it helps us to move towards dedicated frameworks of our own (#155).)

As for the target itself, I guess the question on whether to hide the code with git rm -rf or keeping it visible is also influenced by personal preferences.

[bryceco]

MapView is the centerpiece of the shared code, and is derived from UIView/NSView. It already incorporates a lot of functionality that rightfully belongs in a view controller instead of a view. Can you explain the approach you envision to remove the dependency on UIKit while still keeping that code shared?

[wtimme]

Thanks for your insights! Yes, let me elaborate a bit. If we wanted to represent a color value, we have different options.

Using "bridging macros" as in iosapi.h

This seems to be the current approach. One can either choose to use classes from UIKit or AppKit - either UIColor or NSColor. On iOS, NSColor is being replaced with UIColor and vice versa.

The main issue is that one might use properties that are only available on one of the target systems. For example, NSColor contains the property ignoresAlpha. Because one might not see the need to build the iOS target, they will just use the property and wrongly assume everything's fine. This can become difficult to fix and lower the benefit of sharing the code.

Furthermore, it increases the risks of having commits in which either of the targets is not properly compiling. This can dramatically increase the effort required when finding a bug, since tools such as git-bisect are most effective if the builds and tests run successfully for each commit.

Using Foundation-based values

Both platforms can work with the values from Foundation, so that's what we are limiting ourselves to. Instead of using concrete classes for the color, we could use enums or constants:

@objc enum AppColor: Int {
    case LightText, CallToActionColor
}

The mapping from Color to UIColor (or NSColor) could be done in extensions (or "categories") in Objective-C:

@interface UIColor (AppColor)
+ (UIColor *)colorFromAppColor:(AppColor)appColor;
@end

@implementation UIColor (AppColor)
+ (UIColor *)colorFromAppColor:(AppColor)appColor {
    // switch/case
    // ...
}
@end

In addition to making the code more portable, it allows for easier testing, since we no longer need to compare actual UIColor values against each other.

[1ec5]

Furthermore, it increases the risks of having commits in which either of the targets is not properly compiling.

For what it’s worth, this is something that a continuous integration tool such as Travis, Bitrise, or CircleCI can prevent. The repository can be set up so that no pull request can be merged until both the iOS and macOS builds compile without errors. Of course, that would require addressing this issue first. 😉 Once the macOS build compiles in the first place, keeping it green would involve a smaller amount of work in each PR that touches shared headers.

The Mapbox GL Native project, for instance, has iOS and macOS builds even though most of the developers only routinely develop for iOS. This primarily works because there’s a strong separation between cross-platform files (that mostly only use Foundation types) and platform-specific files. In the few cases where cross-platform files use UIKit/AppKit types, we use conditional compilation macros or platform-specific headers. For UIColor/NSColor, we use a “bridging macro”, because most of the usage in that project involves initializers and properties that are common to both platforms. But the symbolic approach in #156 (comment) could also work if the number of colors is fairly limited.

[wtimme]

Thanks for your input, @1ec5! That sounds like you have experience doing this.

Do you think it's feasible to have a "Shared" framework (cp. #155) that solely depends on Foundation, and have the platform-specific code in the dedicated targets?

most of the developers only routinely develop for iOS

When running unit tests locally, does their scheme also run the test for the respectively other target? (For example, when running the tests while developing the iOS app, are the tests for the macOS target also run?)

[1ec5]

Do you think it's feasible to have a "Shared" framework (cp. #155) that solely depends on Foundation, and have the platform-specific code in the dedicated targets?

Without having looked closely at what this *OS framework would contain, I’d say it’s definitely feasible. It’s a tradeoff: it means one more framework to load, which may or may not be desirable; on the other hand, it means you don’t have to remember to add a file to both iOS and macOS targets.

Solely depending on Foundation can be merely a rule of thumb; if convenience dictates that UIColor/NSColor be used in an otherwise largely Foundation-only class, a modicum of conditional compilation can keep the code in one place.

most of the developers only routinely develop for iOS

When running unit tests locally, does their scheme also run the test for the respectively other target? (For example, when running the tests while developing the iOS app, are the tests for the macOS target also run?)

No, only the iOS tests are run, because a single scheme can only target a single device at a time. We’re relying on the CI bot and contributing documentation to head-off problems before merging.

I should note that gl-native isn’t an ideal example of sharing code across Apple platforms: the iOS and macOS targets live in separate workspaces for historical reasons (mapbox/mapbox-gl-native#5942). A more conventional example would be MapboxDirections.swift, which has a target and scheme for each of the four Apple platforms in the same Xcode project.

(Sorry that I’m just talking instead of offering code here. My to-do list is running long!)

[bryceco]

Just a note that using Catalyst its now possible to run the iOS version on Mac.

[mormahr]

Now that with Catalyst on Big Sur native feeling UIKit apps on the Mac are possible (and the Mac version of Go Map!! isn't currently a thing anyway), i'd suggest removing the Mac target and unifying shared and iOS code and removing the #if TARGET_OS_IPHONE macros to dramatically simplify the codebase.

[bryceco]

I agree its probably okay to delete the Mac port and stick with Catalyst. A couple comments:

• The shared code is pretty foundational and, in a perfect world, could become a standalone framework that could be used in other OSM apps, so I'd rather keep that partition rather than merging everything.
• I haven't tried the most recent Catalyst release, but the previous release has some pretty major performance problems when zooming. This needs to be addressed before the Catalyst port can be considered viable.