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

vs2017ビルド時のビルドログを整理する #790

Merged

Conversation

berryzplus
Copy link
Contributor

目的

ビルド後に実行しているpostBuild.batの出力を整理します。
ログを整理することで、コンパイラ警告やエラーメッセージに集中できるようになります。

バッチの変更だけだとイメージしづらいので、具体的にどうなるかをキャプチャで示します。

変更前

2019-02-17 2

変更後

2019-02-17 1

説明

キャプチャはvs2017でsakura.slnをビルドしたときのpostBuild.bat実行開始付近を撮ったものです。
変更後は zip->dll(exe) だけが出力されるようになります。

このバッチの出力が誰のためのものか?
なにか問題が起こったときに対処するのはどんな人か?

と考えて「これ以上の情報は不要」と判断しました。

@KageShiron さんのPRと修正ファイルが被りますが、修正軸が異なるので別件として検討可能だと思っています。

他バッチから呼び出した際に、後続処理がecho on状態になってしまう。
バッチ内でのon/off制御をやめて、気軽に呼出しできるよう改善する。
7-zip(7z.exe)が出す余計なメッセージ出力を抑制する。
空白を含むパスを考慮した記述に変える。
改善事項
・出力が多すぎて何がしたい処理なのか分かりづらい
・内部変数名が長すぎて処理を追いづらい
・最終パスにファイルがあっても解凍している
・CTAGSの中間パスに誤りがある
@ds14050
Copy link
Contributor

ds14050 commented Feb 20, 2019

全体への意見は置いておいて細部についてだけですが、*zip.bat が動作のために内々に呼び出す find-tools.bat の出力をリダイレクトするのと、*zip.bat の動作結果そのものである CMD_7Z の出力をリダイレクトするのは毛色が違うと思います。

find-tools.bat の出力をその呼び出し元である *zip.bat がリダイレクトして隠したのと同じように、zip.bat の CMD_7Z に関してもその呼び出し元が必要に応じてリダイレクトするのが本来的な修正方法ではないでしょうか。

それはさておいても listzip.bat の CMD_7Z 呼び出しの出力だけがそのままなのは一貫性がないように見えて、背後を貫く論理が読み取れません。

STDOUTを捨てても警告が出せるようになったので7zの出力を捨てないように変更。
@berryzplus
Copy link
Contributor Author

find-tools.bat の出力をその呼び出し元である *zip.bat がリダイレクトして隠したのと同じように、zip.bat の CMD_7Z に関してもその呼び出し元が必要に応じてリダイレクトするのが本来的な修正方法ではないでしょうか。

指摘っぽい書込みだったので対応してみました。

「listzip.batで7z.exeの出力を捨てないのは一貫性がない」はハテナでした。
unzip=解凍が目的、zip=圧縮が目的、listzip=書庫の一覧を出力するのが目的です。
listzipは出力が目的なので、出力捨てたら「何で呼び出すの?」になっちゃいます。

@KENCHjp
Copy link
Member

KENCHjp commented Feb 21, 2019

全体と細部の指摘は置いておいて、VSに名前が出てるのは大丈夫でしょうか?
私は本名さしてるからいいんすけどね。

@berryzplus
Copy link
Contributor Author

特に秘匿しているわけでもないので問題ないです。

サクラエディタの開発に関わることが一般的に恥ずかしいことだというのは否定しません。

それを変えたいからやってるんだっていう。

@KENCHjp
Copy link
Member

KENCHjp commented Feb 21, 2019

私はその筋系の仕事してますが、開発畑ではないので、サクラエディタの名前だすとだいたいどこのお客さんでも使ってるって感じです(笑)
それなりにお硬いお客さんでも。。。

@ds14050
Copy link
Contributor

ds14050 commented Feb 21, 2019

対応ありがとうございます。

  • >NUL を単純に削るだけで postBuild.bat の出力抑制が達成できるのでしょうか。
  • 標準エラーへのリダイレクトはよくわかりません。listzip.bat が出力するリストとその他のメッセージを分ける目的と、その横展開だろうと考えましたが。

