dotnet · rainersigwald · Dec 14, 2023 · ladipro · Dec 19, 2023
diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md
@@ -44,13 +44,6 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 - [AnyHaveMetadataValue returns false when passed an empty list](https://github.com/dotnet/msbuild/pull/8603)
 - [Log item self-expansion](https://github.com/dotnet/msbuild/pull/8581)
 
-### 17.4
-- [Respect deps.json when loading assemblies](https://github.com/dotnet/msbuild/pull/7520)
-- [Consider `Platform` as default during Platform Negotiation](https://github.com/dotnet/msbuild/pull/7511)
-- [Adding accepted SDK name match pattern to SDK manifests](https://github.com/dotnet/msbuild/pull/7597)
-- [Throw warning indicating invalid project types](https://github.com/dotnet/msbuild/pull/7708)
-- [MSBuild server](https://github.com/dotnet/msbuild/pull/7634)
-
 ## Change Waves No Longer In Rotation
 ### 16.8
 - [Enable NoWarn](https://github.com/dotnet/msbuild/pull/5671)
@@ -76,3 +69,11 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 - [Add Microsoft.IO.Redist for directory enumeration](https://github.com/dotnet/msbuild/pull/6771)
 - [Process-wide caching of ToolsetConfigurationSection](https://github.com/dotnet/msbuild/pull/6832)
 - [Normalize RAR output paths](https://github.com/dotnet/msbuild/pull/6533)
+
+### 17.4
+
+- [Respect deps.json when loading assemblies](https://github.com/dotnet/msbuild/pull/7520)
+- [Consider `Platform` as default during Platform Negotiation](https://github.com/dotnet/msbuild/pull/7511)
+- [Adding accepted SDK name match pattern to SDK manifests](https://github.com/dotnet/msbuild/pull/7597)
+- [Throw warning indicating invalid project types](https://github.com/dotnet/msbuild/pull/7708)
+- [MSBuild server](https://github.com/dotnet/msbuild/pull/7634)
diff --git a/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs b/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs
@@ -145,29 +145,6 @@ public void AssertFirstResolverCanResolve()
             _logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("MockSdkResolverWithResolvableSdkPattern1 running");
         }
 
-        [Fact]
-        // Scenario: MockSdkResolver1 has higher priority than MockSdkResolverWithResolvableSdkPattern1 and resolves sdk.
-        public void AssertFirstResolverWithPatternCantResolveChangeWave17_4()
-        {
-            using (TestEnvironment env = TestEnvironment.Create())
-            {
-                ChangeWaves.ResetStateForTests();
-                env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave17_4.ToString());
-                BuildEnvironmentHelper.ResetInstance_ForUnitTestsOnly();
-
-                SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy(includeResolversWithPatterns: true));
-
-                SdkReference sdk = new SdkReference("1sdkName", "referencedVersion", "minimumVersion");
-
-                var result = SdkResolverService.Instance.ResolveSdk(BuildEventContext.InvalidSubmissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath", interactive: false, isRunningInVisualStudio: false, failOnUnresolvedSdk: true);
-
-                result.Path.ShouldBe("resolverpath1");
-                _logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("MockSdkResolver1 running");
-                _logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("MockSdkResolverWithResolvableSdkPattern1 running");
-                ChangeWaves.ResetStateForTests();
-            }
-        }
-
         [Fact]
         // Scenario: MockSdkResolver1 has higher priority than MockSdkResolverWithResolvableSdkPattern1 but MockSdkResolverWithResolvableSdkPattern1 resolves sdk,
         // becuase MockSdkResolver1 is general and MockSdkResolverWithResolvableSdkPattern1 is specific.

diff --git a/src/Build/BackEnd/Components/SdkResolution/SdkResolverLoader.cs b/src/Build/BackEnd/Components/SdkResolution/SdkResolverLoader.cs
@@ -102,22 +102,11 @@ internal virtual IReadOnlyList<SdkResolverManifest> FindPotentialSdkResolversMan
                 var assembly = Path.Combine(subfolder.FullName, $"{subfolder.Name}.dll");
                 bool assemblyAdded = false;
 
-                if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
-                {
-                    // Prefer manifest over the assembly. Try to read the xml first, and if not found then look for an assembly.
-                    assemblyAdded = TryAddAssemblyManifestFromXml(manifest, subfolder.FullName, manifestsList, location);
-                    if (!assemblyAdded)
-                    {
-                        assemblyAdded = TryAddAssemblyManifestFromDll(assembly, manifestsList);
-                    }
-                }
-                else
+                // Prefer manifest over the assembly. Try to read the xml first, and if not found then look for an assembly.
+                assemblyAdded = TryAddAssemblyManifestFromXml(manifest, subfolder.FullName, manifestsList, location);
+                if (!assemblyAdded)
                 {
                     assemblyAdded = TryAddAssemblyManifestFromDll(assembly, manifestsList);
-                    if (!assemblyAdded)
-                    {
-                        assemblyAdded = TryAddAssemblyManifestFromXml(manifest, subfolder.FullName, manifestsList, location);
-                    }
                 }
 
                 if (!assemblyAdded)

diff --git a/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs b/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs
@@ -40,14 +40,6 @@ internal class SdkResolverService : ISdkResolverService
         /// </summary>
         private readonly ConcurrentDictionary<int, ConcurrentDictionary<SdkResolver, object>> _resolverStateBySubmission = new ConcurrentDictionary<int, ConcurrentDictionary<SdkResolver, object>>();
 
-        /// <summary>
-        /// Stores the list of SDK resolvers which were loaded.
-        /// </summary>
-        /// <remarks>
-        /// Need it for supporting the ChangeWave less than <see cref="ChangeWaves.Wave17_4"/>. Remove when move out Wave17_4.
-        /// </remarks>
-        private IReadOnlyList<SdkResolver> _resolversList;
-
         /// <summary>
         /// Stores the loaded SDK resolvers, mapped to the manifest from which they came.
         /// </summary>
@@ -118,27 +110,7 @@ public virtual void ClearCaches()
         /// <inheritdoc cref="ISdkResolverService.ResolveSdk"/>
         public virtual SdkResult ResolveSdk(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath, bool interactive, bool isRunningInVisualStudio, bool failOnUnresolvedSdk)
         {
-            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
-            {
-                return ResolveSdkUsingResolversWithPatternsFirst(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath, interactive, isRunningInVisualStudio, failOnUnresolvedSdk);
-            }
-            else
-            {
-                SdkResult result = ResolveSdkUsingAllResolvers(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath, interactive, isRunningInVisualStudio, out IEnumerable<string> errors, out IEnumerable<string> warnings);
-
-                // Warnings are already logged on success.
-                if (!result.Success)
-                {
-                    if (failOnUnresolvedSdk)
-                    {
-                        loggingContext.LogError(new BuildEventFileInfo(sdkReferenceLocation), "FailedToResolveSDK", sdk.Name, string.Join($"{Environment.NewLine}  ", errors));
-                    }
-
-                    LogWarnings(loggingContext, sdkReferenceLocation, warnings);
-                }
-
-                return result;
-            }
+            return ResolveSdkUsingResolversWithPatternsFirst(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath, interactive, isRunningInVisualStudio, failOnUnresolvedSdk);
         }
 
         /// <remarks>
@@ -266,31 +238,6 @@ private List<SdkResolver> GetResolvers(IList<SdkResolverManifest> resolversManif
             return resolvers;
         }
 
-        private SdkResult ResolveSdkUsingAllResolvers(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath, bool interactive, bool isRunningInVisualStudio, out IEnumerable<string> errors, out IEnumerable<string> warnings)
-        {
-            // Lazy initialize all SDK resolvers
-            if (_resolversList == null)
-            {
-                Initialize(sdkReferenceLocation);
-            }
-
-            TryResolveSdkUsingSpecifiedResolvers(
-                _resolversList,
-                submissionId,
-                sdk,
-                loggingContext,
-                sdkReferenceLocation,
-                solutionPath,
-                projectPath,
-                interactive,
-                isRunningInVisualStudio,
-                out SdkResult sdkResult,
-                out errors,
-                out warnings);
-
-            return sdkResult;
-        }
-
         private bool TryResolveSdkUsingSpecifiedResolvers(
             IReadOnlyList<SdkResolver> resolvers,
             int submissionId,
@@ -395,24 +342,16 @@ internal void InitializeForTests(SdkResolverLoader resolverLoader = null, IReadO
             _specificResolversManifestsRegistry = null;
             _generalResolversManifestsRegistry = null;
             _manifestToResolvers = null;
-            _resolversList = null;
 
             if (resolvers != null)
             {
-                if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
-                {
-                    _specificResolversManifestsRegistry = new List<SdkResolverManifest>();
-                    _generalResolversManifestsRegistry = new List<SdkResolverManifest>();
-                    _manifestToResolvers = new Dictionary<SdkResolverManifest, IReadOnlyList<SdkResolver>>();
+                _specificResolversManifestsRegistry = new List<SdkResolverManifest>();
+                _generalResolversManifestsRegistry = new List<SdkResolverManifest>();
+                _manifestToResolvers = new Dictionary<SdkResolverManifest, IReadOnlyList<SdkResolver>>();
 
-                    SdkResolverManifest sdkResolverManifest = new SdkResolverManifest(DisplayName: "TestResolversManifest", Path: null, ResolvableSdkRegex: null);
-                    _generalResolversManifestsRegistry.Add(sdkResolverManifest);
-                    _manifestToResolvers[sdkResolverManifest] = resolvers;
-                }
-                else
-                {
-                    _resolversList = resolvers;
-                }
+                SdkResolverManifest sdkResolverManifest = new SdkResolverManifest(DisplayName: "TestResolversManifest", Path: null, ResolvableSdkRegex: null);
+                _generalResolversManifestsRegistry.Add(sdkResolverManifest);
+                _manifestToResolvers[sdkResolverManifest] = resolvers;
             }
         }
 
@@ -450,21 +389,6 @@ private object GetResolverState(int submissionId, SdkResolver resolver)
             return null;
         }
 
-        private void Initialize(ElementLocation location)
-        {
-            lock (_lockObject)
-            {
-                if (_resolversList != null)
-                {
-                    return;
-                }
-
-                MSBuildEventSource.Log.SdkResolverServiceInitializeStart();
-                _resolversList = _sdkResolverLoader.LoadAllResolvers(location);
-                MSBuildEventSource.Log.SdkResolverServiceInitializeStop(_resolversList.Count);
-            }
-        }
-
         private void RegisterResolversManifests(ElementLocation location)
         {
             lock (_lockObject)
@@ -522,7 +446,7 @@ private void SetResolverState(int submissionId, SdkResolver resolver, object sta
                     submissionId,
                     _ => new ConcurrentDictionary<SdkResolver, object>(
                         NativeMethodsShared.GetLogicalCoreCount(),
-                        ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4) ? _specificResolversManifestsRegistry.Count + _generalResolversManifestsRegistry.Count : _resolversList.Count));
+                        _specificResolversManifestsRegistry.Count + _generalResolversManifestsRegistry.Count));
 
                 resolverState.AddOrUpdate(resolver, state, (sdkResolver, obj) => state);
             }

diff --git a/src/Build/Evaluation/Expander.cs b/src/Build/Evaluation/Expander.cs
@@ -1614,9 +1614,8 @@ private static string ExpandRegistryValue(string registryExpression, IElementLoc
             {
 #if RUNTIME_TYPE_NETCORE
                 // .NET Core MSBuild used to always return empty, so match that behavior
-                // on non-Windows (no registry), and with a changewave (in case someone
-                // had a registry property and it breaks when it lights up).
-                if (!NativeMethodsShared.IsWindows || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
+                // on non-Windows (no registry).
+                if (!NativeMethodsShared.IsWindows)
                 {
                     return string.Empty;
                 }

diff --git a/src/Build/Evaluation/IntrinsicFunctions.cs b/src/Build/Evaluation/IntrinsicFunctions.cs
@@ -188,9 +188,8 @@ internal static object GetRegistryValue(string keyName, string valueName)
         {
 #if RUNTIME_TYPE_NETCORE
             // .NET Core MSBuild used to always return empty, so match that behavior
-            // on non-Windows (no registry), and with a changewave (in case someone
-            // had a registry property and it breaks when it lights up).
-            if (!NativeMethodsShared.IsWindows || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
+            // on non-Windows (no registry).
+            if (!NativeMethodsShared.IsWindows)
             {
                 return null;
             }
@@ -205,9 +204,8 @@ internal static object GetRegistryValue(string keyName, string valueName, object
         {
 #if RUNTIME_TYPE_NETCORE
             // .NET Core MSBuild used to always return empty, so match that behavior
-            // on non-Windows (no registry), and with a changewave (in case someone
-            // had a registry property and it breaks when it lights up).
-            if (!NativeMethodsShared.IsWindows || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
+            // on non-Windows (no registry).
+            if (!NativeMethodsShared.IsWindows)
             {
                 return defaultValue;
             }
@@ -219,9 +217,8 @@ internal static object GetRegistryValueFromView(string keyName, string valueName
         {
 #if RUNTIME_TYPE_NETCORE
             // .NET Core MSBuild used to always return empty, so match that behavior
-            // on non-Windows (no registry), and with a changewave (in case someone
-            // had a registry property and it breaks when it lights up).
-            if (!NativeMethodsShared.IsWindows || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
+            // on non-Windows (no registry).
+            if (!NativeMethodsShared.IsWindows)
             {
                 return defaultValue;
             }
@@ -242,9 +239,8 @@ internal static object GetRegistryValueFromView(string keyName, string valueName
         {
 #if RUNTIME_TYPE_NETCORE
             // .NET Core MSBuild used to always return empty, so match that behavior
-            // on non-Windows (no registry), and with a changewave (in case someone
-            // had a registry property and it breaks when it lights up).
-            if (!NativeMethodsShared.IsWindows || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
+            // on non-Windows (no registry).
+            if (!NativeMethodsShared.IsWindows)
             {
                 return defaultValue;
             }

diff --git a/src/Build/Logging/BaseConsoleLogger.cs b/src/Build/Logging/BaseConsoleLogger.cs
@@ -891,7 +891,7 @@ public int Compare(object o1, object o2)
 
         public virtual void Shutdown()
         {
-            Traits.LogAllEnvironmentVariables = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDLOGALLENVIRONMENTVARIABLES")) && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4);
+            Traits.LogAllEnvironmentVariables = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDLOGALLENVIRONMENTVARIABLES"));
         }
 
         internal abstract void ResetConsoleLoggerState();

diff --git a/src/Build/Xml/ProjectXmlUtilities.cs b/src/Build/Xml/ProjectXmlUtilities.cs
@@ -98,7 +98,7 @@ internal static bool VerifyValidProjectNamespace(XmlElementWithLocation element)
             }
             else if (string.IsNullOrEmpty(element.NamespaceURI))
             {
-                if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4) && Path.GetExtension(element.Location.File).Equals(".dwproj", StringComparison.OrdinalIgnoreCase))
+                if (Path.GetExtension(element.Location.File).Equals(".dwproj", StringComparison.OrdinalIgnoreCase))
                 {
                     bool validMSBuildProject = true;
                     foreach (XmlNode child in element.ChildNodes)

diff --git a/src/Framework/ChangeWaves.cs b/src/Framework/ChangeWaves.cs
@@ -24,11 +24,10 @@ internal enum ChangeWaveConversionState
     /// For dev docs: https://github.com/dotnet/msbuild/blob/main/documentation/wiki/ChangeWaves-Dev.md
     internal class ChangeWaves
     {
-        internal static readonly Version Wave17_4 = new Version(17, 4);
         internal static readonly Version Wave17_6 = new Version(17, 6);
         internal static readonly Version Wave17_8 = new Version(17, 8);
         internal static readonly Version Wave17_10 = new Version(17, 10);
-        internal static readonly Version[] AllWaves = { Wave17_4, Wave17_6, Wave17_8, Wave17_10 };
+        internal static readonly Version[] AllWaves = { Wave17_6, Wave17_8, Wave17_10 };
 
         /// <summary>
         /// Special value indicating that all features behind all Change Waves should be enabled.

diff --git a/src/Framework/Traits.cs b/src/Framework/Traits.cs
@@ -110,11 +110,8 @@ public Traits()
         /// <summary>
         /// Log all environment variables whether or not they are used in a build in the binary log.
         /// </summary>
-        public static bool LogAllEnvironmentVariables = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDLOGALLENVIRONMENTVARIABLES"))
-#if !TASKHOST
-            && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4)
-#endif
-            ;
+        public static bool LogAllEnvironmentVariables = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDLOGALLENVIRONMENTVARIABLES"));
+
         /// <summary>
         /// Log property tracking information.
         /// </summary>

diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs
@@ -240,7 +240,6 @@ string[] args
 
             int exitCode;
             if (
-                ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4) &&
                 Environment.GetEnvironmentVariable(Traits.UseMSBuildServerEnvVarName) == "1" &&
                 !Traits.Instance.EscapeHatches.EnsureStdOutForChildNodesIsPrimaryStdout &&
                 CanRunServerBasedOnCommandLineSwitches(

diff --git a/src/Shared/MSBuildLoadContext.cs b/src/Shared/MSBuildLoadContext.cs
@@ -56,14 +56,11 @@ public MSBuildLoadContext(string assemblyPath)
                 return null;
             }
 
-            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
+            // respect plugin.dll.json with the AssemblyDependencyResolver
+            string? assemblyPath = _resolver?.ResolveAssemblyToPath(assemblyName);
+            if (assemblyPath != null)
             {
-                // respect plugin.dll.json with the AssemblyDependencyResolver
-                string? assemblyPath = _resolver?.ResolveAssemblyToPath(assemblyName);
-                if (assemblyPath != null)
-                {
-                    return LoadFromAssemblyPath(assemblyPath);
-                }
+                return LoadFromAssemblyPath(assemblyPath);
             }
 
             // Fall back to the older MSBuild-on-Core behavior to continue to support
@@ -113,13 +110,10 @@ public MSBuildLoadContext(string assemblyPath)
 
         protected override IntPtr LoadUnmanagedDll(string unmanagedDllName)
         {
-            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4))
+            string? libraryPath = _resolver?.ResolveUnmanagedDllToPath(unmanagedDllName);
+            if (libraryPath != null)
             {
-                string? libraryPath = _resolver?.ResolveUnmanagedDllToPath(unmanagedDllName);
-                if (libraryPath != null)
-                {
-                    return LoadUnmanagedDllFromPath(libraryPath);
-                }
+                return LoadUnmanagedDllFromPath(libraryPath);
             }
 
             return base.LoadUnmanagedDll(unmanagedDllName);