Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding the ability to define a custom, local path for the Roslyn tools #25

Closed
BenedekFarkas opened this issue Nov 17, 2017 · 10 comments
Closed

Comments

@BenedekFarkas
Copy link
Contributor

When adding this package to a project trough the NuGet package manager, the csproj is also modified with a build task to copy the tools to the $(ProjectDir\bin\roslyn) folder. If the solution contains a lot of projects (e.g. in our case Orchard CMS has 85 out of the box and most of them require this for Razor IntelliSense and view compilation to support C# 6 and above), then copying the tools the each of them increases the size of the build folder by over 1GB (and it's also a lot of I/O). This can also affect deployment, since these files should be available runtime for the JITter.

Adding a custom parameter (e.g. compilerPath) to override the wired-in (or the global) compiler path would enable the ability to copy the compiler files to only one of the projects (e.g. the web project) and the other ones could reference that, for example like this:

<compiler language="c#;cs;csharp" extension=".cs" type="Microsoft.CodeDom.Providers.DotNetCompilerPlatform.CSharpCodeProvider, Microsoft.CodeDom.Providers.DotNetCompilerPlatform" warningLevel="4" compilerOptions="/langversion:6 /compilerPath:..\..\bin\roslyn"/>

@BenedekFarkas
Copy link
Contributor Author

I've submitted a PR in the meanwhile. To be honest, I'm entirely satisfied with this solution, but I didn't really find a nicer way without making significant changes to the Compiler class and its descendants.

@Jinhuafei
Copy link
Contributor

Thanks @BenedekFarkas . Actually we've already added support for that. Check this commit (d930103) for more details.

@BenedekFarkas
Copy link
Contributor Author

That commit (Support custom TTL from Environment variable) is not quite connected to this. What you might wanted to refer to is the global compiler path setting using an environment variable, but that's not really applicable here either, as their purpose is a bit different. Would you like more elaboration about my suggestion and PR?

@Jinhuafei
Copy link
Contributor

You are right. The commit I'm referring is "global compiler path setting using an environment variable".

When DotNetCompilerPlatform nupkg is installed, it modifies project file which inserts some tasks to copy Roslyn bit unto bin\Roslyn folder at build time. The compilerPath PR won't solve multi-copies of Roslyn problem. Also, how will you publish your web apps? Are they all deployed to same server?

One solution for reducing the publishing content size is to add a msbuild property which indicates if Roslyn bit should be included into the publish content.

@BenedekFarkas
Copy link
Contributor Author

When DotNetCompilerPlatform nupkg is installed, it modifies project file which inserts some tasks to copy Roslyn bit unto bin\Roslyn folder at build time

Yep, I used that for the main (Web) project and it works fine. The complexity comes where we need to use these libraries in other project of the same solution, see below.


The compilerPath PR won't solve multi-copies of Roslyn problem.

I did not explain how I'm using the changes in my PR in detail, so here it is:
What we have is a solution with a web (Orchard.Web) project (which is what you'd select to publish from VS, otherwise we have a custom proj file with specific MSBuild tasks to create a deployment package (which includes those Roslyn files, so it can be used both development- and run-time) that you can deploy with MSDeploy) and ~80 other project and most of those also need to use Roslyn as follows:

  • Orchard.Web uses it for dynamic code compilation (i.e. you modify a C# code file and it rebuilds the project and dependent projects when you hit F5 in the browser after saving the file).
  • Other projects use it as well (referencing the files in Orchard.Web, using the additional parameter to define its local path):
    • To support C# 6 (and above) for Razor IntelliSense.
    • To support C# 6 (and above) for static (build-time) Razor compilation (using the MvcBuildViews parameter). This is what actually causes trouble, because without the PR, aspnet_compiler can't find the Roslyn files.
    • To support C# 6 (and above) JIT Razor compilation during run-time.

What I wanted to achieve with these changes is that only Orchard.Web should actually copy Roslyn files to its bin folder, then all other projects can use that.

@BenedekFarkas
Copy link
Contributor Author

So what do you think?

HongGit pushed a commit that referenced this issue Mar 9, 2018
* CompilationSettings: C# 6 code styling and making CompilationSettingsHelper.CompilerFullPath public

* Compiler: Adding the ability to define a custom compiler path for the current build process

* Compiler: Fixing that the compiler path wasn't set when the override parameter is not present
HongGit pushed a commit that referenced this issue Mar 9, 2018
…25)" (#34)

* Revert "add support for non web proj (#32)"

This reverts commit b9e1127.

* Revert "include commit link (#31)"

This reverts commit 30f97fb.

* Revert "Adding the ability to define a custom (local) path to Roslyn (#25) (#26)"

This reverts commit e79aa12.
@BenedekFarkas
Copy link
Contributor Author

Re: #26 (comment)

I just tested those changes and it's working great!
Do you have an idea when's the next release will be?

@Jinhuafei
Copy link
Contributor

Thanks for the verification. We are testing the signed build internally. I don't have a concrete release date yet, but it should be out this month.

@BenedekFarkas
Copy link
Contributor Author

Any news on a NuGet release?

@Jinhuafei
Copy link
Contributor

@BenedekFarkas we are targeting to next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants