Excludes or removes in project folder don't apply to recursive includes from a parent folder

[dsplaisted]

Repro project: ProjectFolder.zip

Consider the following project (named Project.proj):

<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <ItemGroup>
    <File Include="..\**" Exclude="Project.proj" />
  </ItemGroup>
  <Target Name="Build">
    <Message Text="@(File)"/>
  </Target>
</Project>

The File include is recursively including all files starting in the parent directory of the project. The Exclude statement is expected to prevent the Project.proj file to be excluded.

However, when you build the project:

EXPECTED: The File item (and hence output message) does not include Project.proj ACTUAL: Project.proj is included in the file item, as ..\ProjectFolder\Project.proj

[cdmihai]

This is not a regression. Both msbuild 14 and current appear to behave the same way here. This is because items produced by an include with globs integrate the fixed dir part of the glob and the recursive expanded part into the item name. So "..\**" will capture the file Project.proj as ..\ProjectFolder\Project.proj. Since Excludes are treated as strings (sometimes :( ), the string ..\path\to\file\Project.proj does not match the string Project.proj and therefore the Exclude does not apply.

To make the Exclude work here you either

• change it to be the same as Include would have produced it (prepend fixed dir part and glob expanded recursive dir part): <File Include="..\**" Exclude="..\ProjectFolder\Project.proj" />
• exclude with a glob pattern: <File Include="..\**" Exclude="**\Project.proj" />

[clairernovotny]

@cdmihai I appreciate the technical detail, but that's just making the user alter their code to an internal weirdness. I'm not saying this is a regression, but I think we're saying that a user would expect Exclude="Project.proj" to work because Project.proj is a valid subset of anything matching ..\**. The fact that certain things are expanded sometimes is irrelevant to user expectations.

[dsplaisted]

I agree with @onovotny that this is counter to user expectations. Given the improvements we've made in this release, people are going to be using wildcards more, so they will be more likely to run into this issue than in previous versions.

This seems like it might be related to #1598.

[cdmihai]

@dsplaisted Unfortunately the issues are related, but the fix is not related because item provenance uses regex matching without hitting the disk, whereas the FileMatcher follows a different code path since it needs to walk the file tree.

Also, since this is a 10 year old bug (msbuild2, which ships with every windows installation, behaves this way, and probably the subsequent msbuilds were made to mimic it), fixing it is scary, since the number of potentially breaking msbuild files in the world is unknown.

This also means that after I fix #1598 there will be a discrepancy between item provenance and actual globbing, with item provenance returning the correct thing, which means that sadly I might have to alter the item provenance to match this bug's behaviour :scream:

I think that the way to go forward here is to wait until we implement fine grained telemetry for msbuild and then log an event when the code path for this bug is called. That way, after collecting events for an year or so we'll know how many users' builds we'd potentially break by fixing this bug.

[clairernovotny]

@cdmihai is there any way to shim/quirk this behavior on the presence of the "Sdk" attribute? There's no existing builds in the wild that use that as it's brand new. Then the telemetry can tell you if the change needs to be shimmed permanently or if it can be applied to all builds?

Then we can have correct behavior for .NET Core projects at least.

[cdmihai]

Discussed this in our daily stand-up. We do like the idea of forking the msbuild evaluation based on the SDK attribute to fix a bunch of wrong things with msbuild, which would be breaking changes otherwise. However, right now the change does not meet the bar for VS2017.

After this release, we may look into ways of forking evaluation and preserving backwards compat.