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

meson: use swiftc instead of swift for building #15442

Merged
merged 1 commit into from
Dec 7, 2024

Conversation

NekoAsakura
Copy link
Contributor

@NekoAsakura NekoAsakura commented Dec 5, 2024

Use swiftc instead of swift for building on macOS. This is requested in #13608.

The compile has been tested and successfully passed on M1 Air running Sequoia 15.1.1(Swift version 6.0.2).

Ref doc: https://github.com/swiftlang/swift/blob/main/docs/Driver.md

Copy link

github-actions bot commented Dec 5, 2024

Download the artifacts for this pull request:

Windows
macOS

@NekoAsakura NekoAsakura changed the title meson.build: use swiftc instead of swift for building meson: use swiftc instead of swift for building Dec 5, 2024
@Akemi
Copy link
Member

Akemi commented Dec 5, 2024

i am wondering:
why was -enable-objc-interop removed? i assume there is no equivalent for swiftc and it 'just works'.
what is the reason for -static?

that's a lot less changes than i anticipated tbh.

[edit]
also on a side note since i always forget it, one can pass --help-hidden to swift and swiftc to get a more exhaustive help output.

@kasper93
Copy link
Contributor

kasper93 commented Dec 5, 2024

I was wondering, if it is possible to use native Meson swift compiler detection? They at least have the swift support defined https://github.com/mesonbuild/meson/blob/master/mesonbuild/compilers/swift.py not sure how it works if at all, but maybe it would be possible to use?

@Akemi
Copy link
Member

Akemi commented Dec 5, 2024

we tried this initially but it didn't work around 3 years ago (somewhere hidden here #8840 or maybe on IRC, #8840 (comment)). i believe one of the biggest blockers was, that we have code that operates in both direction (objective-)c<>swift and the meson integration doesn't support this (eg only swift only worked/works?)

@NekoAsakura
Copy link
Contributor Author

NekoAsakura commented Dec 5, 2024

why was -enable-objc-interop removed?

swiftc does not accept -enable-objc-interop flag. It is unclear whether swiftc automatically detects it or if it defaults to true, but -enable-objc-interop can be seen in the verbose output during compiling.

May related to: https://github.com/swiftlang/swift/blob/464ff30cfae713fab658fbd071954dbd726b678f/lib/DriverTool/sil_func_extractor_main.cpp#L128-L132

Here is an excerpt from verbose output by swiftc -c -v ...

...
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift-frontend \
    -frontend -c \
    ../osdep/mac/application.swift \
    ../osdep/mac/app_hub.swift \
    ../osdep/mac/event_helper.swift \
    ../osdep/mac/input_helper.swift \
    ../osdep/mac/libmpv_helper.swift \
    ../osdep/mac/log_helper.swift \
    ../osdep/mac/menu_bar.swift \
    ../osdep/mac/option_helper.swift \
    ../osdep/mac/precise_timer.swift \
    ../osdep/mac/presentation.swift \
    ../osdep/mac/swift_compat.swift \
    ../osdep/mac/swift_extensions.swift \
    ../osdep/mac/type_helper.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 \
    ../video/out/mac_common.swift \
    ../video/out/mac/metal_layer.swift \
    ../osdep/mac/remote_command_center.swift \
    -primary-file ../osdep/mac/touch_bar.swift \
    -target arm64-apple-macosx15.0 \
    -Xllvm -aarch64-use-tbi \
    -enable-objc-interop \
    -stack-check \
    -sdk /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.1.sdk \
    -I . \
    -I /Users/neko/Projects/Build/mpv \
    -I /opt/homebrew/Cellar/libplacebo/7.349.0/include \
    -I /opt/homebrew/Cellar/ffmpeg/7.1_3/include \
    -color-diagnostics \
    -static \
    -swift-version 5 \
    -O \
    -D HAVE_MACOS_10_15_4_FEATURES \
    -D HAVE_MACOS_11_FEATURES \
    -D HAVE_MACOS_12_FEATURES \
    -D HAVE_MACOS_COCOA_CB \
    -D HAVE_MACOS_TOUCHBAR \
    -D HAVE_MACOS_MEDIA_PLAYER \
    -new-driver-path /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift-driver \
    -empty-abi-descriptor \
    -resource-dir /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift \
    -import-objc-header /var/folders/_y/ch_6sfqx3cb9r6z4yb15dzgw0000gn/T/TemporaryDirectory.KVxuoe/app_bridge_objc.pch \
    -module-name swift \
    -disable-clang-spi \
    -target-sdk-version 15.1 \
    -target-sdk-name macosx15.1 \
    -external-plugin-path '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/usr/lib/swift/host/plugins#/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/usr/bin/swift-plugin-server' \
    -external-plugin-path '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/usr/local/lib/swift/host/plugins#/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/usr/bin/swift-plugin-server' \
    -plugin-path /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/host/plugins \
    -plugin-path /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/local/lib/swift/host/plugins \
    -enable-default-cmo \
    -parse-as-library \
    -o /var/folders/_y/ch_6sfqx3cb9r6z4yb15dzgw0000gn/T/TemporaryDirectory.KVxuoe/touch_bar-1.o

...

what is the reason for -static?

Our target linked library is static(*.o), rather dynamic(*.dylib) by default.
Excerpt from swiftc --help

-emit-library           Emit a linked library
-static                 Make this module statically linkable and make the output of -emit-library a static library.

that's a lot less changes than i anticipated tbh.

I agree.

@NekoAsakura
Copy link
Contributor Author

I haven’t tried it, but it seems that the meson integration doesn’t include all options we need.
Addtionally, swiftc compiler is actually quite intelligent; I originally thought we’d need to make more adjustments, such as specifying the link search path, etc. I didn't see the necessary to use meson integration.

@Akemi
Copy link
Member

Akemi commented Dec 5, 2024

Our target linked library is static(.o), rather dynamic(.dylib) by default.

let me rephrase the question. this was the case before too (.o file), but we didn't need to add the -static flag. is it needed needed now? if it is needed why did it work before? just wondering what this actually changes or what the implications are.

also you need to adjust the commit message, the PR title would be fine.

@NekoAsakura
Copy link
Contributor Author

This is because swift -frontend performs compile only, but swiftc do both compile and linking.

Linking is made by ld/libtool(depends on macOS version), and requires a flag to specify whether the target library is dynamic or static.

In the doc ref I mentioned above:

Part of Swift's model is that a file can implicitly see entities declared elsewhere in the same module as long as they have not been marked private. Consequently, compiling any one file needs some knowledge of all the others. However, we don't want a build model that rules out incremental builds, nor one that forces non-parallel compilation. Consequently, the Swift driver has three stages of subprocesses:

  1. "Frontend jobs"
  2. "Module merging"
  3. Linking

The third step requires a -static flag otherwise it will tries to emit a dynamic lib.

I attached the verbose output (swiftc_verbose.txt). Check it If you'd like to view how it works in the code.

@Akemi
Copy link
Member

Akemi commented Dec 7, 2024

thanks for you work on this one, it's much appreciated.

@Akemi Akemi merged commit 8d20b72 into mpv-player:master Dec 7, 2024
25 checks passed
@Dudemanguy
Copy link
Member

Dudemanguy commented Dec 7, 2024

I didn't see the necessary to use meson integration.

IMO, it would be much nicer to have meson integration if possible for this. Directly building the compiler command kind of sucks and there's a lot of dumb manual things we have to do (e.g. like adding defines for HAVE_MACOS_*). Of course, I don't know if it's even possible for it to work currently. If I remember right, back when I initially tried swift support in meson, one of the blockers was that the meson integration always used swiftc and that wouldn't compile correctly at the time with the swift code. So we stuck to manually calling swift. Nice to see that swiftc at least works now, but I'm sure there would be more that is required than just that.

@NekoAsakura
Copy link
Contributor Author

NekoAsakura commented Dec 7, 2024

I’m not an expert in meson, and meson manual doesn’t include any references to swift. I think I’ll give it a try when I have some free time.

[Edit]
If meson integration use swiftc, it is possible to work because we no longer need to explicitly pass -enable-objc-interop.

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

Successfully merging this pull request may close these issues.

4 participants