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

darwin-arm64プラットフォーム対応 #2112

Merged
merged 4 commits into from
Jun 4, 2024

Conversation

nix6839
Copy link
Contributor

@nix6839 nix6839 commented Jun 2, 2024

内容

GitHub Actionにdarwin-arm64ビルドを追加しました。
このPRのリリースは次のリンクでご覧いただけます: https://github.com/nix6839/voicevox/releases/tag/0.20.0-nix6839-macos-arm64.1
このPRのリリースは次のリンクでご覧いただけます: https://github.com/nix6839/voicevox/releases/tag/0.20.0-nix6839-macos-arm64

関連 Issue

close: #348
VOICEVOXをApple Silicon macOSでネイティブに動作させるという根本的な目的は達成しましたし、Universal2アプリをリリースするには大変な労力がかかると予想されるため、このイシューをクローズしても良いと思います。どう思いますか?

その他

すべて完成しましたが、voicevox_engineが0.20.0をリリースするのを待たなければならないので、今はドラフトです。

他のコミットに関する説明

  • アクションのmacOSビルドでアドホック署名:
    Apple Siliconでネイティブに動作するアプリはコード署名がないと動作しないため、アドホック署名を追加しました。
  • macOSの.appファイルが正しい構造を持つように修正:
    次の部分を修正しました

    Contents/MacOSフォルダには実行ファイルのみが存在するべきですが、
    その他のファイルも含まれていたため、正しい構造ではなく、
    コード署名が失敗しました。

  • preview:
    これは一時的にビルドを動作させるためのコミットです。後で削除する予定です。

@nix6839 nix6839 requested a review from a team as a code owner June 2, 2024 06:42
@nix6839 nix6839 requested review from Hiroshiba and removed request for a team June 2, 2024 06:42
@nix6839
Copy link
Contributor Author

nix6839 commented Jun 2, 2024

Test / e2e-test (macos-latest) (pull_request)が失敗した理由は、voicevox_nemo_negineにmacos-arm64ビルドがないためだと思います。今はそのまま進めても良いと思います。

@nix6839 nix6839 marked this pull request as draft June 2, 2024 07:22
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

プルリクエストありがとうございます!!

VOICEVOXをApple Silicon macOSでネイティブに動作させるという根本的な目的は達成しましたし、Universal2アプリをリリースするには大変な労力がかかると予想されるため、このイシューをクローズしても良いと思います。どう思いますか?

賛成です!理由はいくつかあります。

  • 労力がかかると予想されるため
  • M1 macが発売されてから4年という長めの時間が経過したため
  • 今の容量が1.3GBあり、倍の2.6GBになると流石に大きすぎるため

もし容量の問題が解決したらユニバーサル版の開発を頑張っても良いかもと思っていますが、現状だとクローズが良いと思います!

すべて完成しましたが、voicevox_engineが0.20.0をリリースするのを待たなければならないので、今はドラフトです。

0.20-preview.0エンジンを使って正しく動くのであれば、マージしてもOKです!
そして無事に動くことを確認してから、エンジンの0.20.0とエディタ(このリポジトリ)の0.20.0をリリースします。

.github/workflows/test.yml Outdated Show resolved Hide resolved
@Hiroshiba
Copy link
Member

Hiroshiba commented Jun 2, 2024

@PickledChair @HyodaKazuaki
リプライすみません! またもし良かったらレビューの形で力をお借りできると・・・!! 🙇

以前署名周りの話がちらっと出ていたと思うので、issueを掘り起こしていました。
Pyinstaller v6のときに調査したことがあり、この辺りに記載がありました!
Resourceディレクトリに関してもちょこっと言及がありました。

@nix6839 nix6839 marked this pull request as ready for review June 2, 2024 21:16
@nix6839
Copy link
Contributor Author

nix6839 commented Jun 2, 2024

以前署名周りの話がちらっと出ていたと思うので、issueを掘り起こしていました。
Pyinstaller v6のときに調査したことがあり、この辺りに記載がありました!
Resourceディレクトリに関してもちょこっと言及がありました。

