msbuild icon indicating copy to clipboard operation
msbuild copied to clipboard

Project-to-project reference fails to detect correct multi-target framework if <TargetFramework> is defined

Open jonathanou opened this issue 1 year ago • 13 comments

If a project has both <TargetFramework> and <TargetFrameworks> defined, other projects cannot correctly reference the project's target framework that is different than what <TargetFramework> specifies.

Building the project with both properties defined does correctly produce the binaries for each targeted framework. It's just project-to-project references that doesn't work.

Create project A (ClassLibrary1) like so:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net6.0-windows</TargetFramework>
    <TargetFrameworks>net6.0;net6.0-windows</TargetFrameworks>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

</Project>

and project B (ClassLibrary2) like so:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <ProjectReference Include="..\ClassLibrary1\ClassLibrary1.csproj" />
  </ItemGroup>

</Project>

Project B (ClassLibrary2) will fail to build with the following error:

Project '..\ClassLibrary1\ClassLibrary1.csproj' targets 'net6.0-windows'. It cannot be referenced by a project that targets '.NETCoreApp,Version=v6.0'.	ClassLibrary2	C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Current\Bin\amd64\Microsoft.Common.CurrentVersion.targets	1806	

From the binlog, it seems MSBuild will treat project A as having a single target framework:

image

If I remove <TargetFramework> in project A, then MSBuild correctly treats project A as having multiple target frameworks:

image

jonathanou avatar Jul 29 '22 05:07 jonathanou

From the binlog, it seems MSBuild will treat project A as having a single target framework

Yes, and that's the expected behavior! Do you need to specify singular TF when you already specified TFS?

Nirmal4G avatar Jul 29 '22 10:07 Nirmal4G

Do you need to specify singular TF when you already specified TFS?

My company's build infrastructure specifies a default TF via Directory.Build.props file, so I am not in control of TF having a value that seems to contradict TFS.

To me, it seems even if TFS conflicts with TF, TFS should take precedence? Certainly when I build the project, TFS overrides TF as expected, as I see multiple binaries being built.

jonathanou avatar Jul 29 '22 11:07 jonathanou

When you build the project ClassLibrary1 separately, TFS takes precedence and multi-build comes into play and the inner build replaces the TF specified in the project file, even if TF is some garbage value. It builds and it produces multiple outputs.

But when using project reference, the resolution logic thinks it's a single target build because TF takes precedence there. Yes, this is more of a design issue than an implementation bug. I don't know if the team would allow the change since it's a behavior change, and a huge breaking change at that.

In the meantime, the specific problem with your setup considering the current project reference behavior is that you're trying to reference Windows-specific implementation to a cross-platform library which is not allowed. If you're expecting the project to build as net6.0 instead of net6.0-windows, you can modify the ClassLibrary2 project file as shown below...

<ProjectReference Include="..\ClassLibrary1\ClassLibrary1.csproj" SetTargetFramework="TargetFramework=$(TargetFramework)" />

Nirmal4G avatar Jul 29 '22 14:07 Nirmal4G

If you're expecting the project to build as net6.0 instead of net6.0-windows, you can modify the ClassLibrary2 project file as shown below...

Thanks for the tip! While it works, the disadvantage is every new project reference made to ClassLibrary1 will need this special workaround. It doesn't "just work".

I am currently manually unsetting the target framework in my ClassLibrary1 csproj file by doing this:

<TargetFramework></TargetFramework>
<TargetFrameworks>net6.0;net6.0-windows</TargetFramework>

This allows other projects to reference ClassLibrary1 normally.

However, unsetting the target framework is also not ideal, since every project in our solution that wants to multi-target now needs to know to manually unset the target framework.

But when using project reference, the resolution logic thinks it's a single target build because TF takes precedence there. Yes, this is more of a design issue than an implementation bug. I don't know if the team would allow the change since it's a behavior change, and a huge breaking change at that.

In addition to the build preferring TFS over TF, the Visual Studio UI also shows TFS over TF if both are defined. Here is the ClassLibrary1 properties UI:

image

It just seems to be that if both TF and TFS are defined, TFS generally trumps TF. I feel like the current project reference behavior preferring TF over TFS seems inconsistent with how other areas of MSBuild works.

Hopefully changing the project reference behavior can be considered!

jonathanou avatar Jul 29 '22 16:07 jonathanou

IMO, The precedence of TFS over TF should be consistent. I'm actually in favor of making the ProjectReference-protocol to support multi-target resolution even if a singular target has already been defined. But it comes with its own set of problems; not that we can't solve it.

