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

Rewrite zip_artifacts.bat, build-installer.bat #679

Closed

Conversation

ds14050
Copy link
Contributor

@ds14050 ds14050 commented Dec 4, 2018

どちらのバッチファイルも zip ファイルを含む多くのリソースファイルを扱いますが、リソースのリストとバッチコードを分離することでリソースの管理を容易にします。

postBuild.bat で行われているがエディタのビルドとの関連が不明な bregonig.dll などの解凍処理も不要になります。

@KageShiron さんの pull #630 「[WIP] インストーラの同梱ファイルの調整」とバッティングするのでなんらかの調整が必要です。こちらで取り込むのが簡単ですが KageShiron さんの名前がコミットに残る方法でなければいけないと考えています。

@ds14050 ds14050 added the refactoring リファクタリング 【ChangeLog除外】 label Dec 4, 2018
@ds14050 ds14050 added this to the next release milestone Dec 4, 2018
* Space-containing path handling.
* Working directory independence.
* Make zip files according to a recipe file.
@ds14050 ds14050 changed the title [WIP] Rewrite zip_artifacts.bat, build-installer.bat Rewrite zip_artifacts.bat, build-installer.bat Dec 5, 2018
@ds14050
Copy link
Contributor Author

ds14050 commented Dec 5, 2018

この PR と master で2つのジョブ Win32/Release と x64/Debug のアーティファクト計16個を展開して中身を確認しましたがファイル数が一致しておりファイルサイズも一見したところでは一致していました(※インストーラに 1 KB 以上のファイルの同梱漏れとかはなさそう)。

以前は同梱されていなかった sha256.txt を良かれと思って同梱するようにしていますが、違いはそれだけですのでタイトルから WIP を外しました。

@berryzplus
Copy link
Contributor

判断保留で。

postBuild.bat で行われているがエディタのビルドとの関連が不明な bregonig.dll などの解凍処理も不要になります。

ビルド後すぐにテストで正規表現を使えるようにしたい、という趣旨で入れた解凍処理です。
実用では正規表現なしで使うケースの方が少ないと考えられるので、「使える前提」でデバッグしたいという話だったはず。

いまいま、中身見る気力ないので判断は保留で。

@ds14050
Copy link
Contributor Author

ds14050 commented Dec 5, 2018

ビルド後すぐにテストで正規表現を使えるようにしたい、という趣旨で入れた解凍処理です。

しかしインストーラの作成でも postBuild.bat の生成物を利用しているので MinGW ビルドでインストーラを作成しようとするときにこの依存関係の不適切さが顕在化します。

仮に postBuild.bat の内容は取り除かないとしても不適切な依存関係は取り除いておきましょう。インストーラを配布しないとしても MinGW ビルドだけの例外処理を書かなくて済みます。

中身見る気力ないので判断は保留で。

判断を保留したくなるのはわかりますが、生成物で判断してもいいんだぜ、とは囁いておきます。

#630 の動向もあるので急ぎません。

@KageShiron KageShiron mentioned this pull request Dec 5, 2018
@KageShiron
Copy link
Member

いろいろと作業中の場所がかち合ってしまって申し訳ないです 🙇
うまいこと組み合わせて、ビルドプロセスをシンプルにしていきたいですね

@ds14050
Copy link
Contributor Author

ds14050 commented Dec 5, 2018

コンフリクトの解消は大した手間ではありません。妙に遠慮して引っ込めてしまう方が問題ですので、空気なんて読まずにとりあえず PR を出すのがよいですよ。

サクラエディタの過去の掲示板を読むと、掲示板で差分の交換をしていたみたいです。お前の変更量が一番多いから俺の差分をそっちで取り込んでソースに反映してくれ、みたいなやり取りを見かけました。SVN も CVS もない遠い時代の話です。

@KENCHjp
Copy link
Member

KENCHjp commented Dec 5, 2018

SVN も CVS もない遠い時代の話です。

それしかできなかったといえば、身もふたもないですが、Gitだと、コミットを綺麗につまないといけないのではないか?って強迫観念に駆られてしまいます(笑)>GitHub初心者の私

@ds14050
Copy link
Contributor Author

ds14050 commented Dec 6, 2018

Gitだと、コミットを綺麗につまないといけないのではないか?

