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.

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

qt{5,6}: update for the new Darwin SDK #347216

Merged
merged 8 commits into from
Oct 11, 2024
Merged

Conversation

reckenrode
Copy link
Contributor

Qt 5.15 needs to know the exact location of the OpenGL framework, so it needs updated for the new SDK pattern. I also updated Qt 6. Both are examples of how the new Darwin SDK can simplify derivations and improve support (e.g., Qt WebEngine builds again on Darwin).

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@reckenrode
Copy link
Contributor Author

@K900 pinging you since you’d been interested in this on Matrix.

@emilazy
Copy link
Member

emilazy commented Oct 8, 2024

For future cleaner‐uppers, to avoid making Randy build Qt again: 5.15/qtwebengine-darwin-checks.patch can probably go now, and 5.15/qtwebengine-darwin-no-platform-check.patch can probably be replaced by just rewriting the hardcoded /usr/bin/xcodebuild path to point to our xcbuild.

Copy link
Contributor

@K900 K900 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very cool.

Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reiterating what I said on Matrix: these kinds of diffs are incredibly satisfying and I can’t think of anything better to point people at to explain how much of an improvement the new model is. A couple comments that are more me thinking out loud than anything.

Comment on lines 239 to +241
# FIXME These dependencies shouldn't be needed but can't find a way
# around it. Chromium pulls this in while bootstrapping GN.
++ lib.optionals stdenv.hostPlatform.isDarwin [
libobjc
cctools

# frameworks
ApplicationServices
AVFoundation
Foundation
ForceFeedback
GameController
AppKit
ImageCaptureCore
CoreBluetooth
IOBluetooth
CoreWLAN
Quartz
Cocoa
LocalAuthentication
MediaPlayer
MediaAccessibility
SecurityInterface
Vision
CoreML
OpenDirectory
Accelerate

openbsm
libunwind
];
++ lib.optionals stdenv.hostPlatform.isDarwin [ cctools.libtool ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems probably inaccurate now? Though I don’t really know what it meant to begin with.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m seeing if I can drop it now that xcbuild includes xcrun. I also tried dropping the patches as suggested above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I meant this comment that I don’t really understand:

  # FIXME These dependencies shouldn't be needed but can't find a way
  # around it. Chromium pulls this in while bootstrapping GN.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m seeing if I can go one step further and delete the whole thing. If not, I’ll drop or change the comment.

Copy link
Contributor Author

@reckenrode reckenrode Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. Deleting those would be non-trivial. It requires replacing the qtbase mkspecs patches with different patches to use xcbuild. If someone wants to do that later, that’s fine; but it’s beyond the scope of what I want to do with this PR (which is clean things up and use the new SDK).

openbsm
libunwind
];
++ lib.optionals stdenv.hostPlatform.isDarwin [ cctools.libtool ];

buildInputs = lib.optionals stdenv.hostPlatform.isDarwin [
cups
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I wonder if it just needs the headers?)

Copy link
Contributor Author

@reckenrode reckenrode Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For CUPS? CUPS headers are propagated by the SDK now. I wonder if Chromium is detecting CUPS support due to that.

@emilazy
Copy link
Member

emilazy commented Oct 8, 2024

0002-qtbase-qmake-fix-mkspecs-for-darwin.patch also looks a lot like something that could be dropped or be made much smaller now that we have an actual SDK.

@reckenrode reckenrode force-pushed the reckenrode/darwin-sdk-refactor branch 2 times, most recently from a240201 to 4112f1a Compare October 9, 2024 02:16
Per upstream’s supported Darwin versions, built with the 14.4 SDK
(corresponding to Xcode 15) and a 10.14 deployment target. To ensure
that users of Qt have a compatible SDK and deployment target, propagate
the 10.14 SDK and a 10.14 minimum version. Users that need a newer
version can opt into using it by adding the SDK package to their build
inputs. aarch64-darwin uses 11.0 and 11.3 because those are the oldest
supported SDK and deployment target on that platform.

Note: upstream actually supports 10.13, but 10.14 was chosen as the
minimum and SDK because it ensures that automatic dark mode switching
works for x86_64-darwin users.
This change ensures all Qt modules build with the latest SDK.
Qt is using the new Darwin SDK, which no longer requires overrideSDK to
change the SDK version or set the deployment target.
While the 14.4 SDK works to build the rest of Qt 5, building Qt
WebEngine fails with errors like the following:

    In file included from gen/base/base_jumbo_37.cc:14:
    In file included from ./../../3rdparty/chromium/base/mac/mach_port_rendezvous.cc:16:
    In file included from ../../3rdparty/chromium/base/mac/foundation_util.h:34:
    In file included from /nix/store/sfdyfscmykycv7nfscn551lyl3gf4n27-apple-sdk-14.4/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/ApplicationServices.framework/Headers/ApplicationServices.h:39:
    In file included from /nix/store/sfdyfscmykycv7nfscn551lyl3gf4n27-apple-sdk-14.4/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreText.framework/Headers/CoreText.h:26:
    In file included from /nix/store/sfdyfscmykycv7nfscn551lyl3gf4n27-apple-sdk-14.4/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreText.framework/Headers/CTFramesetter.h:21:
    In file included from /nix/store/sfdyfscmykycv7nfscn551lyl3gf4n27-apple-sdk-14.4/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreText.framework/Headers/CTTypesetter.h:20:
    /nix/store/sfdyfscmykycv7nfscn551lyl3gf4n27-apple-sdk-14.4/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreText.framework/Headers/CTLine.h:138:5: error: unknown type name 'CFAttributedStringRef'; did you mean 'CFMutableStringRef'?
        CFAttributedStringRef attrString ) CT_AVAILABLE(macos(10.5), ios(3.2), watchos(2.0), tvos(9.0));
        ^
    /nix/store/sfdyfscmykycv7nfscn551lyl3gf4n27-apple-sdk-14.4/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreFoundation.framework/Headers/CFBase.h:500:70: note: 'CFMutableStringRef' declared here
    typedef struct CF_BRIDGED_MUTABLE_TYPE(NSMutableString) __CFString * CFMutableStringRef;
                                                                     ^
Per upstream’s supported Darwin versions, built with the 14.4 SDK
(corresponding to Xcode 15) and a 11.0 deployment target. To ensure that
users of Qt have a compatible SDK and deployment target, propagate the
11.3 SDK and a 11.0 minimum version. Users that need a newer version can
opt into using it by adding the SDK package to their build inputs.
This change ensures all Qt modules build with the latest SDK.
Qt Multimediate is being built with a newer SDK that has the new
definition, so don’t bother to patch it out anymore. The symbol is
defined in a backwards compatible way, so it will still work on older
releases that don’t have it in their SDKs.
Qt is using the new Darwin SDK, which no longer requires overrideSDK to
change the SDK version or set the deployment target.
@reckenrode reckenrode force-pushed the reckenrode/darwin-sdk-refactor branch 2 times, most recently from a1adff0 to e3f2829 Compare October 10, 2024 20:25
@emilazy emilazy marked this pull request as ready for review October 10, 2024 22:54
@emilazy emilazy deleted the branch NixOS:staging October 11, 2024 00:01
@emilazy emilazy closed this Oct 11, 2024
@reckenrode reckenrode reopened this Oct 11, 2024
@reckenrode reckenrode changed the base branch from reckenrode/darwin-sdk-refactor to staging October 11, 2024 00:05
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/on-the-future-of-darwin-sdks-or-how-you-can-stop-worrying-and-put-the-sdk-in-build-inputs/50574/15

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from my limited Darwin knowledge

--replace "/System/Library/Frameworks/OpenGL.framework/" "${OpenGL}/Library/Frameworks/OpenGL.framework/" \
--replace "/System/Library/Frameworks/AGL.framework/" "${AGL}/Library/Frameworks/AGL.framework/"
substituteInPlace configure \
--replace-fail '/usr/bin/xcode-select' '${lib.getBin xcbuild}/bin/xcode-select' \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also use lib.getExe'

Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I’ll let @K900 do the merge.

@K900 K900 merged commit ead94a7 into NixOS:staging Oct 11, 2024
40 of 44 checks passed
@reckenrode reckenrode deleted the push-nyvqzpppwmom branch October 11, 2024 13:27
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/title-the-darwin-sdks-have-been-updated/55295/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants