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

qt5: build with Apple SDK 11.0, drop awful patches #181862

Closed
wants to merge 2 commits into from
Closed

qt5: build with Apple SDK 11.0, drop awful patches #181862

wants to merge 2 commits into from

Conversation

K900
Copy link
Contributor

@K900 K900 commented Jul 17, 2022

Description of changes

Please work 🤞

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@K900 K900 requested a review from ttuegel as a code owner July 17, 2022 15:45
@ofborg ofborg bot added 10.rebuild-darwin: 101-500 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Jul 17, 2022
@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

@ofborg build libsForQt5.qtbase libsForQt5.qtwidgets

@K900 K900 mentioned this pull request Jul 17, 2022
Merged
@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

https://github.com/ofborg build libsForQt515.qtbase

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

@ofborg build libsForQt515.qtbase

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

https://github.com/ofborg build libsForQt515.qtbase

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

@ofborg libsForQt515.qtbase

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

@ofborg build libsForQt515.qtbase

veprbl
veprbl reviewed Jul 17, 2022
View reviewed changes
pkgs/development/libraries/qt-5/5.15/qtwebengine-darwin-no-platform-check.patch
# otherwise query for it.
QMAKE_MAC_SDK_VERSION = $$eval(QMAKE_MAC_SDK.$${QMAKE_MAC_SDK}.SDKVersion)
isEmpty(QMAKE_MAC_SDK_VERSION) {
- QMAKE_MAC_SDK_VERSION = $$system("/usr/bin/xcodebuild -sdk $${QMAKE_MAC_SDK} -version SDKVersion 2>/dev/null")
Copy link
Member

Choose a reason for hiding this comment

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

Removing this one can't be right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why?

Copy link
Member

Choose a reason for hiding this comment

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

We don't want to use impure XCode or Command Line Tools that provide xcodebuild (plus, those are not installed on Hydra and many users won't have them). Hence these patches are needed to fix the build system that is otherwise nailed to the Apple tools.

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 pretty sure QMAKE_MAC_SDK_VERSION should be set here though? This block will only be entered if it's not.

@veprbl
Copy link
Member

veprbl commented Jul 17, 2022

Not sure what is this supposed to achieve. The qt515 already works on darwin except for qtwebengine, but I assume you don't plan to fix the latter.

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

@ofborg build libsForQt515.qtbase

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

Not sure what is this supposed to achieve. The qt515 already works on darwin except for qtwebengine, but I assume you don't plan to fix the latter.

It doesn't after updating to the latest KDE patchsets, which fix a lot of Wayland issues on Linux.

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

@ofborg build libsForQt515.qtbase

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

@ofborg build libsForQt515.qtbase

(sorry for the spam, I have no Darwin machine :( )

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

@ofborg build libsForQt515.qtbase

@veprbl
Copy link
Member

veprbl commented Jul 17, 2022

Not sure what is this supposed to achieve. The qt515 already works on darwin except for qtwebengine, but I assume you don't plan to fix the latter.

It doesn't after updating to the latest KDE patchsets, which fix a lot of Wayland issues on Linux.

How can one see the issue? It's not on Hydra:
https://hydra.nixos.org/job/nixpkgs/trunk/qt515.qtbase.x86_64-darwin

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

Check staging-next.

@veprbl
Copy link
Member

veprbl commented Jul 17, 2022

Okay, I see https://hydra.nixos.org/build/183942941. There is no apparent SDK version issue, it will need corrected patches.

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022 •
edited
Loading

I rebased the patches, but it doesn't build now. Someone in #macos:nixos.org suggested it might be easier to build with the SDK that's actually supported upstream.

@K900 K900 marked this pull request as draft July 17, 2022 18:25
@veprbl
Copy link
Member

veprbl commented Jul 17, 2022

Yeah, for some reason it's a popular opinion in the community that the 10.12 SDK Frameworks are somehow deficient, and the potency of the newer version is exaggerated. In my experience, very few indispensable API's were added in the newer versions.

@K900
Copy link
Contributor Author

K900 commented Jul 17, 2022

That's something to bring up with upstream, really. I mostly just want to unblock staging-next right now and this whole thing is already very messy...

@veprbl
Copy link
Member

veprbl commented Jul 17, 2022

There is no open-source upstream for Qt5. Would be strange to route macOS patches through KDE.

This should be a simple fix. @NixOS/darwin-maintainers, anyone has time to look into fixing the qt515.qtbase on staging-next?

@veprbl
Copy link
Member

veprbl commented Jul 19, 2022

See #182035

@K900
Copy link
Contributor Author

K900 commented Jul 20, 2022

Going to close this for now, more stdenv changes seem to be required.

@K900 K900 closed this Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veprbl veprbl veprbl left review comments

@ttuegel ttuegel Awaiting requested review from ttuegel

Assignees
No one assigned
Labels
6.topic: qt/kde 10.rebuild-darwin: 101-500 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@K900 @veprbl