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

MinGWビルドで使用するビルドツールを更新する #1771

Merged
1 commit merged into from Jan 20, 2022
Merged

MinGWビルドで使用するビルドツールを更新する #1771

1 commit merged into from Jan 20, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jan 16, 2022

PR の目的

MinGWビルドで使用しているMinGW-w64-gccを更新します。

カテゴリ

  • ビルド関連
    • Azure Pipelines

PR の背景

#1752 による対応の第2弾です。)

MinGWビルドを行うジョブはWindows Server 2016の仮想環境で実行されていますが、当該環境は3月中に利用できなくなるため、Windows Server 2022環境へ移行する予定です。

しかし、仮想環境に既定でインストールされているMinGW-w64が古く、Windows Server 2022環境でのビルド時に標準ライブラリ関係のコンパイルエラーが生じます。

例:

In file included from C:/ProgramData/Chocolatey/lib/mingw/tools/install/mingw64/lib/gcc/x86_64-w64-mingw32/8.1.0/include/c++/filesystem:37,
                 from D:/a/1/s/sakura_core/util/file.h:33,
                 from D:/a/1/s/sakura_core/io/CFile.h:31,
                 from D:/a/1/s/sakura_core/env/CommonSetting.h:34,
                 from D:/a/1/s/sakura_core/env/DLLSHAREDATA.h:35,
                 from D:/a/1/s/sakura_core/StdAfx.h:108:
C:/ProgramData/Chocolatey/lib/mingw/tools/install/mingw64/lib/gcc/x86_64-w64-mingw32/8.1.0/include/c++/bits/fs_path.h: In member function 'std::filesystem::__cxx11::path& std::filesystem::__cxx11::path::operator/=(const std::filesystem::__cxx11::path&)':
C:/ProgramData/Chocolatey/lib/mingw/tools/install/mingw64/lib/gcc/x86_64-w64-mingw32/8.1.0/include/c++/bits/fs_path.h:237:47: error: no match for 'operator!=' (operand types are 'std::filesystem::__cxx11::path' and 'std::filesystem::__cxx11::path')
    || (__p.has_root_name() && __p.root_name() != root_name()))
                               ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~

PR のメリット

Windows Server 2022 環境でもMinGWビルドができるようになります。

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

仕様・動作説明

これはGCC9以降で修正された既存の不具合によるもの(資料参照)ですが、MinGW-w64の開発チームがSourceForgeにバージョン9以降のインストーラを公開していないこともあるのか、仮想環境には修正前のバージョンがインストールされています。

【2022-01-17T12:52 追記】
幸い、Windows Server 2016 環境ではMSYS2経由で修正後のバージョンが別にインストールされていたため、この問題を回避できていました。しかし、Windows Server 2022 環境では最低限のパッケージしか含まれないらしく、修正後のバージョンはインストールされていません。
このためにビルド時に修正前のバージョンの方が呼び出されることとなり、上記の問題が生じました。
【追記ここまで】

バグチケットの会話でも推奨されている通り、MSYS2経由で最新のツールセット一式を明示的にインストールするようにします。
現在の最新バージョンは11.2です。
Group: mingw-w64-x86_64-toolchain - MSYS2 Packages
Package: mingw-w64-x86_64-gcc - MSYS2 Packages

なお、このPRにより使用するGCCのバージョンが一気に上がりますのでご注意ください。

  • 変更前: x86_64-w64-mingw32-gcc.exe (x86_64-posix-seh-rev0, Built by MinGW-W64 project) 8.1.0
  • 変更後(現時点): x86_64-w64-mingw32-gcc (Rev5, Built by MSYS2 project) 11.2.0

PR の影響範囲

Azure Pipelinesの次のジョブ

  • MinGW
    • MinGW_Debug
    • MinGW_Release

テスト内容

MinGWビルドで

  • 最新のMinGW-w64がインストールされること
  • ビルドに成功すること

関連 issue, PR

参考資料

エラーに関するもの

仮想環境の構成に関するもの(いずれも Mingw-w648.1.0 となっている)

@berryzplus
Copy link
Contributor

berryzplus commented Jan 17, 2022

cppCheckのx64がコケてますね。

cppCheckをx86/x64の両方実施する必要があるのかどうか、良く分からないっす。
結局「静的解析」なので「無意味」とは言わないまでも「必要」ではない気がします。。。


