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

[Discussion] Microsoft.AspNetCore.Mvc.ViewCompilation 1.1.0-msbuild3-update1 released #71

Closed
pranavkm opened this Issue Jan 27, 2017 · 22 comments

Comments

Projects
None yet
7 participants
@pranavkm
Member

pranavkm commented Jan 27, 2017

Discussion for aspnet/Announcements#217

Today, we released a version of Microsoft.AspNetCore.Mvc.Razor.ViewCompilation that supports MsBuild based projects as part of the January 27th RC release of Visual Studio 2017 (https://blogs.msdn.microsoft.com/visualstudio/2017/01/26/update-to-visual-studio-2017-release-candidate).

If you were previously referencing Microsoft.AspNetCore.Mvc.Razor.ViewCompilation.Design in your project.json based applications that was migrated to csproj, your application might have a Microsoft.AspNetCore.Mvc.Razor.ViewCompilation v1.1.0-msbuild3-final package. The update1 release of this package addresses some significant pain points discovered in the 1.1.0-msbuild3-final build of the package. See https://github.com/aspnet/MvcPrecompilation/issues?q=is%3Aissue+milestone%3A1.1.0-msbuild3-update1+is%3Aclosed for a list of the fixed issues.

To enable view compilation in a csproj based application,

  1. Ensure your application is referencing the Microsoft.AspNetCore.Mvc.Razor.ViewCompilation package. For e.g. your csproj would say:
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Razor.ViewCompilation" Version="1.1.0-msbuild3-update" PrivateAssets="All" />

  2. Set the MvcRazorCompileViewsOnPublish property in the project enabling view compilation on publish:

<PropertyGroup>
  ...
  <MvcRazorCompileOnPublish>true</MvcRazorCompileOnPublish>
  ...
</PropertyGroup>

Use #71 for questions and further discussion.

@ctolkien

This comment has been minimized.

Show comment
Hide comment
@ctolkien

ctolkien Jan 28, 2017

Just to clarify, if we're still on project.json we should skip this for now?

Just to clarify, if we're still on project.json we should skip this for now?

@guardrex

This comment has been minimized.

Show comment
Hide comment
@guardrex

guardrex Jan 28, 2017

Oh ... I see the problem. @pranavkm You said in the Announcement to use ...

<MvcRazorCompileViewsOnPublish>true</MvcRazorCompileViewsOnPublish>

but it looks like it should be ...

<MvcRazorCompileOnPublish>true</MvcRazorCompileOnPublish>

I have it working with nightly bits now using that property.
☝️ Produces the DLL. However, it's not finding the views ...

InvalidOperationException: The view 'Index' was not found. The following locations were searched:

🐀 🍔 Roasted rat burgers! 🐀 🍔 ... I fell prey to the case-sensitive view naming issue. When I renamed my index.cshtml to Index.cshtml, it worked. I'm good. Cross-reference: #37

guardrex commented Jan 28, 2017

Oh ... I see the problem. @pranavkm You said in the Announcement to use ...

<MvcRazorCompileViewsOnPublish>true</MvcRazorCompileViewsOnPublish>

but it looks like it should be ...

<MvcRazorCompileOnPublish>true</MvcRazorCompileOnPublish>

I have it working with nightly bits now using that property.
☝️ Produces the DLL. However, it's not finding the views ...

InvalidOperationException: The view 'Index' was not found. The following locations were searched:

🐀 🍔 Roasted rat burgers! 🐀 🍔 ... I fell prey to the case-sensitive view naming issue. When I renamed my index.cshtml to Index.cshtml, it worked. I'm good. Cross-reference: #37

@pranavkm

This comment has been minimized.

Show comment
Hide comment
@pranavkm

pranavkm Jan 28, 2017

Member

@ctolkien - yes, the 1.1.0-preview4-final versions of Microsoft.AspNetCore.Mvc.Razor.ViewCompilation.Design and Microsoft.AspNetCore.Mvc.Razor.ViewCompilation.Tools are the most recent, and possibly the final, set of packages targeting project.json. The MSBuild based tooling is generally incompatible with project.json based projects and vice-versa.

@guardrex, thanks for noticing that! Fixed the announcement.

Member

pranavkm commented Jan 28, 2017

@ctolkien - yes, the 1.1.0-preview4-final versions of Microsoft.AspNetCore.Mvc.Razor.ViewCompilation.Design and Microsoft.AspNetCore.Mvc.Razor.ViewCompilation.Tools are the most recent, and possibly the final, set of packages targeting project.json. The MSBuild based tooling is generally incompatible with project.json based projects and vice-versa.

@guardrex, thanks for noticing that! Fixed the announcement.

@guylando

This comment has been minimized.

Show comment
Hide comment
@guylando

guylando Feb 4, 2017

Hi,
The Microsoft.AspNetCore.Mvc.Razor.Precompilation version 1.1.0-msbuild3-update1 uses version 1.1.0.0 of Microsoft.AspNetCore.Mvc.Razor, Microsoft.AspNetCore.Mvc.Core, Microsoft.AspNetCore.Mvc.Razor.Host.
However in my solution I upgraded those packages to 1.1.1 after the security bulletin which was published.
So I had error regarding not finding 1.1.0.0. I solved them by adding App.Config file to my project and writing there:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
<runtime>
  <!-- Add those redirects here and it will be automatically integrated into exe config and then copied to the Microsoft.AspNetCore.Mvc.Razor.Precompilation config -->
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="Microsoft.AspNetCore.Mvc.Razor" publicKeyToken="adb9793829ddae60" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.1.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="Microsoft.AspNetCore.Mvc.Core" publicKeyToken="adb9793829ddae60" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.1.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="Microsoft.AspNetCore.Mvc.Razor.Host" publicKeyToken="adb9793829ddae60" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.1.0" />
      </dependentAssembly>
    </assemblyBinding>
</runtime>
</configuration>

HOWEVER: Now when publishing, Microsoft.AspNetCore.Mvc.Razor.ViewCompilation.exe is throwing the following errors:
EXEC(6,49): Error CS0826: No best type found for implicitly-typed array
EXEC(6,49): Error CS1503: Argument 1: cannot convert from '?[]' to 'System.Collections.Generic.IReadOnlyList<Microsoft.AspNetCore.Mvc.ApplicationParts.ViewInfo>'

So my guess is that those dependencies changed their code from 1.1.0 to 1.1.1 in a way that currently breaks Microsoft.AspNetCore.Mvc.Razor.ViewCompilation.exe code.

So the easiest solution would be if you can publish a patch to version 1.1.0-msbuild3-update1 which is compiled with the security bulletin versions 1.1.1 of the mentioned libraries.

Or tell me where I can find source code of version 1.1.0-msbuild3-update1 and I will recompile it myself with appropriate references and code fixes.

P.S: I tried also using the newer prerelease versions of Microsoft.AspNetCore.Mvc.Razor.ViewCompilation but it breaks my solution because my solution currently uses versions 1.1.0, 1.1.1 of all libraries.

guylando commented Feb 4, 2017

Hi,
The Microsoft.AspNetCore.Mvc.Razor.Precompilation version 1.1.0-msbuild3-update1 uses version 1.1.0.0 of Microsoft.AspNetCore.Mvc.Razor, Microsoft.AspNetCore.Mvc.Core, Microsoft.AspNetCore.Mvc.Razor.Host.
However in my solution I upgraded those packages to 1.1.1 after the security bulletin which was published.
So I had error regarding not finding 1.1.0.0. I solved them by adding App.Config file to my project and writing there:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
<runtime>
  <!-- Add those redirects here and it will be automatically integrated into exe config and then copied to the Microsoft.AspNetCore.Mvc.Razor.Precompilation config -->
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="Microsoft.AspNetCore.Mvc.Razor" publicKeyToken="adb9793829ddae60" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.1.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="Microsoft.AspNetCore.Mvc.Core" publicKeyToken="adb9793829ddae60" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.1.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="Microsoft.AspNetCore.Mvc.Razor.Host" publicKeyToken="adb9793829ddae60" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-1.1.1.0" newVersion="1.1.1.0" />
      </dependentAssembly>
    </assemblyBinding>
</runtime>
</configuration>

HOWEVER: Now when publishing, Microsoft.AspNetCore.Mvc.Razor.ViewCompilation.exe is throwing the following errors:
EXEC(6,49): Error CS0826: No best type found for implicitly-typed array
EXEC(6,49): Error CS1503: Argument 1: cannot convert from '?[]' to 'System.Collections.Generic.IReadOnlyList<Microsoft.AspNetCore.Mvc.ApplicationParts.ViewInfo>'

So my guess is that those dependencies changed their code from 1.1.0 to 1.1.1 in a way that currently breaks Microsoft.AspNetCore.Mvc.Razor.ViewCompilation.exe code.

So the easiest solution would be if you can publish a patch to version 1.1.0-msbuild3-update1 which is compiled with the security bulletin versions 1.1.1 of the mentioned libraries.

Or tell me where I can find source code of version 1.1.0-msbuild3-update1 and I will recompile it myself with appropriate references and code fixes.

P.S: I tried also using the newer prerelease versions of Microsoft.AspNetCore.Mvc.Razor.ViewCompilation but it breaks my solution because my solution currently uses versions 1.1.0, 1.1.1 of all libraries.

@awcab

This comment has been minimized.

Show comment
Hide comment
@awcab

awcab Feb 25, 2017

Is it possible to have this enabled when just building and not deploying?

awcab commented Feb 25, 2017

Is it possible to have this enabled when just building and not deploying?

@guardrex

This comment has been minimized.

Show comment
Hide comment
@guardrex

guardrex Feb 25, 2017

@awcab If by "deploying" you mean "publishing," a build automatically accompanies publishing. Do you mean "Debug" vs. "Release"? If so, you could set a Condition to control it ...

<MvcRazorCompileOnPublish Condition="'$(Configuration)'=='Debug'">true</MvcRazorCompileOnPublish>

guardrex commented Feb 25, 2017

@awcab If by "deploying" you mean "publishing," a build automatically accompanies publishing. Do you mean "Debug" vs. "Release"? If so, you could set a Condition to control it ...

<MvcRazorCompileOnPublish Condition="'$(Configuration)'=='Debug'">true</MvcRazorCompileOnPublish>
@awcab

This comment has been minimized.

Show comment
Hide comment
@awcab

awcab Feb 25, 2017

@guardrex I did mean publish. Currently I'm only seeing compile errors on package or publish and not just when I build. Maybe I'm missing another setting?

awcab commented Feb 25, 2017

@guardrex I did mean publish. Currently I'm only seeing compile errors on package or publish and not just when I build. Maybe I'm missing another setting?

@guardrex

This comment has been minimized.

Show comment
Hide comment
@guardrex

guardrex Feb 25, 2017

@awcab Ah ... I see. It sounds like something else has gone wrong then. I suggest opening a new issue in this repo with the details about the compile errors you're seeing. If you can setup a small repro to go with the issue, that's often helpful. If setting up a repro project would be very time-consuming, then just describe the situation ... they might not need a repro to help you solve the problem(s).

WRT controlling it on build vs. publish, I'm not aware of how that's done (idk if there is a property you can check). AFAIK publish automatically triggers a build, so there wouldn't be a simple way to distinguish between the two. One of the dev team cats will probably jump in here and tell us if there's a way.

@awcab Ah ... I see. It sounds like something else has gone wrong then. I suggest opening a new issue in this repo with the details about the compile errors you're seeing. If you can setup a small repro to go with the issue, that's often helpful. If setting up a repro project would be very time-consuming, then just describe the situation ... they might not need a repro to help you solve the problem(s).

WRT controlling it on build vs. publish, I'm not aware of how that's done (idk if there is a property you can check). AFAIK publish automatically triggers a build, so there wouldn't be a simple way to distinguish between the two. One of the dev team cats will probably jump in here and tell us if there's a way.

@awcab

This comment has been minimized.

Show comment
Hide comment
@awcab

awcab Feb 25, 2017

@guardrex Just to be clear I'm talking about compile errors in my Views. I want compile errors to fail the build and not have to go through a package/publish step. Currently, publish does trigger a build, but I would like to see these just during a build without a publish. This is the way it happened with pre-Core Asp MVC.

awcab commented Feb 25, 2017

@guardrex Just to be clear I'm talking about compile errors in my Views. I want compile errors to fail the build and not have to go through a package/publish step. Currently, publish does trigger a build, but I would like to see these just during a build without a publish. This is the way it happened with pre-Core Asp MVC.

@guardrex

This comment has been minimized.

Show comment
Hide comment
@guardrex

guardrex Feb 25, 2017

@awcab See if you can chat with @dasMulli or some of the other devs on Slack. It might be quicker to get assistance given that its the weekend. The self-signup is via: http://tattoocoder.com/aspnet-slack-sign-up/

@awcab See if you can chat with @dasMulli or some of the other devs on Slack. It might be quicker to get assistance given that its the weekend. The self-signup is via: http://tattoocoder.com/aspnet-slack-sign-up/

@pranavkm

This comment has been minimized.

Show comment
Hide comment
@pranavkm

pranavkm Feb 25, 2017

Member

@awcab does #83 (comment) address your scenario?

Member

pranavkm commented Feb 25, 2017

@awcab does #83 (comment) address your scenario?

@dasMulli

This comment has been minimized.

Show comment
Hide comment
@dasMulli

dasMulli Feb 25, 2017

So @guardrex asked to make it work for build & publish.. came up with that:

    <Target Name="SetMvcRazorOutputPath">
      <PropertyGroup>
        <MvcRazorOutputPath>$(OutputPath)</MvcRazorOutputPath>
      </PropertyGroup>
    </Target>

    <Target Name="_MvcRazorPrecompileOnBuild"
            DependsOnTargets="SetMvcRazorOutputPath;MvcRazorPrecompile"
            AfterTargets="Build" 
            Condition=" '$(IsCrossTargetingBuild)' != 'true' " />

     <Target Name="IncludePrecompiledViewsInPublishOutput"
            DependsOnTargets="_MvcRazorPrecompileOnBuild"
            BeforeTargets="PrepareForPublish"
            Condition=" '$(IsCrossTargetingBuild)' != 'true' ">
      <ItemGroup>
        <_PrecompiledViewsOutput Include="$(MvcRazorOutputPath)$(MSBuildProjectName).PrecompiledViews.dll" />
        <_PrecompiledViewsOutput Include="$(MvcRazorOutputPath)$(MSBuildProjectName).PrecompiledViews.pdb" />
        <ContentWithTargetPath Include="@(_PrecompiledViewsOutput->'%(FullPath)')"
            RelativePath="%(_PrecompiledViewsOutput.Identity)" 
            TargetPath="%(_PrecompiledViewsOutput.Filename)%(_PrecompiledViewsOutput.Extension)" 
            CopyToPublishDirectory="PreserveNewest" />
      </ItemGroup>
    </Target>

So @guardrex asked to make it work for build & publish.. came up with that:

    <Target Name="SetMvcRazorOutputPath">
      <PropertyGroup>
        <MvcRazorOutputPath>$(OutputPath)</MvcRazorOutputPath>
      </PropertyGroup>
    </Target>

    <Target Name="_MvcRazorPrecompileOnBuild"
            DependsOnTargets="SetMvcRazorOutputPath;MvcRazorPrecompile"
            AfterTargets="Build" 
            Condition=" '$(IsCrossTargetingBuild)' != 'true' " />

     <Target Name="IncludePrecompiledViewsInPublishOutput"
            DependsOnTargets="_MvcRazorPrecompileOnBuild"
            BeforeTargets="PrepareForPublish"
            Condition=" '$(IsCrossTargetingBuild)' != 'true' ">
      <ItemGroup>
        <_PrecompiledViewsOutput Include="$(MvcRazorOutputPath)$(MSBuildProjectName).PrecompiledViews.dll" />
        <_PrecompiledViewsOutput Include="$(MvcRazorOutputPath)$(MSBuildProjectName).PrecompiledViews.pdb" />
        <ContentWithTargetPath Include="@(_PrecompiledViewsOutput->'%(FullPath)')"
            RelativePath="%(_PrecompiledViewsOutput.Identity)" 
            TargetPath="%(_PrecompiledViewsOutput.Filename)%(_PrecompiledViewsOutput.Extension)" 
            CopyToPublishDirectory="PreserveNewest" />
      </ItemGroup>
    </Target>
@pranavkm

This comment has been minimized.

Show comment
Hide comment
@pranavkm

pranavkm Feb 25, 2017

Member

You could possibly replace the second target with the MvcRazorCompileOnPublish property. The one issue with enabling it on build and leaving the precompiled view around is that you lose the edit-cshtml-and-refresh-the-browser experience. If that's something you want to keep, remove the binary from the output path after it's compiled. This should make the behavior analogous to the MvcBuildViews feature from Mvc <= 5.

Member

pranavkm commented Feb 25, 2017

You could possibly replace the second target with the MvcRazorCompileOnPublish property. The one issue with enabling it on build and leaving the precompiled view around is that you lose the edit-cshtml-and-refresh-the-browser experience. If that's something you want to keep, remove the binary from the output path after it's compiled. This should make the behavior analogous to the MvcBuildViews feature from Mvc <= 5.

@dasMulli

This comment has been minimized.

Show comment
Hide comment
@dasMulli

dasMulli Feb 25, 2017

Or replace this from my example

<MvcRazorOutputPath>$(OutputPath)</MvcRazorOutputPath>

with

<MvcRazorOutputPath>$(IntermediateOutputPath)</MvcRazorOutputPath>

That way, the views will be checked on every build but only used when published (since the dll isn't dropped next to the main dll on build).

dasMulli commented Feb 25, 2017

Or replace this from my example

<MvcRazorOutputPath>$(OutputPath)</MvcRazorOutputPath>

with

<MvcRazorOutputPath>$(IntermediateOutputPath)</MvcRazorOutputPath>

That way, the views will be checked on every build but only used when published (since the dll isn't dropped next to the main dll on build).

@guardrex

This comment has been minimized.

Show comment
Hide comment
@guardrex

guardrex Feb 25, 2017

I want compile errors to fail the build

Would it be cleaner to fail a unit test rather than inject that into the project file? If so, what would a unit testing approach look like? (just curious)

guardrex commented Feb 25, 2017

I want compile errors to fail the build

Would it be cleaner to fail a unit test rather than inject that into the project file? If so, what would a unit testing approach look like? (just curious)

@awcab

This comment has been minimized.

Show comment
Hide comment
@awcab

awcab Feb 25, 2017

@pranavkm It would be nice to keep edit and refresh. However, this is part of a service fabric application and so far I lost any sort of edit and refresh, even js and html. If anyone knows how to get that working that would be fantastic.

awcab commented Feb 25, 2017

@pranavkm It would be nice to keep edit and refresh. However, this is part of a service fabric application and so far I lost any sort of edit and refresh, even js and html. If anyone knows how to get that working that would be fantastic.

@awcab

This comment has been minimized.

Show comment
Hide comment
@awcab

awcab Feb 25, 2017

@guardrex IMO unit tests should be used to check runtime behavior. Earlier detection is always better.

awcab commented Feb 25, 2017

@guardrex IMO unit tests should be used to check runtime behavior. Earlier detection is always better.

@guardrex

This comment has been minimized.

Show comment
Hide comment
@guardrex

guardrex Feb 25, 2017

@awcab Just curious: As a feature request, are you hoping for something simple like <MvcRazorCompileOnBuild>? ... that applies notwithstanding how MvcRazorCompileOnPublish is set and notwithstanding the loss of edit-cshtml-and-refresh-the-browser experience. [Things would get a little hairy if the reverse were true ... build is set to false but publish is set to true.]

guardrex commented Feb 25, 2017

@awcab Just curious: As a feature request, are you hoping for something simple like <MvcRazorCompileOnBuild>? ... that applies notwithstanding how MvcRazorCompileOnPublish is set and notwithstanding the loss of edit-cshtml-and-refresh-the-browser experience. [Things would get a little hairy if the reverse were true ... build is set to false but publish is set to true.]

@awcab

This comment has been minimized.

Show comment
Hide comment
@awcab

awcab Feb 26, 2017

@guardrex Yes, I think that's exactly what I would like. Wasn't the the previous default behavior?

awcab commented Feb 26, 2017

@guardrex Yes, I think that's exactly what I would like. Wasn't the the previous default behavior?

@guardrex

This comment has been minimized.

Show comment
Hide comment
@guardrex

guardrex Feb 26, 2017

@awcab idk on the previous behavior. They'll say if they want that in a feature request issue opened separately. What @dasMulli and @pranavkm show is wonderful, but it seems kind'a heavy for such a simple requirement. I just wanted to clarify what would be the simplest public feature from a UE perspective in case this becomes a real ask (i.e., a real feature request).

@awcab idk on the previous behavior. They'll say if they want that in a feature request issue opened separately. What @dasMulli and @pranavkm show is wonderful, but it seems kind'a heavy for such a simple requirement. I just wanted to clarify what would be the simplest public feature from a UE perspective in case this becomes a real ask (i.e., a real feature request).

@awcab

This comment has been minimized.

Show comment
Hide comment
@awcab

awcab Feb 27, 2017

Just FYI, this works well except that compile errors shown in the error list are not hotlinked to the source. I realize this is probably a VS issue.

awcab commented Feb 27, 2017

Just FYI, this works well except that compile errors shown in the error list are not hotlinked to the source. I realize this is probably a VS issue.

@Eilon

This comment has been minimized.

Show comment
Hide comment
@Eilon

Eilon Jun 9, 2017

Member

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.

Member

Eilon commented Jun 9, 2017

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.

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