Support Reference Assemblies in SGEN. #24491

huanwu · 2017-10-06T22:18:08Z

Customer can define the reference assemblies as well as the related types that need pregenerated serializer as the followings.

<ItemGroup>
  <SerializationAssembly Include="MyData06">
    <SerializationType>MyData06.Class1;MyData06.Class3</SerializationType>
  </SerializationAssembly>
  <SerializationAssembly Include="MyData07">
    <SerializationType>MyData07.Class5;MyData07.Class6</SerializationType>
  </SerializationAssembly>
</ItemGroup>

Fix #22937

@shmao @mconnew @zhenlan

shmao · 2017-10-07T01:05:42Z

src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets

+        <TargetSerializationAssembly>%(SerializationAssembly.Identity)</TargetSerializationAssembly>
+        <TargetSerializationType>@(SerializationAssembly-&gt;Metadata('SerializationType'))</TargetSerializationType>
+      </TargetSerializationAssemblyAndType>
+    </ItemGroup>


The ItemGroup above can be simplified as,

<ItemGroup Condition="@(SerializationAssembly)!=''" Label="Parse SerializationAssembly"> <SearchSerializationAssembly Include="@(Reference)"> <AssemblyName>%(SerializationAssembly.Identity)</AssemblyName> <SerializationTypes>%(SerializationAssembly.SerializationType)</SerializationTypes> </SearchSerializationAssembly> <TargetSerializationAssembly Include="@(SearchSerializationAssembly)" Condition="$([System.String]::new('%(SearchSerializationAssembly.Identity)').EndsWith('%(SearchSerializationAssembly.AssemblyName).dll'))" /> </ItemGroup>

Accordingly, the <Exec> at line 53 can be simplified as,

<Exec Command="dotnet Microsoft.XmlSerializer.Generator /force /quiet /assembly:%(TargetSerializationAssembly.Identity) /type:%(TargetSerializationAssembly.SerializationTypes) /out:$(IntermediateOutputPath)" ContinueOnError="true" />

shmao · 2017-10-07T01:13:05Z

src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets

+      <ReferenceSerializerName Include="%(SerializationAssembly.Identity).XmlSerializers" />
+      <ReferenceSerializerDllImmediatePath Include="$(IntermediateOutputPath)%(ReferenceSerializerName.Identity).dll" />
+      <ReferenceSerializerPdbImmediatePath Include="$(IntermediateOutputPath)%(ReferenceSerializerName.Identity).pdb" />
+      <ReferenceSerializerCsImmediatePath Include="$(IntermediateOutputPath)%(ReferenceSerializerName.Identity).cs" />


I think we can keep only one ReferenceSerializerImmediatePath. It's not easy to distinguish the above 3 items. Besides, it seems in some case we cannot use them.

shmao · 2017-10-07T01:23:42Z

src/Microsoft.XmlSerializer.Generator/src/Sgen.cs

@@ -79,7 +79,18 @@ private int Run(string[] args)
                    }
                    else if (ArgumentMatch(arg, "type"))
                    {
-                        types.Add(value);
+                        if (value.Contains(";"))


We don't need to special case value.Contains(";") == true. If value is not empty, we can just do value.Split(';').

shmao · 2017-10-07T01:27:19Z

src/Microsoft.XmlSerializer.Generator/src/Sgen.cs

@@ -368,7 +379,16 @@ private void ImportType(Type type, ArrayList mappings, ArrayList importedTypes,
        private static Assembly LoadAssembly(string assemblyName, bool throwOnFail)
        {
            Assembly assembly = null;
-            string path = Path.GetFullPath(assemblyName);
+            string path;


This can be written as,

string path = Path.IsPathRooted(assemblyName) ? assemblyName : Path.GetFullPath(assemblyName);

shmao · 2017-10-07T01:28:40Z

src/System.Private.Xml/src/System/Xml/Serialization/Compilation.cs

@@ -173,6 +173,11 @@ internal static Assembly LoadGeneratedAssembly(Type type, string defaultNamespac
                name.CodeBase = null;
                name.CultureInfo = CultureInfo.InvariantCulture;
                string serializerPath = Path.Combine(Path.GetDirectoryName(type.Assembly.Location), serializerName + ".dll");
+                if(!File.Exists(serializerPath))


Nit: space after if

huanwu · 2017-10-09T22:24:31Z

@dotnet-bot test OSX x64 Debug Build

shmao · 2017-10-09T22:27:13Z

src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets

  <Target Name="CopySerializer" AfterTargets="PrepareForPublish">
    <Copy Condition="Exists('$(OutputPath)\$(AssemblyName).XmlSerializers.dll')=='true'" SourceFiles="$(OutputPath)\$(AssemblyName).XmlSerializers.dll" DestinationFolder="$(PublishDir)" SkipUnchangedFiles="false" />
  </Target>
+
+ <Target Name="GenerateSerializationAssemblyForReferenceAssembly" AfterTargets="GenerateSerializationAssembly" Condition="@(SerializationAssembly)!=''">


Can you please correct the indent for the changes?

shmao · 2017-10-09T22:29:17Z

src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets

+    <Csc Condition="Exists('$(IntermediateOutputPath)%(ReferenceSerializerName.Identity).cs') == 'true'" ContinueOnError="true" OutputAssembly="$(IntermediateOutputPath)%(ReferenceSerializerName.Identity).dll" References="@(ReferencePath);@(IntermediateAssembly)" EmitDebugInformation="$(DebugSymbols)" Sources="$(IntermediateOutputPath)%(ReferenceSerializerName.Identity).cs" TargetType="Library" ToolExe="$(CscToolExe)" ToolPath="$(CscToolPath)" />
+    <Warning Condition="Exists('$(IntermediateOutputPath)%(ReferenceSerializerName.Identity).dll') != 'true' And Exists('$(IntermediateOutputPath)%(ReferenceSerializerName.Identity).cs') == 'true'" Text="SGEN : warning SGEN1: Fail to compile %(ReferenceSerializerName.Identity).cs. Please follow the instructions in https://go.microsoft.com/fwlink/?linkid=858594 and try again." />
+    <Copy Condition="Exists('%(ReferenceSerializeImmediatePath.Identity).dll') == 'true'" SourceFiles="%(ReferenceSerializeImmediatePath.Identity).dll" DestinationFolder="$(OutputPath)" />
+  </Target>


Do we need to add a target, like the target CopySerializer above, for publishing the generated assemblies?

Add CopySerializerForReferenceAssemblies

shmao · 2017-10-09T22:36:36Z

src/Microsoft.XmlSerializer.Generator/src/Sgen.cs

@@ -79,7 +79,14 @@ private int Run(string[] args)
                    }
                    else if (ArgumentMatch(arg, "type"))
                    {
-                        types.Add(value);
+                        if(value != string.Empty)


Add space after if

shmao · 2017-10-09T22:37:22Z

src/Microsoft.XmlSerializer.Generator/src/Sgen.cs

-                        types.Add(value);
+                        if(value != string.Empty)
+                        {
+                            var typelist = value.Split(';');


'var' -> string[]

mconnew · 2017-10-10T18:14:09Z

src/Microsoft.XmlSerializer.Generator/src/Sgen.cs

-                        types.Add(value);
+                        if(value != string.Empty)
+                        {
+                            string[] typelist = value.Split(';');


';' [](start = 60, length = 3)

Is making sgen handle the semi-colon the way we wish to handle this as this is different from sg? If it is, that's fine but if we want to use a comma in the tool, then we can do the change in msbuild.

Yes, it is different from sg desktop version, which is using /type: type1 /type:type2

mconnew · 2017-10-25T20:26:25Z

<SerializerDllImmediatePath>$(IntermediateOutputPath)$(SerializerName).dll</SerializerDllImmediatePath>

Where you say ImmediatePath do you mean IntermediatePath?

Refers to: src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets:4 in c37e15f. [](commit_id = c37e15f, deletion_comment = False)

mconnew · 2017-10-25T20:26:49Z

<SGenWarningText>SGEN : warning SGEN1: Fail to generate the serializer for $(AssemblyName).dll. Please follow the instructions in https://go.microsoft.com/fwlink/?linkid=858594 and try again.</SGenWarningText>

at

Refers to: src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets:7 in c37e15f. [](commit_id = c37e15f, deletion_comment = False)

mconnew · 2017-10-25T20:30:03Z

src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets

  <Target Name="CleanSerializationAssembly" AfterTargets="CoreClean">
    <Message Text="Cleaning serialization files..." Importance="normal"/>
    <Delete Condition="Exists('$(OutputPath)\$(SerializerName).dll') == 'true'" Files="$(OutputPath)\$(SerializerName).dll" />
  </Target>
+
  <Target Name="CopySerializer" AfterTargets="PrepareForPublish">


CopySerializer [](start = 16, length = 14)

To keep consistent naming with the other targets, this should be CopySerializationAssembly

mconnew · 2017-10-25T20:32:12Z

<SerializerDllImmediatePath>$(IntermediateOutputPath)$(SerializerName).dll</SerializerDllImmediatePath>

Any properties that an end user would not manipulate/specify in their csproj should be prefixed with _

Refers to: src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets:4 in c37e15f. [](commit_id = c37e15f, deletion_comment = False)

mconnew · 2017-10-25T20:33:23Z

src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets

+        <SerializationTypes>%(SerializationAssembly.SerializationType)</SerializationTypes>
+      </SearchSerializationAssembly>
+      <TargetSerializationAssembly Include="@(SearchSerializationAssembly)" Condition="$([System.String]::new('%(SearchSerializationAssembly.Identity)').EndsWith('%(SearchSerializationAssembly.AssemblyName).dll'))" />
+      <ReferenceSerializerName Include="%(SerializationAssembly.Identity).XmlSerializers" />


ReferenceSerializerName [](start = 7, length = 23)

Inconsistent naming. Should this be ReferenceSerializationAssemblyName?

mconnew · 2017-10-25T20:35:32Z

<Delete Condition="Exists('$(SerializerPdbImmediatePath)') == 'true'" Files="$(SerializerPdbImmediatePath)" />

If deleting the dll fails, will the build continue of fail? We should fail otherwise later checks with conditions might not get an error and act like it was successful. This can happen if the application is still running.

Refers to: src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets:11 in c37e15f. [](commit_id = c37e15f, deletion_comment = False)

huanwu · 2017-10-31T18:15:15Z

@dotnet-bot test OSX x64 Debug Build

huanwu · 2017-10-31T18:15:31Z

@dotnet-bot test Windows x64 Debug Build

huanwu · 2017-10-31T18:15:45Z

@dotnet-bot test Windows x86 Release Build

mconnew · 2017-10-31T18:23:50Z

src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets

-    <SGenWarningText>SGEN : warning SGEN1: Fail to generate the serializer for $(AssemblyName).dll. Please follow the instructions in https://go.microsoft.com/fwlink/?linkid=858594 and try again.</SGenWarningText>
+    <_SerializerName>$(AssemblyName).XmlSerializers</_SerializerName>
+    <_SerializerDllIntermediatePath>$(IntermediateOutputPath)$(_SerializerName).dll</_SerializerDllIntermediatePath>
+    <_SerializerPdbIntermediatePath>$(IntermediateOutputPath)$(_SerializerName).pdb</_SerializerPdbIntermediatePath>


For me, Path = folder, Filename = file

mconnew · 2017-10-31T18:24:49Z

src/Microsoft.XmlSerializer.Generator/pkg/build/Microsoft.XmlSerializer.Generator.targets

-    <SerializerCsImmediatePath>$(IntermediateOutputPath)$(SerializerName).cs</SerializerCsImmediatePath>
-    <SGenWarningText>SGEN : warning SGEN1: Fail to generate the serializer for $(AssemblyName).dll. Please follow the instructions in https://go.microsoft.com/fwlink/?linkid=858594 and try again.</SGenWarningText>
+    <_SerializerName>$(AssemblyName).XmlSerializers</_SerializerName>
+    <_SerializerDllIntermediatePath>$(IntermediateOutputPath)$(_SerializerName).dll</_SerializerDllIntermediatePath>


SerializationAssemblyName?

huanwu · 2017-11-02T17:19:57Z

@dotnet-bot test OSX x64 Debug Build

mconnew

huanwu · 2017-11-06T21:45:16Z

@dotnet-bot test OSX x64 Debug Build

* Support Reference Assemblies in SGEN. * Several fixes based on Shin's feedback. * minor change. * Publish serializer for reference assemblies. * Add publish * correct the assembly name to be copied * small format and naming change. * Add a space. * Rename some variables. * Rename some variables.

* Support Reference Assemblies in SGEN. * Several fixes based on Shin's feedback. * minor change. * Publish serializer for reference assemblies. * Add publish * correct the assembly name to be copied * small format and naming change. * Add a space. * Rename some variables. * Rename some variables. Commit migrated from dotnet/corefx@f7ed59d

huanwu added area-Serialization enhancement Product code improvement that does NOT require public API changes/additions labels Oct 6, 2017

huanwu self-assigned this Oct 6, 2017

dnfclas added the cla-already-signed label Oct 6, 2017

shmao suggested changes Oct 7, 2017

View reviewed changes

shmao reviewed Oct 9, 2017

View reviewed changes

mconnew reviewed Oct 10, 2017

View reviewed changes

karelz removed the enhancement Product code improvement that does NOT require public API changes/additions label Oct 11, 2017

mconnew reviewed Oct 25, 2017

View reviewed changes

karelz added this to the 2.1.0 milestone Oct 28, 2017

huanwu added 9 commits October 30, 2017 13:26

Support Reference Assemblies in SGEN.

4a0e8ec

Several fixes based on Shin's feedback.

bc77d87

minor change.

75cbc52

Publish serializer for reference assemblies.

72a8ec0

Add publish

ea4bd4d

correct the assembly name to be copied

6ae6b47

small format and naming change.

8113cb7

Add a space.

08f19fc

Rename some variables.

2d913b5

huanwu force-pushed the SupportReferenceAssemblyInSgen branch from c37e15f to 2d913b5 Compare October 30, 2017 20:26

mconnew reviewed Oct 31, 2017

View reviewed changes

Rename some variables.

d32eda3

shmao approved these changes Nov 1, 2017

View reviewed changes

mconnew approved these changes Nov 6, 2017

View reviewed changes

huanwu merged commit f7ed59d into dotnet:master Nov 6, 2017

Lxiamail modified the milestones: 2.1.0, 3.0, 2.1.x Nov 14, 2018

This was referenced Jan 31, 2020

Sgen cannot find the class in project referenced project dotnet/runtime#26878

Closed

Support /t in the project settings for sgen dotnet/runtime#27326

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Reference Assemblies in SGEN. #24491

Support Reference Assemblies in SGEN. #24491

huanwu commented Oct 6, 2017 •

edited

Loading

shmao Oct 7, 2017

shmao Oct 7, 2017

shmao Oct 7, 2017

shmao Oct 7, 2017

shmao Oct 7, 2017

huanwu commented Oct 9, 2017

shmao Oct 9, 2017

shmao Oct 9, 2017

huanwu Oct 25, 2017

shmao Oct 9, 2017

shmao Oct 9, 2017

mconnew Oct 10, 2017

huanwu Oct 10, 2017

mconnew commented Oct 25, 2017

mconnew commented Oct 25, 2017

mconnew Oct 25, 2017

mconnew commented Oct 25, 2017

mconnew Oct 25, 2017

mconnew commented Oct 25, 2017

huanwu commented Oct 31, 2017

huanwu commented Oct 31, 2017

huanwu commented Oct 31, 2017

mconnew Oct 31, 2017

mconnew Oct 31, 2017

huanwu commented Nov 2, 2017

mconnew left a comment

huanwu commented Nov 6, 2017

Support Reference Assemblies in SGEN. #24491

Support Reference Assemblies in SGEN. #24491

Conversation

huanwu commented Oct 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huanwu commented Oct 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mconnew commented Oct 25, 2017

mconnew commented Oct 25, 2017

Choose a reason for hiding this comment

mconnew commented Oct 25, 2017

Choose a reason for hiding this comment

mconnew commented Oct 25, 2017

huanwu commented Oct 31, 2017

huanwu commented Oct 31, 2017

huanwu commented Oct 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huanwu commented Nov 2, 2017

mconnew left a comment

Choose a reason for hiding this comment

huanwu commented Nov 6, 2017

huanwu commented Oct 6, 2017 •

edited

Loading