Make SdkResolver-provided environment variables take precedence over ambient environment

src/Build.UnitTests/Evaluation/SdkResultEvaluation_Tests.cs

@@ -549,6 +549,121 @@ public void SdkResolverCanSetEnvVarsThatInfluenceBuild()
             instance.GetItems("ExecThingsAsEnvironment").ShouldHaveSingleItem().EvaluatedInclude.ShouldBe("TestEnvVarValue");
         }
 
+        [Fact]
+        public void SdkResolverEnvironmentVariablesOverrideAmbientEnvironmentVariables()
+        {
+            string originalValue = Environment.GetEnvironmentVariable("SDK_TEST_VAR");
+            try
+            {
+                // Set an ambient environment variable
+                Environment.SetEnvironmentVariable("SDK_TEST_VAR", "AmbientValue");
+
+                var projectOptions = SdkUtilities.CreateProjectOptionsWithResolver(new SdkUtilities.ConfigurableMockSdkResolver((_, _, factory) =>
+                    factory.IndicateSuccess(Path.Combine(_testFolder, "Sdk"), "1.0.0", null, null, null, environmentVariablesToAdd: new Dictionary<string, string>
+                            {
+                                { "SDK_TEST_VAR", "SdkValue" }
+                            })));
+
+                string projectContent = """
+                    <Project Sdk="sdkenvvar">
+                        <PropertyGroup>
+                            <CapturedValue>$(SDK_TEST_VAR)</CapturedValue>
+                        </PropertyGroup>
+                        <Target Name="TestTarget">
+                            <PropertyGroup>
+                                <ExecValue>$(SDK_TEST_VAR)</ExecValue>
+                            </PropertyGroup>
+                        </Target>
+                    </Project>
+                    """;
+
+                string projectPath = Path.Combine(_testFolder, "project.proj");
+                File.WriteAllText(projectPath, projectContent);
+
+                string sdkPropsContents = "<Project />";
+                string sdkPropsPath = Path.Combine(_testFolder, "Sdk", "Sdk.props");
+                Directory.CreateDirectory(Path.Combine(_testFolder, "Sdk"));
+                File.WriteAllText(sdkPropsPath, sdkPropsContents);
+
+                string sdkTargetsContents = @"<Project />";
+                string sdkTargetsPath = Path.Combine(_testFolder, "Sdk", "Sdk.targets");
+                File.WriteAllText(sdkTargetsPath, sdkTargetsContents);
+
+                var project = CreateProject(projectPath, projectOptions);
+                var instance = project.CreateProjectInstance();
+
+                _logger.AssertNoErrors();
+                _logger.AssertNoWarnings();
+
+                // The SDK value should override the ambient environment variable
+                instance.GetPropertyValue("CapturedValue").ShouldBe("SdkValue");
+
+                instance.Build(["TestTarget"], [_logger], out var targetOutputs);
+
+                instance.GetPropertyValue("ExecValue").ShouldBe("SdkValue");
+            }
+            finally
+            {
+                Environment.SetEnvironmentVariable("SDK_TEST_VAR", originalValue);
+            }
+        }
+
+        [Fact]
+        public void FirstSdkEnvironmentVariableWinsOverSubsequentSdks()
+        {
+            var projectOptions = SdkUtilities.CreateProjectOptionsWithResolver(new SdkUtilities.ConfigurableMockSdkResolver((sdkReference, _, factory) =>
+            {
+                // First SDK sets the variable
+                if (sdkReference.Name == "FirstSdk")
+                {
+                    return factory.IndicateSuccess(Path.Combine(_testFolder, "Sdk1"), "1.0.0", null, null, null, environmentVariablesToAdd: new Dictionary<string, string>
+                    {
+                        { "SHARED_VAR", "FirstValue" }
+                    });
+                }
+                // Second SDK tries to set it, but should be ignored
+                else if (sdkReference.Name == "SecondSdk")
+                {
+                    return factory.IndicateSuccess(Path.Combine(_testFolder, "Sdk2"), "1.0.0", null, null, null, environmentVariablesToAdd: new Dictionary<string, string>
+                    {
+                        { "SHARED_VAR", "SecondValue" }
+                    });
+                }
+                return null;
+            }));
+
+            string projectContent = """
+                <Project>
+                    <Import Project="Sdk.props" Sdk="FirstSdk"/>
+                    <Import Project="Sdk.props" Sdk="SecondSdk"/>
+                    <PropertyGroup>
+                        <CapturedValue>$(SHARED_VAR)</CapturedValue>
+                    </PropertyGroup>
+                </Project>
+                """;
+
+            string projectPath = Path.Combine(_testFolder, "project.proj");
+            File.WriteAllText(projectPath, projectContent);
+
+            string sdk1PropsContents = "<Project />";
+            string sdk1PropsPath = Path.Combine(_testFolder, "Sdk1", "Sdk.props");
+            Directory.CreateDirectory(Path.Combine(_testFolder, "Sdk1"));
+            File.WriteAllText(sdk1PropsPath, sdk1PropsContents);
+
+            string sdk2PropsContents = "<Project />";
+            string sdk2PropsPath = Path.Combine(_testFolder, "Sdk2", "Sdk.props");
+            Directory.CreateDirectory(Path.Combine(_testFolder, "Sdk2"));
+            File.WriteAllText(sdk2PropsPath, sdk2PropsContents);
+
+            var project = CreateProject(projectPath, projectOptions);
+
+            _logger.AssertNoErrors();
+            _logger.AssertNoWarnings();
+
+            // The first SDK's value should win
+            project.GetPropertyValue("CapturedValue").ShouldBe("FirstValue");
+        }
+
         public void Dispose()
         {
             _env.Dispose();

src/Build/BackEnd/Components/SdkResolution/SdkResult.cs

@@ -92,6 +92,7 @@ public void Translate(ITranslator translator)
                                            keyTranslator: (ITranslator t, ref string s) => t.Translate(ref s),
                                            valueTranslator: SdkResultTranslationHelpers.Translate,
                                            dictionaryCreator: count => new Dictionary<string, SdkResultItem>(count, StringComparer.OrdinalIgnoreCase));
