Repository navigation
Declare minimum macOS version in app bundles - #1398
Conversation
aadef62 to
38cb72f
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The app declares macOS 10.10 as its minimum despite the native launcher targeting macOS 11, allowing startup attempts on unsupported Intel Macs.
No open issues 🎉
What changed in this PR
This PR moves the macOS launcher's minimum-version check into app bundle metadata so Launch Services can enforce it without relying on an API unavailable on older systems.
Changes:
- Declare
LSMinimumSystemVersionas10.10in both macOS app bundles. - Remove the Cocoa launcher’s runtime version check, leaving other platforms unchanged.
| File | Description |
|---|---|
| features/org.eclipse.equinox.executable.feature/library/cocoa/eclipseCocoaMain.c | Remove the runtime macOS version check. |
| features/org.eclipse.equinox.executable.feature/bin/cocoa/macosx/x86_64/Eclipse.app/Contents/Info.plist | Declare the minimum macOS version for Intel. |
| features/org.eclipse.equinox.executable.feature/bin/cocoa/macosx/aarch64/Eclipse.app/Contents/Info.plist | Declare the minimum macOS version for Apple Silicon. |
Review effort: Balanced
That's correct. It was changed in @GaborLajos can you please fix that inconsistency and change the value to 11? Otherwise this change looks good to me and I think it's far superior to have proper OS level metadata defined than checking the OS version in native code. Thank you. @Phillipus do you want to check this? |
|
That might be OK for Eclipse but an RCP app might have it's own |
|
Understand. And of course check for macOS 11 in all places. |
|
If I understand correctly, this PR modifies the Equinox launcher's default Info.plist, so it should also benefit RCP applications using the standard launcher bundle. Products providing their own Info.plist would remain free to specify a different minimum OS version. I can see the benefit of a runtime check for version 11. I originally removed the previous check because it was enforcing 10.10, and that check itself relies on APIs that are not available on pre-10.10 systems. However, if an RCP application defines a higher minimum version in its own Info.plist, the launcher can no longer be the authoritative source of that requirement. In any case, I am happy to follow whichever approach you consider most appropriate. Thanks for your review. |
5d25a42 to
29b4938
Compare
8e00c43 to
51cd061
Compare
There was a problem hiding this comment.
I originally removed the previous check because it was enforcing 10.10, and that check itself relies on APIs that are not available on pre-10.10 systems. However, if an RCP application defines a higher minimum version in its own Info.plist, the launcher can no longer be the authoritative source of that requirement.
Yes, there are always multiple scenarios to consider, but the current should give everyone the best.
The only drawback is that we now have to maintain the minimal required macOS version in four instead of only two places (from which one was already forgotten the last time it changed). But we'll handle that.
Given that 10.10 is now over 10 years old, I think we can consider it save to use API from it. The pinfo should prevent using anything lower before. And custom apps should follow that.
Thank you for this enhancement and also Thanks to Phil for making it better.
Summary
LSMinimumSystemVersionset to11.0to the macOS app bundleInfo.plistfiles for aarch64 and x86_64.The launcher check uses
NSProcessInfo.operatingSystemVersion, which was introduced in macOS 10.10. It therefore cannot reliably display an error on older systems. Declaring the minimum version in the app bundle lets macOS Launch Services enforce the requirement when launching the app.Notes
This change is targeted at the macOS launcher behaviour and does not alter the other platform implementations.