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

Regression in the XmlBaseWriter.cs #74752

Closed
marcin-krystianc opened this issue Aug 29, 2022 · 4 comments · Fixed by #74787
Closed

Regression in the XmlBaseWriter.cs #74752

marcin-krystianc opened this issue Aug 29, 2022 · 4 comments · Fixed by #74787

Comments

@marcin-krystianc
Copy link
Contributor

Description

In this optimization change (XmlBaseWriter.cs), there is a mistake which changes the logic in the XmlBaseWriter.cs. Previously, it checked for the existence of - at the end of a text, but now it is checked at the beginning of the text.

Reproduction Steps

Use .NET 7 preview 1 and run this command for any F# project:
dotnet msbuild /t:build /pp /p:TargetFramework=net7.0 MyFsharpProject.fsharp
It fails to write for example this element (which exists there for about 5 years)

Expected behavior

Shouldn't throw an exception

Actual behavior

throws an exception:

System.ArgumentException: An XML comment cannot contain '--', and '-' cannot be the last character.
   at System.Xml.XmlTextWriter.WriteComment(String text)
   at System.Xml.XmlElement.WriteElementTo(XmlWriter writer, XmlElement el)
   at System.Xml.XmlDocument.Save(XmlWriter w)
   at Microsoft.Build.Evaluation.Project.ProjectImpl.SaveLogicalProject(TextWriter writer)
   at Microsoft.Build.CommandLine.MSBuildApp.BuildProject(String projectFile, String[] targets, String toolsVersion, Dictionary`2 globalProperties, Dictionary`2 restoreProperties, ILogger[] loggers, LoggerVerbosity verbosity, DistributedLoggerRecord[] distributedLoggerRecords, Int32 cpuCount, Boolean enableNodeReuse, TextWriter preprocessWriter, TextWriter targetsWriter, Boolean detailedSummary, ISet`1 warningsAsErrors, ISet`1 warningsNotAsErrors, ISet`1 warningsAsMessages, Boolean enableRestore, ProfilerLogger profilerLogger, Boolean enableProfiler, Boolean interactive, Boolean isolateProjects, GraphBuildOptions graphBuildOptions, Boolean lowPriority, String[] inputResultsCaches, String outputResultsCache, String[] commandLine)
   at Microsoft.Build.CommandLine.MSBuildApp.Execute(String[] commandLine)
Unhandled exception: System.ArgumentException: An XML comment cannot contain '--', and '-' cannot be the last character.
   at System.Xml.XmlTextWriter.WriteComment(String text)
   at System.Xml.XmlElement.WriteElementTo(XmlWriter writer, XmlElement el)
   at System.Xml.XmlDocument.Save(XmlWriter w)
   at Microsoft.Build.Evaluation.Project.ProjectImpl.SaveLogicalProject(TextWriter writer)
   at Microsoft.Build.CommandLine.MSBuildApp.BuildProject(String projectFile, String[] targets, String toolsVersion, Dictionary`2 globalProperties, Dictionary`2 restoreProperties, ILogger[] loggers, LoggerVerbosity verbosity, DistributedLoggerRecord[] distributedLoggerRecords, Int32 cpuCount, Boolean enableNodeReuse, TextWriter preprocessWriter, TextWriter targetsWriter, Boolean detailedSummary, ISet`1 warningsAsErrors, ISet`1 warningsNotAsErrors, ISet`1 warningsAsMessages, Boolean enableRestore, ProfilerLogger profilerLogger, Boolean enableProfiler, Boolean interactive, Boolean isolateProjects, GraphBuildOptions graphBuildOptions, Boolean lowPriority, String[] inputResultsCaches, String outputResultsCache, String[] commandLine)
   at Microsoft.Build.CommandLine.MSBuildApp.Execute(String[] commandLine)
   at Microsoft.Build.CommandLine.MSBuildApp.Main(String[] args)
   at Microsoft.DotNet.Cli.Utils.MSBuildForwardingAppWithoutLogging.ExecuteInProc(String[] arguments)

Regression?

yes

Known Workarounds

no

Configuration

No response

Other information

FYI @GrabYourPitchforks

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 29, 2022
@ghost
Copy link

ghost commented Aug 29, 2022

Tagging subscribers to this area: @dotnet/area-system-xml
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

In this optimization change (XmlBaseWriter.cs), there is a mistake which changes the logic in the XmlBaseWriter.cs. Previously, it checked for the existence of - at the end of a text, but now it is checked at the beginning of the text.