+            translator.TranslateDictionary(ref _environmentVariablesToAdd, count => new Dictionary<string, string>(count, StringComparer.OrdinalIgnoreCase));
 
             translator.Translate(ref _sdkReference);
         }
@@ -112,6 +113,7 @@ public override bool Equals(object obj)
                   _additionalPaths?.Count == result._additionalPaths?.Count &&
                   _propertiesToAdd?.Count == result._propertiesToAdd?.Count &&
                   _itemsToAdd?.Count == result._propertiesToAdd?.Count &&
+                  _environmentVariablesToAdd?.Count == result._environmentVariablesToAdd?.Count &&
                   EqualityComparer<SdkReference>.Default.Equals(_sdkReference, result._sdkReference))
             {
                 if (_additionalPaths != null)
@@ -147,6 +149,17 @@ public override bool Equals(object obj)
                     }
                 }
 
+                if (_environmentVariablesToAdd != null)
+                {
+                    foreach (var envVarToAdd in _environmentVariablesToAdd)
+                    {
+                        if (result._environmentVariablesToAdd[envVarToAdd.Key] != envVarToAdd.Value)
+                        {
+                            return false;
+                        }
+                    }
+                }
+
                 return true;
             }
 
@@ -185,6 +198,14 @@ public override int GetHashCode()
                     hashCode = (hashCode * -1521134295) + itemToAdd.Value.GetHashCode();
                 }
             }
+            if (_environmentVariablesToAdd != null)
+            {
+                foreach (var envVarToAdd in _environmentVariablesToAdd)
+                {
+                    hashCode = (hashCode * -1521134295) + envVarToAdd.Key.GetHashCode();
+                    hashCode = (hashCode * -1521134295) + envVarToAdd.Value.GetHashCode();
+                }
+            }
 
             return hashCode;
         }

src/Build/Definition/Project.cs

