-
Notifications
You must be signed in to change notification settings - Fork 240
Support using the Microsoft.NET.Sdk with dotnet/buildtools #2068
Conversation
- Don't import Microsoft.$(Language).targets in buildtools - the SDK will do that. - Use the Sdk's GenerateAssemblyInfo target when possible Still support non-SDK projects for backwards compatiblity. Note: I removed support for @(AssemblyInfoSource) during GenerateAssemblyInfo because it is no longer used as of dotnet/corefx/pull/30400.
@@ -123,7 +123,7 @@ | |||
|
|||
|
|||
<Import Project="$(CSharpTargetsFile)" | |||
Condition="'$(TargetFrameworkIdentifier)' != '.NETPortable' and '$(MSBuildProjectExtension)' == '.csproj'" /> | |||
Condition="'$(TargetFrameworkIdentifier)' != '.NETPortable' and '$(MSBuildProjectExtension)' == '.csproj' and '$(UsingMicrosoftNETSdk)' != 'true'" /> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -41,8 +41,7 @@ | |||
</PropertyGroup> | |||
|
|||
<Target Name="DecideIfWeNeedToIncludeDllSafeSearchPathAttribute" | |||
Condition="'$(IsDotNetFrameworkProductAssembly)' == 'true' AND '$(IsTestProject)' != 'true'" | |||
BeforeTargets="GenerateAssemblyInfo"> | |||
Condition="'$(IsDotNetFrameworkProductAssembly)' == 'true' AND '$(IsTestProject)' != 'true'"> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@@ -104,14 +104,14 @@ | |||
Condition="'$(MSBuildProjectExtension)' == '.depproj'" /> | |||
|
|||
<Import Project="IL.targets" | |||
Condition="'$(MSBuildProjectExtension)' == '.ilproj' AND '$(SkipImportILTargets)'!='true'" /> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
in order to get all the version properties set correctly. | ||
So overwrite these two properties here, so they are set correctly. | ||
--> | ||
<Company>Microsoft Corporation</Company> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
DependsOnTargets="CreateVersionFileDuringBuild;DecideIfWeNeedToIncludeDllSafeSearchPathAttribute" | ||
BeforeTargets="GetAssemblyAttributes"> | ||
<ItemGroup> | ||
<AssemblyAttribute Include="System.Reflection.AssemblyDefaultAliasAttribute"> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few questions but overall looks reasonable.
Still support non-SDK projects for backwards compatiblity.
Note: I removed support for @(AssemblyInfoSource) during GenerateAssemblyInfo because it is no longer used as of dotnet/corefx/pull/30400.