ビルド時間とアーティファクトサイズの軽減が狙いですが、主としてバッチのリファクタリングです。

[ds14050]

#615 のコミット再構成版です。

コミット構成について

リファクタリングを目的とした最初のコミット(9959259)が以降のコミットのベースになっています。２番目以降のコミットの取捨選択は容易ですが、１番目のコミットなしでは２番目以降のコミットが成り立ちません。

そういうわけでひとつの PR にまとめていますが、コミット毎に採用可否を判断していただければと思います。

大きな修正は１番目だけで、他は小さな変更ばかりです。

[ds14050]

わりと見かけると思うのですが、こういうメッセージで s を脱着するまではしないけれど、括弧を付けずにはいられない(second(s))几帳面さが日本語使いには理解しがたいものがあります。

マーク・ピーターセンさんの古典的名著『日本人の英語』を読むとそういう、(不)定冠詞を軽視した態度は厳に改めるべきものだと反省させられます。英語ネイティブはまず a や the を発話してから、具体的な名詞となる単語を頭の中で探すことがあるらしいですよ。そっちが核なんですね。

この echo メッセージも改めるべきなのかもしれません。

[ds14050]

cmd /C "%Batch%" の代わりに call|"%Batch%" でもいけそうですがあまりに趣味的なのでやめておきます。

[berryzplus]

`%time%‘ の書式がロケール依存なのを嫌って powershell のワンライナー書いた人がいるみたいです。
https://pslabo.hatenablog.com/entry/20150803/p1

とりあえず「使いやすい」からバッチにしてますけど、バッチにこだわる必要はないと思ってます。計時部分だけ wsh とかもアリだと思います。（計時に他プロセス起動していいのか？という根源的なツッコミは置いておいて・・・。）

そもそも、すでに一部pythonを使った処理が入りこんでいますよね？unix系文化では使えるもんはなんでも使う系のビルドスクリプトが多いように思います。c-shellオンリーで外部コマンド禁止みたいな縛りプレイを要求されるのは業務だけであって欲しいです 😢

[berryzplus]

計時機能についてダメ出し、というか単に趣味の問題なんですが。

こうなってるんですけど、

	call :StartWatch
	echo ---- start %Batch% [%StartDate% %StartTime%] ----
	echo.
	cmd /C "call "%Batch%" %Platform% %Config%" ^
	|| exit /b 1
	call :StopWatch
	echo ---- end   %Batch% [%StopDate% %StopTime% / %Elapsed% seconds elapsed.] ----
	echo.
exit /b 0

:StartWatch

こうならんですかね？

	call :StartWatch "%LogFileName%" "%Batch%" %Platform% %Config% || exit /b 1
exit /b 0

:StartWatch
        set LogFileName=%~2
        shift /2
        :: この辺に時刻取得処理がある
	echo ---- start %1 [%StartDate% %StartTime%] ---- > "%LogFileName%"
	echo. > "%LogFileName%"
	cmd /C %*  > "%LogFileName%" || exit /b 1
        :: この辺に時刻取得処理がある
	echo ---- end   %1 [%StopDate% %StopTime% / %Elapsed% seconds elapsed.] ---- > "%LogFileName%"
	echo. > "%LogFileName%"

こういう構成の場合、StopWatch.bat を別ファイルにすることもできると思います。
元のバッチ呼出しに対する変更量も少なめなのもメリットです。

[ds14050]

`%time%‘ の書式がロケール依存なのを嫌って powershell のワンライナー書いた人がいるみたいです。

今のところ %DATE% の差異は確認しましたが %TIME% の違いは確認していません。

こうならんですかね？

目的がよくわかりません。ファイルを分けるにしても、とっちらかるのに勝る理由が欲しいです。

それと以前ここの githash.h に関するやりとりで知ったのですが、ラベル呼び出しをリダイレクトできるらしいです。

call :label > log

[berryzplus]

こうならんですかね？

目的がよくわかりません。ファイルを分けるにしても、とっちらかるのに勝る理由が欲しいです。

start & end を出すのは計時機構の一部だと思います。
外部化しようとしたら計時機構単体の完成度を上げたくなると思います。
使いまわしができることはメリットだと思っています。

[ds14050]

start & end を出していたのは build-all.bat と tests/build-and-test.bat です。そこにタイムスタンプを埋め込みました。

