Visual Studio 2022に対応する #1755
Visual Studio 2022に対応する #1755
Conversation
|
✅ Build sakura 1.0.3976 completed (commit e08fb8457a by @dep5) |
tests/compiletests.targets
Outdated
| @@ -18,6 +18,7 @@ | |||
| <SetEnv name="platform" value="$(Platform)" prefix="false" /> | |||
| <SetEnv name="configuration" value="$(Configuration)" prefix="false" /> | |||
| <SetEnv name="NUM_VSVERSION" value="$(VsVersion)" prefix="false" /> | |||
| <SetEnv name="ARG_VSVERSION" value="$(VsVersion)" prefix="false" /> | |||
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
僕が #1752 で言及した直後にこういったものが出てくると、なんだか不安になってきます。 |
|
#1752 設定する環境変数はNUM_VSVERSIONではなく、ARG_VSVERSIONではないかなと思います。 |
CIの話ではありますが、VS2017のビルドをCIで確認できないということは、VS2017を使い続けることが困難になるということでもあります。特定バージョンでしかビルドできないコードを弾くためにも確認できないといけません。 |
| :: 16 => Visual Studio 2019 | ||
| :: 2017 or 15 => Visual Studio 2017 | ||
| :: 2019 or 16 => Visual Studio 2019 | ||
| :: 2022 or 17 => Visual Studio 2022 |
There was a problem hiding this comment.
ここのコメント修正はどちらでもよいです。
visual studio 的に「バージョン」とは 17.0 のことで 2022 は「商標」にあたります。
ここで指定するのはバージョンなので、2桁を前にしたほうが良いように思います。（対応不要
There was a problem hiding this comment.
そもそも
:: 2017 => Visual Studio 2017
:: 2019 => Visual Studio 2019
+ :: 2022 => Visual Studio 2022
:: 15 => Visual Studio 2017
:: 16 => Visual Studio 2019
+ :: 17 => Visual Studio 2022
とすればよかったような気もします。
| @@ -185,6 +186,8 @@ exit /b | |||
| set CMAKE_G_PARAM=Visual Studio 15 2017 | |||
| ) else if "%NUM_VSVERSION%" == "16" ( | |||
| set CMAKE_G_PARAM=Visual Studio 16 2019 | |||
| ) else if "%NUM_VSVERSION%" == "17" ( | |||
| set CMAKE_G_PARAM=Visual Studio 17 2022 | |||
| ) else ( | |||
| call :set_cmake_gparam_automatically | |||
| ) | |||
There was a problem hiding this comment.
235行目ですが、ここに「17^」を追加する必要がある気がします。
Line 235 in 90951db
There was a problem hiding this comment.
👆の部分は、利用できるMsBuildのパスを列挙する部分です。
ここを修正しないとPRの目的が達成されないのではないかと思います。
vs2019のMsBuildは、vs2022を使ったビルドができる仕様なので、
結果オーライになると予想されますが、vs2022しか入れない端末でのビルドができないんじゃないかと思いました。
There was a problem hiding this comment.
VS2019/VS2022が混在した環境で実行し、vswhereにどんな引数が入るか確認してみました。
引数指定なしの場合
find-tools.bat
vswhere -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath -version [15,16\)
vswhere -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationVersion -latest
vswhere -requires Microsoft.Component.MSBuild -find MSBuild\**\Bin\MSBuild.exe -version [17,18\)
|- CMD_MSBUILD=C:\Program Files\Microsoft Visual Studio\2022\Community\MSBuild\Current\Bin\MSBuild.exe
|- CMAKE_G_PARAM=Visual Studio 17 2022
end
引数に「15」を与えた場合（※実行した環境にVS2017はインストールされていない）
find-tools.bat
vswhere -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath -version [15,16)`
vswhere -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationVersion -latest`
vswhere -requires Microsoft.Component.MSBuild -find MSBuild\**\Bin\MSBuild.exe -version [17,18)`
|- CMD_MSBUILD=C:\Program Files\Microsoft Visual Studio\2022\Community\MSBuild\Current\Bin\MSBuild.exe
|- CMAKE_G_PARAM=Visual Studio 17 2022
end
引数に「16」を与えた場合
find-tools.bat
vswhere -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath -version [16,17)
vswhere -requires Microsoft.Component.MSBuild -find MSBuild\**\Bin\MSBuild.exe -version [16,17)
|- CMD_MSBUILD=C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Current\Bin\MSBuild.exe
|- CMAKE_G_PARAM=Visual Studio 16 2019
end
引数に「17」を与えた場合
find-tools.bat
vswhere -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath -version [17,18)
vswhere -requires Microsoft.Component.MSBuild -find MSBuild\**\Bin\MSBuild.exe -version [17,18)
|- CMD_MSBUILD=C:\Program Files\Microsoft Visual Studio\2022\Community\MSBuild\Current\Bin\MSBuild.exe
|- CMAKE_G_PARAM=Visual Studio 17 2022
end
探索ロジックがまずビルドツールを「指定したバージョンが利用できるか調べ、できなければ最新のものを選ぶ」ようになっており、MSBuildは「ビルドツールの探索で決定したバージョンのものを探す」ようになっているようなので、find-tools.batの実行結果からは問題なく見えています。
しかし、find_msbuild_legacyで実行されるvswhereの呼び出し箇所は、逆に「利用できる一番古いもの」という動作になりそうな気がします（上記と同じ環境で実行しました）。
> vswhere -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationVersion -latest
17.0.32014.148
> vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [15,17)
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community
ビルドツールの探索と同じく、指定バージョンのものが見つからなければ利用可能な最新のものを選ぶようにして良いのではないかと思うのですが。
> vswhere -requires Microsoft.Component.MSBuild -property installationPath -latest
C:\Program Files\Microsoft Visual Studio\2022\Community
There was a problem hiding this comment.
単独環境はこうなりました。
VS2017のみ
> vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [15,16`)
C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise
> vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [15,17`)
C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise
VS2019のみ
> vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [15,16`)
> vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [15,17`)
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise
VS2022のみ
> vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [15,16`)
> vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [15,17`)
There was a problem hiding this comment.
vswhere -versionを使って抽出するには
VS2017のみ
vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [15,16)
VS2019のみ
vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [16,17)
VS2022のみ
vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [17,18)
VS2017とVS2019
vswhere -requires Microsoft.Component.MSBuild -property installationPath -version [15,17)
となります。
1つだけ指定のメジャーバージョンを見つけるにはこう書くようです。
vswhere -version [x,x+1)
There was a problem hiding this comment.
:find_msbuild_legacyについてですが
改めて見直してみたところ、将来のVisual Studioのバージョンで
Msbuild.exeのパスが変更された場合などを想定しているように思います
There was a problem hiding this comment.
find_msbuild_legacyまで到達するシチュエーションが本当にありうるかは不明です。
.vsconfigに書いたコンポーネントのインストールが済んでいることを前提とすれば到達しなさそう。
だとしたら変更しなくても支障ないかも。
|
@kazasaku さん
コメントの意図が分からないです。 「こういったもの」がこのPRを指すのは自明ですが、 個人的には「あきらかにアカンやつ」とは思っていないです。 |
|
指摘ありがとうございます。 わたしも最初そのように変更していたのですが :find_msbuild で CMD_MSBUILD が設定されない場合に 今までARG_VSVERSIONが無かったので サクラエディタがいつまで対応するかは置いておいて、 Visual Studio 2019のみ、2022のみの環境で動くのかは気になっています。 tests/compiletests.targetsで NUM_VSVERSION を残すべきか |
自分の中では「CIも対応したみたいだし、そろそろ検討を始めませんか」と言う段階なので、いきなり対応開始するためのPRが出されたのは「明らかに早すぎる」と感じています。 どうしてもVS2022を使いたいんだとかであれば別ですが。 |
それらtargetsファイルの変更は要らないと言っています。 |
Lines 58 to 75 in 42568b8 ここで NUM_VSVERSIONに選択されたVSのメジャーバージョンが設定され、その値を後からtargetsファイルなどが利用します。ARG_VSVERSIONは、find-tools.batを呼び出す前にあらかじめユーザーが設定したとかでなければ定義されないはずです。
|
|
kazasakuさん その部分の前に Line 39 in 42568b8 が呼ばれています。 setlocalコマンドとendlocalコマンドで囲われた箇所です:msbuildではNUM_VSVERSIONはARG_VSVERSIONの値で上書きされています。ARG_VSVERSIONを指定しない場合、15が設定されます
つぎに わたしの直面したのは15.0と17.0の組み合わせですが、 コマンドラインでのビルドについて考えてみます。CIもこれでしてるのでしょうか？ Line 22 in 42568b8 まずこれが呼ばれるようなので、どのバージョンが選ばれるかは上と同じだと思います。 |
|
find-tools.batでVSVERSIONを受け入れるのは targetsが意図しているのがfind-tools.batにVSVERSIONを渡すことなら
そう言われれば、その通りです。 |
|
実現したことが明確で変更理由に筋が通っているなら導入して問題ない内容だと思っています。 ・vs2022でもビルドできるようにしたい find-tools.batには一貫性のないロジックがあると思うんですが、 バッチでも関数でも同じですが、呼出の引数は最小限とするのが望ましいので、 |
繰り返しになりますが、
ユーザー側の指定の揺れを吸収しつつ値を検証し、正しい値をNUM_VSVERSIONとして設定する仕組みです。 僕の結論：find-tools.batと各環境変数の使い方は今のままで正しい。 |
|
一貫性のあるロジック、ですかね？ find-tools.batを触らないことにしたのでそれ以外を編集することにします。 ここ以外に出てこないので一時的なテストコードと考えて削除しています。 |
|
Kudos, SonarCloud Quality Gate passed!
|
tests/compiletests.targets
Outdated
| <SetEnv name="NUM_VSVERSION" value="$(VsVersion)" prefix="false" /> | ||
| <SetEnv name="ARG_VSVERSION" value="$(VsVersion)" prefix="false" /> |
There was a problem hiding this comment.
不満はたくさん残りますが、これらを撤回していただければいったんapproveすることにします。
…が、事前検討の不足などから混乱を招いているように感じますので、今後はいきなりPRを出したりせずissueを立ててください。
There was a problem hiding this comment.
このファイルが実行されるのは、常にvisual studioの「中」です。
ここから呼ばれたバッチでは「呼出元と同じバージョンのvisual studio」を使います。
ユーザーの利便性のための「2022」に対応する必要はないので「追加不要」ということになります。
|
❌ Build sakura 1.0.3979 failed (commit 41b6dda8ad by @dep5) |
|
2022の導入には反対しませんが、この形で対応しても2022でビルドできることは検証できませんのでご注意ください。 今月中ごろを目標にいったんCI対応の実験PRを出す予定ではいますのでよろしくお願いします。 |
tests/googletest.targets
Outdated
| @@ -37,7 +37,7 @@ | |||
| </PropertyGroup> | |||
| <SetEnv name="platform" value="$(Platform)" prefix="false" /> | |||
| <SetEnv name="configuration" value="$(Configuration)" prefix="false" /> | |||
| <SetEnv name="NUM_VSVERSION" value="$(VsVersion)" prefix="false" /> | |||
| <SetEnv name="ARG_VSVERSION" value="$(VsVersion)" prefix="false" /> | |||
僕は「そんなバグは存在しない」という認識です。 |
|
❌ Build sakura 1.0.3979 failed (commit 0cd30f73eb by @dep5) |
|
kazasaku さん set ARG_VSVERSION=latest つぎに しかし、ARG_VSVERSIONがlatestから実際のバージョンに変わってしまっていますね。 まだテストしてませんが、このような指定にする必要があるかもしれないです。 kazasakuさんはVS2019/VS2022が混在した環境をお持ちなんですよね？ の行を と書き換えながらテストしてみてもらえませんか？ 表にしてくださった |
|
結論として不具合修正でなくなるなら、 |
|
✅ Build sakura 1.0.3982 completed (commit 0cd30f73eb by @dep5) |
tests/compiletests.run.cmd
Outdated
| @@ -5,8 +5,6 @@ set SOURCE_DIR=%~dp0compiletests | |||
| :: find generic tools | |||
| if not defined CMD_VSWHERE call %~dp0..\tools\find-tools.bat | |||
|
|
|||
| set /a NUM_VSVERSION_NEXT=NUM_VSVERSION + 1 | |||
|
|
|||
There was a problem hiding this comment.
これを削ってよいのか疑問です。
本来 [16,17) のように使うための変数と考えられるので、
変数が利用されていないとすれば、find-tools.batが壊れていると思います。
There was a problem hiding this comment.
調査結果： この一文は要りません。
PRの趣旨とは無関係なので、対応からは外すのがベターと思います。
コミットが分かれているならPRに混ぜてもよいはと思います。
There was a problem hiding this comment.
NUM_VSVERSION_NEXTはソースをGrepしてみたところ、find-tools.bat内とここでだけ使用されています。
find-tools.batのあとで使われているのでテストコードのように思えるのですが、どうでしょうか？
コミットは分けたほうがよいですね・・・
There was a problem hiding this comment.
find-tools.batのNUM_VSVERSION_NEXTはendlocalより前にあるので外には出てこないはずです。
find-tools.batから制御が戻った時にはもうなくなっています。
つまりこのコードは要りません。
…が、VS2022対応とは無関係な変更になります。
|
試してみましたところ、find-tools.batに不具合があるように見えます。 vs2017、vs2019、vs2022がインストールされた端末で、 find-tools.bat clear
set NUM_VSVERSION=16
find-tools.bat先にこの不具合を修正したほうがわかりやすいのではないかと思います。 |
|
find-tools.batの不具合修正をしてみました。 やりとりするのが面倒くさいのでPRは出しません。 |
|
berryzplus さん
ARG_VSVERSIONを使う必要があります
不具合とはプログラム本体の動作のことなのですか？ vcx-props/vcxcompat.props を修正しないとVS2022を使うことができません。 調べた結果、問題の原因はARG_VSVERSIONとNUM_VSVERSIONの混用にあると思います find-tools.batの挙動をNUM_VSVERSIONも指定できるように変更する場合、 どちらでも設定できるとなったら、どちらで設定するのが正解ですか？ わたしはfind-tools.batに渡すにはARG_VSVERSIONに一本化すべきと思います。 NUM_VSVERSIONでソースをGrepすると使用されているのは
なので、コマンドラインで |
自分はそれを「find-tools.bat の不具合である」と解釈しました。
違います。バッチの不具合も「不具合修正」としてよいと思います。
「vs2022を使えるようにする」が目的なら、その修正は必須です。
ARG_VSVERSION はバッチに与える引数なので、厳密には環境変数じゃありません。 ARG_VSVERSION ＝ find-tools.batに与えた第１引数の値が "clear" でない場合に、ローカルな環境変数として定義されるものです。（外部から指定できますが、名前付き引数として指定できるわけじゃないです。）
NUM_VSVERSIONとは異なり、ARG_VSVERSIONはバッチ引数に付けるローカルな名前なので、 まぁ結局、vs2022を使えるようにしたいのか、compiletest.targetsのバグ（？）を直したいのかどっちやねん？という感じになります。こういう言い方をしたらあかんのかも知れませんが。 |
|
書き洩らし。
ARG_VSVERSIONはNUM_VSVERSIONを指定するためのものなので、 |
|
VS2017と2022の入った環境で または compiletests.run.cmdに compiletests.targetsの 結果: 結果: 以上から、コマンドプロンプトで環境変数ARG_VSVERSIONを設定したあと、 |
|
❌ Build sakura 1.0.3984 failed (commit 0a87166fdd by @dep5) |
|
パッチありがとうございます。 リンクがうまくできなくてすいませんが
tools\find-tools.mdの44行目
などをご覧ください ARG_VSVERSIONは引数以外の使い方もできますし、それを想定しています。
$(VisualStudioVersion)と間違えてませんか？ https://github.com/sakura-editor/sakura/blob/25281cdf5d5be0174ca4be0923cbee717dc77ba5/tests/compiletests.targets#L16 kazasakuさんもおっしゃったように これを対応しないと、VS2022のCMAKEが使えないのでVS2022対応の一環と思っています。 |
「引数指定がない場合に無条件でVS2017を仮選択する」のはおかしいということかな？ find-tools.bat の話はVS2022対応とは関係ない話だと思うので、issueとして切り出しておきます。 |
そう言った覚えはないです😢
|
ghost
mentioned this pull request
find-tools.batに関する疑問などをまとめ、 #1759 として投稿しました。 |
|
kazasakuさん
で切れていました。すいません #1755 (comment) |
|
指摘していただいた部分だと思うのですが、ドキュメントが変な感じですね。 Line 44 in 25281cd ドキュメントに「環境変数」と書くとグローバルな環境変数と混同してしまうのでよくないと思います。 Lines 23 to 25 in 25281cd バッチファイルの記法で %~1 は「最初の引数から引用符を取り除いた文字列」を意味します。shiftコマンドとか、知らないことにしたほうが説明しやすい概念もあったりなかったり。。。 |
|
mdのファイルはリンクできないのかと思ったのですが、できるのですね。 グローバルな環境変数とはコントロールパネルで設定する項目のことですか？ |
表示したプレビューページの上の方に |
|
Lines 1 to 2 in 138ec8e できました。?も消すのですね。ありがとうございます |
|
force-pushしてしまったのですが、こんな感じになりました。 vs2022対応とは無関係なコミットを7つも積んでいます。 |
1行にまとめられるんですね。とてもいいです。 ということでクローズします。 |
PR の目的
Visual Studio 2022に対応します。
起動しているVisual Studioのバージョンのツールを使うようにします。
カテゴリ
PR の背景
Visual Studio 2022を使うとソリューション(プロジェクト)の再ターゲットが必要になります。
合わせて想定と違うバージョンのツールが選ばれるのにも対応しました。
PR のメリット
そのままビルドできます。
起動したバージョンのツールが選ばれます。
PR のデメリット (トレードオフとかあれば)
対応した以外のビルド方法に影響があるかもしれません。
仕様・動作説明
vcx-props/vcxcompat.propsとtools/find-tools.batに追記しました。
Visual Studio 2017とそれ以上がインストールされている環境でIDEでローカルビルドをすると
サクラエディタの初回ビルドやリビルドの時に
find-tools.batが動いて各種ツールで2017のものが設定されます。
そのままでも特に問題はないですが、調べてみました。
find-tools.batはtests/compiletests.run.cmdで呼ばれており
tests/compiletests.targetsではNUM_VSVERSIONに起動しているVisual Studioのバージョンが指定されています。
置き換えでいいかもしれませんが、ARG_VSVERSIONにも同様の指定をして、環境変数を追加しています。
tests/googletest.targetsにも同様の指定があるので対応した方がよいかも。
PR の影響範囲
対応した以外のビルド方法に影響があるかもしれません。
テスト内容
Visual Studio 2022 IDEのsakuraプロジェクトを右クリックしてビルドします。
テスト1
手順
関連 issue, PR
参考資料