Default items and default removes trample on item groups expressed in csproj

[natemcmaster]

Can we make DefaultItems and DefaultRemoves smarter? Current implementation tramples what the user expresses in their csproj file and requires deep understanding of internal workings of MSBuild to unwind this.

Addition of Defaultitems and DefaultRemoves makes the behavior of ItemGroups in csproj unpredictable.

For example, consider this test application:

Microsoft.DotNet.Tools.Watcher.FunctionalTests.csproj
Tests.cs
TestProjects/
     TestApp/
          TestApp.csproj
          Program.cs

In RC.2, my project defined these items:

<Compile Include="**\*.cs" Exclude="TestProjects\**\*" />
<Content Include="TestProjects\**\*" CopyToOutputDirectory="PreserveNewest" />

DefaultItems causes duplication in Compile groups, and default removes remove items I want in the Content group. To workaround this, I have to change the project to:

  <PropertyGroup>
    <DisableDefaultRemoves>true</DisableDefaultRemoves>
  </PropertyGroup>

  <ItemGroup>
    <Compile Remove="TestProjects\**\*" />

    <Compile Remove="$(DefaultRemoves)" />
    <EmbeddedResource Remove="$(DefaultRemoves)" />
    <None Remove="$(DefaultRemoves)" />
    <Content Remove="$(DefaultRemoves)" />

    <Content Include="TestProjects\**\*" CopyToOutputDirectory="PreserveNewest" />
 </ItemGroup>

This seems excessively ugly.

More details:
Here's the RC.2 project I'm having difficulty porting: https://github.com/aspnet/DotNetTools/blob/cb3c5d77ade283538af441be9792590422c789a5/test/Microsoft.DotNet.Watcher.Tools.FunctionalTests/Microsoft.DotNet.Watcher.Tools.FunctionalTests.csproj#L10

[muratg]

cc @balachir @piotrpMSFT

[natemcmaster]

Example of one implication of this issue: dotnet/project-system#1126.

[rainersigwald]

I hate to sound like a broken record here, but: I still don't think there should be implicit globs.

That said, there's a way to avoid this type of problem: if you're going to do anything non-default with an item group, first make the include explicit, then do your operations on the explicit copy. In other words, excluding a directory should produce this modification to the project:

<PropertyGroup>
  <EnableDefaultCompileItems>false</EnableDefaultCompileItems>
</PropertyGroup>
<ItemGroup>
  <Compile Include="**\*.cs" Exclude="TestProjects\**\*" />
</ItemGroup>

Instead of the Remove stuff above.

Of course, it's hard to socialize a solution like this--even if we fix it in VS-driven edits.

[dsplaisted]

Thanks for raising this issue @natemcmaster. I've sent a PR to fix it.

[srivatsn]

Fixed by #630