merge unnecessary Docker layers

vvakame / docker-review

Re:VIEW build container by docker

MIT License

148 stars 38 forks source link

merge unnecessary Docker layers #29

Closed e-koma closed 5 years ago

e-koma commented 5 years ago

There are many unnecessary ENV / RUN layers, so merged.

Result

delete 30MB image size
- vvakame/review:3.0 806MB
- ekoma/docker-review 776MB

vvakame commented 5 years ago

ありがとうございます。 ENVやらRUNやらを圧縮して容量を削減したい、という方針はOKです。ただ、諸々の記述順序は変えたくないところです。この行はざっくりtex入れてて…次の行でreviewが必要なもの入れて…グラフ系ツール入れて… みたいな感じになっています。これを崩すとなんでこれ入れてるんだっけ…？となってメンテできなくなるからです。また、update.shもメンテし、各バージョン毎のDockerfileにも変更を反映できるようにしていただけると嬉しいです。

e-koma commented 5 years ago

レビューありがとうございます。

記述順序を元通りに合わせました。
update.shを実行して各versionのDockerfileも反映しました。
修正後の状態でdocker buildしてビルドが成功すること確認しました。

（PR出しておいてこんなコメント言うのも恐縮なのですが　修正後の状態がメンテしづらいと思われるようでしたら、　ガッツリRUNコマンドを削るのではなく、　ENV等の本当に最低限のLayerだけ削るだけにしておく、のでもありかなと思いました）

[追記] 最後にforce-pushの痕跡を残してしまいましたが、コメント以降にpushしてしまったcommitを直したかったためです。

vvakame commented 5 years ago

ENV自体のsyntaxを変えてしまっているので、update.shの記述を修正しないといけなさそうですね。例えば、 Re:VIEW 2.4用のディレクトリでも REVIEW_VERSION=3.0.0 のままになっています。

たぶん何らかのツールを使っておられるのかと思うのですが、ちょっと僕の好みにはあわなさそうです。しかしイメージサイズを削減したいのはわかるなぁ…という気持ちです。

e-koma commented 5 years ago

あぁぁそういう意味でしたか。。失礼しました。

e-koma commented 5 years ago

今回、Re:VIEWを使わせていただくにあたって環境構築する際に、 pullするLayerが多くて気になったので、手元直したら容量削減できたのでPRを送った、という経緯です。ただ、そもそも容量削減効果がめっちゃ大きいわけではないので、

ちょっと僕の好みにはあわなさそうです

というのであれば、本PR、Closeしちゃってもいいかなという気持ちになりました。（いったんENVだけ元通りに戻します）

vvakame commented 5 years ago

うーん、ではすみませんが一旦閉じてしまいます…！後ほど振り返ってやるかもしれません。