dotnet / project-system

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

Project System expects TargetFramework property in the csproj itself, and not in an imported props/targets file. #1358

Closed tannergooding closed 5 years ago

tannergooding commented 7 years ago

Description

The default project template looks like the following:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netcoreapp1.0</TargetFramework>
  </PropertyGroup>
</Project>

I have a set of shared props/targets that are imported by every project in my repo. In order for these to be imported properly, I first have to get rid of the Sdk="Microsoft.NET.Sdk" attribute and manually call the appropriate imports. This leaves me with:

<?xml version="1.0" encoding="utf-8"?>
<Project>
  <PropertyGroup>
    <TargetFramework>netcoreapp1.0</TargetFramework>
  </PropertyGroup>
  <Import Project="$(MSBuildSDKsPath)\Microsoft.NET.Sdk\Sdk\Sdk.props" />
  <Import Project="$(MSBuildSDKsPath)\Microsoft.NET.Sdk\Sdk\Sdk.targets" />
</Project>

I can then import my shared props/targets files and things continue to work as expected:

<?xml version="1.0" encoding="utf-8"?>
<Project>
  <Import Project="$(MSBuildThisFileDirectory)..\..\Imports\Common\<RepoName>.Common.props" />
  <PropertyGroup>
    <TargetFramework>netcoreapp1.0</TargetFramework>
  </PropertyGroup>
  <Import Project="$(MSBuildSDKsPath)\Microsoft.NET.Sdk\Sdk\Sdk.props" />
  <Import Project="$(MSBuildThisFileDirectory)..\..\Imports\Common\<RepoName>.Common.targets" />
  <Import Project="$(MSBuildSDKsPath)\Microsoft.NET.Sdk\Sdk\Sdk.targets" />
</Project>

I then generally take this a step further and declare another set of props/targets. These import the Common props/targets and declare any specifics to a particular target framework (so I might have a <RepoName>.Desktop props/targets and a <RepoName>.Portable props/targets for example). These generally define the target framework and additionally import the appropriate framework props/targets: So I would end up with:

\.Portable.props

<?xml version="1.0" encoding="utf-8"?>
<Project>
  <Import Project="$(MSBuildThisFileDirectory)..\Common\<RepoName>.Common.props" />
  <PropertyGroup>
    <TargetFramework>netcoreapp1.0</TargetFramework>
  </PropertyGroup>
  <Import Project="$(MSBuildSDKsPath)\Microsoft.NET.Sdk\Sdk\Sdk.props" />
</Project>

\.Portable.targets

<?xml version="1.0" encoding="utf-8"?>
<Project>
  <Import Project="$(MSBuildThisFileDirectory)..\Common\<RepoName>.Common.targets" />
  <Import Project="$(MSBuildSDKsPath)\Microsoft.NET.Sdk\Sdk\Sdk.targets" />
</Project>

Project.csproj

<?xml version="1.0" encoding="utf-8"?>
<Project>
  <Import Project="$(MSBuildThisFileDirectory)..\..\Imports\Portable\<RepoName>.Portable.props" />
  <Import Project="$(MSBuildThisFileDirectory)..\..\Imports\Portable\<RepoName>.Portable.targets" />
</Project>

Issue

This last step fails and I get a dialog stating the following: image

Expected Behavior

Since the resulting project file is the same overall (just with a level of indirection on certain imports), I would expect the project system to successfully load the project and continue working as normal.

srivatsn commented 7 years ago

Currently, we require the TargetFramework property to be defined in the project to know to load with cps instead of old project system. So your final project is being loaded with the old project system. If you add the TF property to the project this should work fine. Couple of other things:

tannergooding commented 7 years ago

@srivatsn

Currently, we require the TargetFramework property to be defined in the project to know to load with cps instead of old project system. So your final project is being loaded with the old project system. If you add the TF property to the project this should work fine.

Most of the new project system features seem to be working, regardless of where I have defined TargetFramework (such as automatically picking up changes on disk/etc) -- This is on RC3.

Your reponame.props presumably was importing Microsoft.Common.Props and the reponame.targets was importing Microsoft.CSharp.targets before. The sdk props\targets import those as well. So instead of adding both, you should change your reponame props\tragetd to import the sdk props\targets.

I already cleaned that up :smile:

For importing the sdk you can use the new msbuild syntax, since that avoids hard coding the path

This is great, I will update my repo to use the new syntax

srivatsn commented 7 years ago

Do you see the Dependencies node or References node in solution explorer? The code basically does a textual scan of the project file to find the TargetFramework property to decided CPS vs old project system. I'd be very surprised if CPS is loading for that last project.

tannergooding commented 7 years ago

Yes I am. Here is the sample solution: sample.zip

It seems that the only reason it is working is because of the process/exact steps I used.

Specifically, doing the process listed in the original post causes the project to be saved in the solution as follows:

Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Sample", "Sources\Sample\Sample.csproj", "{20063A65-6A05-4299-9B03-F3F982692DCA}"
EndProject

If I remove and add the project back, the solution will change to the following:

Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Sample", "Sources\Sample\Sample.csproj", "{20063A65-6A05-4299-9B03-F3F982692DCA}"
EndProject

