Skip to content

Suppress warnings about breaking changes to Newtonsoft.Json dependency in SignalR #9405

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

Merged
merged 3 commits into from
Apr 16, 2019
Merged

Suppress warnings about breaking changes to Newtonsoft.Json dependency in SignalR #9405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8965e21
Suppress warnings about breaking changes to Newtonsoft.Json dependenc…
natemcmaster Apr 16, 2019
220985e
fixup! Suppress warnings about breaking changes to Newtonsoft.Json de…
natemcmaster Apr 16, 2019
14b2fa0
Update BuildErrors.md
natemcmaster Apr 16, 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
17 changes: 17 additions & 0 deletions docs/BuildErrors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Build Errors
Copy link
Contributor

Choose a reason for hiding this comment

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

👏 👏 👏 👏 👏 👏

------------

This document is for common build errors and how to resolve them.

### Warning BUILD001

> warning BUILD001: Package references changed since the last release...

This warning indicates a breaking change might have been made to a package or assembly due to the removal of a reference which was used
in a previous release of this assembly. See <./ReferenceResolution.md> for how to suppress.

### Error BUILD002

> error BUILD002: Package references changed since the last release...

Similar to BUILD001, but this error is not suppressable. This error only appears in servicing builds, which should not change references between assemblies or packages.
14 changes: 14 additions & 0 deletions docs/ReferenceResolution.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ The requirements that led to this system are:
* Name the .csproj file to match the assembly name.
* Run `build.cmd /t:GenerateProjectList` when adding new projects
* Use [eng/tools/BaseLineGenerator/](/eng/tools/BaselineGenerator/README.md) if you need to update baselines.
* If you need to make a breaking change to dependencies, you may need to add `<SuppressBaselineReference>`.

## Important files

Expand Down Expand Up @@ -67,3 +68,16 @@ Steps for adding a new package dependency to an existing project. Let's say I'm

If you don't know the commit hash of the source code used to produce "0.0.1-beta-1", you can use `000000` as a placeholder for `Sha`
as its value is unimportant and will be updated the next time the bot runs.

## Example: make a breaking change to references

If Microsoft.AspNetCore.Banana in 2.1 had a reference to `Microsoft.AspNetCore.Orange`, but in 3.0 this reference is changing to `Microsoft.AspNetCore.BetterThanOrange`, you would need to make these changes to the .csproj file
Copy link
Contributor

Choose a reason for hiding this comment

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

🍌 is already better than orange (🔶) ❕


```diff
<!-- in Microsoft.AspNetCore.Banana.csproj -->
<ItemGroup>
- <Reference Include="Microsoft.AspNetCore.Orange" /> <!-- the old dependency -->
+ <Reference Include="Microsoft.AspNetCore.BetterThanOrange" /> <!-- the new dependency -->
+ <SuppressBaselineReference Include="Microsoft.AspNetCore.Orange" /> <!-- suppress as a known breaking change -->
</ItemGroup>
```
11 changes: 9 additions & 2 deletions eng/targets/ResolveReferences.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@

<!-- Identify if any references were present in the last release of this package, but have been removed. -->
<UnusedBaselinePackageReference Include="@(BaselinePackageReference)" Exclude="@(Reference);@(_ProjectReferenceByAssemblyName);@(PackageReference)" />
<!-- Only allow suppressing baseline changes in non-servicing builds. -->
<UnusedBaselinePackageReference Remove="@(SuppressBaselineReference)" Condition="'$(IsServicingBuild)' != 'true'"/>

<!--
MSBuild does not provide a way to join on matching identities in a Condition,
Expand Down Expand Up @@ -201,8 +203,13 @@
<_ExplicitPackageReference Remove="@(_ExplicitPackageReference)" />
</ItemGroup>

<Warning Condition="@(UnusedBaselinePackageReference->Count()) != 0"
Text="Package references changed since the last release. This could be a breaking change. References removed:%0A - @(UnusedBaselinePackageReference, '%0A -')" />
<Warning Condition="'$(IsReferenceAssemblyProject)' != 'true' AND '$(IsServicingBuild)' != 'true' AND '%(UnusedBaselinePackageReference.Identity)' != ''"
Code="BUILD001"
Text="Reference to '%(UnusedBaselinePackageReference.Identity)' was removed since the last stable release of this package. This could be a breaking change. See docs/ReferenceResolution.md for instructions on how to update changes to references or suppress this warning if the error was intentional." />

<Error Condition="'$(IsReferenceAssemblyProject)' != 'true' AND '$(IsServicingBuild)' == 'true' AND @(UnusedBaselinePackageReference->Count()) != 0"
Code="BUILD002"
Text="Package references changed since the last release. This could be a breaking change and is not allowed in a servicing update. References removed:%0A - @(UnusedBaselinePackageReference, '%0A -')" />