このPRに関してのコメントですが、
MinGWビルドで使ってるGCCはそもそもMsys2版だった気がします。(既にv10以降を使ってます。
installed softwaresに載ってるMinGW-w64 8.1.0とは違います。

いままでと同じように実施してできないなら対処は必要と思います。
対処方法はざっくり2択で、頑張ってできるようにするか、あきらめるかです。

MinGWビルドをGitHub Actionsに引っ越しするのもアリかな・・・と思っています。
https://github.com/msys2/setup-msys2

@ghost
Copy link
Author

ghost commented Jan 17, 2022

MinGWビルドで使ってるGCCはそもそもMsys2版だった気がします。(既にv10以降を使ってます。
installed softwaresに載ってるMinGW-w64 8.1.0とは違います。

MSYS2にインストールされているパッケージまでは actions/virtual-environments の README に書いてないので気が付きませんでしたが、改めて pacman -Q でリストを出したところ構成が違いました。
Windows Server 2016(262個):windows-2016-msys2-packages.txt
Windows Server 2022(96個):windows-2022-msys2-packages.txt

2016ではMinGW-w64-x86_64-gccが最初から入っていますが、2022では入っていません。
たまたまパスが通ったところに8.1.0があったのでこれを使ったみたいです。

本文修正しておきます。

@ghost
Copy link
Author

ghost commented Jan 17, 2022

2016ではMinGW-w64-x86_64-gccが最初から入っていますが、2022では入っていません

2016ではあらかじめ更新されていたが、2022では自分で更新しなければならなくなった、ということです。

@berryzplus
Copy link
Contributor

Windows Server 2016では動いてしまう原因は、Msys2の構成の違いに加えてこれをやっているからです。

sakura/build-gnu.bat

Lines 27 to 30 in 68c4dc8

@rem https://www.appveyor.com/docs/environment-variables/
@rem path=C:\mingw-w64\x86_64-7.2.0-posix-seh-rt_v5-rev1\mingw64\bin;%path%
path=C:\msys64\usr\bin;%path:C:\msys64\usr\bin;=%
path=C:\msys64\mingw64\bin;%path:C:\msys64\mingw64\bin;=%

27行目:ゴミ
28行目:初期の記述(何故かここだけコメントアウトで残している)
29行目:環境変数PATHの先頭にMsys2の/usr/binフォルダを追加(重複排除)
30行目:環境変数PATHの先頭にMsys2版MinGW-w64のbinを追加(重複排除)

MinGWでも32bit版をビルドしないと気が済まない人からの苦情はなかったように思います・・・。

@@ -29,7 +29,8 @@ jobs:
- script: set
displayName: Show environmental variables for debug

- template: /ci/azure-pipelines/template.steps.install-mingw-w64-gcc.yml
- script: C:\msys64\usr\bin\bash --login -c "pacman -S --noconfirm mingw-w64-x86_64-toolchain"
displayName: Install MinGW-w64 build tools via Pacman
Copy link
Contributor

Choose a reason for hiding this comment

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

指摘です。

実効スクリプトが1行しかないので、無駄なテンプレート化はやめてシンプルにしませんか?
という提案を含んでいると読み取りました。

template.steps.install-mingw-w64-gcc.ymlは現在は「空」ですが、
過去に複数ステップのインストール処理を実装していたことがあり、
今後も増減があると思っています。
(過去は、たとえばgoogletestを入れたりしてました。)

「追加のインストールプロセスが必要になったのでステップを追加します」にこのテンプレートの削除の提案を混ぜるのは適切でないと思いました。

Copy link
Author

@ghost ghost Jan 18, 2022

Choose a reason for hiding this comment

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

PRを分割しろってことかな。

僕にはここでテンプレートを使う理由が分かりませんでした。
たとえインストールするパッケージが複数あったとしても、MinGWビルドでしか使わないのだから、他のジョブから再利用できるようにする必要はないと感じます。

Copy link
Author

Choose a reason for hiding this comment

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

MinGWビルドの不要タスク利用の廃止と抱き合わせたうえでリファクタリング案件として別PRを起こします。

Copy link
Contributor

Choose a reason for hiding this comment

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

セットアップタスクの追加はOKです。
タスクテンプレート化の解除も、多少議論の余地あるもののOKだと思います。

問題は、2点目について言及されてないことなのかな?と思います。
いまはmingwのセットアップタスクはこんだけあります、の中身が空の状態で、
このPRのやりたいことは「タスクを増やす」なはずです。
じゃ、なんで構成まで変わっとんねん!ということです。
(無駄だと思うからです→うんそうだね、と言えてしまうとこが微妙なんですが)

@ghost ghost marked this pull request as draft January 18, 2022 03:27
@ghost ghost added azure pipelines MinGW MinGW labels Jan 18, 2022
@ghost
Copy link
Author

ghost commented Jan 18, 2022

Windows Server 2016では動いてしまう原因は、Msys2の構成の違いに加えてこれをやっているからです。

sakura/build-gnu.bat

Lines 27 to 30 in 68c4dc8

@rem https://www.appveyor.com/docs/environment-variables/
@rem path=C:\mingw-w64\x86_64-7.2.0-posix-seh-rt_v5-rev1\mingw64\bin;%path%
path=C:\msys64\usr\bin;%path:C:\msys64\usr\bin;=%
path=C:\msys64\mingw64\bin;%path:C:\msys64\mingw64\bin;=%

27行目:ゴミ 28行目:初期の記述(何故かここだけコメントアウトで残している) 29行目:環境変数PATHの先頭にMsys2の/usr/binフォルダを追加(重複排除) 30行目:環境変数PATHの先頭にMsys2版MinGW-w64のbinを追加(重複排除)

MinGWでも32bit版をビルドしないと気が済まない人からの苦情はなかったように思います・・・。

同列で語っていいのかは知りませんが、こんなのもあります。

:find_gcc_compilers
set C_COMPILER=C:/msys64/mingw64/bin/gcc.exe
set CXX_COMPILER=C:/msys64/mingw64/bin/g++.exe
goto :EOF

提案なんですが、パスの設定をtemplate.step.install-mingw-w64-gcc.ymlに書きませんか?

@ghost ghost marked this pull request as ready for review January 18, 2022 04:42
@berryzplus
Copy link
Contributor

同列で語っていいのかは知りませんが、こんなのもあります。

これは端折ってあるだけだったはず。
もしMsys2版のMinGW-w64が入ってないと、ここまで来ないからgcc/g++の検索を省略している・・・
ここではCMakeのジェネレータにNinjaを使ってるのでコンパイラのフルパスが必要。

@ghost
Copy link
Author

ghost commented Jan 20, 2022

レビューありがとうございました。いったんマージしてしまいます。
何かありましたらご連絡ください。

@ghost ghost merged commit 249b289 into sakura-editor:master Jan 20, 2022
@ghost ghost deleted the feature/update_mingw_toolchain branch January 20, 2022 23:12
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant