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

Set/Enforce Minimum supported macOS version #20459

Open
xavier2k6 opened this issue Feb 22, 2024 · 15 comments
Open

Set/Enforce Minimum supported macOS version #20459

xavier2k6 opened this issue Feb 22, 2024 · 15 comments
Labels
Build system Issue with the build configuration or scripts (but not the code itself) OS: macOS Issues specific to macOS Project management

Comments

@xavier2k6
Copy link
Member

qBittorrent & operating system versions

qBittorrent: 4.x/master
OS: macOS

What is the problem?

In below PR, a Min. macOS version was set:

However it seemed that this PR may have not fully done what it was intended to do. See #19066 (comment)

In below PR, QMake Support was removed in master

  • Now, there's no set minimum for macOS in master & we need to review/correctly set this.

    qBittorrent/src/app/CMakeLists.txt

    Lines 91 to 92 in 7786e1b

    # This variable name should be changed once qmake is no longer used. Refer to the discussion in PR #14813
    set(MACOSX_DEPLOYMENT_TARGET ${CMAKE_OSX_DEPLOYMENT_TARGET})

Steps to reproduce

No response

Additional context

No response

Log(s) & preferences file(s)

No response
@xavier2k6 xavier2k6 added OS: macOS Issues specific to macOS Project management labels Feb 22, 2024
@xavier2k6 xavier2k6 mentioned this issue Feb 22, 2024
Closed
@xavier2k6 xavier2k6 added the Build system Issue with the build configuration or scripts (but not the code itself) label Feb 22, 2024
@sledgehammer999
Copy link
Member

The Qt docs say that Qt sets the minimum required target to CMAKE_OSX_DEPLOYMENT_TARGET.
Reading the whole thread in #14813 it seems that this:

set(MACOSX_DEPLOYMENT_TARGET ${CMAKE_OSX_DEPLOYMENT_TARGET})

is required for substituting the MACOSX_DEPLOYMENT_TARGET in the Info.plist file. They were forced to leave that var name for compatibility reasons with qmake. Now that qmake is gone, the var name should be changed to something else because the same var name is also used by cmake itself to derive CMAKE_OSX_DEPLOYMENT_TARGET (and this leads to confusion to readers).

@xavier2k6
Copy link
Member Author

@Kolcha Thoughts?! - depending on the Qt version being used, the supported platform will vary....Do you think we should just explicitly set the minimum version in the info.plist so that there's more control over the minimum supported?

@Kolcha
Copy link
Contributor

Kolcha commented Aug 14, 2024 •
edited
Loading

I'm not a macOS/Apple developer at all, I'm just an enthusiast who owned a MacBook for a long time (~7 years) and has some fun with Qt :) so, don't expect an "expert" answer from me.

first of all, CMAKE_OSX_DEPLOYMENT_TARGET is just one of the standard CMake variables, it has nothing to do with Qt.

based on my experience and many pitfalls I faced, this variable is vital, at least if you want to get a "really portable" (works on many devices) application. it definitely should be set during the build process, but can't tell too much about Info.plist, as even Apple doesn't follow their guides for the format of the values in this field. and it doesn't seem to be too important, the only "visible effect" from this - is when you try to open app on too old system its icon will be dimmed and overlayed with "unavailable" icon.

it doesn't need to be the same as Qt defines, even less may work without the issues (but you should be careful, macOS is known for C runtime incompatibility, and it is even not taking into account required features that just may not exist in older runtime versions).

reasonable minimum nowadays is macOS 12, as Xcode 15 produces binaries that can't be launched (app just crashes) on macOS 11 (at least for Qt-based app, tested personally) regardless of "deployment target" (what's behind that CMake variable).

for qBittorrent I even recommend setting it to macOS 13, as qBittorrent extensively uses C++20 features, some of which may be implemented in runtime only starting with macOS 13. but again, this is very important at build time. Info.plist is just metadata who knows exactly used for.

@xavier2k6
Copy link
Member Author

reasonable minimum nowadays is macOS 12

This is almost EOL & GHA Runners for this OS are being deprecated/removed!
See: actions/runner-images#9255 or screenshot below.

300882082-ac6ef1f1-8e18-4af1-a97f-d38cf82885df

for qBittorrent I even recommend setting it to macOS 13

Xcode's Minimum OS Requirements &/or C++XX were used previously to determine our support, for example:

The reason Mojave isn't listed as officially supported is due to:

  1. the requirements of providing universal builds.
  2. macOS 10.14 Mojave is no longer supported by Apple

Xcode 12.2 and later is a requirement for building universal binaries. Earlier versions of Xcode don’t contain the support needed to build and test universal versions

Xcode 12.2 requires >=macOS Catalina 10.15.4

#16910 (comment)

Support for macOS 10.13 (High Sierra) was dropped because Xcode doesn't support C++17 for that version.

https://www.qbittorrent.org/news#thursday-january-19th-2021---qbittorrent-v4.3.3-release

@xavier2k6
Copy link
Member Author

Also in my OP - I mentioned #19066 (comment)

which seems to indicate that minimum target was being set to use the Image version being used by the GHA Runner.

@Kolcha
Copy link
Contributor

Kolcha commented Aug 15, 2024 via email

@Kolcha
Copy link
Contributor

Kolcha commented Aug 15, 2024 via email

@xavier2k6
Copy link
Member Author

you can set it to the lower version than you are using for building, it's totally fine.

There is no minimum being set by qBittorrent currently, that's the thing. (as far as I can tell)

So, we may need to do something explicitly like set(CMAKE_OSX_DEPLOYMENT_TARGET = "13.5") in CMakeLists.txtt.

@Kolcha
Copy link
Contributor

Kolcha commented Aug 15, 2024 via email

@xavier2k6
Copy link
Member Author

nothing sets this option by default

Hence this ticket, we did set it in qmake but support for qmake has since been removed.

@Kolcha
Copy link
Contributor

Kolcha commented Aug 15, 2024 via email

@xavier2k6
Copy link
Member Author

xavier2k6 commented Aug 15, 2024 •
edited
Loading

Qt still sets it to macOS 11 (at least it was so quite recently)

It more than likely depends on Qt version being used - so as I said previously, that can vary.

Supported Platforms: - Qt 6.9
macOS 12, 13, 14, 15
https://doc-snapshots.qt.io/qt6-dev/supported-platforms.html#macos

Supported Platforms: - Qt 6.8
macOS 12, 13, 14
https://doc-snapshots.qt.io/qt6-6.8/supported-platforms.html#macos

Supported Platforms: - Qt 6.7
macOS 11, 12, 13, 14
https://doc-snapshots.qt.io/qt6-6.7/supported-platforms.html#macos

Your comment from #14813 (comment) did suggest to set CMAKE_OSX_DEPLOYMENT_TARGET explicitly.....

@Kolcha
Copy link
Contributor

Kolcha commented Aug 15, 2024 via email

@xavier2k6
Copy link
Member Author

@Kolcha Willing to submit a PR?

Perhaps setting 13.5.0 as minimum....it can be discussed by team/maintainer can have their say.

@Kolcha
Copy link
Contributor

Kolcha commented Aug 15, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Build system Issue with the build configuration or scripts (but not the code itself) OS: macOS Issues specific to macOS Project management
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sledgehammer999 @Kolcha @xavier2k6