Toolset configuration net5.0 (#6220)

eng/Packages.props

@@ -18,6 +18,7 @@
     <PackageReference Update="SourceLink.Create.CommandLine" Version="2.1.2" />
     <PackageReference Update="System.CodeDom" Version="4.4.0" />
     <PackageReference Update="System.Collections.Immutable" Version="5.0.0" />
+    <PackageReference Update="System.Configuration.ConfigurationManager" Version="4.7.0" />
     <PackageReference Update="System.Memory" Version="4.5.4" />
     <PackageReference Update="System.Reflection.Metadata" Version="1.6.0" />
     <PackageReference Update="System.Resources.Extensions" Version="4.6.0" />

src/Build.UnitTests/Definition/ToolsetConfigurationReader_Tests.cs

@@ -1,10 +1,7 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-#if FEATURE_SYSTEM_CONFIGURATION
-
 using System.Configuration;
-using Microsoft.Win32;
 using Microsoft.Build.Collections;
 using Microsoft.Build.Evaluation;
 using Microsoft.Build.Execution;
@@ -683,4 +680,3 @@ private ToolsetConfigurationReader GetStandardConfigurationReader()
 
     }
 }
-#endif

src/Build.UnitTests/Definition/ToolsetReader_Tests.cs

@@ -3,9 +3,7 @@
 
 using System;
 using System.Collections.Generic;
-#if FEATURE_SYSTEM_CONFIGURATION
 using System.Configuration;
-#endif
 using System.IO;
 
 using Microsoft.Build.Collections;

src/Build.UnitTests/Evaluation/ToolsetConfigurationNet5_Tests.cs

@@ -0,0 +1,80 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+#if !FEATURE_SYSTEM_CONFIGURATION
+/*  This test is designed especially to test Configuration parsing in net5.0
+ *  which means it WON'T work in net472 and thus we don't run it in net472 */
+
+using Microsoft.Build.Evaluation;
+using Microsoft.Build.Execution;
+
+using Xunit;
+using System.Collections.Generic;
+using Shouldly;
+
+namespace Microsoft.Build.UnitTests.Evaluation
+{
+    /// <summary>
+    /// Unit tests for MSBuild Net5.0 Configuration Parsing
+    /// </summary>
+    public class ToolsetConfigurationNet5Test
+    {
+        [Fact]
+        // The default ToolsetDefintionLocations is None, which results in only the local which results in only the several included
+        // paths such as SDK path and RoslynTargetPath and nothing else. This behavior is expected and the exact same as before.
+        public void ToolsetDefinitionLocationsIsDefault()
+        {
+            var projectCollection = new ProjectCollection();
+            IDictionary<string, string> toolsetProperties
+                = new Dictionary<string, string>();
+
+            foreach (Toolset toolset in projectCollection.Toolsets)
+            {
+                foreach (KeyValuePair<string, ProjectPropertyInstance> properties in toolset.Properties)
+                {
+                    toolsetProperties[properties.Value.Name] = properties.Value.EvaluatedValue;
+                }
+            }
+
+            toolsetProperties.ShouldContainKey("MSBuildSDKsPath");
+            toolsetProperties.ShouldContainKey("RoslynTargetsPath");
+            toolsetProperties["MSBuildSDKsPath"].ShouldNotBeNullOrEmpty();
+            toolsetProperties["RoslynTargetsPath"].ShouldNotBeNullOrEmpty();
+
+            toolsetProperties.ShouldNotContainKey("VCTargetsPath");
+            toolsetProperties.ShouldNotContainKey("MSBuildToolsRoot");
+            toolsetProperties.ShouldNotContainKey("MSBuildExtensionsPath");
+        }
+
+        [Fact]
+        // With ToolsetDefintionLocations set to ConfigurationFile (Which would only happen in net5.0 if the user decides to set it). 
+        // Most toolsets are available and the MsBuildTools and SDK paths are all in the net5.0 runtime.
+        public void ToolsetDefinitionLocationsIsConfiguration()
+        {
+            var projectCollection = new ProjectCollection(ToolsetDefinitionLocations.ConfigurationFile);
+            IDictionary<string, string> toolsetProperties
+                = new Dictionary<string, string>();
+
+            foreach (Toolset toolset in projectCollection.Toolsets)
+            {
+                foreach (KeyValuePair<string, ProjectPropertyInstance> properties in toolset.Properties)
+                {
+                    toolsetProperties[properties.Value.Name] = properties.Value.EvaluatedValue;
+                }
+            }
+
+            toolsetProperties.ShouldContainKey("MSBuildSDKsPath");
+            toolsetProperties.ShouldContainKey("RoslynTargetsPath");
+            toolsetProperties["MSBuildSDKsPath"].ShouldNotBeNullOrEmpty();
+            toolsetProperties["RoslynTargetsPath"].ShouldNotBeNullOrEmpty();
+
+            toolsetProperties.ShouldContainKey("VCTargetsPath");
+            toolsetProperties.ShouldContainKey("MSBuildToolsRoot");
+            toolsetProperties.ShouldContainKey("MSBuildExtensionsPath");
+            toolsetProperties["VCTargetsPath"].ShouldNotBeNullOrEmpty();
+            toolsetProperties["MSBuildToolsRoot"].ShouldNotBeNullOrEmpty();
+            toolsetProperties["MSBuildExtensionsPath"].ShouldNotBeNullOrEmpty();
+        }
+    }
+}
+#endif