The project guid of 9A19103F-16F7-4668-BE54-9A1E7A4F7556 seems to cause the new project system to be used unconditionally.

srivatsn commented 7 years ago

Note that today the parsing of the csproj to identify the TargetFramework property is textual and that leads to issues like https://developercommunity.visualstudio.com/content/problem/4661/adding-a-comment-in-a-csproj-file-with-text-target.html

The right fix here is to make the Project Selector not use TargetFramework at all.

davkean commented 7 years ago

@jviau Also ran into this yesterday, where he accidentally opt'd in a project.

davkean commented 7 years ago

We should factor in https://github.com/dotnet/project-system/issues/2601 when we come up with a user experience for opting in/out of the new project system.

Pilchie commented 6 years ago

:link: also reported at https://developercommunity.visualstudio.com/content/problem/231745/1556-add-netstandard20-sdk-library-to-solution-and.html

davkean commented 6 years ago

Note, I've made a slight change to the logic so that we can now at least handle conditions on TargetFramework/TargetFrameworks: https://github.com/dotnet/project-system/issues/3587.

davkean commented 6 years ago

We now support conditions on TargetFramework/TargetFrameworks, which means this will now correctly load in the new project system whereas previously you would have got an upgrade prompt or installer prompt:

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

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks Condition=""'$(BuildingInsideVisualStudio)' == 'true'"">netcoreapp2.0</TargetFrameworks>
    <TargetFrameworks Condition=""'$(BuildingInsideVisualStudio)' != 'true'"">net45;netcoreapp2.0</TargetFrameworks>
  </PropertyGroup>

</Project>
martinfletcher commented 6 years ago

@davkean Has this feature made it into visual studio yet?

My issue https://github.com/Microsoft/msbuild/issues/3574 is still happening with latest updates! Is there an extension that needs installing?

davkean commented 6 years ago

This bug is still open.

As a workaround, you can change the GUIDs in the solution: https://github.com/dotnet/project-system/blob/master/docs/opening-with-new-project-system.md#project-type-guids.

davkean commented 6 years ago

Thinking out loud, we cannot evaluate to figure out if this project has a TargetFramework property or not. The performance hit is just too much on every single project. Until we've opt'd all projects into the new project system, I propose that we tweak the algorithm slightly:

Pilchie commented 6 years ago

I like the idea of expanding the check to look for Sdk style projects. Good idea.

alexeyzimarev commented 5 years ago

After compared the migrated version with the original version, I got the projects loading properly when the first line in the csproj file looks like this:

<Project Sdk="Microsoft.NET.Sdk" ToolsVersion="15.0">
jurby commented 5 years ago

Problem is in TargetFramework. When is property in external build props file - build not working in VS 2017. In dotnet cli this setup working.

Not working in VS:

<Project Sdk="Microsoft.NET.Sdk">
  <Import Project="../../../build/NugetProject.build.props" />
  ....

Working in VS:

<Project Sdk="Microsoft.NET.Sdk">
  <Import Project="../../../build/NugetProject.build.props" />
  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
  </PropertyGroup>
tebeco commented 5 years ago

Same in Vs2019 (especially now that the release frequency is now 2-3 per year),

when you want to align TargetFramework in a really simple way :

this is what it looks like : image

Also the "attempted fix" it really awfull and do really bad stuff but i guess that should be part of another issue

MrJul commented 5 years ago

If that can help, I've used the following workaround for months now. Assuming you've already imported a.props file before containing a valid TargetFramework, add this to your .csproj:

<PropertyGroup>
    <TargetFramework>$(TargetFramework)</TargetFramework>
</PropertyGroup>

Visual Studio won't complain anymore when adding the .csproj, and you still have only one file to change for the solution.

davkean commented 5 years ago

Going to dupe this against https://github.com/dotnet/project-system/issues/4314 that's tracking fixing this detection: https://github.com/dotnet/project-system/issues/4314.

bqstony commented 5 years ago

alternative to @MrJul workaround, for me worked to change the used Projecttype id in the solution file to {9A19103F-16F7-4668-BE54-9A1E7A4F7556}:

  1. dotnet sln add pathToProject
  2. change in sln file Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "proj", "proj\proj.csproj", "{D5E5FF98-62C5-4231-B8B0-95F4A12FC033}" to Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "proj", "proj\proj", "{D5E5FF98-62C5-4231-B8B0-95F4A12FC033}"

I Use 16.1.6

deanmarcussen commented 4 months ago

Disappointing this has been closed as duplicate when the linked issue does not describe a solution.

@MrJul solution is great, but with extra weirdness it only works for Sdk="Microsoft.NET.Sdk"> projects, for Sdk="Microsoft.NET.Sdk.Razor"> it had to use the plural version of the property <TargetFrameworks>$(TargetFrameworks)</TargetFrameworks> property.

If that can help, I've used the following workaround for months now. Assuming you've already imported a.props file before containing a valid TargetFramework, add this to your .csproj:

<PropertyGroup>
    <TargetFramework>$(TargetFramework)</TargetFramework>
</PropertyGroup>

Visual Studio won't complain anymore when adding the .csproj, and you still have only one file to change for the solution.