use a custom MSBuild task #971
Conversation
Co-authored-by: Brandon Ording <bording@gmail.com>
|
@bording I cherry-picked this from https://github.com/bording/minver/commits/msbuild-task-spike/ |
| @@ -4,8 +4,12 @@ | |||
| <MSBuildAllProjects Condition="'$(MSBuildAssemblyVersion)' == '' Or '$(MSBuildAssemblyVersion)' < '16.0'">$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects> | |||
| <GenerateNuspecDependsOn>$(GenerateNuspecDependsOn);MinVer</GenerateNuspecDependsOn> | |||
| <GetPackageVersionDependsOn>$(GetPackageVersionDependsOn);MinVer</GetPackageVersionDependsOn> | |||
| <MinVerTaskPath Condition="'$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)..\minver\net6.0\MinVer.dll</MinVerTaskPath> | |||
| <MinVerTaskPath Condition="'$(MSBuildRuntimeType)' != 'Core'">$(MSBuildThisFileDirectory)..\minver\net472\MinVer.dll</MinVerTaskPath> | |||
There was a problem hiding this comment.
@bording this doesn't seem right. The MinVer project isn't targetting net472.
There was a problem hiding this comment.
The MinVer project isn't targetting net472.
It will need to. I didn't bother with changing that in my spike, but you'll need to multi-target and build a net472 version of the task assembly as well to support .NET Framework MSBuild, which is used by Visual Studio.
There was a problem hiding this comment.
Ugh... that's a pain. Does even the latest version of VS use .NET Framework MSBuild?
There was a problem hiding this comment.
Hmmm... what concerns me about this is that the net472 code path will be untested in CI. Local smoke testing would be one option to mitigate that but it's not easy for me to do so since I no longer use Windows. I guess it's unlikely that the net472 version will have problems if the net6 version doesn't but it's the edge cases/unknown unknowns that I'm wary of. I have to say this gives me cold feet about switching to an MSBuild task.
Is the caching part of your spike only possible if we switch to an MSBuild task?
There was a problem hiding this comment.
Hmmm... what concerns me about this is that the net472 code path will be untested in CI
That doesn't have to be true, though. It would certainly be possible to have tests that also verify behavior on net472.
I have to say this gives me cold feet about switching to an MSBuild task.
I don't really understand this, and given the benefits of doing it, I really don't agree.
Is the caching part of your spike only possible if we switch to an MSBuild task?
Yes, it's relying on APIs that only accessible from a task.
There was a problem hiding this comment.
That doesn't have to be true, though. It would certainly be possible to have tests that also verify behavior on
net472.
I guess it is possible, but it would not require running tests conditionally, on Windows only, calling msbuild.exe?
There was a problem hiding this comment.
I'm not sure what sort of tests you're thinking of. Most of the logic would just be testable directly, and the test projects could be multi-targeted, so that would take care of that.
If you want some sort of integration test, then that would need to use dotnet/msbuild.exe and to use the respective task assemblies, but that also seems rather doable?
|
closing in favour of #989 |
closes #112