Skip to content

Avoid configuring PreserveCompilationContext=true in 6.0 apps #16656

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 7, 2021
Merged

Avoid configuring PreserveCompilationContext=true in 6.0 apps #16656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6facda2
Avoid configuring PreserveCompilationContext=true in 6.0 apps
pranavkm Mar 31, 2021
2112f3c
Changes per PR comments
pranavkm Apr 7, 2021
cde791b
Move the target to RazorSDK
pranavkm Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.BeforeCommon.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!--
***********************************************************************************************
Microsoft.NET.Sdk.Razor.BeforeCommon.targets

WARNING: DO NOT MODIFY this file unless you are knowledgeable about MSBuild and have
created a backup copy. Incorrect changes to this file will make it
impossible to load or build your projects from the command-line or the IDE.

Copyright (c) .NET Foundation. All rights reserved.
***********************************************************************************************
-->
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<!-- Configure PreserveCompilationContext and PreserveCompilationReferences defaults for .NET Framework apps and .NET Core apps targets 5.0 and earlier. -->
<PropertyGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework' OR ('$(TargetFrameworkIdentifier)' == '.NETCoreApp' AND $([MSBuild]::VersionLessThan($(TargetFrameworkVersion), 6.0)))">
<!-- Determines if the deps file includes complication context -->
<PreserveCompilationContext Condition="'$(PreserveCompilationContext)' == ''">true</PreserveCompilationContext>

<!-- Determines if the refs folder is produced as part of build \ publish -->
<PreserveCompilationReferences Condition="'$(PreserveCompilationReferences)' == ''">false</PreserveCompilationReferences>
</PropertyGroup>
</Project>
6 changes: 0 additions & 6 deletions src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ Copyright (c) .NET Foundation. All rights reserved.
-->
<RazorGenerateOutputFileExtension>.g.cs</RazorGenerateOutputFileExtension>

<!-- Determines if the deps file includes complication context -->
<PreserveCompilationContext>true</PreserveCompilationContext>

<!-- Determines if the refs folder is produced as part of build \ publish -->
<PreserveCompilationReferences>false</PreserveCompilationReferences>

<!--
List of file extensions that will cause VS to restart the application when RazorSDK is effective.
This property affects C# projects targeting 3.0 or later.
Expand Down
16 changes: 11 additions & 5 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,14 @@ Copyright (c) .NET Foundation. All rights reserved.
Also, enforce that RuntimeIdentifier is always specified for .NETFramework executables.
-->
<Import Project="$(MSBuildThisFileDirectory)Microsoft.NET.RuntimeIdentifierInference.targets" />



<!--
Import targets from RazorSDK if referenced
Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.BeforeCommon.targets
Sdks\Microsoft.NET.Sdk.Razor\build\netstandard2.0\Microsoft.NET.Sdk.Razor.BeforeCommon.targets
-->
<Import Project="$(MSBuildThisFileDirectory)..\..\Microsoft.NET.Sdk.Razor\build\netstandard2.0\Microsoft.NET.Sdk.Razor.BeforeCommon.targets" Condition="'$(UsingMicrosoftNETSdkRazor)' == 'true'" />

<!-- Import workload targets -->
<Import Project="Microsoft.NET.Sdk.ImportWorkloads.targets" Condition="'$(MSBuildEnableWorkloadResolver)' == 'true'" />

Expand Down Expand Up @@ -181,7 +187,7 @@ Copyright (c) .NET Foundation. All rights reserved.
<_FrameworkIdentifierForImplicitDefine Condition=" '$(TargetFrameworkIdentifier)' == '.NETFramework'">NET</_FrameworkIdentifierForImplicitDefine>

<_FrameworkVersionForImplicitDefine>$(TargetFrameworkVersion.TrimStart('vV'))</_FrameworkVersionForImplicitDefine>

<_FrameworkVersionForImplicitDefine>$(_FrameworkVersionForImplicitDefine.Replace('.', '_'))</_FrameworkVersionForImplicitDefine>