Reproduction Steps

Use .NET 7 preview 1 and run this command for any F# project:
dotnet msbuild /t:build /pp /p:TargetFramework=net7.0 MyFsharpProject.fsharp
It fails to write for example this element (which exists there for about 5 years)

Expected behavior

Shouldn't throw an exception

Actual behavior

throws an exception:

System.ArgumentException: An XML comment cannot contain '--', and '-' cannot be the last character.
   at System.Xml.XmlTextWriter.WriteComment(String text)
   at System.Xml.XmlElement.WriteElementTo(XmlWriter writer, XmlElement el)
   at System.Xml.XmlDocument.Save(XmlWriter w)
   at Microsoft.Build.Evaluation.Project.ProjectImpl.SaveLogicalProject(TextWriter writer)
   at Microsoft.Build.CommandLine.MSBuildApp.BuildProject(String projectFile, String[] targets, String toolsVersion, Dictionary`2 globalProperties, Dictionary`2 restoreProperties, ILogger[] loggers, LoggerVerbosity verbosity, DistributedLoggerRecord[] distributedLoggerRecords, Int32 cpuCount, Boolean enableNodeReuse, TextWriter preprocessWriter, TextWriter targetsWriter, Boolean detailedSummary, ISet`1 warningsAsErrors, ISet`1 warningsNotAsErrors, ISet`1 warningsAsMessages, Boolean enableRestore, ProfilerLogger profilerLogger, Boolean enableProfiler, Boolean interactive, Boolean isolateProjects, GraphBuildOptions graphBuildOptions, Boolean lowPriority, String[] inputResultsCaches, String outputResultsCache, String[] commandLine)
   at Microsoft.Build.CommandLine.MSBuildApp.Execute(String[] commandLine)
Unhandled exception: System.ArgumentException: An XML comment cannot contain '--', and '-' cannot be the last character.
   at System.Xml.XmlTextWriter.WriteComment(String text)
   at System.Xml.XmlElement.WriteElementTo(XmlWriter writer, XmlElement el)
   at System.Xml.XmlDocument.Save(XmlWriter w)
   at Microsoft.Build.Evaluation.Project.ProjectImpl.SaveLogicalProject(TextWriter writer)
   at Microsoft.Build.CommandLine.MSBuildApp.BuildProject(String projectFile, String[] targets, String toolsVersion, Dictionary`2 globalProperties, Dictionary`2 restoreProperties, ILogger[] loggers, LoggerVerbosity verbosity, DistributedLoggerRecord[] distributedLoggerRecords, Int32 cpuCount, Boolean enableNodeReuse, TextWriter preprocessWriter, TextWriter targetsWriter, Boolean detailedSummary, ISet`1 warningsAsErrors, ISet`1 warningsNotAsErrors, ISet`1 warningsAsMessages, Boolean enableRestore, ProfilerLogger profilerLogger, Boolean enableProfiler, Boolean interactive, Boolean isolateProjects, GraphBuildOptions graphBuildOptions, Boolean lowPriority, String[] inputResultsCaches, String outputResultsCache, String[] commandLine)
   at Microsoft.Build.CommandLine.MSBuildApp.Execute(String[] commandLine)
   at Microsoft.Build.CommandLine.MSBuildApp.Main(String[] args)
   at Microsoft.DotNet.Cli.Utils.MSBuildForwardingAppWithoutLogging.ExecuteInProc(String[] arguments)

Regression?

yes

Known Workarounds

no

Configuration

No response

Other information

FYI @GrabYourPitchforks

Author: marcin-krystianc
Assignees: -
Labels:

area-System.Xml

Milestone: -

@stephentoub stephentoub added this to the 7.0.0 milestone Aug 29, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 29, 2022
@krwq
Copy link
Member

krwq commented Aug 29, 2022

cc: @GrabYourPitchforks

@GrabYourPitchforks
Copy link
Member

Thanks for the report! I'll send a PR fixing this (and adding regression tests) within a few hours.

@GrabYourPitchforks
Copy link
Member

GrabYourPitchforks commented Aug 29, 2022

Double-checked the original PR to look for additional errors. Buggy lines:

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 29, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 30, 2022
@ghost ghost added in-pr There is an active PR which will close this issue when it is merged and removed in-pr There is an active PR which will close this issue when it is merged labels Aug 30, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.