Skip to content

MacビルドをPyInstallerへ移行し、Windowsとビルドジョブを統合する#446

Merged
y-chan merged 8 commits intoVOICEVOX:pyinstallerfrom
y-chan:feature/pyinstaller-mac
Aug 12, 2022
Merged

MacビルドをPyInstallerへ移行し、Windowsとビルドジョブを統合する#446
y-chan merged 8 commits intoVOICEVOX:pyinstallerfrom
y-chan:feature/pyinstaller-mac

Conversation

@y-chan
Copy link
Copy Markdown
Member

@y-chan y-chan commented Aug 9, 2022

内容

題の通り
まだMacでしか試していないので、とりあえずLinuxは無視してWindowsとMacのみビルドジョブ統合しました。

関連 Issue

ref #439
ref #443

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 9, 2022

Coverage Result

Resultを開く
Name Stmts Miss Cover
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/acoustic_feature_extractor.py 75 0 coverage-100%
voicevox_engine/dev/synthesis_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/synthesis_engine/mock.py 36 2 coverage-94%
voicevox_engine/full_context_label.py 162 3 coverage-98%
voicevox_engine/kana_parser.py 86 1 coverage-99%
voicevox_engine/model.py 154 7 coverage-95%
voicevox_engine/mora_list.py 4 0 coverage-100%
voicevox_engine/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/preset/Preset.py 12 0 coverage-100%
voicevox_engine/preset/PresetLoader.py 34 1 coverage-97%
voicevox_engine/preset/init.py 3 0 coverage-100%
voicevox_engine/synthesis_engine/init.py 5 0 coverage-100%
voicevox_engine/synthesis_engine/core_wrapper.py 199 159 coverage-20%
voicevox_engine/synthesis_engine/make_synthesis_engines.py 48 39 coverage-19%
voicevox_engine/synthesis_engine/synthesis_engine.py 133 12 coverage-91%
voicevox_engine/synthesis_engine/synthesis_engine_base.py 66 9 coverage-86%
voicevox_engine/user_dict.py 131 10 coverage-92%
voicevox_engine/utility/init.py 3 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 35 3 coverage-91%
voicevox_engine/utility/engine_root.py 9 2 coverage-78%
TOTAL 1203 248 coverage-79%

@y-chan
Copy link
Copy Markdown
Member Author

y-chan commented Aug 9, 2022

スクリーンショット 2022-08-10 5 07 34

GitHub Actionsでビルドした結果をダウンロードして実行した結果
直接は実行できない(実行権限が足りない。7zで解凍しても同様。artifactを直に落としてくるのはzipだから...?今までと異なる...?)。
手元ではlibcore周りは入れずに試したので気づかなかったが、libcoreが依存するライブラリが足りてなかった...?
今まではスクリプトで直してたor直す必要がない箇所だった...?
さらっと確認した感じ、それ以外は問題なく動く動きそうでした。
ビルドの後の後処理スクリプトを書かれた @PickledChair さん、ご意見いただけると助かります。

Copy link
Copy Markdown
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!!

かなり丁寧に見てみました。良さそう!
もしよかったら @PickledChair さんや @aoirint さんにも見てもらえると頼もしそうです!

Comment on lines -48 to -50
- name: Install CCache
shell: bash
run: brew install ccache
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

これなくて良いんですね。nuitka用っぽみ。

Comment thread .github/workflows/build.yml
Comment thread .github/workflows/build.yml
Comment thread .github/workflows/build.yml Outdated
@PickledChair
Copy link
Copy Markdown
Member

@y-chan

私も実行権限や dylib のロードの仕組みなどについて決して詳しいわけではないので、いつも手探りで動く状態に持っていったりしています……それでも考えられることを以下に書きます。

GitHub Actionsでビルドした結果をダウンロードして実行した結果
直接は実行できない(実行権限が足りない。7zで解凍しても同様。artifactを直に落としてくるのはzipだから...?今までと異なる...?)。

