From 6facda2e1ecddcd7a4c588dfeae97af1f403b5cd Mon Sep 17 00:00:00 2001 From: Pranav K Date: Wed, 31 Mar 2021 13:45:55 -0700 Subject: [PATCH 1/3] Avoid configuring PreserveCompilationContext=true in 6.0 apps The goal is to avoid producing the side-effects of setting PreserveCompilationContext=true (additional content in deps file and ref assemblies) unless the app expects it. Previously the option was configured as part of Razor's SDK.props. This was done as a hack around target evaluation ordering - GenerateDependencyFile and PreserveCompilationReferences are defined in M.NET.Sdk's targets and use PreserveCompilationContext during their initialization. Unfortunately configuring it there makes it difficult to make any tfm specific decisions or de-activate it if the user did not explicitly configure it. This change adds a hook to initialize PreserveCompilationContext early on during .NET SDK that allows configuring defaults for 5.0 and earlier apps. --- ...itializePreserveCompilationContext.targets | 21 ++++++++++ .../Targets/Sdk.Razor.CurrentVersion.props | 8 +--- .../Microsoft.NET.Build.Tasks/sdk/Sdk.targets | 14 +++---- ...oft.NET.PreserveCompilationContext.targets | 13 +++++- .../Microsoft.NET.Sdk.BeforeCommon.targets | 14 ++----- .../BuildIntegrationTest.cs | 13 ++---- .../MvcBuildIntegrationTestLegacy.cs | 40 +++++++++++++++++++ 7 files changed, 89 insertions(+), 34 deletions(-) create mode 100644 src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets diff --git a/src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets b/src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets new file mode 100644 index 000000000000..541750abdeae --- /dev/null +++ b/src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets @@ -0,0 +1,21 @@ + + + + + + true + + + false + + diff --git a/src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.props b/src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.props index 795c769a0681..06c0d4b83127 100644 --- a/src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.props +++ b/src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.props @@ -67,12 +67,6 @@ Copyright (c) .NET Foundation. All rights reserved. --> .g.cs - - true - - - false - $(CustomCollectWatchItems);_RazorSdkCustomCollectWatchItems + + <_NETSdkInitializePreserveCompilationTargets>$(MSBuildThisFileDirectory)Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.targets b/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.targets index c521e2b2c8cf..5f3715982760 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.targets @@ -6,7 +6,7 @@ WARNING: DO NOT MODIFY this file unless you are knowledgeable about MSBuild and 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. +Copyright (c) .NET Foundation. All rights reserved. *********************************************************************************************** --> @@ -15,7 +15,7 @@ Copyright (c) .NET Foundation. All rights reserved. true - + $(MSBuildToolsPath)\Microsoft.Common.targets - - + - + $(MSBuildThisFileDirectory)..\..\NuGet.Build.Tasks.Pack\buildCrossTargeting\NuGet.Build.Tasks.Pack.targets $(MSBuildThisFileDirectory)..\..\NuGet.Build.Tasks.Pack\build\NuGet.Build.Tasks.Pack.targets true - + + Condition="Exists('$(NuGetBuildTasksPackTargets)') AND '$(ImportNuGetBuildTasksPackTargetsFromSdk)' == 'true'"/> diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PreserveCompilationContext.targets b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PreserveCompilationContext.targets index 99e5bf241e5e..8d6e37a4ac50 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PreserveCompilationContext.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PreserveCompilationContext.targets @@ -6,15 +6,24 @@ WARNING: DO NOT MODIFY this file unless you are knowledgeable about MSBuild and 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. +Copyright (c) .NET Foundation. All rights reserved. *********************************************************************************************** --> + + + refs - + $(PreserveCompilationContext) + + + $(PreserveCompilationContext) - - + @@ -105,11 +104,6 @@ Copyright (c) .NET Foundation. All rights reserved. false - - - $(PreserveCompilationContext) - - publish @@ -181,7 +175,7 @@ Copyright (c) .NET Foundation. All rights reserved. <_FrameworkIdentifierForImplicitDefine Condition=" '$(TargetFrameworkIdentifier)' == '.NETFramework'">NET <_FrameworkVersionForImplicitDefine>$(TargetFrameworkVersion.TrimStart('vV')) - + <_FrameworkVersionForImplicitDefine>$(_FrameworkVersionForImplicitDefine.Replace('.', '_')) <_FrameworkVersionForImplicitDefine Condition=" '$(TargetFrameworkIdentifier)' == '.NETFramework'">$(_FrameworkVersionForImplicitDefine.Replace('_', '')) @@ -214,9 +208,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)) " /> diff --git a/src/Tests/Microsoft.NET.Sdk.Razor.Tests/BuildIntegrationTest.cs b/src/Tests/Microsoft.NET.Sdk.Razor.Tests/BuildIntegrationTest.cs index b31eed5532f9..b72c12888998 100644 --- a/src/Tests/Microsoft.NET.Sdk.Razor.Tests/BuildIntegrationTest.cs +++ b/src/Tests/Microsoft.NET.Sdk.Razor.Tests/BuildIntegrationTest.cs @@ -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); @@ -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(); diff --git a/src/Tests/Microsoft.NET.Sdk.Razor.Tests/MvcBuildIntegrationTestLegacy.cs b/src/Tests/Microsoft.NET.Sdk.Razor.Tests/MvcBuildIntegrationTestLegacy.cs index a1f535f09a5f..9905b016dadd 100644 --- a/src/Tests/Microsoft.NET.Sdk.Razor.Tests/MvcBuildIntegrationTestLegacy.cs +++ b/src/Tests/Microsoft.NET.Sdk.Razor.Tests/MvcBuildIntegrationTestLegacy.cs @@ -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; @@ -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() + { + 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); + } + } } } From 2112f3c5d586b9731feda144acecc24c79f76ebf Mon Sep 17 00:00:00 2001 From: Pranav K Date: Wed, 7 Apr 2021 11:55:03 -0700 Subject: [PATCH 2/3] Changes per PR comments --- ...itializePreserveCompilationContext.targets | 21 ------------------- .../Targets/Sdk.Razor.CurrentVersion.props | 2 -- .../Microsoft.NET.Build.Tasks/sdk/Sdk.targets | 14 ++++++------- ...oft.NET.PreserveCompilationContext.targets | 13 ++---------- .../Microsoft.NET.Sdk.BeforeCommon.targets | 8 +++++++ ...crosoft.NET.Sdk.Razor.BeforeCommon.targets | 21 +++++++++++++++++++ 6 files changed, 38 insertions(+), 41 deletions(-) delete mode 100644 src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets create mode 100644 src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Razor.BeforeCommon.targets diff --git a/src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets b/src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets deleted file mode 100644 index 541750abdeae..000000000000 --- a/src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets +++ /dev/null @@ -1,21 +0,0 @@ - - - - - - true - - - false - - diff --git a/src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.props b/src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.props index 06c0d4b83127..a28e616ff935 100644 --- a/src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.props +++ b/src/RazorSdk/Targets/Sdk.Razor.CurrentVersion.props @@ -77,8 +77,6 @@ Copyright (c) .NET Foundation. All rights reserved. Target used by dotnet-watch to resolve additional items. --> $(CustomCollectWatchItems);_RazorSdkCustomCollectWatchItems - - <_NETSdkInitializePreserveCompilationTargets>$(MSBuildThisFileDirectory)Microsoft.NET.Sdk.Razor.InitializePreserveCompilationContext.targets diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.targets b/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.targets index 5f3715982760..c521e2b2c8cf 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/sdk/Sdk.targets @@ -6,7 +6,7 @@ WARNING: DO NOT MODIFY this file unless you are knowledgeable about MSBuild and 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. +Copyright (c) .NET Foundation. All rights reserved. *********************************************************************************************** --> @@ -15,7 +15,7 @@ Copyright (c) .NET Foundation. All rights reserved. true - + $(MSBuildToolsPath)\Microsoft.Common.targets - - + - + $(MSBuildThisFileDirectory)..\..\NuGet.Build.Tasks.Pack\buildCrossTargeting\NuGet.Build.Tasks.Pack.targets $(MSBuildThisFileDirectory)..\..\NuGet.Build.Tasks.Pack\build\NuGet.Build.Tasks.Pack.targets true - + + Condition="Exists('$(NuGetBuildTasksPackTargets)') AND '$(ImportNuGetBuildTasksPackTargetsFromSdk)' == 'true'"/> diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PreserveCompilationContext.targets b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PreserveCompilationContext.targets index 8d6e37a4ac50..99e5bf241e5e 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PreserveCompilationContext.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PreserveCompilationContext.targets @@ -6,24 +6,15 @@ WARNING: DO NOT MODIFY this file unless you are knowledgeable about MSBuild and 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. +Copyright (c) .NET Foundation. All rights reserved. *********************************************************************************************** --> - - - refs - + $(PreserveCompilationContext) - - - $(PreserveCompilationContext) + + + @@ -104,6 +107,11 @@ Copyright (c) .NET Foundation. All rights reserved. false + + + $(PreserveCompilationContext) + + publish diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Razor.BeforeCommon.targets b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Razor.BeforeCommon.targets new file mode 100644 index 000000000000..97a992ab31f0 --- /dev/null +++ b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Razor.BeforeCommon.targets @@ -0,0 +1,21 @@ + + + + + + true + + + false + + From cde791b4d9cccfa2d8874850b5318a52b9887c0e Mon Sep 17 00:00:00 2001 From: Pranav K Date: Wed, 7 Apr 2021 14:41:14 -0700 Subject: [PATCH 3/3] Move the target to RazorSDK --- .../Targets}/Microsoft.NET.Sdk.Razor.BeforeCommon.targets | 0 .../targets/Microsoft.NET.Sdk.BeforeCommon.targets | 8 ++++++-- 2 files changed, 6 insertions(+), 2 deletions(-) rename src/{Tasks/Microsoft.NET.Build.Tasks/targets => RazorSdk/Targets}/Microsoft.NET.Sdk.Razor.BeforeCommon.targets (100%) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Razor.BeforeCommon.targets b/src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.BeforeCommon.targets similarity index 100% rename from src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Razor.BeforeCommon.targets rename to src/RazorSdk/Targets/Microsoft.NET.Sdk.Razor.BeforeCommon.targets diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets index 3947b560457d..ab1f746c1b65 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets +++ b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets @@ -66,8 +66,12 @@ Copyright (c) .NET Foundation. All rights reserved. --> - - + +