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

mpv: enable swift support #214944

Merged
merged 3 commits into from
Mar 6, 2023
Merged

mpv: enable swift support #214944

merged 3 commits into from
Mar 6, 2023

Conversation

azuwis
Copy link
Contributor

@azuwis azuwis commented Feb 6, 2023

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
Contributor Author

azuwis commented Feb 6, 2023

Related #160828 #204979

@wegank
Copy link
Member

wegank commented Feb 6, 2023

@ofborg build mpv mpv.passthru.tests

@stephank
Copy link
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
Member

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

Nonetheless, I wish good luck.

@azuwis
Copy link
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.

Copy link
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.

@wegank wegank added the 6.topic: darwin Running or building packages on Darwin label Feb 14, 2023
Copy link
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
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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants