From 387316fed644b1deeeeef0b385ae52b68aab3151 Mon Sep 17 00:00:00 2001
From: Ankit Jain <ankit.jain@xamarin.com>
Date: Thu, 10 Nov 2016 13:26:27 -0500
Subject: [PATCH] [xbuild] Don't reevaluate project when setting metadata in a
 dynamic ..

.. item.

If we have a project, that sets some properties or items at runtime,
like in a target:

```
        <Target Name="Build">
                <PropertyGroup>
                        <Bar>Bar01</Bar>
                </PropertyGroup>
```

.. such a property can be accessed as `$(Bar)` subsequently.

Then at later point, a target tries to update metadata on an item group
with existing items, like:

```
	<ItemGroup>
		<FooItem Include="xyz" />
	</ItemGroup>

	<Target Name="Foo">
                <ItemGroup>
                        <FooItem>
                                <SomeMetadata>MetadataValue</SomeMetadata>
                        </FooItem>
                </ItemGroup>
```

.. then it is seen that the value for the earlier created `$(Bar)` disappears!
So, if we try to print the value of `$(Bar)` right after that item
group, it would appear as `''`!

The issue is that the metadata update in target `Foo` caused the project
to get re-evaluated, which meant that items/properites created after the
project load were lost! We should not be reevaluating the project when
setting metadata on the basis of dynamic items, like in the target
`Foo`.

These dynamic item groups in a target are represented as
`BuildItemTask`, and these create BuildItems which have their
`IsDynamic` property set. And this is used to avoid re-evaluating the
project in `BuildItem.SetMetadata`.

But when updating metadata (`BuildItem.UpdateMetadata`), we need to
update metadata on the *existing* items, which might not have been
created from such dynamic item groups! So, their `IsDynamic==false`.
Hence, trying to `SetMetadata` on such items would cause the project to
be reevaluated, and thus properties/items like `$(Bar)` would be lost!

Fixes: https://bugzilla.xamarin.com/show_bug.cgi?id=45137

In this particular case, `Bar` was the `DesignTimeBuild` property, which
was losing it's value when `ProjectReference`'s `AdditionalProperties`
metadata was set in:

```
 <Target Name="BclBuildAddProjectReferenceProperties"
    <ItemGroup>
      <ProjectReference>
         <AdditionalProperties>$(_BclBuildProjectReferenceProperties);%(ProjectReference.AdditionalProperties)</AdditionalProperties>
     </ProjectReference>
    </ItemGroup>
```
---
 .../Microsoft.Build.BuildEngine/BuildItem.cs  | 16 ++++++++---
 .../Microsoft.Build.BuildEngine/TargetTest.cs | 27 +++++++++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/BuildItem.cs b/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/BuildItem.cs
index f663120bf79..82af83b1523 100644
--- a/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/BuildItem.cs
+++ b/mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/BuildItem.cs
@@ -226,6 +226,14 @@ namespace Microsoft.Build.BuildEngine {
 		public void SetMetadata (string metadataName,
 					 string metadataValue,
 					 bool treatMetadataValueAsLiteral)
+		{
+			SetMetadata (metadataName, metadataValue, treatMetadataValueAsLiteral, false);
+		}
+
+		void SetMetadata (string metadataName,
+					 string metadataValue,
+					 bool treatMetadataValueAsLiteral,
+					 bool fromDynamicItem)
 		{
 			if (metadataName == null)
 				throw new ArgumentNullException ("metadataName");
@@ -251,9 +259,11 @@ namespace Microsoft.Build.BuildEngine {
 			} else if (HasParentItem) {
 				if (parent_item.child_items.Count > 1)
 					SplitParentItem ();
-				parent_item.SetMetadata (metadataName, metadataValue, treatMetadataValueAsLiteral);
+				parent_item.SetMetadata (metadataName, metadataValue, treatMetadataValueAsLiteral, fromDynamicItem);
 			}
-			if (FromXml || HasParentItem) {
+
+			// We don't want to reevalute the project for dynamic items
+			if (!fromDynamicItem && !IsDynamic && (FromXml || HasParentItem)) {
 				parent_item_group.ParentProject.MarkProjectAsDirty ();
 				parent_item_group.ParentProject.NeedToReevaluate ();
 			}
@@ -374,7 +384,7 @@ namespace Microsoft.Build.BuildEngine {
 					continue;
 				
 				foreach (string name in evaluatedMetadata.Keys) {
-					item.SetMetadata (name, (string)evaluatedMetadata [name]);
+					item.SetMetadata (name, (string)evaluatedMetadata [name], false, IsDynamic);
 				}
 
 				AddAndRemoveMetadata (project, item);
diff --git a/mcs/class/Microsoft.Build.Engine/Test/Microsoft.Build.BuildEngine/TargetTest.cs b/mcs/class/Microsoft.Build.Engine/Test/Microsoft.Build.BuildEngine/TargetTest.cs
index 459bb66e6d5..8f2aa2e7b28 100644
--- a/mcs/class/Microsoft.Build.Engine/Test/Microsoft.Build.BuildEngine/TargetTest.cs
+++ b/mcs/class/Microsoft.Build.Engine/Test/Microsoft.Build.BuildEngine/TargetTest.cs
@@ -698,6 +698,33 @@ namespace MonoTests.Microsoft.Build.BuildEngine {
 				</Project>", "D");
 		}
 
+		[Test]
+		public void ItemGroupInsideTarget_UpdateMetadata ()
+		{
+			ItemGroupInsideTarget (
+				@"<Project ToolsVersion=""4.0"" xmlns=""http://schemas.microsoft.com/developer/msbuild/2003"">
+					<ItemGroup>
+						<ProjectReference Include='xyz'/>
+					</ItemGroup>
+
+					<Target Name='Main' DependsOnTargets='CreateBar'>
+						<Message Text='Before: $(Bar)'/>
+						<ItemGroup>
+							<ProjectReference>
+								<AdditionalProperties>A=b</AdditionalProperties>
+							</ProjectReference>
+						</ItemGroup>
+						<Message Text='After: $(Bar)'/>
+					</Target>
+
+					<Target Name='CreateBar'>
+						<PropertyGroup>
+							<Bar>Bar01</Bar>
+						</PropertyGroup>
+					</Target>
+				</Project>", 2, "Before: Bar01", "After: Bar01");
+		}
+
 		[Test]
 		public void ItemGroupInsideTarget_Batching ()
 		{
-- 
2.25.1