src/Build.UnitTests/Microsoft.Build.Engine.UnitTests.csproj

@@ -16,6 +16,7 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <PackageReference Include="System.Configuration.ConfigurationManager" />
     <PackageReference Include="Shouldly" />
     <PackageReference Include="Microsoft.CodeAnalysis.Build.Tasks" />
     <PackageReference Include="NuGet.Frameworks" >
@@ -44,17 +45,12 @@
       <SetTargetFramework Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework' and '$(MonoBuild)' == 'true'">TargetFramework=$(FullFrameworkTFM)</SetTargetFramework>
       <SetTargetFramework Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework'">TargetFramework=net5.0</SetTargetFramework>
     </ProjectReference>
-
-    <Reference Include="System.Configuration" Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'" />
   </ItemGroup>
 
   <ItemGroup>
     <Compile Include="..\Shared\FxCopExclusions\Microsoft.Build.Shared.Suppressions.cs">
       <ExcludeFromStyleCop>true</ExcludeFromStyleCop>
     </Compile>
-
-    <Compile Remove="Definition\ToolsetConfigurationReaderTestHelper.cs" />
-    <Compile Include="Definition\ToolsetConfigurationReaderTestHelper.cs" Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'" />
 
     <Compile Include="..\Shared\UnitTests\BuildEventArgsExtension.cs">
       <!-- Extension methods -->

src/Build/Definition/ProjectCollection.cs

@@ -1729,15 +1729,13 @@ private void CreateLoggingService(int maxCPUCount, bool onlyLogCriticalEvents)
             _loggingService.OnlyLogCriticalEvents = onlyLogCriticalEvents;
         }
 
-#if FEATURE_SYSTEM_CONFIGURATION
         /// <summary>
         /// Reset the toolsets using the provided toolset reader, used by unit tests
         /// </summary>
         internal void ResetToolsetsForTests(ToolsetConfigurationReader configurationReaderForTestsOnly)
         {
             InitializeToolsetCollection(configReader:configurationReaderForTestsOnly);
         }
-#endif
 
 #if FEATURE_WIN32_REGISTRY
         /// <summary>