外部バッチ化して引数としてテンプレート文字列を受け取り、キーワード(!WATCHDATE! と !WATCHTIME! エクスクラメーションマークがキーです)をタイムスタンプで置換して出力して、出力先はバッチ呼び出し元に制御してもらう案もありますが、むしろ build-all.bat と tests/build-and-test.bat を統合することで StartWatch/StopWatch の重複をなくそうと考えています。

他で引き合いが出てきてから汎用ツール化するのでいいと思います。

[ds14050]

単体ファイル化(Watch.bat)するとこうです。

@echo off
setlocal ENABLEDELAYEDEXPANSION

call :%~1Watch %~2 %~3 %~4 %~5 %~6 %~7 %~8 %~9^
 || exit /b 1
exit /b 0

:StartWatch
	set StartDate=%DATE%
	set StartTime=%TIME: =0%
	if not "%*" == "" echo %*
exit /b 0

:StopWatch
	rem Be aware of space-padding hours and octal number literals
	rem when calculating the Elapsed.

	set StopDate=%DATE%
	set StopTime=%TIME: =0%
	set /A Elapsed=(1%StopTime:~0,2%  * 3600 + 1%StopTime:~3,2%  * 60 + 1%StopTime:~6,2%) ^
	              -(1%StartTime:~0,2% * 3600 + 1%StartTime:~3,2% * 60 + 1%StartTime:~6,2%)
	if not "%StartDate%" == "%StopDate%" (
		set /A Elapsed=%Elapsed% + 60 * 3600
	)
	if not "%*" == "" echo %*
exit /b 0

こうやって使います。

>call Watch Start "Starting at ^^!StartDate^^! ^^!StartTime^^!"

[ds14050]

setlocal したら Start と Stop の間で環境変数が維持されないでしょう。setlocal を外して親環境を汚染しようとしても新しいコマンドインタープリタ上で実行されていればそれも叶いません。決まった名前の一時ファイルを使用すると一度に一か所からしか使えません。どうやって汎用ツール化するのかわからなくなってしまいました(今のところそのつもりはありませんが)。

[ds14050]

遅延環境変数を利用したい CMAKE_PATH_FIX が呼び出し元の PATH 環境変数に影響を与えるためにこの場所で setlocal しましたが、endlocal & set PATH=%PATH% することにより setlocal を CMAKE_PATH_FIX 内部のみで完結させられます。

[berryzplus]

あらためて内容確認しましたが、これ #653 と内容被りますね。
入れる前提で考えると事前調整が必要と思われます。

全体的な総括として、修正前後のバッチ記述の技術レベルに差がありすぎて、何がどう変わっているかを把握するのが難しいコミットだと思いました。

取りうる対応として2通り。

1. 動いてるからこれで行こうぜ！で突き進むパターン
2. 変更内容をぼくらが理解できるまで説明するパターン

問題発生時に即対応できるなら 1. でもいいと思ってます。少なくとも個人的には。

あと、全体見渡してみて気付いた点ですが、
googletest submoduleの更新がPRに含まれています。
もしも意図する変更であれば、これはPRを分けるべきと思います。

[ds14050]

こんばんは。お久しぶりです>@berryzplus さん

これ #653 と内容被りますね。

そうでしょうか。Files Changed > Jump to... からファイル一覧が見られますが、見事にかぶりなしに見えました。

1. 動いてるからこれで行こうぜ！で突き進むパターン
2. 変更内容をぼくらが理解できるまで説明するパターン

リファクタリングを行う１コミット目は、動いている一事を以て「行こうぜ！」と考えてもらえるとありがたいです。説明は望むところですが、テクニカルな説明に終始する予感がします。

２コミット目以降はたとえば「Release と Debug でビルド内容を分ける」ことについて、いま分ける必要があるのか、分けるとしてその分配など、一度は検討してもらいたいと思っています。

---

force-push について。

現在の master に rebase して googletest の変更を取り除きました。内容の修正は計時機能に関して時刻が 23:59:59 から 00:00:00 に巻き戻るときに加算する補正値を (24 * 3600) 秒にしたことのみです。動いているかどうかは AppVeyor 待ちです。

IE誤動作のため。request changesをdismissしたかっただけです。

[berryzplus]

あと、全体見渡してみて気付いた点ですが、
googletest submoduleの更新がPRに含まれています。
もしも意図する変更であれば、これはPRを分けるべきと思います。

これが直ってたのでdismissしようとしたらIE11が暴走...orz

[ds14050]

気がついたので最新の master に rebase しました。