New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

#7713: Dynamic C# Compilation, Static and Dynamic Razor Compilation with Roslyn (1.10.x) #8103

Merged
merged 31 commits into from Sep 19, 2018

Conversation

Projects
None yet
2 participants
@BenedekFarkas
Member

BenedekFarkas commented Sep 4, 2018

The PR looks pretty huge, but most of the changes are repetitive across projects. Let's see what we started from, what we wanted to achieve and how we did it.

Important: If you're using MSBuild directly, make sure that your PATH variable contains the path to the latest available MSBuild, as it needs a fairly recent version. If you have Visual Studio 2017 installed, then it's usually C:\Program Files (x86)\Microsoft Visual Studio\2017\Professional\MSBuild\15.0\Bin. Otherwise, e.g. on a build server, you have to install the build tools separately. Here's a link to the 2017 version.

Initial problems

  • Razor files could not be compiled in advance, i.e. errors that could be prevented with static code analysis can only be discovered by actually running the code (or looking at it 馃槃).
  • Dynamic Compilation was using the now deprecated compiler service (pre-Roslyn), which means that it doesn't support C# language features above version 5. This meant that while you can write e.g. C# 6 (or even 7.3 with the latest version) code in Visual Studio (and you can compile it manually), Dynamic Compilation becomes useless. Same goes for Razor code (which has issues with IntelliSense in this case as well).

What do we want?

  • To use the latest available C# language version in C# and Razor files and have IntelliSense support it.
  • To have Dynamic Compilation and the ASP.NET compiler be able to handle the above.

A step in the right direction

An earlier iteration of these changes was already merged to 1.10.x and dev. It used two NuGet packages:

The problem with this was an "integration boom", where the configuration for Dynamic Compilation and Static Razor Compilation collided: The latter uses the ASP.NET Compiler, which (due to the configuration necessary for Razor IntelliSense) by default is looking for Roslyn in the extension's bin folder. However, Roslyn was not there (it was only included for Orchard.Web, because that's the starting point of Dynamic Compilation), because it didn't make sense to include ~16 MBs of files for every extension (a lot of compile-time I/O and deployment packages get bloated, so it would be a mess). The issue was that the DotNetCompilerPlatform package did not have a configuration option to define the path to the Roslyn files. I submitted an issue then a PR to fix that, but it was not accepted, then some time later the package's developers implemented it a bit differently (and their solution is actually better). After that we just needed a new release for the package (which took quite some time), so let's jump back to present time...

What changed this time?

Version 2.0.0 of the DotNetCompilerPlatform was shipped and now it actually includes the Roslyn package (version 2.8.2). This means that C# 7.3 is supported (but there's already a newer release - 2.9.0 - that improves C# 7.3 compilation).

So the changes in the PR are the following:

  • A few minor fixes in various projects (mostly incorrect references).
  • Orchard.proj:
    • The Compile build task makes use of the MvcBuildViews parameter of MSBuild.
    • Added a separate build task (BuildViews) for that Compiles with "MvcBuildViews" enabled ("true" means that Razor compilation will happen as well).
  • Every csproj file (except for the ones that belong to the Azure solution): <LangVersion>latest</LangVersion> added to both the Debug and Release build targets, so VS and MSBuild allows the usage of the latest minor version of C# (e.g. VS 2017 to allow 7.3 instead of 7.0).
  • Every extension with at least one Razor file:
    • packages.config: Microsoft.CodeDom.Providers.DotNetCompilerPlatform version upgrade.
    • csproj: Reference updated according to the change in packages.config.
    • web.config:
      • Added App Setting to tell the ASP.NET compiler where to find the Roslyn tools (required for Razor pre-compilation).
      • Compiler configuration updated to allow the latest C# version (for Razor IntelliSense).
  • Orchard.Web:
    • packages.config: Microsoft.Net.Compilers is no longer included separately.
    • Web.config: Compiler configuration updated to allow the latest C# version (for Dynamic Compilation).
  • Code generation templates updated according to these changes.

