DbNull serialization with tests and forward #23897
Conversation
ViktorHofer
added
* NO MERGE *
|
@dotnet-bot test this please (new clr) |
|
presumably because CoreRT changes have not reached here. You could put this behind #if UAP (if you modify the csproj) and you will need to do that anyway to port to 2.0. |
| @@ -20,6 +20,7 @@ | |||
| [assembly:System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Collections.Generic.ShortEnumEqualityComparer<>))] | |||
| [assembly:System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Collections.Generic.LongEnumEqualityComparer<>))] | |||
| [assembly:System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Collections.ListDictionaryInternal))] | |||
| [assembly:System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.UnitySerializationHolder))] | |||
There was a problem hiding this comment.
@weshaggard are you fine with that change? You can find the type here: https://github.com/dotnet/coreclr/blob/master/src/mscorlib/shared/System/UnitySerializationHolder.cs
There was a problem hiding this comment.
And any idea why I didn't need a TypeForwardedFrom attribute on the UnitySerializationHolder? Is it because it sets the type itself with SerializationInfo.SetType(Type)?
There was a problem hiding this comment.
Yes I'm fine with this change as it is along the lines as the other ones here. As for the TypeForwardedFrom question I'm not sure I would expect if you serialize on core it would end up with System.Private.CoreLib as the assembly it is from, is that not what is happening?
There was a problem hiding this comment.
That's what's so interesting about it. It doesn't contain the QualifiedAssemblyName, so it's the optimized version without the attribute:
�����ÿÿÿÿ��������������System.UnitySerializationHolder�����Data UnityType
AssemblyName����
����������
There was a problem hiding this comment.
Interesting I don't know why that is. Your the expert here now :) My only guess is that the binary serializer omits assembly names for the core assembly? Not sure though. It is probably worth debugging though the code to understand further.
There was a problem hiding this comment.
I have a guess but need to verify it. Thanks Wes :)
Which corert version is release/2.0.0 taking again? I couldn't find a branch for it. Is it stored in vsts? I will disable the TypeForward and the UnitySerializationHolder test in the commit against the release branch. I expect the corert mirror changes now to be in (at least I hope so). |
ViktorHofer
force-pushed
the
DbNullSerialization
branch
from
d9161ec to
ecf8116
Compare
| @@ -67,6 +67,14 @@ public void ValidateAgainstBlobs(object obj, string[] blobs) | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void UnitySerializationHolderWithAssemblySingleton() | |||
There was a problem hiding this comment.
I want to move this test to System.Runtime.Tests but I will wait with that until I commited my change into the release/2.0.0 branch because it requires the bigger change #23940 to handle "blobs" outside of S.R.Serialization.Formatters.
ViktorHofer
force-pushed
the
DbNullSerialization
branch
from
ecf8116 to
4d2bd54
Compare
|
@danmosemsft if Wes is ok with change in the mscorlib façade then I believe we can merge it into master. I will then create the release/2.0.0 PRs for both coreclr and corefx. The corefx one with the additional conditional compilation for uap/uapaot. |
ViktorHofer
removed
the
* NO MERGE *
|
Just occurred to me is it worth adding a test for a DataTable containing a DBNull (since that is what it is serializable for) |
* DbNull tests and forward * UnitySerializationHolder deserialization test added Commit migrated from dotnet/corefx@9ab8189
Fixes https://github.com/dotnet/corefx/issues/23398
Waiting for new coreclr build ingestion: dotnet/coreclr#13845