opserver / Opserver

Stack Exchange's Monitoring System
https://opserver.github.io/Opserver/
MIT License
4.51k stars 827 forks source link

Remove dependency of Microsoft.Net.Compilers NuGet Package #271

Closed PropelledDog closed 6 years ago

PropelledDog commented 7 years ago

When I build Opserver with the C# 6.0 Roslyn compiler, compiler executables are generated in the bin/roslyn folder. I have narrowed it down that it must be because of the NuGet package: Microsoft.Net.Compilers, but removing this package breaks Opserver and generates errors in compiler generated files.

How can I build Opserver without these Roslyn Compiler executables being generated? References: https://stackoverflow.com/questions/45308445/build-with-roslyn-but-leave-the-compile-at-runtime-executables-at-the-door

NickCraver commented 7 years ago

Why do you want to remove this dependency? It's there so views can be properly compiled and use C# 6/7 features.

NickCraver commented 7 years ago

Why do you want to remove this dependency? It's there so views can be properly compiled and use C# 6/7 features.

PropelledDog commented 7 years ago

I am porting Opserver over into an application that can benefit from it. The software allows for hosting of such a project, but it does not allow executables to be uploaded and/or run at runtime due to security reasons.

I have been looking into this NuGet package and from what I have found, it is used to build on environments that don't have C#6.0 on them (i.e. it ignores the compiler version installed on the environment and uses the one bundled from within the NuGet package), but my environment has all the proper set up.

Reference: https://stackoverflow.com/questions/34533613/what-is-the-purpose-of-microsoft-net-compilers

NickCraver commented 7 years ago

@dhoynoski The problem is to support your environment use case, I have to hose everyone not in the same boat. I plan to drop the package in the ASP.NET Core move, but that's still a little ways off (I'm porting lib dependencies now). Before then, people get unexpected runtime errors and don't know why and generally broken builds all over randomly isn't an ideal situation.

PropelledDog commented 7 years ago

@NickCraver Why do you need this NuGet package specifically? If I am compiling with Visual Studio 2017 with C#7.0 compiler, why can it not even build in Visual Studios without this NuGet package? I haven't even gotten to the porting part. I just can't have these executables attached.

NickCraver commented 7 years ago

@dhoynoski Because views can be compiled, injected, etc. on the server at runtime. This will be more important with plugins later as well. That's why they're deployed in the bin folder at all.

PropelledDog commented 7 years ago

@NickCraver Is there a way to compile that not at runtime, but everything at compile time (i.e. to avoid deploying these compiler executables)?

NickCraver commented 7 years ago

@dhoynoski Not without dropping support for users. I don't have stats about who's compiling with which tooling (nor in the documentation in general that great here), so I'm very wary of any such change. If you want to force view compilation on a fork though, it'd likely be okay provided you're n the latest build tooling.

PropelledDog commented 7 years ago

@NickCraver What changes would I have to make to my local copy of Opserver to force total View compilation at compile time instead of runtime? Are there compiler specific flags I am missing to accomplish this?

NickCraver commented 7 years ago

@dhoynoski The base views are already compiled, so if injection isn't needed (most don't), then remove the CodeDom and compilers packages and remove those lines from the .csproj (very top, very bottom of the file). NuGet used to handle this removal but no longer does in v4+.

PropelledDog commented 7 years ago

I removed the CodeDom and Compilers package and then modified the .csproj files for both the Operserver and Opserver.Core, but compiling with Visual Studios c#7.0 compiler still won't build (it also won't build if I just compiler with the c#6.0 compiler).

I removed the following lines from both the Opserver.csproj and Opserver.Core.csproj:

At the top:

<Import Project= "..\packages\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.1.0.4\build\net45\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.props" Condition="Exists('..\packages\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.1.0.4\build\net45\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.props')" />

<Import Project="..\packages\Microsoft.Net.Compilers.2.1.0\build\Microsoft.Net.Compilers.props" Condition="Exists('..\packages\Microsoft.Net.Compilers.2.1.0\build\Microsoft.Net.Compilers.props')" />

At the bottom:

<Error Condition= "!Exists('..\packages\Microsoft.Net.Compilers.2.1.0\build\Microsoft.Net.Compilers.props')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\Microsoft.Net.Compilers.2.1.0\build\Microsoft.Net.Compilers.props'))" />

<Error Condition= "!Exists('..\packages\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.1.0.4\build\net45\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.props')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.1.0.4\build\net45\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.props'))" />

These are the only lines I removed and nothing more. Did I miss something? Did I do something wrong?

NickCraver commented 7 years ago

@dhoynoski What does "won't build" mean? Please give me error messages...

PropelledDog commented 7 years ago

I just noticed, NuGet removes those lines automatically actually if you do it through Visual Studios.

When I click build, the following errors are produced:

1.) Severity Code Description Project File Line Suppression State Error CS1525 Invalid expression term ',' Opserver c:\Users\dhoynoski\AppData\Local\Temp\Temporary ASP.NET Files\temp\36c6a5bc\1575aeed\App_Code.zcypk1cv.1.cs 1 Active