こちらの部分について詳しく説明するつもりだったのですが、うっかり忘れてしまいました😅
本来は voicevox_engineを .appにするか、または単一実行ファイル(pyinstaller の onefileで作成)にすることで解決しようと考えていました。しかし、どちらの方法も簡単にできるものではなく、試行錯誤の末に断念し、現在の方向に切り替えました。
時間ができたときに、上記の二つの方法のいずれかに変更するのがスマートだと思います。

@nix6839 nix6839 requested a review from Hiroshiba June 2, 2024 21:26
@PickledChair
Copy link
Member

PickledChair commented Jun 3, 2024

この PR のテストビルドは私の環境でも正常に動作しました。また、macOS 向けビルドにおいて app bundle の構造を適切に整理することは望ましいことだと思います。

ただし、もしその作業を進めるなら、次の理由で別の Issue に切り離して作業を進めた方が良いと感じました:

  • 今回の PR による改善はまだ不完全である:
    • 確かに Contents/MacOS フォルダには実行ファイル(あるいは app bundle )のみが含まれているのが標準的です。ただしこれを徹底するなら、Contents/MacOS にまだ残っている README.txt もどうすべきか考えた方が良いことになります。
    • また、vv-engine フォルダには多くの共有ライブラリが含まれていますが、同じ方針で整理を進めるなら、これらも Contents/Frameworks に移動すべきファイルだと考えられます。しかし、エンジンのパッケージングには PyInstaller を使用しているので、配布後のエンジンにおいて run ファイルと run が依存している共有ライブラリの位置関係を変更するのは大変です(備考: run ファイル自体も実行ファイルなので、Contents/Resources にそれを置くのは不適切に思えますが、helper アプリケーションの位置は自由度が高いように見えます。Electron 製のアプリでは Electron の helper app が Contents/Frameworks にありますし、App Store の外部で配布されているアプリケーションでは Contents/Resources に実行ファイルが配置されている例も時々見かけます)。
    • エンジンを .app にするか onefile にすれば、エンジンをそのまま Contents/MacOS 内に置いても不自然ではありませんが、言及されている通りどちらも簡単ではありません。また、onefile オプションの指定では別の問題が発生します。(ところで、現在使用している PyInstaller は v5 ですが、v6 にアップデートすると問題はさらに複雑になります。それが原因で以前に revert が行われました。)
  • ファイルの位置関係に関して、他のプラットフォーム (Windows, Linux) との差異が生じる:
    • エンジンの存在している位置が他のプラットフォームと異なるようになるだけなので、現状はそこまで複雑な事態にはなりません。幸い、現在の VOICEVOX はエンジンの存在している位置に依存しない設計になっている(実はエンジン側の方で些細な部分ではありますが依存していることを発見しました……。しかし主要な機能には影響がないので急いで対応する必要はなさそうです)ので、エンジンを Contents/Resources に移しても正常に動作するように見えます。
    • しかし、他のプラットフォームのことを考慮せずに macOS 向けに独自の変更を加え続けると、将来的にメンテナンスが困難になる可能性があります(今回は問題にはならなかったものの、現在の VOICEVOX は VOICEVOX エディタの実行ファイルと同じディレクトリ階層に vv-engine があるのが暗黙の前提になっている気がします)。したがって、実行ファイル・共有ライブラリ・リソースファイルなどの位置を整理する場合、最終的にはそれぞれのプラットフォームについて最適な配置を考え、統一されたインターフェースを整え、それをはっきり文書化する必要がありそうです。関連する作業を1つのタイムラインで追える Issue が必要だと思いました。

