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

VOICEVOX ENGINEの実行コマンドをrun.exeから./runに変更する #265

Closed
wants to merge 1 commit into from

Conversation

aoirint
Copy link
Member

@aoirint aoirint commented Sep 24, 2021

内容

Linux用実行バイナリでも、VOICEVOXから相対パスにあるVOICEVOX ENGINEを実行できるようにします。

関連 Issue

ref #218

@aoirint aoirint changed the title VOICEVOXエンジンの実行コマンドをrun.exeから./runに変更する VOICEVOX ENGINEの実行コマンドをrun.exeから./runに変更する Sep 24, 2021
@aoirint aoirint marked this pull request as ready for review September 24, 2021 21:54
@Hiroshiba
Copy link
Member

どんなwindows環境でも、exe拡張子を書かなくてもちゃんと動くものなのかが若干気になりました。

@aoirint
Copy link
Member Author

aoirint commented Sep 25, 2021

どんなwindows環境でも、exe拡張子を書かなくてもちゃんと動くものなのか

たしかにそうですね。

一応、child_process.execFileの処理を追ってみました。

調査の結果としては、挙動がWindows(Win32 API)ではなく、Node.js(libuv)に依存するため、動きます。
しかしこのPRは、OS間でENGINEの実行コマンドを共通化できるメリットがありますが、
将来的に、ENGINEの実行ファイルの存在チェックがしにくくなるというデメリットがあるように思いました。

child_process.execFileの処理

Windowsでは、拡張子を指定せずにchild_process.execFileを呼び出したときの挙動は、Windows(Win32 API)ではなく、Node.jsが内部で使っているライブラリのlibuvの仕様に依存します。

libuvは、ファイル名に拡張子がない場合、ファイルの存在をチェックして、.comまたは.exeの拡張子を付加することを試みる仕様になっています。

補完される拡張子は、Win32 API CreateProcessWや環境変数 PATHEXTとは関係なく、libuvによって決まります。

環境変数PATHEXTについては、Node.js v0.xのときの議論がありました。このときから、Node.js内部でlibuvを使っていて、.exeを補完する仕様については変わっていないように思われます。

一方で、libuvが内部で呼び出しているWin32 APIのCreateProcessWでは、ディレクトリパスを含むファイル名が指定されたとき、.exeは補完されない仕様のようです。

lpApplicationName
...
If the file name does not contain an extension, .exe is appended.
...
If the file name ends in a period (.) with no extension, or if the file name contains a path, .exe is not appended.

調査の結果

結果として、コードは動きますが、
WindowsのCreateProcessWに合わせるならば.exeを明示した方がよい、
Node.jsのchild_process.execFile(libuv)に合わせるならば.exeを明示しなくてもよい、といった形でしょうか。

このPRは、OS間でENGINEの実行コマンドを共通化して、開発しやすくする意図のものです。

個人的には、このPRが加える変更とは反しますが、ENGINE_PATHを実行ファイルの場所を指定している、ととらえれば、.exeを省略しない方が自然ではある気がします。また実行せずに、ENGINEの実行ファイルの存在チェックができます。

将来的には、ENGINEのパスを.envとは別の設定(UI上)で指定できるようになったり、内部的にVOICEVOXのインストールとは別にENGINEをダウンロードする仕様になる可能性はあると思います。
そのときには拡張子を含むフルパスで扱い、実行とは別にファイルの存在チェックをする可能性もあると思うので、そちらに合わせるならば、.exeを省略せず、OSごとにファイル名を切り替える方がいいのかな、と思いました。

その場合、 #266 のような形でビルド時の環境変数またはyamlを使って .env.production ファイルを切り替えるか、ビルド時に .env.production を直接書き換える形になるのかなと思います。

child_process.execFileの処理の流れ

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 25, 2021

調査ありがとうございます!!とても勉強になりました。

仰るとおり、ENGINE_PATHの名前を守るためにも、OSごとに何らかの方法で切り替えるのが良さそうだなと感じました。

そうですね・・・ちょっと実験的になってしまって申し訳ないのですが、 https://github.com/Hiroshiba/voicevox/pull/266 では実装をビルド時の仕様に合わせたので、こちらはビルド時のコマンドを実装に合わせてみる(コードはそのままに、Github Actions上でOSごとの.envを作るなどで対応する)というのはどうでしょう。
設定ファイルで行くか他の方法が良いかを決める今後の判断の所感も得られるかなと思っています。

@aoirint
Copy link
Member Author

aoirint commented Sep 25, 2021

#266 では実装をビルド時の仕様に合わせたので、こちらはビルド時のコマンドを実装に合わせてみる
設定ファイルで行くか他の方法が良いかを決める今後の判断の所感も得られる

ひとまず、その方針でいいと思いました!

Linuxの自動ビルドのPRで対応すると思うので、このPRはCloseします。

@aoirint aoirint closed this Sep 25, 2021
@aoirint aoirint deleted the patch-linux-run branch February 28, 2022 00:58
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.

None yet

2 participants