「listzip.batで7z.exeの出力を捨てないのは一貫性がない」はハテナでした。
unzip=解凍が目的、zip=圧縮が目的、listzip=書庫の一覧を出力するのが目的です。
listzipは出力が目的なので、出力捨てたら「何で呼び出すの?」になっちゃいます。

listzip.bat において CMD_7Z が出力するリストが目的なら、他の zip.bat, unzip.bat において CMD_7Z が出力するステータス報告も、主ではなくとも目的の一部だと思います。7z.exe が解凍・圧縮という目的に付随してユーザーへのフィードバックとして出力しているものですから。それをバッチの一利用者である postBuild.bat の都合に合わせてサイレントモードを唯一の選択肢にするのはやり方が違うのではないか、と考えました。

@berryzplus
Copy link
Contributor Author

•>NUL を単純に削るだけで postBuild.bat の出力抑制が達成できるのでしょうか。

postBuild.bat の呼出側には (間違って) 元々>NULを付けてたので達成できます。

•標準エラーへのリダイレクトはよくわかりません。listzip.bat が出力するリストとその他のメッセージを分ける目的と、その横展開だろうと考えましたが。

標準エラー出力は、イレギュラーな通知を表示するために使います。
*zip.batがechoするメッセージのうち、今回修正したものは「警告」にあたるので、stdout(=1)ではなくstderr(=2)に吐くのがベターだと思います。こうしておくことで、7zipのない環境でvs2017ビルドを行った場合に、イレギュラーな処理が行われたことに気付けるようになります。

それをバッチの一利用者である postBuild.bat の都合に合わせてサイレントモードを唯一の選択肢にするのはやり方が違うのではないか、と考えました。

これはその通り、なので普通に使うこともできるように修正しました。