私も VOICEVOX の app bundle の構造が標準的ではないことを把握していましたが、開発の進めやすさを考慮して、他のプラットフォーム向けに近い構造のまま放置していました(残念なことに bundle の構造について詳しいわけではなく、問題を先送りにしていました)。最近も開発が活発に続いていますが、ファイルの配置については以前より大きな変更の可能性が少なくなってきていると個人的に思うので、この問題について取り組んでも良いタイミングかもしれません。問題解決のモチベーションとしては、bundle の構造についての Apple の文書にあるように、コード署名をするときの問題を避けられることが挙げられます ("If you put content in the wrong location, you may encounter hard-to-debug code signing and distribution problems.") 。

一方で、bundle の構造の整理をどこまで真面目にやるかは論点の1つになると思います。現状でも動作は可能であり、コード署名で問題が発生せず App Store で配布することもない場合は現状で十分という見方もあるからです(これが現在までの私の怠惰な姿勢でした)。

(コード署名の話題で気づいたのですが、今回 vv-engineContents/Resources に移したのは、コード署名の時に Contents/MacOS にある vv-engine が障害となったからでしょうか? もしそうであれば、一時的な措置として今回の変更を取り入れるのが良いと思います。そうでなければ、この PR ではこれまでの構造のままでも良いと思いました。 @Hiroshiba どうでしょうか……? )

@nix6839
Copy link
Contributor Author

nix6839 commented Jun 3, 2024

コード署名の話題で気づいたのですが、今回 vv-engine を Contents/Resources に移したのは、コード署名の時に Contents/MacOS にある vv-engine が障害となったからでしょうか? もしそうであれば、一時的な措置として今回の変更を取り入れるのが良いと思います。

はい移さないとコード署名が失敗します。本文に書いておいたつもりでしたが、今見たらカットされていましたので修正しました。

@nix6839
Copy link
Contributor Author

nix6839 commented Jun 3, 2024

コミットメッセージの本文の誤字を修正するために強制プッシュしました。

@PickledChair
Copy link
Member

PickledChair commented Jun 3, 2024

はい移さないとコード署名が失敗します。

変更の意図を理解しました! Contents/Resourcesvv-engine を移すのは必要な措置なので賛成です。

Copy link
Contributor

@HyodaKazuaki HyodaKazuaki left a comment

Choose a reason for hiding this comment

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

私の環境だとzipファイルvoicevox-macos-arm64-cpu-0.20.0-nix6839-macos-arm64.zipは「壊れているため開けません」と表示されて起動できませんでした
dmgファイルVOICEVOX.0.20.0-nix6839-macos-arm64-arm64.dmgは起動できました。

Universal2 binaryについては私も調べてみましたが、コード署名以外に特にメリットはなさそうです。
そのため、universal2 binaryにする必要は今のところないと考えて良さそうです。

バンドルについては、別のIssueで議論することに賛成です。
個人的には、JREが同梱されているアプリが参考になるのではないかと考えています。
例えばあるJREが同梱されたアプリでは、Pluginsの中にJREが入っていました。
SomeApp.app/Contents/Plugins/Java.runtime

@nix6839
Copy link
Contributor Author

nix6839 commented Jun 3, 2024

私の環境だとzipファイルvoicevox-macos-arm64-cpu-0.20.0-nix6839-macos-arm64.zipは「壊れているため開けません」と表示されて起動できませんでした

ありがとうございます! コードの署名箇所をzipファイルを生成する前に移動しました。

@nix6839
Copy link
Contributor Author

nix6839 commented Jun 3, 2024

新しいリリース: https://github.com/nix6839/voicevox/releases/tag/0.20.0-nix6839-macos-arm64.1
これでzipファイルが正常に動作します。

@nix6839 nix6839 requested a review from HyodaKazuaki June 3, 2024 15:26
Comment on lines +325 to +327
- name: Ad hoc code signing
if: endsWith(matrix.installer_artifact_name, '-dmg') # macOS
run: codesign --force --deep -s - prepackage/VOICEVOX.app
Copy link
Member

@Hiroshiba Hiroshiba Jun 3, 2024

Choose a reason for hiding this comment

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

ちょっとお聞きしたいことがあります!

