Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
using System;
using System.Collections.Generic;

namespace Microsoft.XmlSerializer.Generator
{
/// <summary>
/// MSBuild task to run the Microsoft.XmlSerializer.Generator directly,
/// without spawning a new process.
/// </summary>
public class GenerateXmlSerializers : Task
{
/// <summary>
/// Path to the target assembly (usually the output DLL).
/// </summary>
[Required]
public string AssemblyPath { get; set; }

/// <summary>
/// Optional path to a response file containing additional generator arguments.
/// </summary>
public string RspFilePath { get; set; }

/// <summary>
/// Force regeneration of serializers even if they exist.
/// </summary>
public bool Force { get; set; }

/// <summary>
/// Run in quiet mode with minimal output.
/// </summary>
public bool Quiet { get; set; }

/// <summary>
/// If true, errors during serializer generation will be suppressed.
/// </summary>
public bool IgnoreErrors { get; set; }

/// <summary>
/// Executes the XmlSerializer generator.
/// </summary>
public override bool Execute()
{
var args = new List<string>();

if (!string.IsNullOrEmpty(AssemblyPath))
args.Add(AssemblyPath);

if (Force)
args.Add("--force");

if (Quiet)
args.Add("--quiet");
Comment on lines +53 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

These two are always true and we should update Sgen to remove all code that would run if they were false.


if (!string.IsNullOrEmpty(RspFilePath))
args.Add(RspFilePath);

Sgen sgen = new Sgen();
Sgen.InfoWriter = new InfoTextWriter(Log);
Sgen.WarningWriter = new WarningTextWriter(Log);
Sgen.ErrorWriter = new ErrorTextWriter(Log);

int exitCode = sgen.Run(args.ToArray());
Comment on lines +62 to +67
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that Sgen is no longer a CLI tool, you should add properties to the MSBuild task and to the Sgen class, and not construct command-line arguments.


if (exitCode != 0)
{
Log.LogError("XmlSerializer failed with exit code {0}.", exitCode);
}

return IgnoreErrors || exitCode == 0;
}
}
}
95 changes: 95 additions & 0 deletions src/libraries/Microsoft.XmlSerializer.Generator/src/LogWriter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.IO;
using System.Text;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;

namespace Microsoft.XmlSerializer.Generator
{
internal sealed class InfoTextWriter : TextWriter
Copy link
Contributor

Choose a reason for hiding this comment

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

You can combine the three TextWriters with a single one like this:

    internal sealed class DelegatedTextWriter(Action<string> fWrite) : TextWriter
    {
        public override Encoding Encoding => Encoding.UTF8;

        public override void Write(string? value)
        {
            if (!string.IsNullOrEmpty(value))
            {
                fWrite(value)
            }
        }