<Error Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework' AND '%(Reference.Identity)' != '' AND ! Exists('%(Reference.Identity)') AND '$(DisablePackageReferenceRestrictions)' != 'true'"
Code="MSB3245"
Expand Down
5 changes: 5 additions & 0 deletions src/SignalR/clients/csharp/Client.Core/src/Microsoft.AspNetCore.SignalR.Client.Core.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@
<Reference Include="System.Threading.Channels" />
</ItemGroup>

<ItemGroup Condition="'$(AspNetCoreMajorMinorVersion)' == '3.0'">
<!-- This dependency was replaced by Protocols.NewtonsoftJson between 3.0 and 2.2. This suppression can be removed after 3.0 is complete. -->
Copy link
Member

Choose a reason for hiding this comment

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

Should we file an issue to do this after branching for 3.0? I guess it's not a big deal if we don't remove it right away.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, such an issue is likely to get lost between now and RTM if it has to wait in the backlog. But, if you create a 4.0 milestone for it, that'll work.

Copy link
Member

Choose a reason for hiding this comment

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

We don't need to file an issue, this will go away when we make System.Text.Json the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't hurt if it stays around, but feel free to file an issue to clean up later. I'm writing these suppressions with the condition that checks version to makes sure the code doesn't become a problem later when we branch for 3.1 and beyond.

<SuppressBaselineReference Include="Microsoft.AspNetCore.SignalR.Protocols.Json" />
</ItemGroup>

</Project>
5 changes: 5 additions & 0 deletions ...lR/common/Http.Connections.Common/src/Microsoft.AspNetCore.Http.Connections.Common.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,9 @@
<Reference Include="System.Runtime.CompilerServices.Unsafe" />
</ItemGroup>

<ItemGroup Condition="'$(AspNetCoreMajorMinorVersion)' == '3.0'">
<!-- This dependency was replaced by System.Text.Json between 3.0 and 2.2. This suppression can be removed after 3.0 is complete. -->
<SuppressBaselineReference Include="Newtonsoft.Json" />
</ItemGroup>

</Project>
5 changes: 5 additions & 0 deletions src/SignalR/common/Protocols.Json/src/Microsoft.AspNetCore.SignalR.Protocols.Json.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,9 @@
<Reference Include="System.Runtime.CompilerServices.Unsafe" />
</ItemGroup>

<ItemGroup Condition="'$(AspNetCoreMajorMinorVersion)' == '3.0'">
Copy link
Member

Choose a reason for hiding this comment

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

This is technically a new package in 3.0.

We replaced the old Microsoft.AspNetCore.SignalR.Protocols.Json package with the Microsoft.AspNetCore.SignalR.Protocols.NewtonsoftJson package and then removed the old one.

So this is now a new package and probably shouldn't have a baseline associated with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even so, it's the same package ID and the build system (and users) see this as a breaking change. It's okay since this was planned. This documents that intention in the code.

<!-- This dependency was replaced by System.Text.Json between 3.0 and 2.2. This suppression can be removed after 3.0 is complete. -->
<SuppressBaselineReference Include="Newtonsoft.Json" />
</ItemGroup>

</Project>
5 changes: 5 additions & 0 deletions src/SignalR/common/SignalR.Common/src/Microsoft.AspNetCore.SignalR.Common.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,9 @@
<Reference Include="System.Runtime.CompilerServices.Unsafe" />
</ItemGroup>

<ItemGroup Condition="'$(AspNetCoreMajorMinorVersion)' == '3.0'">
<!-- This dependency was replaced by System.Text.Json between 3.0 and 2.2. This suppression can be removed after 3.0 is complete. -->
<SuppressBaselineReference Include="Newtonsoft.Json" />
</ItemGroup>

</Project>
1 change: 1 addition & 0 deletions version.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<PreReleaseBrandingLabel>Preview $(PreReleasePreviewNumber)</PreReleaseBrandingLabel>
<ExperimentalVersionPrefix>0.3.$(AspNetCorePatchVersion)</ExperimentalVersionPrefix>
<BlazorComponentsVersionPrefix>0.9.$(AspNetCorePatchVersion)</BlazorComponentsVersionPrefix>
<AspNetCoreMajorMinorVersion>$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion)</AspNetCoreMajorMinorVersion>
<VersionPrefix>$(AspNetCoreMajorVersion).$(AspNetCoreMinorVersion).$(AspNetCorePatchVersion)</VersionPrefix>

<!-- ANCM versioning is intentionally 10 + AspNetCoreMajorVersion because earlier versions of ANCM shipped as 8.x. -->
Expand Down