No error for Item update in target

[dasMulli]

Ran into this while creating a workaround to patch an existing target (https://github.com/dotnet/cli/issues/6397#issuecomment-347989664) and naively using the item update syntax inside a target to update the link metadata of an item.
However, using the update syntax inside a target seems to update the metadata of all items, which is unexpected.

Steps to reproduce

Project file

<Project DefaultTargets="PrintResults">
  <ItemGroup>
    <SomeStaticItem Include="Item1" SomeMeta="MetaVal1" />
    <SomeStaticItem Include="Item2" SomeMeta="MetaVal2" />
    <SomeStaticItem Include="Item3" SomeMeta="MetaVal3" />

    <SomeStaticItem Update="Item2" SomeMeta="ChangedMetaVal2" />
  </ItemGroup>

  <Target Name="CreateRuntimeUpdatedItems">
    <ItemGroup>
      <SomeRuntimeItem Include="@(SomeStaticItem)" />
      <SomeRuntimeItem Update="Item2" SomeMeta="ChangedMetaVal2" />
    </ItemGroup>
  </Target>

  <Target Name="PrintResults" DependsOnTargets="CreateRuntimeUpdatedItems">
    <Message Importance="high" Text="static item: %(SomeStaticItem.Identity): SomeMeta=%(SomeStaticItem.SomeMeta)" />
    <Message Importance="high" Text="runtiime item:%(SomeRuntimeItem.Identity): SomeMeta=%(SomeRuntimeItem.SomeMeta)" />
  </Target>
</Project>

Command line

dotnet msbuild

Expected behavior

Error for Update syntax not being allowed inside targets

or

  static item: Item1: SomeMeta=MetaVal1
  static item: Item2: SomeMeta=ChangedMetaVal2
  static item: Item3: SomeMeta=MetaVal3
  runtiime item:Item1: SomeMeta=MetaVal1
  runtiime item:Item2: SomeMeta=ChangedMetaVal2
  runtiime item:Item3: SomeMeta=MetaVal3

Actual behavior

  static item: Item1: SomeMeta=MetaVal1
  static item: Item2: SomeMeta=ChangedMetaVal2
  static item: Item3: SomeMeta=MetaVal3
  runtiime item:Item1: SomeMeta=ChangedMetaVal2
  runtiime item:Item2: SomeMeta=ChangedMetaVal2
  runtiime item:Item3: SomeMeta=ChangedMetaVal2

Environment data

dotnet msbuild /version output: tested on 15.5.179.9764 and 15.6.12.27473 .

OS info:

If applicable, version of the tool that invokes MSBuild (Visual Studio, dotnet CLI, etc): macOS 10.13.2, dotnet cli

[rainersigwald]

Looks like a duplicate of #1618, but that should have been fixed long ago (by throwing an error on the not-actually-supported Update-inside-a-target syntax).

If you change your target to use syntax like #1618 (comment), it should work.

I repro the problem with 15.1.548.43366, so it looks like the error never worked :(

[dasMulli]

yep the old syntax works perfectly, I assumed that since there was no Update metadata added it actually tried to update (which is exactly what's discussed on the linked issue).

[dasMulli]

(updated the issue a bit to focus on the error)

[dasMulli]

@rainersigwald I'm afraid adding the error now would break at least
https://github.com/dotnet/sdk/blob/cb4cf799e0afd5a6c8e0630638f550b95e3eb35e/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.NET.ConflictResolution.targets#L47
https://github.com/dotnet/sdk/blob/cb4cf799e0afd5a6c8e0630638f550b95e3eb35e/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets#L450-L453
https://github.com/dotnet/sdk/blob/cb4cf799e0afd5a6c8e0630638f550b95e3eb35e/src/Tasks/Microsoft.NET.Build.Tasks/build/Microsoft.PackageDependencyResolution.targets#L578-L580

[rainersigwald]

@dasMulli Yup, looks like it. Unfortunately, that means we can't do this in a minor update as it'd be a breaking change.

[tmat]

@rainersigwald We should fix this in Dev16.

Bugs like these make msbuild very hard to use. If you want to avoid breaking changes please consider introducing some compat switch that can be used to opt-in new behavior.

[nguerrera]

it looks like the error never worked :(

Was it ever implemented? AFAICT, #1618 was closed because it was included in #1124 (another bug), not because it was fixed.

[nguerrera]

Maybe we should consider making it actually work as one expects rather than erroring in Dev16? The examples shown above so far would not break. I am nervous as always about breaking changes, so we should search for more data on whether there are cases of an update on anything but the entire set, that are actually depending on their bug.