Testing

  1. Development

    • Open solution in VS. Build, run, setup.
    • Start Orchard CLI, enable Orchard.CodeGeneration and codegen a module.
    • You'll have a new module that is fully C# 7.3-ready, regardless of building with Dynamic Compilation or in VS. C# code files and Razor files have proper IntelliSense support. Existing modules are also configured for all this, so Orchard contributions benefit from all of it as well.
  2. Deployment to Production with some extras

    • After deploying the application to e.g. an Azure App Service, Razor JIT compilation will still run with Roslyn, so you'll get same results as in development.
    • If you enable the appropriate components in HostComponents.config, you can even codegen a module on the server (e.g. by running Orchard.exe from Kudu) and it will dynamically compile and run. This also means that you can deploy any extension by dropping it into the Modules/Themes folder on the server if you want to test it quickly (basically this is what the Gallery is for, but not every open source module is uploaded there).

Next steps

If this PR is accepted and merged into 1.10.x, then 1.10.x needs to be merged into dev. Then the remaining modules (those that only included in dev) need to be upgraded the same way as these ones.

Future upgrades

All the csproj and web.config files are configured to always use the latest available C# version. This can mean different things depending on whether you want to use Dynamic Compilation or not:

  • Static C# compilation (in VS) will be limited by VS itself. See the Roslyn GitHub Wiki for version pairings.

  • Dynamic Compilation (both C# and Razor) and Static Razor Compilation (BuildViews target or using the MvcBuildViews MSBuild parameter) depends on the version of the Microsoft.CodeDom.Providers.DotNetCompilerPlatform package (or actually, it depends on which version of Microsoft.Net.Compilers (= Roslyn) is included). The aforementioned version pairing document also tells you which package version supports which C# version.

The latter also means that theoretically, you can use an earlier version of Visual Studio and still be able to use recent language versions, e.g. Visual Studio 2015 (and thus the static C# compilation) is limited to C# 6, but Dynamic Compilation can currently handle everything up to 7.3. I haven't tested this, though.

So upgrading this pipeline is really just upgrading the Microsoft.CodeDom.Providers.DotNetCompilerPlatform package and its usage to the new version (which is basically a search and replace in the packages.config, web.config and csproj files).

LombiqTechnologies and others added some commits Jun 12, 2017

Extension csprojs: Setting up CopyLocal behavior for Orchard.Framewor鈥
鈥 and Orchard.Core references

to depend on the MvcBuildViews build parameter (also Orchard.Layouts in some cases)
Extension csprojs: Removing unnecessary round-trip when defining the 鈥
鈥ath for view compilation

(which also broke view compilation for Orchard.Core)
Orchard.Dashboards: Changing System.Web.Mvc version reference in the 鈥
鈥eb.config to be the same as other extensions
Orchard.Templates: Fixing Parts.Shape.cshtml by removing unused code 鈥
鈥hat also causes error during view compilation
Merge branch '1.10.x' into issue/7713
# Conflicts:
#	Orchard.proj
#	src/Orchard.Web/Orchard.Web.csproj
Removing Microsoft.Net.Compilers package and Roslyn tools are now cop鈥
鈥ed from the DotNetCompilerPlatform package
Fixing that Razor IntelliSense and Dynamic Compilation should enforce鈥
鈥 the usage of the latest available C# version: 7.3

Reason: Microsoft.CodeDom.Providers.DotNetCompilerPlatform 2.0.0 is shipped containing Microsoft.Net.Compilers 2.8.2

@BenedekFarkas BenedekFarkas requested a review from sebastienros Sep 4, 2018

@BenedekFarkas BenedekFarkas changed the title from 7713: Dynamic C# Compilation, Static and Dynamic Razor Compilation with Roslyn (1.10.x) to #7713: Dynamic C# Compilation, Static and Dynamic Razor Compilation with Roslyn (1.10.x) Sep 4, 2018

@BenedekFarkas

This comment has been minimized.

Show comment
Hide comment
@BenedekFarkas

BenedekFarkas Sep 5, 2018

Member

Updated my previous comment with the "Testing" and "Future upgrades" section and opening remarks about MSBuild.

Member

BenedekFarkas commented Sep 5, 2018

Updated my previous comment with the "Testing" and "Future upgrades" section and opening remarks about MSBuild.

@BenedekFarkas BenedekFarkas merged commit be80214 into 1.10.x Sep 19, 2018

1 check passed

license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment