Make GenerateResource look up resgen.exe in 10.0 SDK#6336
Make GenerateResource look up resgen.exe in 10.0 SDK#6336benvillalobos merged 3 commits intomainfrom
Conversation
GenerateResource tests were failing when only VS 2019 and 10.0 Windows SDK is installed. I suspect the task itself would also fail if run with ExecuteAsTool. The problem is that we started look up with .NET 4.6.1 and lower. Start lookup with .NET 4.8 instead and fallback down the chain. This should still find all earlier SDKs too. Introduce latest versions of Visual Studio too.
| /// compatibility implications. | ||
| /// </summary> | ||
| VersionLatest = Version462, | ||
| VersionLatest = Version48, |
There was a problem hiding this comment.
Everything looks good except this. I'm not sure it should change in a minor update for compatibility reasons. But I'm only saying that as the author of that comment that says not to, not that I remember why not :(.
There was a problem hiding this comment.
IIRC it's because enum values get baked into calling assemblies and we wanted it to be possible to use VersionLatest and compile against the latest MSBuild while still running on an earlier patch. Especially given AzDO/GitHub silent VS version updates that still seems like a good conservative policy.
| Version471 = 12, | ||
| Version472 = 13, | ||
| Version48 = 14, | ||
| VersionLatest = 14, |
There was a problem hiding this comment.
IIRC this is also something that shouldn't change mid-major-version.
| // VS16 | ||
| { (dotNetFrameworkVersion451, visualStudioVersion160), (dotNetFrameworkVersion45, visualStudioVersion160) }, | ||
| { (dotNetFrameworkVersion452, visualStudioVersion160), (dotNetFrameworkVersion451, visualStudioVersion160) }, | ||
| { (dotNetFrameworkVersion46, visualStudioVersion160), (dotNetFrameworkVersion451, visualStudioVersion160) }, |
There was a problem hiding this comment.
This matches above. But do we know why it's not falling back to 452?
| /// compatibility implications. | ||
| /// </summary> | ||
| VersionLatest = Version462, | ||
| VersionLatest = Version48, |
There was a problem hiding this comment.
IIRC it's because enum values get baked into calling assemblies and we wanted it to be possible to use VersionLatest and compile against the latest MSBuild while still running on an earlier patch. Especially given AzDO/GitHub silent VS version updates that still seems like a good conservative policy.
|
|
||
| if (String.IsNullOrEmpty(_sdkToolsPath)) | ||
| { | ||
| _resgenPath = ToolLocationHelper.GetPathToDotNetFrameworkSdkFile("resgen.exe", TargetDotNetFrameworkVersion.Version35); |
There was a problem hiding this comment.
This being specifically 3.5 makes me a bit nervous. Was it just never changed and usually overridden from project logic, so we never noticed?
There was a problem hiding this comment.
Turns out we need to leave Version35 here. I've added a comment explaining what is going on.
This actually fixes the unit-tests on a machine with only SDK 10.0 installed.
|
Turns out, this commit alone is what fixes the unit-tests for me: Technically we don't need the first two commits but if I've done the work anyway, I suppose let's keep them. |
Forgind
added
the
merge-when-branch-open
| Version160, | ||
|
|
||
| /// <summary> | ||
| /// Visual Studio "Dev17" |
There was a problem hiding this comment.
| /// Visual Studio "Dev17" | |
| /// Visual Studio 2022 "Dev17" |
nit 😉
5 participants
GenerateResource tests were failing when only VS 2019 and 10.0 Windows SDK is installed. I suspect the task itself would also fail if run with ExecuteAsTool.
The problem is that we started look up with .NET 4.6.1 and lower. Start lookup with .NET 4.8 instead and fallback down the chain. This should still find all earlier SDKs too.
Introduce latest versions of Visual Studio too.
Fixes #6266