build: PyInstallerをv6へ更新#1766
Conversation
Hiroshiba
left a comment
Hiroshiba
left a comment
There was a problem hiding this comment.
@tarepan @aoirint
すみません、レビューのお力をお借りできると非常に心強いです！ 🙇
@aoirint さんに処理が大丈夫そうかを、 @tarepan さんにコメントや文脈のドキュメントは十分そうかをレビューいただければと思ってます！！
このPRはかなり文脈が難しいと思うので、ちょっと状況を掘り返してみました！
- １回マージされたけど、課題があったのでリバートした
- そもそもPyInstaller v6にするのは課題が２つある
- v6にすることでディレクトリ構造が変わる
- v6の仕様変更で、macOSエディタで動かない
macOSエディタで動かないの方は #828 の調査で解決していることがわかりました！
ディレクトリ構造が変わる方はrevertする前のプルリクエストですでに解決していて、このPRはほぼ当時のPRと同じというわけです。
ただ、このプルリクエストが問題ないことを把握するには、結構深いコンテクストがいると思います。
まず、ディレクトリ構造に関してですが、このissueの解説のように、PyInstaller 6のアプデでサブディレクトリの中にいろんなものが保存されるようになりました。
これはPython単体アプリとしては不要なものを隠すので素晴らしいのですが、エンジンとしてはエディタからマルチエンジンとして使える必要があるので、互換性を維持するためにルートディレクトリに残しておかないといけないものが結構あったりします。
（あとDLLファイルとかも同じルートに置いておかないとエラーになる）
というのがrun.specの「# 実行ファイル作成後の処理」になります！
レビューの参考になれば幸いです！ 🙇
tarepan
left a comment
tarepan
left a comment
There was a problem hiding this comment.
👍️
長年の懸案であった v6 対応が実現しています。great!
🤔
将来のコントリビュータがより楽にメンテできるよう、コメントを改良できそうです。
| target_arch=None, | ||
| codesign_identity=None, | ||
| entitlements_file=None, | ||
| contents_directory="engine_internal", # 実行時に"sys._MEIPASS"が参照するディレクトリ名 |
There was a problem hiding this comment.
ここのメモは「今そうなってる」というwhatはわかるけど、結局なぜこの値なのかのwhyがわからないかもと感じました！
･･････これちなみになんで指定が必要なんでしたっけ･･･。
無指定だと変なディレクトリ名になるから･･････？
There was a problem hiding this comment.
#828 (comment)
以前のissueで決まったことだからとしか言えないような…
無指定だと_internalになります。
正直なところこの引数にこれ以上詳細な説明を書くことに疑問を感じています。
引数の意味はPyInstallerのドキュメントを確認するべきですし、ディレクトリ名も外部からアクセスするべきではないと分かる名前ならなんでもいいからです。
engine_internalに深い意味がなく、この文字列を使ってアクセスしようとするべきではないという情報を短く書くなら今のコメントしか思い浮かばないです。
There was a problem hiding this comment.
あ、なるほどです！！ありがとうございます！！
engine_internal、当初の思いにあったかは不明ですが、「エンジンの中身である」ということをわかりやすくしたかったのかもです。
デフォルトの_internalでも良いけど、何のディレクトリ由来だったかわからなくなるという思いでengine_internalもアリかも。
･･･だったけど、今のエディタはエンジンをvv-engineディレクトリに放り込むから、別に_internalでも良いんですよねぇ･･･････。
engine_internalにして、かつコメントは今の形で良さそうに思いました！！
文脈とてもよく理解できました、ありがとうございました！！！！！
3 participants
内容
PyInstaller v6へ更新を行います。
関連 Issue
その他
ほぼ #857 と同じです。