dotnet / project-system

The .NET Project System for Visual Studio
MIT License
970 stars 389 forks source link

Infer <Link> missing attributes for files outside of project root #5765

Open danmoseley opened 4 years ago

danmoseley commented 4 years ago

Visual Studio Version: 16.4.1

Summary: Right now, if a project contains a file outside of the project file's subtree, it is displayed as if it was in the project folder. If there are more than a few of these, one likely wants them to be in some kind of structure and right now that can only be achieved by manually adding many "Link" attributes.

Eg., in a project we have ~50 entries like this. In this case it happens the paths are <repo-root>\src\Common\Interop\... and the project is in <repo-root>\src\Microsoft.Win32.SystemEvents

    <Compile Include="$(CommonPath)\Interop\Windows\Interop.Errors.cs">
      <Link>Common\Interop\Windows\Interop.Errors.cs</Link>
    </Compile>
    <Compile Include="$(CommonPath)\Interop\Windows\User32\Interop.Constants.cs">
      <Link>Common\Interop\Windows\User32\Interop.Constants.cs</Link>
    </Compile>

etc. It makes no difference whether Link is an attribute or an element - you can see how this is a burden to write, maintain, and it adds noise to the project and changes to it. But what is being done here is so mechanical. Can we improve the heuristic so that we can use Link less?

Perhaps if Link is not present, instead of "put everything at the root" we could use a heuristic like "remove any prefix common to the project path, then represent remainder". In this case, there is a common prefix: <repo-root>\src. Removing that and representing the rest would put the files in the solution explorer inside Common\Interop\Windows, which is what we want in this case.

drewnoakes commented 4 years ago

Does MSBuild currently need this metadata, or is it just for VS?

jkotas commented 4 years ago

Build does not need this metadata, it is just for VS.

danmoseley commented 4 years ago

Motivational example: https://github.com/dotnet/runtime/blob/5d8f92435ebfeab67b41aba0bce12e1e8af3187e/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems#L1100-L1442 (this is a file imported into a csproj, but it shouldn't matter)

davkean commented 4 years ago

@danmosemsft We have LinkBase support which will simplify this:

    <ItemGroup>
        <Compile Include="..\..\System\Numerics\**\*.cs" LinkBase="System\Numerics" />
    </ItemGroup>

It works with Update, so you should be able to write something like the following that will prevent you from needing to opt into globs for the include if you want to explicitly pick individual source files:

    <ItemGroup>
        <Compile Include="..\..\System\Numerics\Vector.cs" />   
    <Compile Update="..\..\System\Numerics\**\*.cs" LinkBase="System\Numerics" />
    </ItemGroup>

I'll leave this open to track maybe being smarter about this, but I think LinkBase is a pretty good workaround.

davkean commented 4 years ago

Playing around with this, we do already support this via globs, just not individual items.

The following:

  <ItemGroup>
    <Compile Include="..\Common\**\*.cs" />
  </ItemGroup>

Results in a tree like:

image

Going deeper into the tree:

  <ItemGroup>
    <Compile Include="..\Common\VS\**\*.cs" />
  </ItemGroup>

Results in:

image

You can actually make use of this to do this individual items by globbing (I don't want to see what this does to evaluation time!).

<Compile Include="$(CommonPath)\**\Interop.Errors.cs">

This will result in Interop\Native becoming the LinkBase.

I'll leave this open, but this bug basically becomes "do something smarter with individual includes".