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

最新のWindows SDKでビルドできるようにする #1621

Merged
merged 3 commits into from
Apr 16, 2021

Conversation

dep5
Copy link
Contributor

@dep5 dep5 commented Apr 11, 2021

PR の目的

Visual Studio 2019で最新のWindows SDKを使えるようにします

カテゴリ

  • ビルド関連
  • ローカルビルド

PR の背景

Visual Studio 2019でのローカルビルドする時Windows SDKの最新バージョンのみをインストールした状態では
指定のバージョンのSDKを入れるようにエラーが出ます

PR のメリット

Windows SDKの最新バージョンが使えます
サクラエディタのビルドでしか使わないバージョンのWindows SDKを入れる必要がなくなります

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

Windows SDKの最新バージョンでは使えない機能があるかもしれません
その時に改めてバージョンを指定してもよいのでは、と思います

仕様・動作説明

Visual Studio 2017では指定のWindows SDKのバージョンを使い
Visual Studio 2019では最新のWindows SDKを使えるようになります

PR の影響範囲

テスト内容

テスト1

手順

関連 issue, PR

参考資料

@AppVeyorBot
Copy link

Build sakura 1.0.3661 completed (commit 545e3a2aad by @dep5)

@berryzplus
Copy link
Contributor

はじめましてっす。

  • PR趣旨には賛同です。
  • 修正内容について
    • この内容で最新のWindows 10 SDKが使われるようになるのは正しい認識です。
    • この内容だとソリューションを開いたときのエラー(?)は消えないです。
      👇これを削る必要があります。
      "microsoft.visualstudio.component.windows10sdk.17763",

@dep5
Copy link
Contributor Author

dep5 commented Apr 12, 2021

ありがとうございます。削りました

@ghost
Copy link

ghost commented Apr 12, 2021

それ、削っていいんですかね。
.vsconfigだけではビルド可能な状態に持っていけないことになりますが。
(README.mdの記述による)

VS2019のデフォルトインストールされるSDKバージョンが、どこかのタイミングで18362から変更されたので、「SDKバージョンの固定をやめてVSの構成を原因としたビルドエラーの可能性を減らす」という提案だと理解しました。

@AppVeyorBot
Copy link

Build sakura 1.0.3662 completed (commit f188088b67 by @dep5)

@dep5
Copy link
Contributor Author

dep5 commented Apr 12, 2021

エラーを気にせず使っていたのでわかりませんでしたが、
SDKを入れずに使っている人のための項目ということですね。
コミットの度にチェックが毎回走るのが申し訳ないので、あとで調べてから修正したいと思います

@ghost
Copy link

ghost commented Apr 12, 2021

SDKを入れずに使っている人のための項目

README.mdの記述から考えると、「サクラエディタのビルド環境を簡単に構築するための項目」と表現できると思います。
必要とするコンポーネントを記述した.vsconfigファイルをインストーラーのコマンドラインにオプションとして渡すと、それらを自動でインストールしてくれます。
2019以降なら、sakura.slnを開いたときに書いてあるもののインストールされていないコンポーネントがあれば通知が表示されます。

したがって、このファイルからSDKのコンポーネントに関する記述を削れば、SDKがインストールされていなくても通知されなくなるので、後から手動で入れる手間が発生します。

@ghost ghost mentioned this pull request Apr 12, 2021
@dep5
Copy link
Contributor Author

dep5 commented Apr 13, 2021

試しにVisual StudioインストーラーでWindows SDKのチェックを全部外してみましたが
古いSDKが残っているようで、まっさらな状態での動作チェックができませんでした。
vcxcompat.props の項目を変更しないとビルド自体できないのに比べると
.vsconfigはエラーを気にしなければビルド可能で、必要な機能とのことでいじらないでおくことにします。

@sonarcloud
Copy link

sonarcloud bot commented Apr 13, 2021

Kudos, SonarCloud 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.3666 completed (commit 9749c44f95 by @dep5)

@ghost
Copy link

ghost commented Apr 13, 2021

.vsconfigはエラーを気にしなければ

気になるので質問させてください。これでしょうか?

スクリーンショット 2021-04-13 235507

@dep5 dep5 marked this pull request as ready for review April 14, 2021 11:53
@dep5
Copy link
Contributor Author

dep5 commented Apr 14, 2021

.vsconfigのエラーとはソリューションエクスプローラーの
その黄色い表示のつもりでした。紛らわしくてすいません
.vsconfigのwindows10sdk.17763を削ると黄色の表示は消えました。
SDK無しで開くとどうなるかをチェックしたかったのですが、思うようにチェックできませんでした。

Visual StudioインストーラーでSDKの最新バージョンのみにチェックを入れ直しています。
現在新規インストールした時に最新版がデフォルトインストールなのかはわかりません。

.vsconfigの更新ありがとうございました。

@ghost
Copy link

ghost commented Apr 14, 2021

画像のメッセージはエラーではないのでビルドができるのであれば気にしなくても大丈夫だとは思います。

ご確認ありがとうございます。

@berryzplus
Copy link
Contributor

berryzplus commented Apr 14, 2021

ビルド環境を特定バージョンのwin10SDKに制限する必要は必ずしもないので、変更は適切と思います。

そもそも、ビルドで使える機能はWindows7相当の機能に限定されるようになっていますし、仮にこの設定で最新windowsの機能を利用したPRを投げた場合には、CIでvs2017ビルドが失敗して異変を察知できる仕組みになっています。

特に反対意見等なければ、明日あたりマージしてしまおうと思います。

@berryzplus
Copy link
Contributor

マージしちゃいます。

@berryzplus berryzplus merged commit 6b30d35 into sakura-editor:master Apr 16, 2021
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.

4 participants