search

T4MVC / R4MVC

R4MVC is a Roslyn code generator for ASP.NET Core MVC apps that creates strongly typed helpers that eliminate the use of literal strings in many places
Apache License 2.0
157 stars 48 forks source link

Upgrade to netstandard2.0 and aspnetcore 2.0 #56

Closed valeriob closed 7 years ago

valeriob commented 7 years ago

It still does not work 😢 [Failure] Msbuild failed when processing the file 'E:\GitHub\R4MVC\src\R4MvcHostApp\R4MvcHostApp.csproj' with message: The SDK 'Microsoft.NET.Sdk.Web' specified could not be found. E:\GitHub\R4MVC\src\R4MvcHostApp\R4MvcHostApp.csproj}

artiomchi commented 7 years ago

Unfortunately, this pull request can't be merged, since it's

Also, the problem you're describing should be an issue. I've created #57 so that we can track this now

artiomchi commented 7 years ago

P.S. I've created a branch with the .net core 2.0 upgrade. I'm not putting it into my main dev branch (or making a pull request for it here) for now, until we can find a workaround for the MSBuild issues.

https://github.com/artiomchi/R4MVC/tree/feature/dotnet_core_2

valeriob commented 7 years ago

Hi @artiomchi ofc it does not work, i was just starting a conversation 😄 Thanks for tracking the issue and starting the 2.0 branch

kevinkuszyk commented 7 years ago

@valeriob thanks for opening this PR early to start the conversation.

A couple of questions:

  1. Do we need this now? Will it break users who want to use R4MVC with MVC Core 1.x?
  2. Can we / should we cross compile for both netstandard1.6 and netstandard2.0? (My understanding is we should target the lowest netstandard possible).

I suspect the build errors may be caused by the build agent not having the .net core 2 preview bits installed. If you edit the appveyor.yml file you can switch to an image that has the preview bits installed when it's available.

artiomchi commented 7 years ago

@kevinkuszyk At least for the time being, we should cross compile against both (currently we're building against netstandard1.6 and .net 4.5.1)

ASP.NET Core 1.0/1.1 are based on Net Standard 1.6 ASP.NET Core 2.0 will be based on Net Standard 2.0

2.0 isn't an upgrade to 1.6, same as 1.6 is a to 1.5, starting from 2.0 they made some changes to the dependencies (from what I understood in the early docs), so it would be best to keep both.

valeriob commented 7 years ago

@kevinkuszyk i do not think you can break anyone since it's not released 😄 I targeted netstandard2.0 because aspnetcore 2.0 is based on netstandard2.0, so i was just trying to compile. You're right, it should target the lowest possible to support you audience.

The real question is about aspnetcore 1.1 vs 2.0, if you can target both easy very well, other way i would go with 2.0. I think that early adopters of aspnetcore 1.1 wont remain there for long after 2.0 comes out.

artiomchi commented 7 years ago

@kevinkuszyk @valeriob Well.. This is an interesting point, actually, and the answer isn't necessarily clear-cut.

Say, we make an assumption that if someone runs on .NET Core 1.1, they'll be using AspNetCore 1.1, and if they're running on .NET Core 2.0, they're using AspNetCore 2.0.

If that's the case, we'll then just multi target the libraries, and apply the right dependencies to the right targets (so one nuget package, the .NET Core 1.1 target references AspNetCore 1.1 and the .NET Core 2.0 will reference AspNetCore 2.0.

Sounds perfect.. Except.. Someone might have a .NET Core 2.0 project, still running AspNetCore 1.1 (you'd say it's silly, but I've seen worse things in the enterprise world, where you can't always update an app already in production, etc). Worse yet, we're also supporting the full .NET framework. When compiled against that, there's no way to know what AspNetCore version they would be running, so there's no way to make sure the right package will be referenced.

The only choice we have is to assume that users will use the latest major version of the AspNetCore framework, and with each major upgrade of said framework, we'll release a major version bump of our lib.

All that considered, we're currently in pre-release stage, so we can have some leeway with this. As such, I suggest that we concentrate on AspNetCore 1.1 and reference it when compiled against .NET Core 1.1 and the full .NET framework. We can also add a .NET Core 2.0 target, referencing AspNetCore. Before the final release, we'll decide which framework will be left in, depending what will be live from Microsoft.

Achieving this is actually quite trivial, and only requires a single file changed - the R4Mvc.csproj to the following:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>netstandard1.6;netstandard2.0;net451</TargetFrameworks>
    <!-- [...] -->
  </PropertyGroup>
  <!-- [...] -->
  <ItemGroup Condition="'$(TargetFramework)'!='netstandard2.0'">
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Core" Version="1.1.3" />
    <PackageReference Include="Microsoft.AspNetCore.Mvc.TagHelpers" Version="1.1.3" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)'=='netstandard2.0'">
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Core" Version="2.0.0-preview2-final" />
    <PackageReference Include="Microsoft.AspNetCore.Mvc.TagHelpers" Version="2.0.0-preview2-final" />
  </ItemGroup>
</Project>

Considering AppVeyor can't compile this anyway, we should park this for the time being. When we do get to it, the changes required to update everything else to .NET Core 2.0 are quite simple (I've managed to get it to work on my branch now, even if it requires a temporary hack to get around the MSBuild issues)

valeriob commented 7 years ago

I think that if you want to support both, you should have a version of r4mvc 1.x to target netstandard1.6 and aspnetcore 1.1 and r4mvc package 2.x target netstandard2.0 and aspnetcore 2.x. If the burden is not high let's do it, but if it must delay the project i say just go with 2.0.

I share my experience and the one of ppl i know. People who use aspnetcore are early adopter and nobody i know would start a project today with aspnetcore 1.1. So existing 1.1 project already have a link generating solution or they did not bother (small projects), and to be honest, who is on 1.1 untill now, it will upgrade to 2.0 when it will rtm. Me and many others want to bring many important long lasting projects to aspnetcore and we are targeting 2.0 (we need to rebuild auth pipleines, signalr is not present in 1.1, etc...). We are willing to test it but pls just give priority to 2.0 branch, we are holding back so much stuff because of this 😄

artiomchi commented 7 years ago

@valeriob I completely understand you. I've got a couple projects already using 2.0 as well because of the changes they introduced. And like I said - I already have a branch of R4MVC that uses AspNetCore 2.0.

The two main issues that prevent us adding 2.0 support to the main pipeline are:

The moment these are sorted, I'll do my best to make sure that 2.0 is supported by R4MVC without side-effects :)

artiomchi commented 7 years ago

I should mention that this issue doesn't affect build time (since the project builds just fine). It will affect the users when they try to run the r4mvc generation on their projects. This process invokes msbuild, and this is where the issue arises. Unless every single user has the same build number of .net core 2.0 preview in the same path - the process will fail :(

P.S. sure, they could add an environment variable with the right path on their machine, but that's terribly clunky

valeriob commented 7 years ago

thanks @artiomchi !

artiomchi commented 7 years ago

By the way, I suggest we close this PR, and follow #58 instead