@@ -4466,9 +4466,11 @@ public IItemDefinition<ProjectMetadata> GetItemDefinition(string itemType)
             /// <param name="value">Environment variable value.</param>
             public void AddSdkResolvedEnvironmentVariable(string name, string value)
             {
-                // If the property has already been set as an environment variable or by another SDK, we do not overwrite it.
-                if (EnvironmentVariablePropertiesDictionary?.Contains(name) == true
-                    || SdkResolvedEnvironmentVariablePropertiesDictionary?.Contains(name) == true)
+                ErrorUtilities.VerifyThrowArgumentLength(name);
+                ErrorUtilities.VerifyThrowArgumentNull(value);
+
+                // If another SDK already set it, we do not overwrite it.
+                if (SdkResolvedEnvironmentVariablePropertiesDictionary?.Contains(name) == true)
                 {
                     return;
                 }
@@ -4478,6 +4480,7 @@ public void AddSdkResolvedEnvironmentVariable(string name, string value)
                 SdkResolvedEnvironmentVariablePropertiesDictionary ??= new();
                 SdkResolvedEnvironmentVariablePropertiesDictionary.Set(property);
 
+                // SDK-resolved environment variables override ambient environment variables.
                 SetProperty(name, value, isGlobalProperty: false, mayBeReserved: false, loggingContext: null);
             }
 

src/Build/Instance/ProjectInstance.cs

@@ -1387,14 +1387,11 @@ PropertyDictionary<ProjectPropertyInstance> IEvaluatorData<ProjectPropertyInstan
 
         public void AddSdkResolvedEnvironmentVariable(string name, string value)
         {
-            // If the property has already been set as an environment variable, we do not overwrite it.
-            if (_environmentVariableProperties.Contains(name))
-            {
-                _loggingContext.LogComment(MessageImportance.Low, "SdkEnvironmentVariableAlreadySet", name, value);
-                return;
-            }
+            ErrorUtilities.VerifyThrowArgumentLength(name);
+            ErrorUtilities.VerifyThrowArgumentNull(value);
+
             // If another SDK already set it, we do not overwrite it.
-            else if (_sdkResolvedEnvironmentVariableProperties?.Contains(name) == true)
+            if (_sdkResolvedEnvironmentVariableProperties?.Contains(name) == true)
             {
                 _loggingContext.LogComment(MessageImportance.Low, "SdkEnvironmentVariableAlreadySetBySdk", name, value);
                 return;
@@ -1406,8 +1403,18 @@ public void AddSdkResolvedEnvironmentVariable(string name, string value)
 
             _sdkResolvedEnvironmentVariableProperties.Set(property);
 
-            // Only set the local property if it does not already exist, prioritizing regular properties defined in XML.
-            if (GetProperty(name) is null)
+            // SDK-resolved environment variables override ambient environment variables.
+            bool overridingAmbient = _environmentVariableProperties.Contains(name);
+            if (overridingAmbient)
+            {
+                _environmentVariableProperties.Remove(name);
+                _loggingContext.LogComment(MessageImportance.Low, "SdkEnvironmentVariableOverridingAmbient", name, value);
+            }
+
+            // Set the property, overriding ambient environment variables but not regular properties defined in XML.
+            // If we're overriding an ambient variable, or if the property doesn't exist, set it.
+            // Otherwise, prioritize regular properties defined in XML.
+            if (overridingAmbient || GetProperty(name) is null)
             {
                 ((IEvaluatorData<ProjectPropertyInstance, ProjectItemInstance, ProjectMetadataInstance, ProjectItemDefinitionInstance>)this)
                    .SetProperty(name, value, isGlobalProperty: false, mayBeReserved: false, loggingContext: _loggingContext, isEnvironmentVariable: true, isCommandLineProperty: false);

src/Build/Resources/Strings.resx

@@ -1392,6 +1392,9 @@ Errors: {3}</value>
   <data name="SdkEnvironmentVariableSet" xml:space="preserve">
     <value>An SDK attempted set the environment variable "{0}" to "{1}".</value>
   </data>
+  <data name="SdkEnvironmentVariableOverridingAmbient" xml:space="preserve">
+    <value>An SDK set the environment variable "{0}" to "{1}", overriding the ambient environment variable value.</value>
+  </data>
   <data name="UnrecognizedParentElement" xml:space="preserve">
     <value>MSB4189: &lt;{1}&gt; is not a valid child of the &lt;{0}&gt; element.</value>
     <comment>{StrBegin="MSB4189: "}</comment>