[xbuild] Don't reevaluate project when setting metadata in a dynamic ..
authorAnkit Jain <ankit.jain@xamarin.com>
Thu, 10 Nov 2016 18:26:27 +0000 (13:26 -0500)
committerAnkit Jain <ankit.jain@xamarin.com>
Thu, 10 Nov 2016 23:20:36 +0000 (18:20 -0500)
.. 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>
```

mcs/class/Microsoft.Build.Engine/Microsoft.Build.BuildEngine/BuildItem.cs
mcs/class/Microsoft.Build.Engine/Test/Microsoft.Build.BuildEngine/TargetTest.cs

index f663120bf798730223c944db359caa7873771dc5..82af83b15232988b1fd1d50b1b18e80d2bc09536 100644 (file)
@@ -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);
index 459bb66e6d5cd3cde9d2007433d0e8aaecb1a480..8f2aa2e7b28daf2b2f4f41ae052bae1b3097e233 100644 (file)
@@ -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 ()
                {