Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add: Specify minimum version for macOS #9689

Merged
merged 1 commit into from
Nov 11, 2021
Merged

Add: Specify minimum version for macOS #9689

merged 1 commit into from
Nov 11, 2021
+2 −0

Conversation

orudge
Copy link
Contributor

@orudge orudge commented Nov 11, 2021 •
edited by TrueBrain
Loading

Motivation / Problem

OpenTTD will open on macOS versions earlier than 10.14, but will crash.

Fixes #9688.

Description

This adds a minimum version to the Info.plist so the Finder will not allow the application to launch. It will display a minimum version warning instead.

Limitations

Tested on macOS 10.11 on my ancient MacBook Pro; with this patch it was greyed out and displayed a warning, without the patch it opened and crashed.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@LordAro
Copy link
Member

LordAro commented Nov 11, 2021

Is 10.14 our actual minimum version, or just the minimum version of our builds?

@orudge
Copy link
Contributor Author

orudge commented Nov 11, 2021

Let me try to build on my ancient Mac and find out...

@orudge
Copy link
Contributor Author

orudge commented Nov 11, 2021

Let me try to build on my ancient Mac and find out...

My ancient Mac isn't able to compile modern OpenTTD, even attempting various newer versions of gcc and clang from MacPorts. I don't have a system with 10.12 or 10.13 on, so couldn't tell you if either of those would work. I'd personally be inclined to accept the patch and if anybody complains, we can change it. Alternatively, I can change it so that it's only applied for the builds that we do if preferred.

@nielsmh
Copy link
Contributor

nielsmh commented Nov 11, 2021

Patching the Plist during the CI build sounds a little more reasonable to me, if it's theoretically possible to still build with a toolchain that can target older macOS versions.

@TrueBrain
Copy link
Member

We use std::variant, and as mentioned in the PRs introducing that, it means you need at least 10.14. Due to how MacOS works this is not really related to a compiler but with the libstdc++ that is shipped with MacOS. It is missing std::bad_variant_access, and that is not fixable by the average user.

We were under the impression our framework bump to 10.14 would already fix it, but clearly this is not the case. So yeah, this is the right solution :)

I am not aware of any theoretical solution @nielsmh ; if you have anything on that I would love to know. But from our investigation a few months ago, this really is not solvable by any toolchain :(

@michicc michicc merged commit 2f72eb0 into OpenTTD:master Nov 11, 2021
@TrueBrain TrueBrain mentioned this pull request Nov 11, 2021
Closed
@michicc michicc added the backport requested This PR should be backport to current release (RC / stable) label Jan 1, 2022
TrueBrain pushed a commit to TrueBrain/OpenTTD that referenced this pull request Jan 3, 2022
@orudge @TrueBrain
Add: Specify minimum version for macOS (OpenTTD#9689)
dd2a612
@TrueBrain TrueBrain mentioned this pull request Jan 3, 2022
Merged
TrueBrain pushed a commit that referenced this pull request Jan 5, 2022
@orudge @TrueBrain
Add: Specify minimum version for macOS (#9689)
280f54c
@TrueBrain TrueBrain added backported This PR is backported to a current release (RC / stable) and removed backport requested This PR should be backport to current release (RC / stable) labels Jan 5, 2022
This was referenced Dec 16, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TrueBrain TrueBrain

Assignees
No one assigned
Labels
backported This PR is backported to a current release (RC / stable)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Crash]: Crash on startup / 12.1 / MacOs
5 participants
@orudge @LordAro @nielsmh @TrueBrain @michicc