        public override void WriteLine(string? value) => Write(value);
    }

And then create them in the task like Sgen.InfoWriter = new DelegatedTextWriter(Log.LogError);.

{
private readonly TaskLoggingHelper _log;

public InfoTextWriter(TaskLoggingHelper log)
{
_log = log ?? throw new ArgumentNullException(nameof(log));
}

public override Encoding Encoding => Encoding.UTF8;

public override void Write(string? value)
{
if (!string.IsNullOrEmpty(value))
{
_log.LogMessage(MessageImportance.High, value);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should decide if the messages should have high importance, or which of them should.

}
}

public override void WriteLine(string? value)
{
if (!string.IsNullOrEmpty(value))
{
_log.LogMessage(MessageImportance.High, value);
}
}
}

internal sealed class WarningTextWriter : TextWriter
{
private readonly TaskLoggingHelper _log;

public WarningTextWriter(TaskLoggingHelper log)
{
_log = log ?? throw new ArgumentNullException(nameof(log));
}

public override Encoding Encoding => Encoding.UTF8;

public override void Write(string? value)
{
if (!string.IsNullOrEmpty(value))
{
_log.LogWarning(value);
}
}

public override void WriteLine(string? value)
{
if (!string.IsNullOrEmpty(value))
{
_log.LogWarning(value);
}
}
}

internal sealed class ErrorTextWriter : TextWriter
{
private readonly TaskLoggingHelper _log;

public ErrorTextWriter(TaskLoggingHelper log)
{
_log = log ?? throw new ArgumentNullException(nameof(log));
}

public override Encoding Encoding => Encoding.UTF8;

public override void Write(string? value)
{
if (!string.IsNullOrEmpty(value))
{
_log.LogError(value);
}
}

public override void WriteLine(string? value)
{
if (!string.IsNullOrEmpty(value))
{
_log.LogError(value);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
Expand All @@ -22,22 +22,24 @@
</PropertyGroup>

<ItemGroup>
<Compile Include="GenerateXmlSerializers.cs" />
<Compile Include="LogWriter.cs" />
<Compile Include="Sgen.cs" />
</ItemGroup>

<ItemGroup>
<!-- Set an empty PackagePath to place this file in the root of the package -->
<None Include="build\prefercliruntime"
PackagePath=""
Pack="true" />
<None Include="build\Microsoft.XmlSerializer.Generator.targets"
PackagePath="build"
Pack="true" />
<None Include="build\dotnet-Microsoft.XmlSerializer.Generator.runtimeconfig.json"
PackagePath="lib\netstandard2.0"
Pack="true" />
<None Include="build\prefercliruntime" PackagePath="" Pack="true" />
<None Include="build\Microsoft.XmlSerializer.Generator.targets" PackagePath="build" Pack="true" />
<None Include="build\dotnet-Microsoft.XmlSerializer.Generator.runtimeconfig.json" PackagePath="lib\netstandard2.0" Pack="true" />
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make sure a .deps.json file is included in the package. You can see this guide for the best practices when creating MSBuild task packages.

Copy link
Member

Choose a reason for hiding this comment

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

We've had to abandon this approach because the Task was being run in msbuild running on .NET Framework when building in Visual Studio. We can't do that because we depend on implementation that lives in the .NET runtime to do some of the work. Would including a .deps.json file allow us to run the Task using .NET when building on Visual Studio?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, that's not yet supported and is tracked in dotnet/msbuild#4834.

</ItemGroup>

<ItemGroup>
<PackageDownloadAndReference Include="Microsoft.Build.Framework" Version="$(MicrosoftBuildFrameworkVersion)" Folder="ref/netstandard2.0" />
<PackageDownloadAndReference Include="Microsoft.Build.Utilities.Core" Version="$(MicrosoftBuildUtilitiesCoreVersion)" Folder="ref/netstandard2.0" />
</ItemGroup>

<Import Project="$(RepositoryEngineeringDir)PackageDownloadAndReference.targets" />
<Import Project="GenerateThisAssemblyCs.targets" />
<Import Project="GenerateNupkgProps.targets" />

Expand Down
60 changes: 31 additions & 29 deletions src/libraries/Microsoft.XmlSerializer.Generator/src/Sgen.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ public static int Main(string[] args)

private static string s_references = string.Empty;
private static readonly Dictionary<string, string> s_referencedic = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
public static TextWriter InfoWriter { get; internal set; } = Console.Out;
public static TextWriter WarningWriter { get; internal set; } = Console.Out;
public static TextWriter ErrorWriter { get; internal set; } = Console.Error;
Comment on lines +28 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be instance fields? Also, an Action<string> looks like a better abstraction. Even better, you can use a TaskLoggingHelper here.


private int Run(string[] args)
internal int Run(string[] args)
{
string assembly = null;
var types = new List<string>();
Expand Down Expand Up @@ -177,15 +180,15 @@ private int Run(string[] args)
{
foreach (string err in errs)
{
Console.Error.WriteLine(FormatMessage(parsableErrors, true, SR.Format(SR.Warning, err)));
ErrorWriter.WriteLine(FormatMessage(parsableErrors, true, SR.Format(SR.Warning, err)));
}
}

if (args.Length == 0 || assembly == null)
{
if (assembly == null)
{
Console.Error.WriteLine(FormatMessage(parsableErrors, false, SR.Format(SR.ErrMissingRequiredArgument, SR.Format(SR.ErrAssembly, "assembly"))));
ErrorWriter.WriteLine(FormatMessage(parsableErrors, false, SR.Format(SR.ErrMissingRequiredArgument, SR.Format(SR.ErrAssembly, "assembly"))));
}

WriteHelp();
Expand All @@ -194,8 +197,8 @@ private int Run(string[] args)

if (disableRun)
{
Console.WriteLine("This tool is not intended to be used directly.");
Console.WriteLine("Please refer to https://go.microsoft.com/fwlink/?linkid=858594 on how to use it.");
InfoWriter.WriteLine("This tool is not intended to be used directly.");
InfoWriter.WriteLine("Please refer to https://go.microsoft.com/fwlink/?linkid=858594 on how to use it.");
return 0;
}

Expand Down Expand Up @@ -254,7 +257,7 @@ private static void GenerateFile(List<string> typeNames, string defaultNamespace
Type type = assembly.GetType(typeName);
if (type == null)
{
Console.Error.WriteLine(FormatMessage(parsableerrors, false, SR.Format(SR.ErrorDetails, SR.Format(SR.ErrLoadType, typeName, assemblyName))));
ErrorWriter.WriteLine(FormatMessage(parsableerrors, false, SR.Format(SR.ErrorDetails, SR.Format(SR.ErrLoadType, typeName, assemblyName))));
}

types[typeIndex++] = type;
Expand All @@ -275,7 +278,7 @@ private static void GenerateFile(List<string> typeNames, string defaultNamespace
{
if (verbose)
{
Console.WriteLine(SR.Format(SR.ImportInfo, type.Name, i + 1, types.Length));
InfoWriter.WriteLine(SR.Format(SR.ImportInfo, type.Name, i + 1, types.Length));
}

bool isObsolete = false;
Expand All @@ -300,7 +303,7 @@ private static void GenerateFile(List<string> typeNames, string defaultNamespace
{
if (verbose)
{
Console.Out.WriteLine(FormatMessage(parsableerrors, true, SR.Format(SR.InfoIgnoreType, type.FullName)));
InfoWriter.WriteLine(FormatMessage(parsableerrors, true, SR.Format(SR.InfoIgnoreType, type.FullName)));
WriteWarning(e, parsableerrors);
}

Expand Down Expand Up @@ -372,7 +375,7 @@ private static void GenerateFile(List<string> typeNames, string defaultNamespace

if (method == null)
{
Console.Error.WriteLine(FormatMessage(parsableerrors: false, warning: false, message: SR.GenerateSerializerNotFound));
ErrorWriter.WriteLine(FormatMessage(parsableerrors: false, warning: false, message: SR.GenerateSerializerNotFound));
}
else
{
Expand Down Expand Up @@ -404,22 +407,21 @@ private static void GenerateFile(List<string> typeNames, string defaultNamespace
{
if (!silent)
{
Console.Out.WriteLine(SR.Format(SR.InfoFileName, codePath));
Console.Out.WriteLine(SR.Format(SR.InfoGeneratedFile, assembly.Location, codePath));
InfoWriter.WriteLine(SR.Format(SR.InfoFileName, codePath));
InfoWriter.WriteLine(SR.Format(SR.InfoGeneratedFile, assembly.Location, codePath));
}
}
else
{
Console.Out.WriteLine(FormatMessage(parsableerrors, false, SR.Format(SR.ErrGenerationFailed, assembly.Location)));
InfoWriter.WriteLine(FormatMessage(parsableerrors, false, SR.Format(SR.ErrGenerationFailed, assembly.Location)));
}
}
else
{
Console.Out.WriteLine(FormatMessage(parsableerrors, true, SR.Format(SR.InfoNoSerializableTypes, assembly.Location)));
InfoWriter.WriteLine(FormatMessage(parsableerrors, true, SR.Format(SR.InfoNoSerializableTypes, assembly.Location)));
}
}


private static bool ArgumentMatch(string arg, string formal)
{
// Full name format, eg: --assembly
Expand Down Expand Up @@ -459,7 +461,7 @@ private static void ImportType(Type type, string defaultNamespace, List<XmlMappi

if (verbose)
{
Console.Out.WriteLine(FormatMessage(parsableerrors, true, SR.Format(SR.InfoIgnoreType, type.FullName)));
InfoWriter.WriteLine(FormatMessage(parsableerrors, true, SR.Format(SR.InfoIgnoreType, type.FullName)));
WriteWarning(e, parsableerrors);
}

Expand Down Expand Up @@ -489,22 +491,22 @@ private static Assembly LoadAssembly(string assemblyName)
private static void WriteHeader()
{
// do not localize Copyright header
Console.WriteLine($".NET Xml Serialization Generation Utility, Version {ThisAssembly.InformationalVersion}]");
InfoWriter.WriteLine($".NET Xml Serialization Generation Utility, Version {ThisAssembly.InformationalVersion}]");
}

private void WriteHelp()
{
Console.Out.WriteLine(SR.HelpDescription);
Console.Out.WriteLine(SR.Format(SR.HelpUsage, this.GetType().Assembly.GetName().Name.Substring("dotnet-".Length)));
Console.Out.WriteLine(SR.HelpDevOptions);
Console.Out.WriteLine(SR.Format(SR.HelpAssembly, "-a", "--assembly"));
Console.Out.WriteLine(SR.Format(SR.HelpType, "--type"));
Console.Out.WriteLine(SR.Format(SR.HelpProxy, "--proxytypes"));
Console.Out.WriteLine(SR.Format(SR.HelpForce, "--force"));
Console.Out.WriteLine(SR.Format(SR.HelpOut, "-o", "--out"));

Console.Out.WriteLine(SR.HelpMiscOptions);
Console.Out.WriteLine(SR.Format(SR.HelpHelp, "-h", "--help"));
InfoWriter.WriteLine(SR.HelpDescription);
InfoWriter.WriteLine(SR.Format(SR.HelpUsage, this.GetType().Assembly.GetName().Name.Substring("dotnet-".Length)));
InfoWriter.WriteLine(SR.HelpDevOptions);
InfoWriter.WriteLine(SR.Format(SR.HelpAssembly, "-a", "--assembly"));
InfoWriter.WriteLine(SR.Format(SR.HelpType, "--type"));
InfoWriter.WriteLine(SR.Format(SR.HelpProxy, "--proxytypes"));
InfoWriter.WriteLine(SR.Format(SR.HelpForce, "--force"));
InfoWriter.WriteLine(SR.Format(SR.HelpOut, "-o", "--out"));

InfoWriter.WriteLine(SR.HelpMiscOptions);
InfoWriter.WriteLine(SR.Format(SR.HelpHelp, "-h", "--help"));
}

Comment on lines 491 to 511
Copy link
Contributor

Choose a reason for hiding this comment

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

These are not required, now that Sgen is no longer a CLI tool.

Suggested change
private static void WriteHeader()
{
// do not localize Copyright header
Console.WriteLine($".NET Xml Serialization Generation Utility, Version {ThisAssembly.InformationalVersion}]");
InfoWriter.WriteLine($".NET Xml Serialization Generation Utility, Version {ThisAssembly.InformationalVersion}]");
}
private void WriteHelp()
{
Console.Out.WriteLine(SR.HelpDescription);
Console.Out.WriteLine(SR.Format(SR.HelpUsage, this.GetType().Assembly.GetName().Name.Substring("dotnet-".Length)));
Console.Out.WriteLine(SR.HelpDevOptions);
Console.Out.WriteLine(SR.Format(SR.HelpAssembly, "-a", "--assembly"));
Console.Out.WriteLine(SR.Format(SR.HelpType, "--type"));
Console.Out.WriteLine(SR.Format(SR.HelpProxy, "--proxytypes"));
Console.Out.WriteLine(SR.Format(SR.HelpForce, "--force"));
Console.Out.WriteLine(SR.Format(SR.HelpOut, "-o", "--out"));
Console.Out.WriteLine(SR.HelpMiscOptions);
Console.Out.WriteLine(SR.Format(SR.HelpHelp, "-h", "--help"));
InfoWriter.WriteLine(SR.HelpDescription);
InfoWriter.WriteLine(SR.Format(SR.HelpUsage, this.GetType().Assembly.GetName().Name.Substring("dotnet-".Length)));
InfoWriter.WriteLine(SR.HelpDevOptions);
InfoWriter.WriteLine(SR.Format(SR.HelpAssembly, "-a", "--assembly"));
InfoWriter.WriteLine(SR.Format(SR.HelpType, "--type"));
InfoWriter.WriteLine(SR.Format(SR.HelpProxy, "--proxytypes"));
InfoWriter.WriteLine(SR.Format(SR.HelpForce, "--force"));
InfoWriter.WriteLine(SR.Format(SR.HelpOut, "-o", "--out"));
InfoWriter.WriteLine(SR.HelpMiscOptions);
InfoWriter.WriteLine(SR.Format(SR.HelpHelp, "-h", "--help"));
}

private static string FormatMessage(bool parsableerrors, bool warning, string message)
Expand All @@ -524,7 +526,7 @@ private static string FormatMessage(bool parsableerrors, bool warning, string co

private static void WriteError(Exception e, bool parsableerrors)
{
Console.Error.WriteLine(FormatMessage(parsableerrors, false, e.Message));
ErrorWriter.WriteLine(FormatMessage(parsableerrors, false, e.Message));
if (e.InnerException != null)
{
WriteError(e.InnerException, parsableerrors);
Expand All @@ -533,7 +535,7 @@ private static void WriteError(Exception e, bool parsableerrors)

private static void WriteWarning(Exception e, bool parsableerrors)
{
Console.Out.WriteLine(FormatMessage(parsableerrors, true, e.Message));
WarningWriter.WriteLine(FormatMessage(parsableerrors, true, e.Message));
if (e.InnerException != null)
{
WriteWarning(e.InnerException, parsableerrors);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
<Delete Condition="Exists('$(_SerializerPdbIntermediateFolder)') == 'true'" Files="$(_SerializerPdbIntermediateFolder)" ContinueOnError="true"/>
<Delete Condition="Exists('$(_SerializerCsIntermediateFolder)') == 'true'" Files="$(_SerializerCsIntermediateFolder)" ContinueOnError="true"/>
<Message Text="Running Serialization Tool" Importance="normal" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Message Text="Running Serialization Tool" Importance="normal" />

Not strictly needed, MSBuild binary logs keep track of when tasks start.

<Exec Command="dotnet Microsoft.XmlSerializer.Generator &quot;$(IntermediateOutputPath)$(AssemblyName)$(TargetExt)&quot; --force --quiet $(_SgenRspFilePath)" ContinueOnError="true"/>
<UsingTask TaskName="GenerateXmlSerializers" AssemblyFile="$(MSBuildThisFileDirectory)/../lib/netstandard2.0/dotnet-Microsoft.XmlSerializer.Generator.dll" />
Copy link
Contributor

Choose a reason for hiding this comment

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

UsingTask must be moved outside a Target node.

<GenerateXmlSerializers AssemblyPath="$(IntermediateOutputPath)$(AssemblyName)$(TargetExt)" RspFilePath="$(_SgenRspFilePath)" Force="true" Quiet="true" IgnoreErrors="true" />
<Warning Condition="Exists('$(_SerializerCsIntermediateFolder)') != 'true'" Text="$(_SGenWarningText)" />
<Csc Condition="Exists('$(_SerializerCsIntermediateFolder)') and !Exists('$(_CscRspFilePath)')" ContinueOnError="true" OutputAssembly="$(_SerializerDllIntermediateFolder)" References="@(ReferencePath);@(IntermediateAssembly)" Optimize="$(Optimize)" EmitDebugInformation="$(DebugSymbols)" Sources="$(_SerializerCsIntermediateFolder);$(_SerializerCsAssemblyInfoIntermediateFolder)" TargetType="Library" ToolExe="$(CscToolExe)" ToolPath="$(CscToolPath)" DisabledWarnings="$(_SerializationAssemblyDisabledWarnings)"/>
<Csc Condition="Exists('$(_SerializerCsIntermediateFolder)') and Exists('$(_CscRspFilePath)')" ContinueOnError="true" OutputAssembly="$(_SerializerDllIntermediateFolder)" References="@(ReferencePath);@(IntermediateAssembly)" Optimize="$(Optimize)" EmitDebugInformation="$(DebugSymbols)" Sources="$(_SerializerCsIntermediateFolder);$(_SerializerCsAssemblyInfoIntermediateFolder)" TargetType="Library" ResponseFiles="$(_CscRspFilePath)" ToolExe="$(CscToolExe)" ToolPath="$(CscToolPath)" DisabledWarnings="$(_SerializationAssemblyDisabledWarnings)"/>
Expand Down
Loading