Skip to content

Commit 986f8ec

Browse files
jeffklrainersigwald
jeffkl
and
rainersigwald
authored
Do not import project extensions during restore (#9748)
* Do not import project extensions during restore Fixes #9512 Co-authored-by: Rainer Sigwald <raines@microsoft.com> * Add ChangeWave condition --------- Co-authored-by: Rainer Sigwald <raines@microsoft.com>
1 parent 868165d commit 986f8ec

6 files changed

Lines changed: 91 additions & 3 deletions

File tree

‎src/MSBuild/XMake.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1774,7 +1774,10 @@ private static BuildResult ExecuteRestore(string projectFile, string toolsVersio
17741774
// Add/set a property with a random value to ensure that restore happens under a different evaluation context
17751775
// If the evaluation context is not different, then projects won't be re-evaluated after restore
17761776
// The initializer syntax can't be used just in case a user set this property to a value
1777-
restoreGlobalProperties["MSBuildRestoreSessionId"] = Guid.NewGuid().ToString("D");
1777+
restoreGlobalProperties[MSBuildConstants.MSBuildRestoreSessionId] = Guid.NewGuid().ToString("D");
1778+
1779+
// Add a property to indicate that a Restore is executing
1780+
restoreGlobalProperties[MSBuildConstants.MSBuildIsRestoring] = bool.TrueString;
17781781

17791782
// Create a new request with a Restore target only and specify:
17801783
// - BuildRequestDataFlags.ClearCachesAfterBuild to ensure the projects will be reloaded from disk for subsequent builds

‎src/Shared/Constants.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,17 @@ internal static class MSBuildConstants
6969

7070
internal const string MSBuildDummyGlobalPropertyHeader = "MSBuildProjectInstance";
7171

72+
/// <summary>
73+
/// A property set during an implicit restore (/restore) or explicit restore (/t:restore) to ensure that the evaluations are not re-used during build
74+
/// </summary>
75+
internal const string MSBuildRestoreSessionId = nameof(MSBuildRestoreSessionId);
76+
77+
/// <summary>
78+
/// A property set during an implicit restore (/restore) or explicit restore (/t:restore) to indicate that a restore is executing.
79+
/// </summary>
80+
internal const string MSBuildIsRestoring = nameof(MSBuildIsRestoring);
81+
82+
7283
/// <summary>
7384
/// The most current VSGeneralAssemblyVersion known to this version of MSBuild.
7485
/// </summary>

‎src/Tasks.UnitTests/ProjectExtensionsImportTestBase.cs‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.IO;
66
using System.Linq;
77
using Microsoft.Build.Evaluation;
8+
using Microsoft.Build.Shared;
89
using Shouldly;
910
using Xunit;
1011

@@ -68,6 +69,57 @@ public void DoesNotImportProjectIfNotExist()
6869
project.GetPropertyValue(PropertyNameToSignalImportSucceeded).ShouldBeEmpty();
6970
}
7071

72+
[Fact]
73+
public void DoesNotImportProjectIfRestoring()
74+
{
75+
ObjectModelHelpers.CreateFileInTempProjectDirectory(ImportProjectPath, BasicProjectImportContents);
76+
77+
Project project = ObjectModelHelpers.LoadProjectFileInTempProjectDirectory(ObjectModelHelpers.CreateFileInTempProjectDirectory(_projectRelativePath, $@"
78+
<Project DefaultTargets=`Build` ToolsVersion=`msbuilddefaulttoolsversion`>
79+
<PropertyGroup>
80+
<{MSBuildConstants.MSBuildIsRestoring}>true</{MSBuildConstants.MSBuildIsRestoring}>
81+
</PropertyGroup>
82+
83+
<Import Project=`$(MSBuildBinPath)\Microsoft.Common.props` />
84+
85+
<Import Project=`$(MSBuildBinPath)\Microsoft.CSharp.targets` />
86+
</Project>
87+
"));
88+
89+
string projectExtensionsPath = project.GetPropertyValue("MSBuildProjectExtensionsPath");
90+
91+
projectExtensionsPath.ShouldNotBeNullOrWhiteSpace();
92+
Directory.Exists(projectExtensionsPath).ShouldBeTrue();
93+
project.GetPropertyValue(PropertyNameToEnableImport).ShouldBe(bool.FalseString, StringCompareShould.IgnoreCase);
94+
project.GetPropertyValue(PropertyNameToSignalImportSucceeded).ShouldBeEmpty();
95+
}
96+
97+
[Fact]
98+
public void ImportsProjectIfRestoringAndExplicitlySet()
99+
{
100+
ObjectModelHelpers.CreateFileInTempProjectDirectory(ImportProjectPath, BasicProjectImportContents);
101+
102+
Project project = ObjectModelHelpers.LoadProjectFileInTempProjectDirectory(ObjectModelHelpers.CreateFileInTempProjectDirectory(_projectRelativePath, $@"
103+
<Project DefaultTargets=`Build` ToolsVersion=`msbuilddefaulttoolsversion`>
104+
<PropertyGroup>
105+
<{PropertyNameToEnableImport}>true</{PropertyNameToEnableImport}>
106+
<{MSBuildConstants.MSBuildIsRestoring}>true</{MSBuildConstants.MSBuildIsRestoring}>
107+
</PropertyGroup>
108+
109+
<Import Project=`$(MSBuildBinPath)\Microsoft.Common.props` />
110+
111+
<Import Project=`$(MSBuildBinPath)\Microsoft.CSharp.targets` />
112+
</Project>
113+
"));
114+
115+
string projectExtensionsPath = project.GetPropertyValue("MSBuildProjectExtensionsPath");
116+
117+
projectExtensionsPath.ShouldNotBeNullOrWhiteSpace();
118+
Directory.Exists(projectExtensionsPath).ShouldBeTrue();
119+
project.GetPropertyValue(PropertyNameToEnableImport).ShouldBe(bool.TrueString, StringCompareShould.IgnoreCase);
120+
project.GetPropertyValue(PropertyNameToSignalImportSucceeded).ShouldBe(bool.TrueString, StringCompareShould.IgnoreCase);
121+
}
122+
71123
/// <summary>
72124
/// Ensures that even if the MSBuildProjectExtensionsPath exists, the extensions are not imported if the functionality is disabled via the <see cref="PropertyNameToEnableImport"/>.
73125
/// </summary>

‎src/Tasks/Microsoft.Common.CrossTargeting.targets‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,13 @@ Copyright (C) Microsoft Corporation. All rights reserved.
227227
build.
228228
-->
229229
<PropertyGroup>
230+
<!--
231+
Don't import project extensions during restore because NuGet restore generates them. Importing them during restore will embed
232+
the pre-restore files in the binary log and then NuGet won't be able to embed the generated one after restore. If some other
233+
project extension mechanism wants to import project extensions during restore, they need to explicitly set ImportProjectExtensionTargets
234+
-->
235+
<ImportProjectExtensionTargets Condition="$([MSBuild]::AreFeaturesEnabled('17.10')) And '$(ImportProjectExtensionTargets)' == '' And '$(MSBuildIsRestoring)' == 'true'">false</ImportProjectExtensionTargets>
236+
230237
<ImportProjectExtensionTargets Condition="'$(ImportProjectExtensionTargets)' == ''">true</ImportProjectExtensionTargets>
231238
</PropertyGroup>
232239

‎src/Tasks/Microsoft.Common.props‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@ Copyright (C) Microsoft Corporation. All rights reserved.
5959
-->
6060
<MSBuildProjectExtensionsPath Condition="'$([System.IO.Path]::IsPathRooted($(MSBuildProjectExtensionsPath)))' == 'false'">$([System.IO.Path]::Combine('$(MSBuildProjectDirectory)', '$(MSBuildProjectExtensionsPath)'))</MSBuildProjectExtensionsPath>
6161
<MSBuildProjectExtensionsPath Condition="!HasTrailingSlash('$(MSBuildProjectExtensionsPath)')">$(MSBuildProjectExtensionsPath)\</MSBuildProjectExtensionsPath>
62+
63+
<!--
64+
Don't import project extensions during restore because NuGet restore generates them. Importing them during restore will embed
65+
the pre-restore files in the binary log and then NuGet won't be able to embed the generated one after restore. If some other
66+
project extension mechanism wants to import project extensions during restore, they need to explicitly set ImportProjectExtensionProps
67+
-->
68+
<ImportProjectExtensionProps Condition="$([MSBuild]::AreFeaturesEnabled('17.10')) And '$(ImportProjectExtensionProps)' == '' And '$(MSBuildIsRestoring)' == 'true'">false</ImportProjectExtensionProps>
69+
6270
<ImportProjectExtensionProps Condition="'$(ImportProjectExtensionProps)' == ''">true</ImportProjectExtensionProps>
6371
<_InitialMSBuildProjectExtensionsPath Condition=" '$(ImportProjectExtensionProps)' == 'true' ">$(MSBuildProjectExtensionsPath)</_InitialMSBuildProjectExtensionsPath>
6472
</PropertyGroup>
@@ -134,4 +142,4 @@ Copyright (C) Microsoft Corporation. All rights reserved.
134142
<DisableLogTaskParameterItemMetadata_WriteLinesToFile_Lines>true</DisableLogTaskParameterItemMetadata_WriteLinesToFile_Lines>
135143
</PropertyGroup>
136144

137-
</Project>
145+
</Project>

‎src/Tasks/Microsoft.Common.targets‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ Copyright (C) Microsoft Corporation. All rights reserved.
2929
management system can write out multiple files but the order of the import is alphabetic because MSBuild sorts the list.
3030
-->
3131
<PropertyGroup>
32+
<!--
33+
Don't import project extensions during restore because NuGet restore generates them. Importing them during restore will embed
34+
the pre-restore files in the binary log and then NuGet won't be able to embed the generated one after restore. If some other
35+
project extension mechanism wants to import project extensions during restore, they need to explicitly set ImportProjectExtensionTargets
36+
-->
37+
<ImportProjectExtensionTargets Condition="$([MSBuild]::AreFeaturesEnabled('17.10')) And '$(ImportProjectExtensionTargets)' == '' And '$(MSBuildIsRestoring)' == 'true'">false</ImportProjectExtensionTargets>
38+
3239
<ImportProjectExtensionTargets Condition="'$(ImportProjectExtensionTargets)' == ''">true</ImportProjectExtensionTargets>
3340
</PropertyGroup>
3441

@@ -54,4 +61,4 @@ Copyright (C) Microsoft Corporation. All rights reserved.
5461

5562
<Import Project="$(CustomAfterDirectoryBuildTargets)" Condition="'$(CustomAfterDirectoryBuildTargets)' != ''" />
5663

57-
</Project>
64+
</Project>

0 commit comments

Comments
 (0)