--force --deepでのcodesignは、署名済みファイルもad-hoc署名で上書きされてしまいますか?
もし上書きされるなら、顕著な問題ではありませんが、避けておきたい問題かもしれません。

--forceでのcodesignが必要なファイルのみad-hoc署名する、ということはできそうでしょうか・・・?
vv-engine/runだけ署名すれば問題ない・・・?)

難しければ、FIXMEコメントを書きつつ様子見でも良いかもしれません。

Copy link
Contributor Author

@nix6839 nix6839 Jun 4, 2024

Choose a reason for hiding this comment

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

  • コード署名前のファイル情報:
    コード署名前のファイル情報
  • コード署名後のファイル情報:
    コード署名後のファイル情報

両者の違いを見ると、以下の変更が発生しました。

  • Contents/_CodeSignature/ディレクトリの追加
  • Contents/MacOS/ディレクトリ内の実行ファイル(README.txtを除く)の最終変更日時とファイルサイズの変更(コード署名されたと推測)

質問に答えると

--force --deepでのcodesignは、署名済みファイルもad-hoc署名で上書きされてしまいますか?
もし上書きされるなら、顕著な問題ではありませんが、避けておきたい問題かもしれません。

他のファイルには問題ありませんが、7zzが既に署名されていた場合は上書きされると推測されます。

--forceでのcodesignが必要なファイルのみad-hoc署名する、ということはできそうでしょうか・・・?
vv-engine/runだけ署名すれば問題ない・・・?)

.appディレクトリを署名するには--deepフラグが必要であり、この場合、--forceの有無に関わらず上記のような動作をします。したがって、答えは不可能です。

ただし、解決策があります。上記に挙げられた箇所以外では署名が行われないようなので、署名したくないファイルを他の場所(例:ResourcesFrameworksPlugins?(ここはよくわかりませんが))に移動すれば良いと思います。

結局これも.appディレクトリの構造に関することなので、別にFIXMEコメントを書かなくても大丈夫でしょうか?

Copy link
Member

@PickledChair PickledChair Jun 4, 2024

Choose a reason for hiding this comment

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

他のファイルには問題ありませんが、7zzが既に署名されていた場合は上書きされると推測されます。

7zz の署名情報を確認してみました。7zz は macOS の場合、ビルド時に次のリンクからダウンロードされたものをパッケージに含めます:

url = "https://www.7-zip.org/a/7z2107-mac.tar.xz";

この 7zz の署名情報を確認したところ、以下のように Signature=adhoc と表示されているので、今のところ署名を上書きしても問題はないと考えられます。今後証明書を用意してコード署名することがある場合は、むしろ積極的に署名しなければなりません:

$ codesign -dv ./7zz
Executable=/Users/graysuitcase/Downloads/7zz
Identifier=7zz
Format=Mach-O universal (x86_64 arm64)
CodeDirectory v=20400 size=19420 flags=0x20002(adhoc,linker-signed) hashes=604+0 location=embedded
Signature=adhoc
Info.plist=not bound
TeamIdentifier=not set
Sealed Resources=none
Internal requirements=none

Copy link
Member

Choose a reason for hiding this comment

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

なるほどです!!

そもそも、企業により正しく署名されたファイルがadhoc署名に置き換わってしまう、ということを懸念していました。
今手元で調べてみると、Pythonですらadhoc署名でした。
libonnxruntime.dylibはadhoc署名すらされていませんでした!
つまり「企業により正しく署名されたファイル」はほとんど無い、というふうに考えが変わりました。

@nix6839 情報ありがとうございます!

なるほど、.appの署名は特別だったんですね!!
丁寧にありがとうございます。FIXMEもなくて大丈夫だと思います!

@PickledChair 検証ありがとうございます!
たぶん *.dylib系もadhoc署名されていると思います。もともとadhocか、署名なしだと思いますが 😇

@Hiroshiba
Copy link
Member

@PickledChair
詳しくありがとうございます!!
昔一度調べたのですが、やはりMacOS・Resources・Frameworkディレクトリ周りは難しいですね・・・。

