Skip to content
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.

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

XmlSerializer.GenerateSerializer should not be in System.Private.Xml #1413

Open
stephentoub opened this issue Sep 19, 2019 · 6 comments
Open
Assignees
Milestone

Comments

@stephentoub
Copy link
Member

System.Private.Xml contains all of the code for generating an assembly as the core logic behind the sgen tool:
https://github.com/dotnet/corefx/tree/master/src/Microsoft.XmlSerializer.Generator
The vast majority of the functionality here isn't used by anything other than sgen, which calls XmlSerializer.GenerateSerializer via reflection, and is the only caller of that method. This functionality should be removed from XmlSerializer in System.Private.Xml and moved into sgen.

@tamlin-mike
Copy link

tamlin-mike commented Nov 6, 2019

I maintain code that uses System.Xml.Serialization.XmlSerializer.GenerateSerializer (directly, not using reflection) for a case sgen is unable to handle.

Could completely ripping this function out break f.ex. the Microsoft.XmlSerializer.Generator nuget package?

@StephenBonikowsky StephenBonikowsky transferred this issue from dotnet/corefx Jan 7, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Serialization untriaged New issue has not been triaged by the area owner labels Jan 7, 2020
@StephenBonikowsky StephenBonikowsky added this to the Future milestone Jan 7, 2020
@StephenBonikowsky StephenBonikowsky added assembly-size and removed untriaged New issue has not been triaged by the area owner labels Jan 7, 2020
@StephenBonikowsky StephenBonikowsky added this to Future P2 (Backlog) in WCF Owned Areas Jan 7, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@stephentoub stephentoub added linkable-framework Issues associated with delivering a linker friendly framework and removed untriaged New issue has not been triaged by the area owner assembly-size labels Feb 25, 2020
@StephenBonikowsky
Copy link
Member

@mconnew Please comment on this issue and resolve as appropriate.

@eerhardt
Copy link
Member

I maintain code that uses System.Xml.Serialization.XmlSerializer.GenerateSerializer (directly, not using reflection)

@tamlin-mike - can you explain this more? Are you only doing that on the .NET Framework? This method is internal in .NET Core.

internal static bool GenerateSerializer(Type[] types, XmlMapping[] mappings, Stream stream)

@tamlin-mike
Copy link

This method is internal in .NET Core.

Yeah, that turned out to be the problem. It was public in Framework. Unfortunately the code started to depend on that functionality. Then the need arose to use some other assembly compiled to netstandard 2.0 IIRC, and much gnashing of teeth ensued. Ended up throwing the MS tool out and writing our own serialization generator (-frontend) to get the required cross-platform compatibility we needed (fw + netstd). Unfortunately that now depends on that older specific version of the mentioned nuget package -- that will likely be removed sooner or later, or become incompatible with later versions of some runtime -- so probability is high we'll either be forced to rewrite it all from scratch or simply burn that use of XML at the stake. To quote Alien "Nuke it from space. Only way to be sure".

@eerhardt
Copy link
Member

When I spoke with @mconnew it seemed that this issue would take a lot of work, and is pretty low on the priority list.

With #35547, we have addressed the size concern with having to root XmlSerializer.GenerateSerializer. I'm going to remove the linkable-framework label, as this doesn't issue no longer needs to be tracked in the linkablity work.

@eerhardt eerhardt removed the linkable-framework Issues associated with delivering a linker friendly framework label May 20, 2020
@HongGit HongGit added this to the Future milestone Jul 1, 2020
@StephenMolloy StephenMolloy moved this from Future (Backlog) to SGen in WCF Owned Areas Jul 27, 2021
MichalStrehovsky pushed a commit to MichalStrehovsky/runtime that referenced this issue Dec 9, 2021
Also for ARM use D0-D31 registers for encoding S0-S31
@HongGit HongGit modified the milestones: Future, 8.0.0 Jul 7, 2022
@KalleOlaviNiemitalo
Copy link

#56589 seems to be a duplicate of this issue.

Microsoft.XmlSerializer.Generator has "rollForward": "LatestMajor", which makes it use the XmlSerializer.GenerateSerializer method of the highest installed version of .NET Runtime (PR #40216, causes #90913). If the XmlSerializer.GenerateSerializer method were deleted from a future version of .NET Runtime, then installing that would indeed break the build of projects that use an older version of Microsoft.XmlSerializer.Generator, even if those projects also use an older version of .NET SDK.

I think the C# code generation should be moved into the Microsoft.XmlSerializer.Generator package. The XmlSerializer.GenerateSerializer method would then be called only by older versions of Microsoft.XmlSerializer.Generator, and its implementation should be reverted to a maximally compatible version, perhaps to how it was in .NET Core 3.1.0. Each project that wants Microsoft.XmlSerializer.Generator to generate code that uses newer features of C# or .NET Runtime, such as Span<T>, would have to upgrade Microsoft.XmlSerializer.Generator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

9 participants