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

GoogleTestを更新する #1779

Merged
3 commits merged into from Jan 29, 2022
Merged

GoogleTestを更新する #1779

3 commits merged into from Jan 29, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jan 28, 2022

PR の目的

CMakeの警告メッセージに対処するため、GoogleTestを更新する。

カテゴリ

  • 不具合修正

PR の背景

先日find-toolsのログを #1759 に投稿しましたが、その時に次のようなメッセージが出ていることに気が付きました。

4>CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required):
4>  Compatibility with CMake < 2.8.12 will be removed from a future version of
4>  CMake.
4>
4>  Update the VERSION argument <min> value or use a ...<max> suffix to tell
4>  CMake that the project does not need compatibility with older versions.
4>
4>
4>CMake Deprecation Warning at googlemock/CMakeLists.txt:42 (cmake_minimum_required):
4>  Compatibility with CMake < 2.8.12 will be removed from a future version of
4>  CMake.
4>
4>  Update the VERSION argument <min> value or use a ...<max> suffix to tell
4>  CMake that the project does not need compatibility with older versions.
4>
4>
4>CMake Deprecation Warning at googletest/CMakeLists.txt:49 (cmake_minimum_required):
4>  Compatibility with CMake < 2.8.12 will be removed from a future version of
4>  CMake.
4>
4>  Update the VERSION argument <min> value or use a ...<max> suffix to tell
4>  CMake that the project does not need compatibility with older versions.

ログにある通りですが、GoogleTest側のCMakeLists.txtの内容に関するものです。

CMakeでは「2.8.12より前のバージョンとの互換性を廃止する」計画があり、3.19以降のバージョンでは予告メッセージが表示されます。
Deprecated and Removed Features | CMake 3.19 Release Notes
現在サクラエディタで使っているGoogleTestは2018年11月21日版ですが、この版はCMake 2.8.8に対応しているため予告メッセージの表示対象となりました。
しかし、この仕様変更予告は2020年12月29日版ですでに対応済みであり、これ以降の版では表示されません。

これを機に最新のGoogleTestへ更新を行いたいです。

PR のメリット

PR のデメリット (トレードオフとかあれば)

仕様・動作説明

サブモジュールの更新と、それに伴うビルドエラーへの対処を実施しています。

targetsファイルとMakefileの変更について

つい先日GoogleTestのビルド設定に変更( google/googletest@f64cf6b )があり、CMakeのDEBUG_POSIFIXオプションが設定されなくなりました。
GoogleTestのライブラリファイルはデバッグ版とリリース版でファイル名が異なっていましたが、この変更によりどちらも同じファイル名になります。
サクラエディタ側ではビルド構成に応じてファイル名にプレフィクスを付加する対応を行っていますが、不要になりましたので削除しました。

補足情報

Visual Studioに付属するCMakeのバージョンは、手元で確認した限りでは次の通りでした。

  • VS 2019 (16.11.9): cmake version 3.20.21032501-MSVC_2
  • VS 2022 (17.0.5): cmake version 3.21.21080301-MSVC_2

PR の影響範囲

単体テスト

テスト内容

  1. PRをチェックアウトし、git submodule updateを実行してサブモジュールを更新する
  2. Visual Studioを起動してソリューションをビルドし、出力ペインに上記メッセージが表示されないこと、ビルドが成功することを確認する
    • リビルドを行って繰り返し確認してください。
  3. テストエクスプローラーを使用して単体テストを実行し、テストに成功することを確認する。

関連 issue, PR

#636 前回は2018年11月23日に行いました。

参考資料

@sonarcloud
Copy link

sonarcloud bot commented Jan 28, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@AppVeyorBot
Copy link

Build sakura 1.0.4011 completed (commit 5d86f04141 by @kazasaku)

Copy link
Contributor

@sanomari sanomari left a comment

Choose a reason for hiding this comment

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

良いと思います。

なんでNuGet使わないんでしょう?とか
そろそろvcpkgの採用を検討してもよいのでは?とか
周辺課題がありそうに思いました。

@ghost ghost marked this pull request as ready for review January 28, 2022 11:09
@ghost
Copy link
Author

ghost commented Jan 28, 2022

CIが通ったのでDraft外します。

周辺課題

なんか単体テスト周りを触るたびに(他への移行も含めて)話題が出る気がしますね。

@beru beru added the UnitTest label Jan 28, 2022
Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

LGTM

gtestd.libを作って参照させる機構は頑張って作ったような気がしますが、消すのは構わないです。

これはlibフォルダにdebugとreleaseの両方を置くための「伝統スタイル」ですが、最近はdebugビルドはreleaseビルドと別フォルダに出力するようになってきたので「ファイル名まで変えるの意味なくね?」になったと予想します。

@berryzplus
Copy link
Contributor

記憶があやふやですが、vs2017に付属するCMakeのバージョンは当初から 3.x 系だった気がします。

CMake対応の検討PRに、やたらと古いrequireの記載があり、
「古いバージョンのCMakeでのビルドを保証するつもりか?」
という意図の指摘をした記憶があります。

@ghost
Copy link
Author

ghost commented Jan 29, 2022

レビューありがとうございました。マージします。

vs2017に付属するCMakeのバージョンは当初から 3.x 系だった気がします

リリースノートだと3.11に更新されているらしいですが、もっと新しいかもしれません。

@ghost ghost merged commit 8353cd2 into sakura-editor:master Jan 29, 2022
@ghost ghost deleted the feature/update_googletest branch January 29, 2022 08:47
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants