Skip to content

Convert ComHost tasks to use System.Text.Json instead of Newtonsoft.Json #3087

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Convert ComHost tasks to use System.Text.Json instead of Newtonsoft.Json #3087

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f7f53dc
Add reference to Microsoft.Bcl.Json.Sources source package.
jkoritzinsky Apr 4, 2019
ed58b77
Convert reg-free com manifest generation to System.Text.Json.
jkoritzinsky Apr 5, 2019
b781924
Move ComHost tasks over to System.Text.Json.
jkoritzinsky Apr 5, 2019
1f5ef15
Add TODO remove comment.
jkoritzinsky Apr 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add reference to Microsoft.Bcl.Json.Sources source package.
  • Loading branch information
@jkoritzinsky
jkoritzinsky committed Apr 4, 2019
commit f7f53dc3f663ef1937296de9d19bc5f009b59b8a
11 changes: 11 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/Microsoft.NET.Build.Tasks.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<OutDirName>$(Configuration)\Sdks\$(PackageId)\tools</OutDirName>
<NoPackageAnalysis>true</NoPackageAnalysis>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<NoWarn>3021</NoWarn>
</PropertyGroup>

<Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
Expand Down Expand Up @@ -46,6 +47,10 @@
<PackageReference Include="Microsoft.Extensions.DependencyModel" Version="$(MicrosoftExtensionsDependencyModelVersion)" />
<PackageReference Include="NuGet.ProjectModel" Version="$(NuGetProjectModelVersion)" />
<PackageReference Include="NuGet.Build.Tasks.Pack" Version="$(NuGetBuildTasksPackVersion)" ExcludeAssets="All" />
<PackageReference Include="Microsoft.Bcl.Json.Sources" Version="4.6.0-preview3.19128.7" GeneratePathProperty="true">
<PrivateAssets>All</PrivateAssets>
</PackageReference>
<PackageReference Include="System.Memory" Version="4.5.2" />
Copy link
Contributor

@nguerrera nguerrera Apr 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super kean on more copy-local assemblies for the desktop build. We ran into issues with System.Memory and S.RC.Unsafe and mismatched versions in PresentationBuildTasks.dll. I don't know if it's worth adding more dependencies if we can't remove NewtonSoft.Json. And we can't because NuGet depends on it.

Copy link
Contributor

@nguerrera nguerrera Apr 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd wait for our dependencies to stop using newtonsoft.json. This is going to just create more DLLs to ship and load, not fewer.

Copy link

@ahsonkhan ahsonkhan Apr 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can move to 3.0.

The decision to port is up to you, but for clarity, if this moves to 3.0 and the built-in library is being used, then there won't be extra copy-local assemblies like System.Memory.

Copy link
Contributor

@nguerrera nguerrera Apr 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can't be 3.0 only. It can be 3.0 + 472. We have to run in Framework for VS msbuild

</ItemGroup>

<!-- System.Reflection.Metadata is in-box for .NET Core 2.0, so we only need to reference it for .NET Framework -->
Expand Down Expand Up @@ -81,6 +86,12 @@
<None Include="..\Common\Resources\xlf\**\*" LinkBase="Resources\xlf" />
</ItemGroup>

<Target Name="RemoveSystemTextJsonResourcesFromXlf" BeforeTargets="GetXlfSources">
<ItemGroup>
<EmbeddedResource Update="$(PkgMicrosoft_Bcl_Json_Sources)/**/*.resx" XlfInput="false" Condition="'$(PkgMicrosoft_Bcl_Json_Sources)' != ''" />
</ItemGroup>
</Target>

<Target Name="PrepareAdditionalFilesToLayout" BeforeTargets="AssignTargetPaths">
<PropertyGroup>
<_NugetBuildTasksPackPath>$(NuGetPackageRoot)nuget.build.tasks.pack\$(NuGetBuildTasksPackVersion)</_NugetBuildTasksPackPath>
Expand Down