Skip to content

mpv: enable swift support#214944

Merged
Atemu merged 3 commits intoNixOS:masterfrom
azuwis:mpv
Mar 6, 2023
Merged

mpv: enable swift support#214944
Atemu merged 3 commits intoNixOS:masterfrom
azuwis:mpv

Conversation

@azuwis
Copy link
Copy Markdown
Contributor

@azuwis azuwis commented Feb 6, 2023 •
edited by AndersonTorres
Loading

Description of changes

Since swift is available on master now, I've rebased @stephank's work on mpv stephank@4ce183c to master, and made this pull request for discussion.

When I used nix-tree to check the closure size of the built mpv, I've found swift lib references clang, and make the closure size pretty big:

Screenshot 2023-02-06 at 22 19 14

$ ls -l /nix/store/2cp55whf6v3631512w4hlsrc4v7r8w36-swift-5.7.3-lib/lib/swift
total 0
dr-xr-xr-x  6 root nixbld 192 Jan  1  1970 FrameworkABIBaseline
dr-xr-xr-x  5 root nixbld 160 Jan  1  1970 _InternalSwiftScan
dr-xr-xr-x  5 root nixbld 160 Jan  1  1970 _InternalSwiftStaticMirror
dr-xr-xr-x  5 root nixbld 160 Jan  1  1970 _InternalSwiftSyntaxParser
dr-xr-xr-x  4 root nixbld 128 Jan  1  1970 apinotes
lrwxr-xr-x  1 root nixbld  78 Jan  1  1970 clang -> /nix/store/avrf44cq2cr18vlvv20aa1pc85cl7rzg-clang-wrapper-14.0.6/resource-root
dr-xr-xr-x 28 root nixbld 896 Jan  1  1970 macosx
dr-xr-xr-x 12 root nixbld 384 Jan  1  1970 migrator
dr-xr-xr-x 27 root nixbld 864 Jan  1  1970 shims

Is that expected?

Removing preConfigure will make the closure size even bigger:

