Msbuild: XmlPoke task should save changes when only attribute values are changed

[kygagner]

The XmlPoke task only saves the xml document it is modifying if changes have been made, however with certain XPath expressions the changes are erroneously not counted, so while the poke does occur, it will not be saved unless other changes are made which are counted. The underlying problem is likely misuse of a XPathNodeIterator, which is not guaranteed to work properly if modifications are being made to the nodes as they are iterated, and misuse of the Count property on the XPathNodeIterator.

Here is a small example that reproduces the problem.

• Save the files listed below into the same directory
• Run MSBuild /v:detailed test.targets on the command line
• Observe output.xml
• Remove <replaceme/> from line 3 in test.xml
• Run MSBuild and observe output.xml again

You will notice that the value of the attribute on line 2 of test.xml is replaced due to the /root/element/@attr[.='replaceme'] portion of the XPath expression only if the <replaceme/> element exists in the document. This is because the poke due to /root/replaceme is counted, whereas the replacement of the attribute value is not. The poke is always visible in the build output.

_test.targets_

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
    <Target Name="Build">
        <Copy SourceFiles="test.xml" DestinationFiles="output.xml"/>
        <XmlPoke Query="/root/element/@attr[.='replaceme']|/root/replaceme" Value="X" XmlInputPath="output.xml"/>
    </Target>
</Project>

_test.xml_

<root>
  <element attr="replaceme" />
  <replaceme/>
</root>

[kygagner]

Other users on StackOverflow have discovered this problem as well.
MSBuild - XPath - XmlPeek can read but XmlPoke can not write

[atykhyy]

This issue is the more annoying because just four extra lines in src/Tasks/XmlPoke.cs would dispose of it:

@@ -199,12 +199,14 @@ public override bool Execute()
             }

             XPathNodeIterator iter = nav.Select(expr);
+            int count = 0;

             while (iter.MoveNext())
             {
                 try
                 {
                     iter.Current.InnerXml = _value.ItemSpec;
+                    count++;
                     Log.LogMessageFromResources(MessageImportance.Low, "XmlPoke.Replaced", iter.Current.Name, _value.ItemSpec);
                 }
                 catch (Exception e)
@@ -219,9 +221,9 @@ public override bool Execute()
                 }
             }

-            Log.LogMessageFromResources(MessageImportance.Normal, "XmlPoke.Count", iter.Count);
+            Log.LogMessageFromResources(MessageImportance.Normal, "XmlPoke.Count", count);

-            if (iter.Count > 0)
+            if (count > 0)
             {
 #if RUNTIME_TYPE_NETCORE
                 using (Stream stream = File.Create(_xmlInputPath.ItemSpec))

I donate this diff to anyone willing to go through the rigmarole of building and submitting a PR.