バッチでログの詳細度を制御するのには限界があるので、なるべく早く既成のビルドユーティリティを活用する方向にシフトしていきたいです。CMakeやMsBuildならオプションでログの詳細度を制御できるので verbose が欲しい人と silent でいい人が共存できるようになります(たぶん。

@berryzplus
Copy link
Contributor Author

私はその筋系の仕事してますが、開発畑ではないので、サクラエディタの名前だすとだいたいどこのお客さんでも使ってるって感じです(笑)

誤解をまねく言い方でした。利用者からみたサクラエディタは素晴らしいアプリです。
クライアントの皆様には是非サクラエディタを使っていただきたい。そして、メモ帳以外にテキストエディタが入ってない作業用PCをぼくらにあてがうのはやめていただきたい。(力説!

ぼくが言ったのは「コードを読み書きできる人から見て」の話です。

サクラエディタのコードを読めるレベルは、業務経験でプログラマ5年以上だと思います。
入れ替わりの激しい業界なので5年以上プログラマは、それだけでもうアレです。
たいていの人はそこに至る前にプログラミングから離れて、違う仕事をするようになります。
長いことプログラマで居続けることはとても難しいです。
それでも、5年、10年と続けていくうちにようやく辿り着ける領域があると思っています。
サクラエディタを読み解くにはそういった経験が必要だと考えています。

しかし、そういうポテンシャルを持った人から見たサクラエディタプロジェクトってどーよ?
と考えると先のコメントが出てくるわけです。
github移行後のリリースが無事行われれば、状況が変わってくるように思っています。
サクラエディタに関わることがなんで恥か、についてはまた別の機会に語ります。

Copy link
Contributor

@ds14050 ds14050 left a comment

Choose a reason for hiding this comment

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

自分は postBuild.bat には興味がなく、いいようにしてもらえばいいと考えています。そういう無責任な態度なので PR の是非には関わらないつもりでしたが、そろそろ日が経つので代表して Approve しようかと思います。

@KENCHjp
Copy link
Member

KENCHjp commented Feb 26, 2019

と考えると先のコメントが出てくるわけです。

中のソースがアレなのは経緯があるので置いておいて、GitHub上でのやり取りは業務系のプログラマには全然知らない世界かなと思います。

プログラムからクラスチェンジしてSEって昔は言われていましたが、今はプログラマ(特に業務アプリ)はチーム開発とか開発手法とかなんたら指向とかで特化したスキルに精通していないと、SEが片手間でやれる世界では全然ないかと思っています。

集中力が継続できるかどうかは、若さに頼る部分をのぞくと、もう適正としかいいようがないかもしれませんが。。。

>別の機会に語ります。

Onで雑談したいっすね(笑)

@berryzplus
Copy link
Contributor Author

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

自分は postBuild.bat には興味がなく、いいようにしてもらえばいいと考えています。そういう無責任な態度なので PR の是非には関わらないつもりでしたが、そろそろ日が経つので代表して Approve しようかと思います。

「無責任」って言葉から、責任の取り方についてちょっと考えました。

サクラエディタはフリーソフトで、その機能や挙動、アプリを使用したことにより発生するあらゆる事象に対して、開発者は責任を負わないことになっています。どんなすっごくエラい人が物言いしても、法的にはまったくの「無能力」です。法律屋さんは「責任を取る資格」を「責任能力」という用語で語るので「無能力」って言い方になります。プログラマ的には「無能」って言われるとイラッとしますが、そういう意味じゃないです。

責任を取る能力がないからといって、なんかあったときに何もしなくていいか?というと違う気がします。「責任ある対応」として何をすべきかについては意見の分かれるところだと思いますが、少なくともなんかしなくちゃいけない気がしてます。

たぶん、なんですけど「なんかあったらなんかする!」と覚悟を決めることが責任ある対応への第一歩なのかなと思ってます。「悪り。」と謝るだけかもしれません。より良い解決に尽力することになるのかもしれません。とりあえず、「なんかする」と決めることが責任を果たす準備になるんじゃないかと思ってます。

@berryzplus berryzplus merged commit f3c940e into sakura-editor:master Mar 2, 2019
@berryzplus berryzplus deleted the feature/reduce_unused_loggings branch March 2, 2019 09:21
@m-tmatma m-tmatma added this to the next release milestone Mar 7, 2019
@ds14050
Copy link
Contributor

ds14050 commented Apr 21, 2019

postBuild.batの整理 に問題を見つけました。

installer\externals\bregonig\bron412.zip がビルド要件になっている件について #388」において @yoshinrt さんから bron412.zip の展開をインストーラビルド時に移動してはどうかという提案がありましたが、そのときは「bron412.zip の解凍が必要ないときは bron412.zip の解凍をスキップする #390」ことで決着しました。

yoshinrt さんの提案は故のあることで、build-installer.bat は postBuild.bat が展開する一時ファイルに依存する造りになっています。そのことの是非はともかくそういう関係があるので、(出力フォルダの)bregonig.dll の存在だけを確認して一時フォルダへの展開をスキップしてしまうと、build-installer.bat の期待を満たせなくなってしまいます。

そのことが「AppVeyor のビルドキャッシュを利用してビルド時間を削減(※およそ5分)できるようなオプションを用意します。 #650」でキャッシュを有効にしたビルドによって明らかになりました。

PR を出します。

ds14050 added a commit to ds14050/sakura-clone that referenced this pull request Apr 21, 2019
関連 sakura-editor#388 「installer\externals\bregonig\bron412.zip がビルド要件になっている件について」
関連 sakura-editor#390 「bron412.zip の解凍が必要ないときは bron412.zip の解凍をスキップする」
関連 sakura-editor#790 「vs2017ビルド時のビルドログを整理する」
ds14050 added a commit that referenced this pull request Apr 24, 2019
* 関連 #388 「installer\externals\bregonig\bron412.zip がビルド要件になっている件について」
* 関連 #390 「bron412.zip の解凍が必要ないときは bron412.zip の解凍をスキップする」
* 関連 #790 「vs2017ビルド時のビルドログを整理する」
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
…nused_loggings

vs2017ビルド時のビルドログを整理する
HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
…#860)

* 関連 sakura-editor#388 「installer\externals\bregonig\bron412.zip がビルド要件になっている件について」
* 関連 sakura-editor#390 「bron412.zip の解凍が必要ないときは bron412.zip の解凍をスキップする」
* 関連 sakura-editor#790 「vs2017ビルド時のビルドログを整理する」
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.

4 participants