Screenshot 2023-02-06 at 22 27 09

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/)
  • 23.05 Release Notes (or backporting 22.11 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.

@azuwis
Copy link
Copy Markdown
Contributor Author

azuwis commented Feb 6, 2023

Related #160828 #204979

wegank
wegank reviewed Feb 6, 2023
View reviewed changes
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Feb 6, 2023
@azuwis azuwis force-pushed the mpv branch 2 times, most recently from 53b6d6a to 5988220 Compare February 6, 2023 14:56
@wegank wegank mentioned this pull request Feb 6, 2023
Closed
13 tasks
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. labels Feb 6, 2023
@wegank
Copy link
Copy Markdown
Member

wegank commented Feb 6, 2023

@ofborg build mpv mpv.passthru.tests

@stephank
Copy link
Copy Markdown
Contributor

stephank commented Feb 6, 2023

Thanks for point out the closure size! It's something I'll make a PR for separately.

I'm seeing the same build failure as ofBorg on x86_64-darwin, but I don't understand it yet.

@AndersonTorres
Copy link
Copy Markdown
Member

AndersonTorres commented Feb 6, 2023

Unfortunately (or not) I can't possibly comment, because this is too Darwin-specific to me.

Nonetheless, I wish good luck.

@azuwis
Copy link
Copy Markdown
Contributor Author

azuwis commented Feb 7, 2023

The error log on x86_64-darwin is different before and after commit 64bce1d

Before(https://logs.nix.ci/?key=nixos/nixpkgs.214944&attempt_id=5ab1c3e7-d414-4bbe-9a9a-0bce635c1dc4):

[21/452] Generating generated/osdep/swift_targets with a custom command
FAILED: generated/osdep/macOS_swift.swiftmodule generated/osdep/macOS_swift.h generated/osdep/macOS_swift.o 
/nix/store/5mldrl986vi6b8lk3gdw8r5mjxqx5i9j-swift-wrapper-5.7.3/bin/swift -frontend -c -sdk /nix/store/bfgqhz5n5ad83h9bqxzvhz158fb03vyz-SDKs/MacOSX10.12.sdk -enable-objc-interop -emit-objc-header -parse-as-library -O -D HAVE_MACOS_10_11_FEATURES -module-name macOS_swift -emit-module-path generated/osdep/macOS_swift.swiftmodule -import-objc-header /private/tmp/nix-build-mpv-0.35.1.drv-0/source/osdep/macOS_swift_bridge.h -emit-objc-header-path generated/osdep/macOS_swift.h -o generated/osdep/macOS_swift.o ../osdep/macos/libmpv_helper.swift ../osdep/macos/log_helper.swift ../osdep/macos/mpv_helper.swift ../osdep/macos/swift_compat.swift ../osdep/macos/swift_extensions.swift ../video/out/mac/common.swift ../video/out/mac/title_bar.swift ../video/out/mac/view.swift ../video/out/mac/window.swift ../video/out/cocoa_cb_common.swift ../video/out/mac/gl_layer.swift ../osdep/macos/remote_command_center.swift -I. -I/private/tmp/nix-build-mpv-0.35.1.drv-0/source
../osdep/macos/libmpv_helper.swift:18:8: error: no such module 'Cocoa'
import Cocoa
       ^
Please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the project and the crash backtrace.

After(https://logs.nix.ci/?key=nixos/nixpkgs.214944&attempt_id=76802592-760c-47e1-9141-4465b7c78f4c):

[21/452] Generating generated/osdep/swift_targets with a custom command
FAILED: generated/osdep/macOS_swift.swiftmodule generated/osdep/macOS_swift.h generated/osdep/macOS_swift.o 
/nix/store/5mldrl986vi6b8lk3gdw8r5mjxqx5i9j-swift-wrapper-5.7.3/bin/swift -frontend -c -sdk /nix/store/y421982rvq7fwfcygxhwala06fxq4sdm-SDKs/MacOSX11.0.sdk -enable-objc-interop -emit-objc-header -parse-as-library -O -D HAVE_MACOS_10_11_FEATURES -D HAVE_MACOS_10_14_FEATURES -module-name macOS_swift -emit-module-path generated/osdep/macOS_swift.swiftmodule -import-objc-header /private/tmp/nix-build-mpv-0.35.1.drv-0/source/osdep/macOS_swift_bridge.h -emit-objc-header-path generated/osdep/macOS_swift.h -o generated/osdep/macOS_swift.o ../osdep/macos/libmpv_helper.swift ../osdep/macos/log_helper.swift ../osdep/macos/mpv_helper.swift ../osdep/macos/swift_compat.swift ../osdep/macos/swift_extensions.swift ../video/out/mac/common.swift ../video/out/mac/title_bar.swift ../video/out/mac/view.swift ../video/out/mac/window.swift ../video/out/cocoa_cb_common.swift ../video/out/mac/gl_layer.swift ../osdep/macos/remote_command_center.swift -I. -I/private/tmp/nix-build-mpv-0.35.1.drv-0/source
<module-includes>:1:9: note: in file included from <module-includes>:1:
#import "Headers/OpenGL.h"
        ^
/nix/store/7i2mvhisb2wnjcvc9jvzlxn78mlsiazq-apple-framework-OpenGL-11.0.0/Library/Frameworks/OpenGL.framework/Headers/OpenGL.h:31:147: error: pasting formed '__MAC_10.0', an invalid preprocessing token
extern CGLError CGLChoosePixelFormat(const CGLPixelFormatAttribute *attribs, CGLPixelFormatObj OPENGL_NULLABLE * OPENGL_NONNULL pix, GLint *npix) OPENGL_DEPRECATED(10.0, 10.14);
                                                                                                                                                  ^
/nix/store/xgb5hzizdmrh83672gxn5llx74vqz0r3-apple-framework-OpenGL/Library/Frameworks/OpenGL.framework/Headers/OpenGLAvailability.h:11:74: note: expanded from macro 'OPENGL_DEPRECATED'
#define OPENGL_DEPRECATED(from, to) __OSX_AVAILABLE_BUT_DEPRECATED(__MAC_##from, __MAC_##to, __IPHONE_NA, __IPHONE_NA)
                                                                         ^

I can reproduce the same error using nix build .#legacyPackages.x86_64-darwin.mpv on a aarch64-darwin machine.

Atemu
Atemu approved these changes Feb 7, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@Atemu Atemu left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 214944 run on aarch64-darwin 1

1 package built:
  • mpv

Diff LGTM and it works just fine on aarch64-darwin.

Closure size optimisation can come later, that's not a hard blocker IMO.

Once the x86 issue is fixed, this is good to go.

@mweinelt mweinelt added the 12.approvals: 1 This PR was reviewed and approved by one person. label Feb 8, 2023
@wegank wegank added the 6.topic: darwin Running or building packages on Darwin label Feb 14, 2023
ylh
ylh reviewed Feb 27, 2023
View reviewed changes
Atemu
Atemu reviewed Feb 27, 2023
View reviewed changes
Atemu
Atemu approved these changes Feb 28, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@Atemu Atemu left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 214944 run on aarch64-darwin 1

5 packages marked as broken and skipped:
  • hydrus
  • jellyfin-mpv-shim
  • mpc-qt
  • plex-mpv-shim
  • sublime-music
2 packages failed to build:
  • mnemosyne
  • subtitleedit
11 packages built:
  • ani-cli
  • anki
  • cplay-ng
  • curseradio
  • dmlive
  • mpv-unwrapped
  • python310Packages.mpv
  • python311Packages.mpv
  • somafm-cli
  • wtwitch
  • ytfzf

@wegank
Copy link
Copy Markdown
Member

wegank commented Mar 6, 2023

Result of nixpkgs-review pr 214944 run on aarch64-darwin 1

5 packages marked as broken and skipped:
  • hydrus
  • jellyfin-mpv-shim
  • mpc-qt
  • plex-mpv-shim
  • sublime-music
13 packages built:
  • ani-cli
  • anki
  • cplay-ng
  • curseradio
  • dmlive
  • mnemosyne
  • mpv-unwrapped
  • python310Packages.mpv
  • python311Packages.mpv
  • somafm-cli
  • subtitleedit
  • wtwitch
  • ytfzf

@Atemu Atemu merged commit 1cd0291 into NixOS:master Mar 6, 2023
@azuwis azuwis deleted the mpv branch April 11, 2023 06:23
reckenrode added a commit to reckenrode/nixpkgs that referenced this pull request Jul 26, 2023
@reckenrode
mpv: enable Swift support on x86_64-darwin
1b26b41 
Swift support doesn’t seem to work with back-deployed dylibs, so set the
deployment target to 10.15 to force Swift to link against the system
Swift libraries. The change to use the Swift stdenv fixes the errors
reported in NixOS#214944.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wegank wegank wegank approved these changes

@Atemu Atemu Atemu approved these changes

@AndersonTorres AndersonTorres Awaiting requested review from AndersonTorres

@tadeokondrak tadeokondrak Awaiting requested review from tadeokondrak

@globin globin Awaiting requested review from globin

@fpletz fpletz Awaiting requested review from fpletz

@Ma27 Ma27 Awaiting requested review from Ma27

+1 more reviewer

@ylh ylh ylh left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@azuwis @wegank @stephank @AndersonTorres @ylh @Atemu @mweinelt