However, unsetting the target framework is also not ideal, since every project in our solution that wants to multi-target now needs to know to manually unset the target framework.

I get it but understand that if TFS takes precedence always, setting the TF centrally doesn't make all the projects build to that. It'll build for all targets and that might take up your build time.

From what I understand, the role of the TF in your setup is more like a default target rather than a single target. If that's the case and you have a mix of projects that have both TF and TFS, rename all TF to TFS. Even a single target can be specified in TFS. There's a slight overhead involved in build time but it's worth in maintaining the source better.

Nirmal4G avatar Jul 30 '22 03:07 Nirmal4G

If that's the case and you have a mix of projects that have both TF and TFS, rename all TF to TFS. Even a single target can be specified in TFS.

That's a good point! I did not think about that. One potential problem I can currently think of is if any project decides to override the default target framework with a different, single, TF, they must know to put the override in TFS, not TF. But the good thing is modifying TF through Visual Studio project UI will correctly modify TFS in the csproj file (since Visual Studio prefers TFS if it is defined in the project).

I think a lot of these problems could be avoided if TFS is used as the only way to specify TF, even if the project only intends to target a single TF. But I guess it's probably too late to remove support for <TargetFramework> altogether at this point...

jonathanou avatar Jul 30 '22 09:07 jonathanou

I think they left TF alone since a lot of application projects use single target. Only libraries use multiple targets. As I already mentioned, it has a performance penalty, minuscule but still an overhead to be considered, especially in large solutions (> 100 single target projects). Anyway, it's a trade-off that falls on us, maintainers!

Nirmal4G avatar Jul 30 '22 09:07 Nirmal4G

Project files should specify only TargetFramework or TargetFrameworks, and not both.

My company's build infrastructure specifies a default TF via Directory.Build.props file, so I am not in control of TF having a value that seems to contradict TFS.

Unsetting via <TargetFramework /> is probably the best you can do here.

I think a lot of these problems could be avoided if TFS is used as the only way to specify TF, even if the project only intends to target a single TF. But I guess it's probably too late to remove support for <TargetFramework> altogether at this point...

There is also a performance implication: the "outer" build triggered when there are only TargetFrameworks collects information about the individual TargetFrameworks to build, where the "inner" build of a single TF doesn't have to care about that.

We spent some time in team triage today trying to think of a way to warn or message the "TF xor TFs" requirement but didn't come up with anything that sounded particularly great.

rainersigwald avatar Aug 04 '22 16:08 rainersigwald

We spent some time in team triage today trying to think of a way to warn or message the "TF xor TFs" requirement but didn't come up with anything that sounded particularly great.

@dsplaisted I think the SDK team should come up with something, be it a warning or ProjectReference behaviour change. Any solution is fine as long as it's understandable from User PoV.

Nirmal4G avatar Aug 04 '22 16:08 Nirmal4G

@rainersigwald a simple verification of unnecessary single TF value in TFs property could be:

<Target Name="ValidateTargetFrameworks" AfterTargets="_ComputeTargetFrameworkItems" Condition=" '$(TargetFrameworks)' != '' ">
  <Warning Condition="'@(_TargetFramework->Count())' &lt;= 1"
           File="$(MSBuildProjectFile)"
           Text="&lt;TargetFrameworks&gt; property requires multiple frameworks. For a single value use &lt;TargetFramework&gt;" />
  <Warning Condition="'@(_TargetFramework->Count())' != '@(_TargetFrameworkNormalized->Count())' "
           File="$(MSBuildProjectFile)"
           Text="Remove duplicates from &lt;TargetFrameworks&gt;" />
</Target>

Do you think it could be shipped as a MSBuild change wave feature?

stan-sz avatar Apr 02 '24 09:04 stan-sz

That sounds like a breaking change (adding a warning) for a legal situation (doing a "foreach" with one item isn't ideal but doesn't cause very many problems). I would expect a very high bar for such a thing. (Adding it as a BuildCheck/analyzer would make sense to me.)

rainersigwald avatar Apr 04 '24 14:04 rainersigwald

In one C# project, I deliberately set TargetFrameworks with only one framework because I wanted to Directory.Build.props to read TargetFramework in the inner build and set OutputPath and some other things.

KalleOlaviNiemitalo avatar Apr 04 '24 14:04 KalleOlaviNiemitalo

That sounds like a breaking change (adding a warning) for a legal situation (doing a "foreach" with one item isn't ideal but doesn't cause very many problems). I would expect a very high bar for such a thing. (Adding it as a BuildCheck/analyzer would make sense to me.)

Pinning your comment to #1777 :)

stan-sz avatar Apr 05 '24 06:04 stan-sz