これは画像の chmod 775 run したら実行できた部分と対応していますか? これに関してはこれまでの artifact や release で提供されている 7z も同じだと思うので、これまでと比べて変化があったわけではないと思います(エディタのビルドでも実行権限を付与し直しています https://github.com/VOICEVOX/voicevox/blob/07cf75495277fc8b0dd87fe57659f4883cf15c7a/.github/workflows/build.yml#L491-L500

手元ではlibcore周りは入れずに試したので気づかなかったが、libcoreが依存するライブラリが足りてなかった...?
今まではスクリプトで直してたor直す必要がない箇所だった...?

nuitka を使用していた時とは異なり、pyinstaller は今回は基本的に依存関係を適切に解決していそうです。なので nuitka の時に行っていたビルド後のスクリプト実行は不要になった可能性が高いです。

ただし、libonnxruntime.dylib をロードしようとしている時に libonnxruntime.dylib を見つけられていないようですね。画像では ln -s libonnxruntime.dylib libonnxruntime.1.10.0.dylib を実行した後に onnxruntime のロードが可能になったように見えます(ここから、これさえ解決すればエンジンが起動可能であるため、その他の依存関係は適切に処理されていそうと想像しています)。これは手元でも同じになることを確認しました。

これに関して、nuitka ビルドとの差異を確認したところ、libcore.dylib の他の dylib への依存関係の情報が若干違っていることがわかりました。

nuitka ビルド内の libcore.dylib および voicevox_core で配布されている元の libcore.dylib:

$ otool -L libcore.dylib
libcore.dylib:
	@rpath/libcore.dylib (compatibility version 0.0.0, current version 0.0.0)
	@rpath/libonnxruntime.1.10.0.dylib (compatibility version 0.0.0, current version 1.10.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 904.4.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.60.1)

pyinstaller ビルド内の libcore.dylib:

$ otool -L libcore.dylib
libcore.dylib:
	@loader_path/libcore.dylib (compatibility version 0.0.0, current version 0.0.0)
	@loader_path/libonnxruntime.1.10.0.dylib (compatibility version 0.0.0, current version 1.10.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 904.4.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.60.1)

このことから、pyinstaller が @rpath の部分を @loader_path に変更していると考えられます(おそらく --add-binary オプションの仕様)。直感的には @loader_path に変更されてもロード時に libonnxruntime.dylib が見つかりそうですが、おそらく @loader_path/libonnxruntime.1.10.0.dylib がリンク先として指定されている場合、ファイル名が libonnxruntime.1.10.0.dylib でなければならないように見えます(一方、@rpath/libonnxruntime.1.10.0.dylib が指定されている場合はファイル名が libonnxruntime.dylib でも良いように見える)(完全に想像でしかないですが……)。

以上の考察から、次の3つのどれかで解決しそうな気がします(手元ではまだ確認していないので、ダメだったらすみません……!):

  • オプションを --add-binary "download/core/libcore.dylib:." の代わりに --add-data "download/core/libcore.dylib:." にする(リンク先の情報の編集をさせない)
  • ビルド後に install_name_tool -change @loader_path/libonnxruntime.1.10.0.dylib @rpath/libonnxruntime.1.10.0.dylib dist/run/libcore.dylib をする(変更されたリンク先の情報を元に戻す)
  • onnxruntime を含めるためのオプションを --add-binary "download/onnxruntime/lib/libonnxruntime.1.10.0.dylib:." にする(ファイル名が libonnxruntime.1.10.0.dylib の状態であれば onnxruntime が見つかるようなので)

@y-chan
Copy link
Copy Markdown
Member Author

y-chan commented Aug 10, 2022

これは画像の chmod 775 run したら実行できた部分と対応していますか? これに関してはこれまでの artifact や release で提供されている 7z も同じだと思うので、これまでと比べて変化があったわけではないと思います(エディタのビルドでも実行権限を付与し直しています

であれば大丈夫そうです...!

nuitka を使用していた時とは異なり、pyinstaller は今回は基本的に依存関係を適切に解決していそうです。なので nuitka の時に行っていたビルド後のスクリプト実行は不要になった可能性が高いです。

スクリプトの削除もこのPRでしてしまったほうが良いのかな...?:thinking:

pyinstaller が @rpath の部分を @loader_path に変更していると考えられます

なるほど...! 実際に試した感じ、--add-dataを用いることで解決できました..!
(--add-binaryオプションは、どうやらdylibをuniversalから、ビルドしたバイナリのarchへの矯正もしているようです...!? なんか成果物の容量が100MBどころか400MBも減ったと思ったらそういうことか...)

余談ですが、調べた感じ--target-architecture "universal2"というオプション指定ができるようで、ちょっと頑張ればUniversalビルドも作れそう...?(でもRust版が確かuniversalビルドできないんでしたっけ)

ところで、specファイルの件なんですが、OSごとにファイルが生えそうというのと、CIの設定をspecに書き込んじゃうと手元でビルドするのにファイル名書き換えが必要で嫌かなーと思っていたんですが、今バージョン値を書き換えるのに使ってる感じで、CI内で依存のファイル名をsed使って書き換えるようにすれば、:;しか使えない問題も解決できそうで、いろいろ楽になりそう(独自にビルドするときは自分でバイナリ準備してもらうような形になりそうですが...)と思っているのですが、どうでしょう...:eyes:

@PickledChair
Copy link
Copy Markdown
Member

スクリプトの削除もこのPRでしてしまったほうが良いのかな...?🤔

無くしてしまっても大丈夫かもしれません……!

余談ですが、調べた感じ--target-architecture "universal2"というオプション指定ができるようで、ちょっと頑張ればUniversalビルドも作れそう...?(でもRust版が確かuniversalビルドできないんでしたっけ)

なるほど、universal2 対応できるかもしれないですね。ただ、actions/setup-python#271 の Issue を見つけたのですが、オプションの指定だけでは不足のようで、Python インタプリタ自体も universal2 ビルドである必要がありそうです(あまり分かりやすくは書いていないのですが、PyInstaller 公式のドキュメントもそのような感じのことが読み取れました https://pyinstaller.org/en/stable/usage.html?highlight=--target-architecture#cmdoption-target-architecture )。あとはサードパーティライブラリが共有ライブラリを含んでいた場合も、その共有ライブラリが universal バイナリである必要がありそうな気がします。実験してみないとうまく行くかどうかはなんとも言えない感じですね……。

また、コアの universal バイナリを作るとしたら、Rust 自体は universal ビルドに対応していないので、x64 版と aarch64 版の各 artifact から lipo コマンドにより universal バイナリを作る、という感じになりそうな気がします(参考記事: https://qiita.com/asfdrwe/items/40370f181fea93cfc855

ところで、specファイルの件なんですが、OSごとにファイルが生えそうというのと、CIの設定をspecに書き込んじゃうと手元でビルドするのにファイル名書き換えが必要で嫌かなーと思っていたんですが、今バージョン値を書き換えるのに使ってる感じで、CI内で依存のファイル名をsed使って書き換えるようにすれば、:や;しか使えない問題も解決できそうで、いろいろ楽になりそう(独自にビルドするときは自分でバイナリ準備してもらうような形になりそうですが...)と思っているのですが、どうでしょう...👀

どのようなやり方が最もやりやすいか、なかなか難しいですね……。個人的に考えていたのは、CI ビルドでも手元のビルドでも、リソースや共有ライブラリの配置を特定のディレクトリに限定してしまって、spec ファイルはそれを前提に作る、というようなことです。ただ「:や;しか使えない問題」等が解決すれば、直接オプション指定が楽というのも感じるので、バランスが難しいところですね……。

@y-chan
Copy link
Copy Markdown
Member Author

y-chan commented Aug 10, 2022

無くしてしまっても大丈夫

了解です!みた感じ成果物は正しく動いているみたいなので削除してしまいますね...!

universal2 対応できるかも
Python インタプリタ自体も universal2 ビルドである必要がありそう
共有ライブラリが universal バイナリである必要がありそう
x64 版と aarch64 版の各 artifact から lipo コマンドにより universal バイナリを作る

ここら辺はNuitkaの時にUniversalビルド試そうとした時もそうだったので、なんとなく把握しています... (ここにトライアンドエラーの結果が散らかっており...w)
少なくとも、別PRで考えることになると思うので、一旦は無視します...!

CI ビルドでも手元のビルドでも、リソースや共有ライブラリの配置を特定のディレクトリに限定してしまって、spec ファイルはそれを前提に作る

なるほど、確かにそうしてしまうのもありかも。修正範囲はREADMEだけで済みそうですし、LGTMです。
あと、specファイルは実質Pythonなので、os.envionとか使って、特定の環境変数が指定されてる時だけバイナリを取り込むようにとかすれば、いいかもです...!

@Hiroshiba
Copy link
Copy Markdown
Member

Hiroshiba commented Aug 10, 2022

たしかに 全部 ある程度specでやってしまうのもいいかなと思いました!

@y-chan y-chan force-pushed the feature/pyinstaller-mac branch from 7059173 to deeea6a Compare August 10, 2022 17:03
@y-chan y-chan force-pushed the feature/pyinstaller-mac branch from deeea6a to a563396 Compare August 10, 2022 17:10
@Hiroshiba Hiroshiba self-requested a review August 10, 2022 17:20
@y-chan
Copy link
Copy Markdown
Member Author

y-chan commented Aug 10, 2022

specファイルを用いてビルドスクリプトを統一・簡略化し、加えて総容量を更に100MBほど減らしました。

PyInstaller Nuitka
image image

Copy link
Copy Markdown
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!!

Comment thread run.spec
Comment thread .github/workflows/build.yml
Comment thread run.spec
Comment thread run.spec
@y-chan
Copy link
Copy Markdown
Member Author

y-chan commented Aug 12, 2022

コメントいただいた件を変更してみました!

@Hiroshiba
Copy link
Copy Markdown
Member

すごく良いと思います!!

@y-chan
Copy link
Copy Markdown
Member Author

y-chan commented Aug 12, 2022

Linuxのジョブ一本化試したいのでマージしてしまいます!

@y-chan y-chan merged commit be26951 into VOICEVOX:pyinstaller Aug 12, 2022
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.

3 participants