[release/6.0] Fix XXHash for stripe size #61923
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Tagging subscribers to this area: @dotnet/area-system-io |
|
@ericstj I feel like I heard that release/6.0 has some automatic detection for OOB servicing. Am I recalling correctly, or do we need to do something explicit still? |
|
Here's a document: https://github.com/dotnet/runtime/blob/4684a1dbab50ea759eaf970302fc9c1f7007bbc2/docs/project/library-servicing.md TL;DR To System.IO.Hashing.csproj add <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<ServicingVersion>1</ServicingVersion>cc @safern |
|
OK, so "more distributed" but not quite "no brainer". Danke for the pointer. |
| @@ -4,6 +4,8 @@ | |||
| <Nullable>enable</Nullable> | |||
| <TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;net461</TargetFrameworks> | |||
| <IsPackable>true</IsPackable> | |||
| <GeneratePackageOnBuild>true</GeneratePackageOnBuild> | |||
| <ServicingVersion>1</ServicingVersion> | |||
| <!-- TODO: Remove when the package ships with .NET 6. --> | |||
| <DisablePackageBaselineValidation>true</DisablePackageBaselineValidation> | |||
There was a problem hiding this comment.
You can remove this while you are here (and in main). That will enable compat checking with the 6.0.0 package.
There was a problem hiding this comment.
Looks like something still wants it to check compat with 5.0.0:
src/libraries/System.IO.Hashing/src/System.IO.Hashing.csproj : error NU1102: Unable to find package System.IO.Hashing with version (= 5.0.0)
There was a problem hiding this comment.
In order for it to look for the 6.0.0 package on the main branch you'd need this change: #61437
There was a problem hiding this comment.
Jinx 😁 Maybe the way to handle this is to instead remove this property from System.IO.Hashing in #61437 and then bring that whole change to servicing?
There was a problem hiding this comment.
Main was refactored so we would have to do something a little different on servicing, but yeah, I can do something on release/6.0 so that it runs package validation against the 6.0.0 version of the package:
Line 13 in e9036b0
There was a problem hiding this comment.
I included the change directly on this PR
safern
added
the
NO-MERGE
|
Added no merge as the branch is currently closed. |
Pilchie
added
Servicing-approved
safern
removed
the
NO-MERGE
|
@ericstj is this good to merge? |
|
Yes this should be good to merge now. |
ghost
locked as resolved and limited conversation to collaborators
Backport of #61881 to release/6.0
/cc @bartonjs @Tornhoof
Customer Impact
Customers who use the new XxHash32 or XxHash64 types and pass a multiple of 16 (or 32 for XxHash64) to the static hash methods will get a different answer than if they used the instance methods. The answer from the static methods doesn't interoperate with other implementations of XxHash32/XxHash64.
Testing
New unit tests that pass in data that is an even multiple of the stripe size.
Risk
Low, it's a simple fix on a new type (6.0) and adds a regression-prevention test. Delaying the change increases the risk as we will have more compatibility concerns.