2.) Severity Code Description Project File Line Suppression State Error CS1525 Invalid expression term '.' Opserver c:\Users\dhoynoski\Desktop\Opserver Testing\Opserver\Opserver\App_Code\Helpers.cshtml 1 Active

3.) Severity Code Description Project File Line Suppression State Error CS1003 Syntax error, ':' expected Opserver c:\Users\dhoynoski\Desktop\Opserver Testing\Opserver\Opserver\App_Code\Helpers.cshtml 1 Active

4.) Severity Code Description Project File Line Suppression State Error CS1002 ; expected Opserver c:\Users\dhoynoski\Desktop\Opserver Testing\Opserver\Opserver\App_Code\Helpers.cshtml 1 Active

5.) Severity Code Description Project File Line Suppression State Error CS1525 Invalid expression term ')' Opserver c:\Users\dhoynoski\Desktop\Opserver Testing\Opserver\Opserver\App_Code\Helpers.cshtml 1 Active

PropelledDog commented 7 years ago

@NickCraver Are you able to force compilation all at compile-time, instead of at runtime?

NickCraver commented 7 years ago

Ah the App_Code folder special snowflake...yeah I'm unaware of a way around this until MVC 6 (ASP.NET Core) migration. You're welcome to try everything to get that working, but AFAIK it's not a supported scenario and it's why the NuGet packages exist.

PropelledDog commented 7 years ago

Okay, I just want to get my thinking straight here on the reason why I basically can't avoid using this NuGet package. The reason why we need these two NuGet packages, is, in your own words, because:

"Because views can be compiled, injected, etc. on the server at runtime."

However, it wouldn't make any sense if you couldn't just compile everything at compile time, like any other programming language, so there has to be another reason why Opserver must use this NuGet package. I am currently using .NET Framework 4.7 with the C#7.0 compiler, so I do, in fact, have access to all the features of C#7.0 and below without having to resort to using the NuGet package Microsoft.Net.Compilers (this is the reason why is exists in the first place); I am able to create projects targeting these versions without this NuGet packages.

Where in the actual code are the dependencies and why are they there? With me being on a platform that supports all versions of C#, I don't understand why I have to depend on another package for this project, when I have never had to without C#7.0/6.0 applications.

Lastly, what does "AFAIK" stand for?

NickCraver commented 7 years ago

@dhoynoski Are you using App_Code folder with helpers with C# > 5 in ASP.NET MVC 5 projects? It's special, and AFAIK (as far as I know) there is no workaround for this outside the packages. The "solution" is to use a newer version of ASP.NET (which is ASP.NET Core). We'll get there eventually, but not anytime soon. There's a ton to do for such a move and all dependencies are not in place yet.

PropelledDog commented 7 years ago

Opserver is currently using App_Code with Helpers and Opserver and Opserver.Core is written in C# version 6.0. I checked the version of MVC and it adds the following assembly to web.config:

<add assembly="System.Web.Mvc, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31BF3856AD364E35" />

But it also is adding in the following: <dependentAssembly> <assemblyIdentity name="System.Web.Mvc" publicKeyToken="31bf3856ad364e35" /> <bindingRedirect oldVersion="0.0.0.0-5.2.3.0" newVersion="5.2.3.0" /> </dependentAssembly>

I am not completely sure if it is using version 4 or 5, but the references show System.Web.MVC being used on version 5.2.3.0.

Can you explain exactly what is going on? Are there syntactical implementation differences that makes using the C#7.0 compiler built into Visual Studios 2017 with .NET Framework 4.7 not able to parse the App_Code files accordingly? What are the fundamental differences between the compiler supplied in the NuGet package and the one commercially available on Microsoft's website? I understand it doesn't work, I just want to know WHY it doesn't. The compiler is giving syntax errors (i.e. invalid expression(s)) in these App_Code files, so this is why I ask.

Lastly, I'll close the issue once I clear up this misunderstanding, but if I was to hypothetically downgrade Opserver and Opserver.Core to only be using C#5.0 feature sets and below (i.e. manually replace C#6.0 features with their equivalent C#5.0 implementation), could I theoretically then compile without these NuGet packages?

NickCraver commented 7 years ago

@dhoynoski That's just how the ASP.NET framework works, it uses another compiler (built-in) which isn't C# 6+ aware. So you have to replace the compiler, and that's what the package does.

Yes you can downgrade everything to C# 5, but please be aware, the next step of evolution for Opserver is ASP.NET Core, so you'll definitely be forked so heavily that it's an EOL fork. One of the many changes of ASP.NET Core means this particular thing won't be an issue anymore, as there is no split of the compilation approaches and we can cleanly compiler the entire application. Additionally, these pieces in helpers can be tag helpers instead and much cleaner, so the specific post-dynamic issue is just gone entirely.

I'm working now on all the libraries used in .NET Core to make this possible (Opserver is just one project I'm responsible for, that's why replies are slower than I'd like here).

ajaysadyaldoeacc commented 2 years ago

This error usually comes when you upgrade the below libraries by nuget

Microsoft.CodeDom.Providers.DotNetCompilerPlatform

Microsoft.Net.Compilers

Guys, If your project is not loading due to this error. just go to that project location by explorer and delete the obj folder manually.

Then right-click on the project and RELOAD AGAIN.