一方で、bundle の構造の整理をどこまで真面目にやるかは論点の1つになると思います。

個人的には、そこそこメンテナンス性が高ければ、わりと「動けば良い」と思っています!
というのも理由が2つあって、1つはエンジンの構成とmacOSが重視するポイントを含めると結構ころころ変わるので結局都度調整が必要そうなのと、もう1つはもし可能であれば将来的にエンジンは同梱しない形にしたいので問題を迂回できそうなためです。

とはいえ、もし余力があればbundleの理想の形を整理し、そこから現状どうなっているかを把握しておけると嬉しいかもではあります。
その場合はissueで議論をまとめて、あとはdocs以下かあるいはbuld.yaml内のコメントとしてドキュメントにまとめる形が良さそう?

(あと個人的に、今の形であればエンジン側をpyinstaller v6にしても大丈夫なのかは若干気になってます。もし大丈夫なのであれば、v6にするissue建てたいな~と!)

@nix6839 nix6839 requested a review from Hiroshiba June 4, 2024 01:07
nix6839 and others added 4 commits June 4, 2024 10:14
Contents/MacOSフォルダには実行ファイルのみが存在するべきですが、
その他のファイルも含まれていたため、正しい構造ではなく、
コード署名が失敗しました。
Co-Authored-By: Hiroshiba <hihokaruta@gmail.com>
@nix6839
Copy link
Contributor Author

nix6839 commented Jun 4, 2024

最新のメインブランチを基にリベースして強制プッシュ

Copy link
Contributor

@HyodaKazuaki HyodaKazuaki left a comment

Choose a reason for hiding this comment

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

LGTMです

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

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

LGTM!!!

@nix6839 @PickledChair @HyodaKazuaki
皆様ありがとうございました!!!

自動ビルドの結果が楽しみですね・・・!

@Hiroshiba Hiroshiba merged commit ebdb854 into VOICEVOX:main Jun 4, 2024
9 checks passed
@nix6839 nix6839 deleted the macos-arm64 branch June 4, 2024 16:00
@Hiroshiba
Copy link
Member

Hiroshiba commented Jun 4, 2024

@nix6839 ちょっとご相談が・・・!
VOICEVOX公式Xのポストで、 @nix6839 さんのXアカウントを言及させていただいても良いでしょうか? 🙇

ニーズのある機能が実装されたときにSNSで言及しておりまして、今回のプルリクエストもツイートしたいと思っています。
https://x.com/search?q=%23VOICEVOX%E9%96%8B%E7%99%BA%E7%8A%B6%E6%B3%81
もしよかったらそこで @nix6839 さんのXアカウントをツイート文に含めて紹介させていただきたいのですが、いかがでしょうか・・・?

(RPされたり、ユーザーからリプライで届く感謝の言葉をお届けできればという意図と、あと開発者が分かるので新規コミッターの方がOSS開発に興味を持ってくださる導線を増やせればという意図があります・・・!)

こんな内容を予定しています・・・!

#VOICEVOX開発状況 
arm64 版 Mac(M1/M2/M3)用のビルドが追加されました  🎉(今後のアップデートで実装されます。)
【開発者: @nix6839 】
https://github.com/VOICEVOX/voicevox/pull/2112

もしXアカウントを載せてもよければ、アカウントを教えて下さい!
ダメであればGithubアカウントを載せさせていただければ!

@nix6839
Copy link
Contributor Author

nix6839 commented Jun 4, 2024

もしXアカウントを載せてもよければ、アカウントを教えて下さい!
ダメであればGithubアカウントを載せさせていただければ!

私はXのアカウントを持っていません🥲 GitHubのアカウントはご自由にお願いします!

@Hiroshiba
Copy link
Member

承知しました!

ご報告忘れていました! こちらにてポストさせていただきました!🙏
https://x.com/voicevox_pj/status/1798222017373880549

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.

Macユニバーサルアプリ版のリリース:Release a Mac Universal App version.
4 participants