From 74c90d637f5c4539b3fb3fd86f0e504bcdb8fc51 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 8 Jul 2024 16:16:38 +0000 Subject: [PATCH] [automated] Merge branch 'vs17.11' => 'main' (#10279) * Final branding for 17.11 (#10270) * Final branding and public API version update * Update the regex for initial commit detection * Disable CustomAnalyzerTest * Delete CompatibilitySuppressions file * Add inter-branch merge flow file (#10274) * Add version to BuildResult 2 (#10288) Fixes #10208 Context We are adding a version field to this class to make the ResultsCache backwards compatible with at least 2 previous releases (meaning the newer VS can read a cache created by older VS). The cache is not forwards compatible (older versions of VS cannot read cache created by newer versions). The adding of a version field is done without a breaking change in 3 steps, each separated with at least 1 intermediate release. Execution plan: 1st step (done): Add a special key to the _savedEnvironmentVariables dictionary during the serialization. A workaround overload of the TranslateDictionary function is created to achieve it. The presence of this key will indicate that the version is serialized next. When serializing, add a key to the dictionary and serialize a version field. Do not actually save the special key to dictionary during the deserialization but read a version as a next field if it presents. 2nd step: Stop serialize a special key with the dictionary _savedEnvironmentVariables using the TranslateDictionary function workaround overload. Always serialize and de-serialize the version field. Continue to deserialize _savedEnvironmentVariables with the TranslateDictionary function workaround overload in order not to deserialize dictionary with the special keys. 3rd step: Stop using the TranslateDictionary function workaround overload during _savedEnvironmentVariables deserialization. Changes Made 1st step from above description. Testing Unit tests, manual tests, experimental insertion * Add CompatibilitySuppressions.xml --------- Co-authored-by: AR-May <67507805+AR-May@users.noreply.github.com> Co-authored-by: Farhad Alizada <104755925+f-alizada@users.noreply.github.com> Co-authored-by: Jan Krivanek --- .github/workflows/inter-branch-merge-flow.yml | 15 +++ .vsts-dotnet-ci.yml | 6 +- .../BackEnd/ResultsCache_Tests.cs | 46 ++++++- .../Components/Caching/ResultsCache.cs | 9 +- src/Build/BackEnd/Shared/BuildResult.cs | 88 ++++++++++++- src/Build/CompatibilitySuppressions.xml | 32 +++++ src/BuildCheck.UnitTests/EndToEndTests.cs | 2 +- src/Framework/BinaryTranslator.cs | 121 ++++++++++++++++++ src/Framework/ITranslator.cs | 13 ++ src/Framework/Traits.cs | 8 ++ 10 files changed, 327 insertions(+), 13 deletions(-) create mode 100644 .github/workflows/inter-branch-merge-flow.yml diff --git a/.github/workflows/inter-branch-merge-flow.yml b/.github/workflows/inter-branch-merge-flow.yml new file mode 100644 index 0000000000..68fdef4127 --- /dev/null +++ b/.github/workflows/inter-branch-merge-flow.yml @@ -0,0 +1,15 @@ +name: Inter-branch merge workflow +on: + push: + branches: + - vs1** + +permissions: + contents: write + pull-requests: write + +jobs: + Merge: + uses: dotnet/arcade/.github/workflows/inter-branch-merge-base.yml@main + with: + configuration_file_path: '.config/git-merge-flow-config.jsonc' \ No newline at end of file diff --git a/.vsts-dotnet-ci.yml b/.vsts-dotnet-ci.yml index daf200ae88..6df5156b63 100644 --- a/.vsts-dotnet-ci.yml +++ b/.vsts-dotnet-ci.yml @@ -15,11 +15,11 @@ jobs: $isVersionBumped = $false if ($changedVersionsFile -ne $null) { $difference = git diff HEAD~1 $versionsFile - $changedContent = $difference -join " " + $changedContent = $difference -join "%" # 'DotNetFinalVersionKind' is expected to be added only during the initial setup of the release branch - $initialCommitPattern = '-\s*\d+\.\d+\.\d+<\/VersionPrefix> \+\s*\d+\.\d+\.\d+<\/VersionPrefix>.*release<\/DotNetFinalVersionKind>' + $initialCommitPattern = '-\s*\d+\.\d+\.\d+<\/VersionPrefix>%.*\+\s*\d+\.\d+\.\d+<\/VersionPrefix>release<\/DotNetFinalVersionKind>' $isInitialCommit = $changedContent -match $initialCommitPattern - $pattern = '-\s*\d+\.\d+\.(?\d+)<\/VersionPrefix>.* \+\s*\d+\.\d+\.(?\d+)<\/VersionPrefix>' + $pattern = '-\s*\d+\.\d+\.(?\d+)<\/VersionPrefix>.*%\+\s*\d+\.\d+\.(?\d+)<\/VersionPrefix>' if (!($isInitialCommit) -and ($changedContent -match $pattern)) { try { $previousPatch = [Convert]::ToInt32($Matches.previous) diff --git a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs index ac42139912..7fc43eccc5 100644 --- a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs +++ b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs @@ -383,6 +383,7 @@ namespace Microsoft.Build.UnitTests.BackEnd } } + // Serialize latest version and deserialize latest version of the cache [Theory] [MemberData(nameof(CacheSerializationTestData))] public void TestResultsCacheTranslation(object obj) @@ -393,12 +394,49 @@ namespace Microsoft.Build.UnitTests.BackEnd var copy = new ResultsCache(TranslationHelpers.GetReadTranslator()); - copy.ResultsDictionary.Keys.ToHashSet().SetEquals(resultsCache.ResultsDictionary.Keys.ToHashSet()).ShouldBeTrue(); + CompareResultsCache(resultsCache, copy); + } - foreach (var configId in copy.ResultsDictionary.Keys) + [Theory] + [InlineData(1, 1)] // Serialize version 0 and deserialize version 0 + [InlineData(1, 0)] // Serialize version 0 and deserialize latest version + public void TestResultsCacheTranslationAcrossVersions(int envValue1, int envValue2) + { + using (var env = TestEnvironment.Create()) { - var copiedBuildResult = copy.ResultsDictionary[configId]; - var initialBuildResult = resultsCache.ResultsDictionary[configId]; + env.SetEnvironmentVariable("MSBUILDDONOTVERSIONBUILDRESULT", $"{envValue1}"); + + // Create a ResultsCache + var request1 = new BuildRequest(1, 2, 3, new[] { "target1" }, null, BuildEventContext.Invalid, null); + var request2 = new BuildRequest(4, 5, 6, new[] { "target2" }, null, BuildEventContext.Invalid, null); + + var br1 = new BuildResult(request1); + var br2 = new BuildResult(request2); + br2.AddResultsForTarget("target2", BuildResultUtilities.GetEmptyFailingTargetResult()); + + var resultsCache = new ResultsCache(); + resultsCache.AddResult(br1.Clone()); + resultsCache.AddResult(br2.Clone()); + + resultsCache.Translate(TranslationHelpers.GetWriteTranslator()); + + env.SetEnvironmentVariable("MSBUILDDONOTVERSIONBUILDRESULT", $"{envValue2}"); + Traits.UpdateFromEnvironment(); + + var copy = new ResultsCache(TranslationHelpers.GetReadTranslator()); + + CompareResultsCache(resultsCache, copy); + } + } + + private void CompareResultsCache(ResultsCache resultsCache1, ResultsCache resultsCache2) + { + resultsCache2.ResultsDictionary.Keys.ToHashSet().SetEquals(resultsCache1.ResultsDictionary.Keys.ToHashSet()).ShouldBeTrue(); + + foreach (var configId in resultsCache2.ResultsDictionary.Keys) + { + var copiedBuildResult = resultsCache2.ResultsDictionary[configId]; + var initialBuildResult = resultsCache1.ResultsDictionary[configId]; copiedBuildResult.SubmissionId.ShouldBe(initialBuildResult.SubmissionId); copiedBuildResult.ConfigurationId.ShouldBe(initialBuildResult.ConfigurationId); diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs index 34480ff214..e34dd90c5b 100644 --- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs +++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs @@ -350,9 +350,14 @@ namespace Microsoft.Build.BackEnd /// The candidate build result. /// True if the flags and project state filter of the build request is compatible with the build result. private static bool AreBuildResultFlagsCompatible(BuildRequest buildRequest, BuildResult buildResult) - { + { + if (buildResult.BuildRequestDataFlags is null) + { + return true; + } + BuildRequestDataFlags buildRequestDataFlags = buildRequest.BuildRequestDataFlags; - BuildRequestDataFlags buildResultDataFlags = buildResult.BuildRequestDataFlags; + BuildRequestDataFlags buildResultDataFlags = (BuildRequestDataFlags) buildResult.BuildRequestDataFlags; if ((buildRequestDataFlags & FlagsAffectingBuildResults) != (buildResultDataFlags & FlagsAffectingBuildResults)) { diff --git a/src/Build/BackEnd/Shared/BuildResult.cs b/src/Build/BackEnd/Shared/BuildResult.cs index 56458bc630..776179d0f0 100644 --- a/src/Build/BackEnd/Shared/BuildResult.cs +++ b/src/Build/BackEnd/Shared/BuildResult.cs @@ -9,6 +9,7 @@ using System.Linq; using Microsoft.Build.BackEnd; using Microsoft.Build.Shared; using Microsoft.Build.Shared.FileSystem; +using Microsoft.Build.Framework; namespace Microsoft.Build.Execution { @@ -31,6 +32,9 @@ namespace Microsoft.Build.Execution /// /// Contains the current results for all of the targets which have produced results for a particular configuration. /// + /// + /// When modifying serialization/deserialization, bump the version and support previous versions in order to keep backwards compatible. + /// public class BuildResult : BuildResultBase, INodePacket, IBuildResults { /// @@ -75,6 +79,14 @@ namespace Microsoft.Build.Execution /// private ConcurrentDictionary _resultsByTarget; + /// + /// Version of the build result. + /// + /// + /// Allows to serialize and deserialize different versions of the build result. + /// + private int _version = Traits.Instance.EscapeHatches.DoNotVersionBuildResult ? 0 : 1; + /// /// The request caused a circular dependency in scheduling. /// @@ -98,6 +110,16 @@ namespace Microsoft.Build.Execution /// private Dictionary? _savedEnvironmentVariables; + /// + /// When this key is in the dictionary , serialize the build result version. + /// + private const string SpecialKeyForVersion = "=MSBUILDFEATUREBUILDRESULTHASVERSION="; + + /// + /// Set of additional keys tat might be added to the dictionary . + /// + private static readonly HashSet s_additionalEntriesKeys = new HashSet { SpecialKeyForVersion }; + /// /// Snapshot of the current directory from the configuration this result comes from. /// This should only be populated when the configuration for this result is moved between nodes. @@ -117,6 +139,9 @@ namespace Microsoft.Build.Execution /// /// The flags provide additional control over the build results and may affect the cached value. /// + /// + /// Is optional, the field is expected to be present starting 1. + /// private BuildRequestDataFlags _buildRequestDataFlags; private string? _schedulerInducedError; @@ -395,7 +420,10 @@ namespace Microsoft.Build.Execution /// Gets the flags that were used in the build request to which these results are associated. /// See for examples of the available flags. /// - public BuildRequestDataFlags BuildRequestDataFlags => _buildRequestDataFlags; + /// + /// Is optional, this property exists starting 1. + /// + public BuildRequestDataFlags? BuildRequestDataFlags => (_version > 0) ? _buildRequestDataFlags : null; /// /// Returns the node packet type. @@ -597,8 +625,62 @@ namespace Microsoft.Build.Execution translator.Translate(ref _projectStateAfterBuild, ProjectInstance.FactoryForDeserialization); translator.Translate(ref _savedCurrentDirectory); translator.Translate(ref _schedulerInducedError); - translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase); - translator.TranslateEnum(ref _buildRequestDataFlags, (int)_buildRequestDataFlags); + + // This is a work-around for the bug https://github.com/dotnet/msbuild/issues/10208 + // We are adding a version field to this class to make the ResultsCache backwards compatible with at least 2 previous releases. + // The adding of a version field is done without a breaking change in 3 steps, each separated with at least 1 intermediate release. + // + // 1st step (done): Add a special key to the _savedEnvironmentVariables dictionary during the serialization. A workaround overload of the TranslateDictionary function is created to achieve it. + // The presence of this key will indicate that the version is serialized next. + // When serializing, add a key to the dictionary and serialize a version field. + // Do not actually save the special key to dictionary during the deserialization, but read a version as a next field if it presents. + // + // 2nd step: Stop serialize a special key with the dictionary _savedEnvironmentVariables using the TranslateDictionary function workaround overload. Always serialize and de-serialize the version field. + // Continue to deserialize _savedEnvironmentVariables with the TranslateDictionary function workaround overload in order not to deserialize dictionary with the special keys. + // + // 3rd step: Stop using the TranslateDictionary function workaround overload during _savedEnvironmentVariables deserialization. + if (_version == 0) + { + // Escape hatch: serialize/deserialize without version field. + translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase); + } + else + { + Dictionary additionalEntries = new(); + + if (translator.Mode == TranslationDirection.WriteToStream) + { + // Add the special key SpecialKeyForVersion to additional entries indicating the presence of a version to the _savedEnvironmentVariables dictionary. + additionalEntries.Add(SpecialKeyForVersion, String.Empty); + + // Serialize the special key together with _savedEnvironmentVariables dictionary using the workaround overload of TranslateDictionary: + translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase, ref additionalEntries, s_additionalEntriesKeys); + + // Serialize version + translator.Translate(ref _version); + } + else if (translator.Mode == TranslationDirection.ReadFromStream) + { + // Read the dictionary using the workaround overload of TranslateDictionary: special keys (additionalEntriesKeys) would be read to additionalEntries instead of the _savedEnvironmentVariables dictionary. + translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase, ref additionalEntries, s_additionalEntriesKeys); + + // If the special key SpecialKeyForVersion present in additionalEntries, also read a version, otherwise set it to 0. + if (additionalEntries is not null && additionalEntries.ContainsKey(SpecialKeyForVersion)) + { + translator.Translate(ref _version); + } + else + { + _version = 0; + } + } + } + + // Starting version 1 this _buildRequestDataFlags field is present. + if (_version > 0) + { + translator.TranslateEnum(ref _buildRequestDataFlags, (int)_buildRequestDataFlags); + } } /// diff --git a/src/Build/CompatibilitySuppressions.xml b/src/Build/CompatibilitySuppressions.xml index 1fd3a16692..8aa3259bc7 100644 --- a/src/Build/CompatibilitySuppressions.xml +++ b/src/Build/CompatibilitySuppressions.xml @@ -1,6 +1,37 @@  + + + CP0002 + M:Microsoft.Build.Execution.BuildResult.get_BuildRequestDataFlags + lib/net472/Microsoft.Build.dll + lib/net472/Microsoft.Build.dll + true + + + CP0002 + M:Microsoft.Build.Execution.BuildResult.get_BuildRequestDataFlags + lib/net8.0/Microsoft.Build.dll + lib/net8.0/Microsoft.Build.dll + true + + + CP0002 + M:Microsoft.Build.Execution.BuildResult.get_BuildRequestDataFlags + ref/net472/Microsoft.Build.dll + ref/net472/Microsoft.Build.dll + true + + + CP0002 + M:Microsoft.Build.Execution.BuildResult.get_BuildRequestDataFlags + ref/net8.0/Microsoft.Build.dll + ref/net8.0/Microsoft.Build.dll + true + + + CP0002 F:Microsoft.Build.Experimental.BuildCheck.BuildAnalyzerResultSeverity.Info @@ -85,4 +116,5 @@ ref/net8.0/Microsoft.Build.dll true + \ No newline at end of file diff --git a/src/BuildCheck.UnitTests/EndToEndTests.cs b/src/BuildCheck.UnitTests/EndToEndTests.cs index 4436fd2436..4fb719fc9a 100644 --- a/src/BuildCheck.UnitTests/EndToEndTests.cs +++ b/src/BuildCheck.UnitTests/EndToEndTests.cs @@ -221,7 +221,7 @@ public class EndToEndTests : IDisposable _env.SetEnvironmentVariable("MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION", "1"); } - [Theory] + [Theory(Skip = "https://github.com/dotnet/msbuild/issues/10277")] [InlineData("AnalysisCandidate", new[] { "CustomRule1", "CustomRule2" })] [InlineData("AnalysisCandidateWithMultipleAnalyzersInjected", new[] { "CustomRule1", "CustomRule2", "CustomRule3" }, true)] public void CustomAnalyzerTest(string analysisCandidate, string[] expectedRegisteredRules, bool expectedRejectedAnalyzers = false) diff --git a/src/Framework/BinaryTranslator.cs b/src/Framework/BinaryTranslator.cs index d3ae387822..a2a72ede9e 100644 --- a/src/Framework/BinaryTranslator.cs +++ b/src/Framework/BinaryTranslator.cs @@ -22,6 +22,14 @@ namespace Microsoft.Build.BackEnd /// internal static class BinaryTranslator { + /// + /// Presence of this key in the dictionary indicates that it was null. + /// + /// + /// This constant is needed for a workaround concerning serializing BuildResult with a version. + /// + private const string SpecialKeyForDictionaryBeingNull = "=MSBUILDDICTIONARYWASNULL="; + #nullable enable /// /// Returns a read-only serializer. @@ -590,6 +598,53 @@ namespace Microsoft.Build.BackEnd dictionary = (Dictionary)copy; } + /// + /// Translates a dictionary of { string, string } with additional entries. The dictionary might be null despite being populated. + /// + /// The dictionary to be translated. + /// The comparer used to instantiate the dictionary. + /// Additional entries to be translated + /// Additional entries keys + /// + /// This overload is needed for a workaround concerning serializing BuildResult with a version. + /// It deserializes additional entries together with the main dictionary. + /// + public void TranslateDictionary(ref Dictionary dictionary, IEqualityComparer comparer, ref Dictionary additionalEntries, HashSet additionalEntriesKeys) + { + if (!TranslateNullable(dictionary)) + { + return; + } + + int count = _reader.ReadInt32(); + dictionary = new Dictionary(count, comparer); + additionalEntries = new(); + + for (int i = 0; i < count; i++) + { + string key = null; + Translate(ref key); + string value = null; + Translate(ref value); + if (additionalEntriesKeys.Contains(key)) + { + additionalEntries[key] = value; + } + else if (comparer.Equals(key, SpecialKeyForDictionaryBeingNull)) + { + // Presence of special key SpecialKeyForDictionaryBeingNull indicates that the dictionary was null. + dictionary = null; + + // If the dictionary is null, we should have only two keys: SpecialKeyForDictionaryBeingNull, SpecialKeyForVersion + Debug.Assert(count == 2); + } + else if (dictionary is not null) + { + dictionary[key] = value; + } + } + } + public void TranslateDictionary(ref IDictionary dictionary, NodePacketCollectionCreator> dictionaryCreator) { if (!TranslateNullable(dictionary)) @@ -1261,6 +1316,72 @@ namespace Microsoft.Build.BackEnd TranslateDictionary(ref copy, (NodePacketCollectionCreator>)null); } + /// + /// Translates a dictionary of { string, string } adding additional entries. + /// + /// The dictionary to be translated. + /// The comparer used to instantiate the dictionary. + /// Additional entries to be translated. + /// Additional entries keys. + /// + /// This overload is needed for a workaround concerning serializing BuildResult with a version. + /// It serializes additional entries together with the main dictionary. + /// + public void TranslateDictionary(ref Dictionary dictionary, IEqualityComparer comparer, ref Dictionary additionalEntries, HashSet additionalEntriesKeys) + { + // Translate whether object is null + if ((dictionary is null) && ((additionalEntries is null) || (additionalEntries.Count == 0))) + { + _writer.Write(false); + return; + } + else + { + // Translate that object is not null + _writer.Write(true); + } + + // Writing a dictionary, additional entries and special key if dictionary was null. We need the special key for distinguishing whether the initial dictionary was null or empty. + int count = (dictionary is null ? 1 : 0) + + (additionalEntries is null ? 0 : additionalEntries.Count) + + (dictionary is null ? 0 : dictionary.Count); + + _writer.Write(count); + + // If the dictionary was null, serialize a special key SpecialKeyForDictionaryBeingNull. + if (dictionary is null) + { + string key = SpecialKeyForDictionaryBeingNull; + Translate(ref key); + string value = string.Empty; + Translate(ref value); + } + + // Serialize additional entries + if (additionalEntries is not null) + { + foreach (KeyValuePair pair in additionalEntries) + { + string key = pair.Key; + Translate(ref key); + string value = pair.Value; + Translate(ref value); + } + } + + // Serialize dictionary + if (dictionary is not null) + { + foreach (KeyValuePair pair in dictionary) + { + string key = pair.Key; + Translate(ref key); + string value = pair.Value; + Translate(ref value); + } + } + } + public void TranslateDictionary(ref IDictionary dictionary, NodePacketCollectionCreator> dictionaryCreator) { if (!TranslateNullable(dictionary)) diff --git a/src/Framework/ITranslator.cs b/src/Framework/ITranslator.cs index edb6e96dfc..edf5b47765 100644 --- a/src/Framework/ITranslator.cs +++ b/src/Framework/ITranslator.cs @@ -319,6 +319,19 @@ namespace Microsoft.Build.BackEnd /// The comparer used to instantiate the dictionary. void TranslateDictionary(ref Dictionary dictionary, IEqualityComparer comparer); + /// + /// Translates a dictionary of { string, string } adding additional entries. + /// + /// The dictionary to be translated. + /// The comparer used to instantiate the dictionary. + /// Additional entries to be translated + /// Additional entries keys + /// + /// This overload is needed for a workaround concerning serializing BuildResult with a version. + /// It serializes/deserializes additional entries together with the main dictionary. + /// + void TranslateDictionary(ref Dictionary dictionary, IEqualityComparer comparer, ref Dictionary additionalEntries, HashSet additionalEntriesKeys); + void TranslateDictionary(ref IDictionary dictionary, NodePacketCollectionCreator> collectionCreator); void TranslateDictionary(ref Dictionary dictionary, StringComparer comparer); diff --git a/src/Framework/Traits.cs b/src/Framework/Traits.cs index 8e9d1e09d0..9bca9afa1a 100644 --- a/src/Framework/Traits.cs +++ b/src/Framework/Traits.cs @@ -359,6 +359,14 @@ namespace Microsoft.Build.Framework /// public readonly bool UseMinimalResxParsingInCoreScenarios = Environment.GetEnvironmentVariable("MSBUILDUSEMINIMALRESX") == "1"; + /// + /// Escape hatch to ensure msbuild produces the compatible build results cache without versioning. + /// + /// + /// Escape hatch for problems arising from https://github.com/dotnet/msbuild/issues/10208. + /// + public readonly bool DoNotVersionBuildResult = Environment.GetEnvironmentVariable("MSBUILDDONOTVERSIONBUILDRESULT") == "1"; + private bool _sdkReferencePropertyExpansionInitialized; private SdkReferencePropertyExpansionMode? _sdkReferencePropertyExpansionValue;