-
Notifications
You must be signed in to change notification settings - Fork 167
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
bregonig.dll のファイル内容とタイムスタンプを比較する #335
bregonig.dll のファイル内容とタイムスタンプを比較する #335
Conversation
build-installer.bat で全部行っているので build-installer.bat が間違ってるのは検出できないかも。 ただ、直接登録した bregonig.dll とアーカイブの中の bregonig.dll の不一致は検出できるので |
実害は発生してなかった認識ですが、 |
1点確認です。
ここに引っかかりました。 「タイムスタンプが一致したら内容も一致とみなす」な比較手法はよくあります。 内容が同じならタイムスタンプが違ってても問題ないですよね?(ここが認識違い?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
変更目的と変更内容は大筋で問題ないと思います。
実現方法に疑問点あるため、一旦コメントだけつけます。
build-installer.bat
Outdated
|
||
@rem compare file contents | ||
set COMPARE_RESULT=0 | ||
fc %DLL_BREGONIG_0% %DLL_BREGONIG_1% 1>nul 2>&1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fc /B path1 path2
な気がします。 →詳しくはコマンドプロンプトで help fc
) | ||
copy %INSTALLER_RESOURCES_BRON_DLL%\*.dll %INSTALLER_WORK%\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
横展ってわけではないですが /B
バイナリモード指定をしたほうがよさそうです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
全体的に抜けているので、他のバッチファイルも含めて一括で対応します。
無人稼働前提なのに 本題と関係ない部分なので通しちゃおうと思っていますが、 |
bregonig を bron412.zip とアーカイブのままで登録している理由の一つに 外部提供のバイナリファイルのタイムスタンプは重要な情報なので保持したいというが理由です。 #332 では、bron412.zip に入っている DLL ではなく、bregonig.dll が直接登録されている |
コピー前にコピー先ディレクトリが存在していたら、 |
安全側に倒すために /Y をつけたほうがいいですが、 |
Lines 10 to 14 in 5540caf
の部分です。 |
/Y と /B に関して #338 を登録しました。 |
fc に /B をつける対応は push 済みです |
了解です。「内容一致 && タイムスタンプ一致」にしている理由を把握しました。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTMです。
…egonig-dll-and-zip bregonig.dll のファイル内容とタイムスタンプを比較する
bregonig.dll のファイル内容とタイムスタンプを比較する
#332 のような不具合を防ぐ
確認手順
ビルド成功の確認
build-all.bat Win32 Release
build-all.bat x64 Release
ビルド失敗の確認 (Win32)
echo "" > installer\externals\bregonig\Win32\bregonig.dll
build-all.bat Win32 Release
ビルド失敗してエラー解消後のビルド成功の確認 (Win32)
git checkout installer\externals\bregonig\Win32\bregonig.dll
build-all.bat Win32 Release
ビルド失敗の確認 (x64)
echo "" > installer\externals\bregonig\x64\bregonig.dll
build-all.bat x64 Release
ビルド失敗してエラー解消後のビルド成功の確認 (x64)
git checkout installer\externals\bregonig\x64\bregonig.dll
build-all.bat x64 Release