@@ -1757,9 +1755,7 @@ internal void ResetToolsetsForTests(ToolsetRegistryReader registryReaderForTests
 #if FEATURE_WIN32_REGISTRY
                 ToolsetRegistryReader registryReader = null,
 #endif
-#if FEATURE_SYSTEM_CONFIGURATION
                 ToolsetConfigurationReader configReader = null
-#endif
                 )
         {
             _toolsets = new Dictionary<string, Toolset>(StringComparer.OrdinalIgnoreCase);
@@ -1769,9 +1765,7 @@ internal void ResetToolsetsForTests(ToolsetRegistryReader registryReaderForTests
 #if FEATURE_WIN32_REGISTRY
                     registryReader,
 #endif
-#if FEATURE_SYSTEM_CONFIGURATION
                     configReader,
-#endif
                     EnvironmentProperties, _globalProperties, ToolsetLocations);
 
             _toolsetsVersion++;

src/Build/Definition/ToolsetConfigurationReader.cs

@@ -253,7 +253,11 @@ private static Configuration ReadApplicationConfiguration()
         {
             // When running from the command-line or from VS, use the msbuild.exe.config file.
             if (BuildEnvironmentHelper.Instance.Mode != BuildEnvironmentMode.None &&
+ // This FEATURE_SYSTEM_CONFIGURATION is needed as OpenExeConfiguration for net5.0 works differently, without this condition unit tests won't pass.
+ // ConfigurationManager.OpenExeConfiguration in net5.0 will find testhost.exe instead which does not contain any configuration and therefore fail.
+#if FEATURE_SYSTEM_CONFIGURATION
                 !BuildEnvironmentHelper.Instance.RunningTests &&
+#endif
                 FileSystems.Default.FileExists(BuildEnvironmentHelper.Instance.CurrentMSBuildConfigurationFile))
             {
                 var configFile = new ExeConfigurationFileMap { ExeConfigFilename = BuildEnvironmentHelper.Instance.CurrentMSBuildConfigurationFile };

src/Build/Definition/ToolsetReader.cs

@@ -76,7 +76,6 @@ internal abstract class ToolsetReader
             get;
         }
 
-#if FEATURE_WIN32_REGISTRY || FEATURE_SYSTEM_CONFIGURATION
         /// <summary>
         /// Gathers toolset data from the registry and configuration file, if any:
         /// allows you to specify which of the registry and configuration file to
@@ -88,12 +87,9 @@ internal static string ReadAllToolsets(Dictionary<string, Toolset> toolsets, Pro
 #if FEATURE_WIN32_REGISTRY
                 null,
 #endif
-#if FEATURE_SYSTEM_CONFIGURATION
                 null,
-#endif
                 environmentProperties, globalProperties, locations);
         }
-#endif
 
         /// <summary>
         /// Gathers toolset data from the registry and configuration file, if any.
@@ -105,9 +101,7 @@ internal static string ReadAllToolsets
 #if FEATURE_WIN32_REGISTRY
             ToolsetRegistryReader registryReader,
 #endif
-#if FEATURE_SYSTEM_CONFIGURATION
             ToolsetConfigurationReader configurationReader,
-#endif
             PropertyDictionary<ProjectPropertyInstance> environmentProperties,
             PropertyDictionary<ProjectPropertyInstance> globalProperties,
             ToolsetDefinitionLocations locations
@@ -124,7 +118,6 @@ ToolsetDefinitionLocations locations
             string overrideTasksPathFromConfiguration = null;
             string defaultOverrideToolsVersionFromConfiguration = null;
 
-#if FEATURE_SYSTEM_CONFIGURATION
             if ((locations & ToolsetDefinitionLocations.ConfigurationFile) == ToolsetDefinitionLocations.ConfigurationFile)
             {
                 if (configurationReader == null)
@@ -137,7 +130,6 @@ ToolsetDefinitionLocations locations
                     initialProperties, true /* accumulate properties */, out overrideTasksPathFromConfiguration,
                     out defaultOverrideToolsVersionFromConfiguration);
             }
-#endif
 
             string defaultToolsVersionFromRegistry = null;
             string overrideTasksPathFromRegistry = null;

src/Build/Definition/ToolsetRegistryReader.cs

@@ -3,11 +3,8 @@
 
 #if FEATURE_WIN32_REGISTRY
 
-using Microsoft.Win32;
 using System;
 using System.Collections.Generic;
-using System.IO;
-using System.Security;
 
 using Microsoft.Build.Shared;
 using error = Microsoft.Build.Shared.ErrorUtilities;
@@ -346,4 +343,4 @@ private static string GetValue(RegistryKeyWrapper wrapper, string valueName)
         }
     }
 }
-#endif
+#endif

src/Build/Microsoft.Build.csproj

@@ -31,6 +31,7 @@
     <ProjectReference Include="..\StringTools\StringTools.csproj" />
 
     <PackageReference Include="System.Collections.Immutable" />
+    <PackageReference Include="System.Configuration.ConfigurationManager" />
     <PackageReference Include="System.Threading.Tasks.Dataflow" />
     <PackageReference Include="System.Text.Json" />
 
@@ -39,7 +40,6 @@
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
     <PackageReference Include="Microsoft.VisualStudio.Setup.Configuration.Interop" />
-    <Reference Include="System.Configuration" />
     <Reference Include="System.IO.Compression" />
     <PackageReference Include="System.Memory" />
   </ItemGroup>
@@ -420,8 +420,8 @@
     <Compile Include="Definition\ResolvedImport.cs" />
     <Compile Include="Definition\SubToolset.cs" />
     <Compile Include="Definition\Toolset.cs" />
-    <Compile Include="Definition\ToolsetConfigurationReader.cs" Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'" />
-    <Compile Include="..\Shared\ToolsetElement.cs" Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
+    <Compile Include="Definition\ToolsetConfigurationReader.cs" />
+    <Compile Include="..\Shared\ToolsetElement.cs">
       <ExcludeFromStyleCop>true</ExcludeFromStyleCop>
     </Compile>
     <Compile Include="Definition\ToolsetPropertyDefinition.cs" />

src/Build/Utilities/RegistryKeyWrapper.cs

@@ -3,10 +3,6 @@
 #if FEATURE_WIN32_REGISTRY
 
 using System;
-using System.Collections.Generic;
-using System.Globalization;
-using System.IO;
-using System.Security;
 
 using Microsoft.Build.Shared;
 using Microsoft.Win32;

src/MSBuild/MSBuild.csproj

@@ -221,12 +221,14 @@
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
   </ItemGroup>
+  <ItemGroup>
+    <PackageReference Include="System.Configuration.ConfigurationManager" />
+  </ItemGroup>
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
     <!-- File for Assemblies we depend on -->
     <Reference Include="System" />
     <Reference Include="System.Core" />
     <Reference Include="System.Xml" />
-    <Reference Include="System.Configuration" />
     <PackageReference Include="LargeAddressAware" PrivateAssets="All" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework'">

src/MSBuild/XMake.cs

@@ -4,9 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
-#if FEATURE_SYSTEM_CONFIGURATION
 using System.Configuration;
-#endif
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;

src/Shared/FrameworkLocationHelper.cs

@@ -4,17 +4,11 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
-#if FEATURE_SYSTEM_CONFIGURATION
-using System.Configuration;
-#endif
 using System.IO;
 using System.Linq;
 using System.Runtime.Versioning;
 using Microsoft.Win32;
 
-#if FEATURE_SYSTEM_CONFIGURATION
-using PropertyElement = Microsoft.Build.Evaluation.ToolsetElement.PropertyElement;
-#endif
 using Microsoft.Build.Shared.FileSystem;
 
 namespace Microsoft.Build.Shared

src/Shared/ToolsetElement.cs

@@ -3,17 +3,13 @@
 
 using System;
 using System.Collections.Generic;
-#if FEATURE_SYSTEM_CONFIGURATION
 using System.Configuration;
-#endif
 using System.IO;
 using Microsoft.Build.Collections;
 using Microsoft.Build.Shared;
 
 namespace Microsoft.Build.Evaluation
 {
-#if FEATURE_SYSTEM_CONFIGURATION
-
     /// <summary>
     /// Helper class for reading toolsets out of the configuration file.
     /// </summary>
@@ -708,5 +704,4 @@ public string DefaultOverrideToolsVersion
             }
         }
     }
-#endif
 }

src/Tasks.UnitTests/Copy_Tests.cs

@@ -2397,7 +2397,9 @@ public void CopyToDestinationFolderWithSymbolicLinkCheck()
 
             if (NativeMethodsShared.IsWindows)
             {
+#pragma warning disable CA1416 // Suppress Warning saying that WindowsPrincipal might not be compatible on Windows (Which shouldn't be an issue...)
                 if (!new WindowsPrincipal(WindowsIdentity.GetCurrent()).IsInRole(new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null)))
+#pragma warning restore CA1416 // Suppress Warning saying that WindowsPrincipal might not be compatible on Windows (Which shouldn't be an issue...)
                 {
                     isPrivileged = false;
                     Assert.True(true, "It seems that you don't have the permission to create symbolic links. Try to run this test again with higher privileges");

src/Utilities/Microsoft.Build.Utilities.csproj

@@ -22,12 +22,11 @@
     <ProjectReference Include="..\StringTools\StringTools.csproj" />
 
     <PackageReference Include="System.Collections.Immutable" />
+    <PackageReference Include="System.Configuration.ConfigurationManager" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETStandard'">
     <PackageReference Include="Microsoft.VisualStudio.Setup.Configuration.Interop" />
-
-    <Reference Include="System.Configuration" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETStandard'">