とりあえず git rebase -i で後からなんとかなります(笑)。直近5コミットの整理なら git rebase -i HEAD~5 です。コミットの分割だけはやや面倒ですが、並び替え・削除・併合なら git が開いたテキストの編集だけでほぼ完了します。

プルリクエストの目的が絞られていて、その構成コミットが目的を逸脱していないなら、マージする際に全コミットを1つにまとめる Squash and merge することで一括整理もできます。それなら予め整理しておく必要もありません。

Git は Subversion よりもコミットに対して厳格性を求めていないように見えます。Subversion のコミットが清書であり GitHub における master へのマージに相当するなら、Git のコミットは日々の作業記録程度の位置づけで取り扱いが可能です。

それが可能なのはローカルリポジトリへ積むコミットに限った話で、GitHub などの公開場所へ git push する前には git rebase することが推奨されるとは思いますが、squash を前提にするなら省略してもいいでしょう。

自分はよく working. というコミットメッセージを最初に書き、同じ目的の追加作業は --amend オプションによって前のコミットと併合しコミットメッセージを書く手間を省いています。

などということを 4/20 に初めて git コマンドを打ち込んだ人間が書いています。

@KENCHjp
Copy link
Member

KENCHjp commented Dec 6, 2018

@ds14050 さん

ああ、こういう書き込みが非常に有益です<超初心者な私

@KageShiron
Copy link
Member

改めて読んでみましたが、結構複雑で将来修正しにくいのではと思います。

と書きつつ、人によってバッチファイルに対する感情にだいぶ開きがありそうな気はしてます。私はバッチファイル(bashスクリプトとかもそうですが)は極力減らして、多少DRYを無視してでも単純化したい派のようです。

@ds14050
Copy link
Contributor Author

ds14050 commented Dec 9, 2018

よく構造化されて理解すべき範囲が分割・限定されたコードだと思うんですけどね、欲目でしょうかね。

将来の修正と言いますけれど、ニーズの大枠が変わらなければ TSV ファイルを編集するだけなんですよ。

まあ、使ってくれる人がいなければ意味がないのでごり押しはできません。

@berryzplus
Copy link
Contributor

あらためて読み返してみました。
これ難しいですね~ww
不満は postBuild.bat の解凍&コピー処理が消されていることだけです。
逆にそんだけなのでそこだけ辻褄合わせてもらえれば問題ないと思います。

@ds14050
Copy link
Contributor Author

ds14050 commented Dec 9, 2018

不満は postBuild.bat の解凍&コピー処理が消されていることだけです。

KageShiron さんの #681 に既視感を感じた部分です。コピー処理に対するニーズはすでに把握していますのでご安心を。

@KageShiron
Copy link
Member

構造的にはかなりきれいだと思います。しかし、私としてはコマンドを並べて時々if程度がバッチファイルの限界だと思ってます。

また、common-sakura.issやCompress-Archive(や7zip)にファイル名を指定できる機能があることも含めて、現時点のフローをバッサリと削れるのではという気がしています。↓
#681 (comment)

@KENCHjp
Copy link
Member

KENCHjp commented Dec 9, 2018

Batも色々しらべると、いろんなことはできるのですが、なるべくシンプルにしたいなというは賛同します。(Batでテトリスとか、tail -fもどきとか作ったことはありますが。。。)

@m-tmatma m-tmatma removed this from the v2.4.0 milestone Mar 27, 2019
@m-tmatma
Copy link
Member

いったん milestone を外します。

※ コンフリクトしています。

@ds14050
Copy link
Contributor Author

ds14050 commented Mar 28, 2019

いったん milestone を外します。

了解です。milestone ってとりあえず付けるものではなく、次のバージョンに反映されるものないしは反映させるつもりのものに付けるものだったんですね。WIP だとかとりあえず晒しておこうというものには付けないようにします。

この PR は #787 「[WIP]build-installerとzipArtifacts.batの簡略化」が取り込まれたときが年貢の納め時だと思っています。独自レシピファイルより 7z が扱うリストファイルの方が扱いやすいうえにコードフリーですから。

@berryzplus
Copy link
Contributor

このPRは賞味期限切れだと思うので閉じておきます。

@berryzplus berryzplus closed this Sep 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring リファクタリング 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants