Update Project Template #490
Conversation
- Adjust Project Sdk definition syntax to match how other SDK-style projects are structured, to reduce end-user confusion. - Add a PreBuild target that corrects a build-time bug in Visual Studio that triggers an unnecessary NuGet restore failure.
- Change Name to SqlTargetName, which is baked into `Microsoft.Data.Tools.Schema.SqlTasks.targets` to control the output file name & Dacpac metadata name. - Add DacVersion to help developers understand how to control the versioning.
| <Target Name="PreBuild" BeforeTargets="PreBuildEvent" Condition="'$(MSBuildRuntimeType)' != 'Core'"> | ||
| <Message Importance="high" Text="Ensuring the $(MSBuildThisFileDirectory)\obj\project.assets.json file is removed, if necessary, so that the database project can be built through VisualStudio SSDT without errors" /> | ||
| <Delete Files="$(MSBuildThisFileDirectory)\obj\project.assets.json" /> |
There was a problem hiding this comment.
If I understand correctly, this target would be triggered for every build. @Ri7Sh Do you see any issues with that?
There was a problem hiding this comment.
@robertmclaws Can you please raise an issue describing the bug as well?
There was a problem hiding this comment.
The code originally came from this post: #180 (comment) and fixes issue #180.
If you think it would be better in the SDK.targets file, I'm happy to do another check-in that moves it there.
There was a problem hiding this comment.
ah - clarifying in this thread - this doesn't fix support in VS, it fixes the specific case in VS when you try to build the project after having built it on the same machine with .NET 8 (cli or VS Code). This would be good to resolve @Ri7Sh but let's make sure we converge on an approach in the .NET template as well as in the VS template.
There was a problem hiding this comment.
@dzsquared I believe that is correct. For more context, if you build in VS the first time, it will build correctly.
But let's say you're trying to verify the build on the CLI so you're sure it will run in your CI/CD pipeline. So you run a dotnet build in the VS Terminal.
The next time you will build in Visual Studio, you will get the following error:
Your project does not reference ".NETFramework,Version=v4.7.2" framework. Add a reference to ".NETFramework,Version=v4.7.2" in the "TargetFrameworks" property of your project file and then re-run NuGet restore.
But that is not explicitly a .NET 8 problem, as this happened while I was trying to move to an SDK-style project in a .NET 9 RC2 app. I personally believe it is an issue in the downstream targets file that sneaks in a .NET Framework reference to get the compiler to work, which I believe is a mistake. But I can't find where to contribute a fix to that targets file.
Including this in the project template makes sure that error doesn't happen.
if you would like I can take this out of this PR and put it in a separate one... however this PR contained everything I needed to get a build working consistently both locally and in Azure DevOps, which is why this fix was also included here, so it could get into customer's hands ASAP during the preview, and you cold maybe move the fix somewhere more appropriate later.
HTH!
| <PropertyGroup> | ||
| <SqlTargetName>SqlProject1</SqlTargetName> | ||
| <DacVersion>1.0.0.0<DacVersion> |
There was a problem hiding this comment.
This is already defined in Microsoft.Data.Tools.Schema.SqlTasks.targets. Is this just to add visibility to this property?
There was a problem hiding this comment.
It's that changing the Name tag has zero effect on changing the file name, while setting this property changes both the Name in the Metadata of the Dacpac AND the filename.
The SDK.targets file either needs to use Name to set the SqlTargetName, or just use the SSDT targets property directly.
For DacVersion, yes it's in the Targets file but only if the version is not set. Setting <Version> in the project also has no effect on this property. So again, the SDK targets file either needs to take the Version and/or PackageVersion property and use those to set the DacVersion... or just let the user set the DacVersion directly and have THAT property set the others.
Either way, the only way right now for an end user to know these properties exist is to dig through the Microsoft.Data.Tools.Schema.SqlTasks.targets file in the NuGet package... which is not helpful.
|
|
||
| <PropertyGroup> | ||
| <SqlTargetName>SqlProject1</SqlTargetName> |
There was a problem hiding this comment.
The default for SqlTargetName is actually just $(MSBuildProjectName) so it does seem like the Name is not honored.
There was a problem hiding this comment.
Yes, that is why I changed it. Changing the name did not change the filename at all. So instead of using a property that literally nothing else uses (Name), I bubbled up the existing property from SSDT.
NametoSqlTargetNameto directly control the output file name and metadata (fixes Dacpac file name does not match<Name />tag #491).DacVersionto allow the developer to control the metdata version.PreBuildtarget that corrects a build-time bug in Visual Studio which triggers an unnecessary NuGet restore failure.