<_FrameworkVersionForImplicitDefine Condition=" '$(TargetFrameworkIdentifier)' == '.NETFramework'">$(_FrameworkVersionForImplicitDefine.Replace('_', ''))</_FrameworkVersionForImplicitDefine>
Expand Down Expand Up @@ -214,9 +220,9 @@ Copyright (c) .NET Foundation. All rights reserved.
<_CompatibleFrameworkVersions Include="@(_SupportedFrameworkVersions)" Condition=" $([MSBuild]::VersionLessThanOrEquals(%(Identity), $(TargetFrameworkVersion))) " />
<_FormattedCompatibleFrameworkVersions Include="@(_CompatibleFrameworkVersions)" Condition=" '$(TargetFrameworkIdentifier)' == '.NETCoreApp' or '$(TargetFrameworkIdentifier)' == '.NETStandard' " />
<_FormattedCompatibleFrameworkVersions Include="@(_CompatibleFrameworkVersions->'%(Identity)'->Replace('.', ''))" Condition=" '$(TargetFrameworkIdentifier)' == '.NETFramework' " />
<_ImplicitDefineConstant Include="@(_FormattedCompatibleFrameworkVersions->'$(_FrameworkIdentifierForImplicitDefine)%(Identity)_OR_GREATER'->Replace('.', '_'))"
<_ImplicitDefineConstant Include="@(_FormattedCompatibleFrameworkVersions->'$(_FrameworkIdentifierForImplicitDefine)%(Identity)_OR_GREATER'->Replace('.', '_'))"
Condition=" '$(TargetFrameworkIdentifier)' != '.NETCoreApp' or $([MSBuild]::VersionGreaterThanOrEquals(%(_FormattedCompatibleFrameworkVersions.Identity), 5.0)) " />
<_ImplicitDefineConstant Include="@(_FormattedCompatibleFrameworkVersions->'NETCOREAPP%(Identity)_OR_GREATER'->Replace('.', '_'))"
<_ImplicitDefineConstant Include="@(_FormattedCompatibleFrameworkVersions->'NETCOREAPP%(Identity)_OR_GREATER'->Replace('.', '_'))"
Condition=" '$(TargetFrameworkIdentifier)' == '.NETCoreApp' and $([MSBuild]::VersionLessThan(%(_FormattedCompatibleFrameworkVersions.Identity), 5.0)) " />
</ItemGroup>
</Target>
Expand Down
13 changes: 4 additions & 9 deletions src/Tests/Microsoft.NET.Sdk.Razor.Tests/BuildIntegrationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public void Build_WithP2P_CopiesRazorAssembly()
}

[Fact]
public void Build_WithViews_ProducesDepsFileWithCompilationContext_ButNoReferences()
public void Build_CompilationContextAndRefsDirectoryAreNotPreserved()
{
var testAsset = "RazorSimpleMvc";
var projectDirectory = CreateAspNetSdkTestAsset(testAsset);
Expand All @@ -132,14 +132,9 @@ public void Build_WithViews_ProducesDepsFileWithCompilationContext_ButNoReferenc
var depsFilePath = Path.Combine(outputPath, "SimpleMvc.deps.json");
var dependencyContext = ReadDependencyContext(depsFilePath);

// Pick a couple of libraries and ensure they have some compile references
var packageReference = dependencyContext.CompileLibraries.First(l => l.Name == "System.Diagnostics.DiagnosticSource");
packageReference.Assemblies.Should().NotBeEmpty();

var projectReference = dependencyContext.CompileLibraries.First(l => l.Name == "SimpleMvc");
projectReference.Assemblies.Should().NotBeEmpty();

dependencyContext.CompilationOptions.Defines.Should().Contain(customDefine);
var library = Assert.Single(dependencyContext.CompileLibraries);
Assert.Empty(library.Assemblies);
Assert.Empty(dependencyContext.CompilationOptions.Defines);

// Verify no refs folder is produced
new DirectoryInfo(Path.Combine(outputPath, "publish", "refs")).Should().NotExist();
Expand Down
40 changes: 40 additions & 0 deletions src/Tests/Microsoft.NET.Sdk.Razor.Tests/MvcBuildIntegrationTestLegacy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

using System;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using System.Xml.Linq;
using FluentAssertions;
using Microsoft.Extensions.DependencyModel;
using Microsoft.NET.TestFramework;
using Microsoft.NET.TestFramework.Assertions;
using Microsoft.NET.TestFramework.Commands;
Expand Down Expand Up @@ -106,5 +108,43 @@ public virtual void Publish_IncludesRefAssemblies_WhenCopyRefAssembliesToPublish

new FileInfo(Path.Combine(outputPath, "refs", "System.Threading.Tasks.Extensions.dll")).Should().Exist();
}

[CoreMSBuildOnlyFact]
public void Build_ProducesDepsFileWithCompilationContext_ButNoReferences()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests run on .NET Core 2.1, 3.1 and 5.0. So we know we haven't regressed this scenario there. We don't have a netframework app in our test matrix, but I manually verified that the behavior there is also unchanged.

{
var testAsset = $"Razor{TestProjectName}";
var projectDirectory = CreateAspNetSdkTestAsset(testAsset);

var customDefine = "AspNetSdkTest";
var build = new BuildCommand(projectDirectory);
build.Execute($"/p:DefineConstants={customDefine}").Should().Pass();

var outputPath = build.GetOutputDirectory(TargetFramework, "Debug").ToString();

var depsFile = new FileInfo(Path.Combine(outputPath, $"{TestProjectName}.deps.json"));
depsFile.Should().Exist();
var dependencyContext = ReadDependencyContext(depsFile.FullName);

// Ensure some compile references exist
var packageReference = dependencyContext.CompileLibraries.First(l => l.Name == "System.Runtime.CompilerServices.Unsafe");
packageReference.Assemblies.Should().NotBeEmpty();

var projectReference = dependencyContext.CompileLibraries.First(l => l.Name == TestProjectName);
projectReference.Assemblies.Should().NotBeEmpty();

dependencyContext.CompilationOptions.Defines.Should().Contain(customDefine);

// Verify no refs folder is produced
new DirectoryInfo(Path.Combine(outputPath, "publish", "refs")).Should().NotExist();
}

private static DependencyContext ReadDependencyContext(string depsFilePath)
{
var reader = new DependencyContextJsonReader();
using (var stream = File.OpenRead(depsFilePath))
{
return reader.Read(stream);
}
}
}
}